Skip to content

Patched DF 52.5.0 (revision d) - #103

Open
lilic wants to merge 20 commits into
base-df-upgrade-ver5250from
upgrade-df-ver5250-d
Open

lilic wants to merge 20 commits into
base-df-upgrade-ver5250from
upgrade-df-ver5250-d

Conversation

@lilic

@lilic lilic commented Aug 24, 2026

Copy link
Copy Markdown
Member

Revision c plus two forward-ported fixes that landed on the DF 51 line (upgrade-df-ver5100-a) on 2026-08-14, after revision c was cut. Both are customer-reported bugs that main already carries via influxdata/influxdb_pro#5221 — without forward-porting them, merging the 52.5 upgrade would silently drop both.

New commits on top of revision c

  1. fix: prevent LEAD/LAG IGNORE NULLS panic without null bitmap (#23706) — cherry-pick of fix: prevent LEAD/LAG IGNORE NULLS panic without null bitmap (#23706) #102. Upstream fix (fix: prevent LEAD/LAG IGNORE NULLS panic without null bitmap apache/datafusion#23706, authored by xudong.w), cherry-picked to the 51 line by @egillespie. Fixes a panic reported by a Cloud Dedicated customer in influxdata/EAR#7049. Applies cleanly; not present in the 52.5.0 base.

  2. fix: fast paths for UnionExec schema recomputation — cherry-pick of fix: fast paths for UnionExec schema recomputation (O(n²) planning on wide unions) #100 (@reidkaufmann). Two fast paths in UnionExec / union_schema that eliminate the O(n²)-in-child-count schema recomputation during physical planning of wide unions. Root-caused via influxdata/EAR#7010 (Vitol): a dedup-heavy LIMIT 1 plan over ~4,177 per-partition union branches took ~50s to plan and ~0.06s to execute; with the fix, planning drops to ~1.7s. Behaviour-identical — same plan. Applies cleanly; not present in the 52.5.0 base.

One adaptation was needed: the accompanying benches/union_schema.rs is reformatted for this branch's rustfmt. The 52.x line uses edition-2024 import sorting, so criterion_group/criterion_main and ExecutionPlan order differently than on the 51 line. Two lines, folded into the cherry-pick.

Unchanged — the 10 patches described in #99, including the to_timestamp timezone revert (influxdata/EAR#6765) and the OneOf signature diagnostics cherry-pick.

Per convention this PR is a review surface only — do not merge. Consumers pin e9e2e6d36bf63e39f746f82ee9b33c47feb65361 by rev

alamb and others added 12 commits May 6, 2026 02:23
Includes fix for FixedSizeBinary LEFT JOIN bug
- apache/arrow-rs#8981

Cherry-picked test and API updates from
- apache#19355
…e#19904)

## Which issue does this PR close?

<!--
We generally require a GitHub issue to be filed for all bug fixes and
enhancements and this helps us generate change logs for our releases.
You can link an issue to this PR using the GitHub syntax. For example
`Closes apache#123` indicates that this PR will close issue apache#123.
-->

- Part of apache#19798

## Rationale for this change

<!--
Why are you proposing this change? If this is already explained clearly
in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand
your changes and offer better suggestions for fixes.
-->

This is an edge case, would prefer to fix upstream in arrow-rs instead
of having handling code here, so just disable test for now.

- arrow-rs issue: apache/arrow-rs#9227

## What changes are included in this PR?

<!--
There is no need to duplicate the description in the issue here but it
is sometimes worth providing a summary of the individual changes in this
PR.
-->

Disable edge-case array_union SLT

## Are these changes tested?

<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code

If tests are not included in your PR, please explain why (for example,
are they covered by existing tests)?
-->

Test related change

## Are there any user-facing changes?

<!--
If there are user-facing changes then we may require documentation to be
updated before approving the PR.
-->

No.

<!--
If there are any breaking changes to public APIs, please add the `api
change` label.
-->
Hot-fix for security_audit CI on upgrade-df-ver5250-a. Addresses 5 RUSTSEC
advisories that landed between 2026-04-14 and 2026-04-27, after upstream DataFusion
52.5.0 was tagged:

- RUSTSEC-2026-0098 — rustls-webpki: name constraints for URI names
- RUSTSEC-2026-0099 — rustls-webpki: name constraints accepted for wildcard certs
- RUSTSEC-2026-0104 — rustls-webpki: reachable panic in CRL parsing
- RUSTSEC-2026-0112 — astral-tokio-tar: PAX header desync
- RUSTSEC-2026-0113 — astral-tokio-tar: unpack_in chmod via symlink

Bumps Cargo.lock only (no Cargo.toml dep range changes):
- astral-tokio-tar 0.6.0 -> 0.6.1
- rustls-webpki 0.103.10 -> 0.103.13

When upstream backports these (or the next 52.x release ships them), this
commit will auto-drop on rebase.
Cherry-pick of apache#21032. Adapted for 52.5.0:

- Two unused imports in datafusion-expr's test module (`IntervalUnit`,
  `logical_int64`) compile-fail under -D warnings; dropped.
- Two sql-integration tests and several sqllogictest expected-error
  patterns assumed the friendlier Display impl from apache#20605
  was already on 52.5.0 (it isn't); updated those patterns to the
  pre-apache#20605 verbose `TypeSignatureClass::Native(LogicalType(...))` shape
  and to the post-apache#21032 `failed with: ...` (no quoted inner message)
  shape.
- Also includes the author's own follow-up CI fix (was a separate commit
  upstream, folded here).

Signed-off-by: yaommen <myanstu@163.com>
Patches RUSTSEC-2026-0145 (PAX header desynchronization, GHSA-3cv2-h65g-fgmm).
Transitive dev-dep of `datafusion-cli` via `testcontainers` /
`testcontainers-modules`; Cargo.lock-only update.
…23706)

## Which issue does this PR close?

- Closes apache#23705.

## Rationale for this change

Arrow arrays containing no null values commonly omit the null bitmap.
The whole-partition evaluation path for `LEAD` and `LAG` with `IGNORE
NULLS` unconditionally unwrapped that optional bitmap, causing a panic
for valid non-null input arrays.

## What changes are included in this PR?

- Fall back to the existing regular shift implementation when the input
has no null bitmap.
- Add a regression test covering both `LEAD` and `LAG` with an array
that explicitly has no null bitmap.

## Are these changes tested?

Yes.

## Are there any user-facing changes?

`LEAD` and `LAG` with `IGNORE NULLS` no longer panic when the input
Arrow array has no null bitmap. There are no public API changes.

(cherry picked from commit a8d1af6)
UnionExec::try_new recomputes the union schema via union_schema() on
every construction, including with_new_children rebuilds performed by
optimizer passes. For wide unions whose children share one schema
(generated UNION ALL, unions of per-partition scans) this makes
physical planning O(n^2) in child count.

Two fast paths:
- union_schema(): if every input schema is pointer- or content-equal
  to the first, return the first schema (merging N identical schemas
  is the identity operation).
- UnionExec::with_new_children(): when child count and per-position
  child schemas are unchanged, reuse the existing schema. Plan
  properties are always recomputed, since they can legitimately change
  when schemas do not. (DataFusion 53 has an analogous properties
  optimization upstream, apache#19792; this variant targets
  the 51 line.)

Adds a union_schema benchmark covering shared-Arc, content-equal, and
adversarial (last child differs) shapes. All existing union unit tests
pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@mhilton mhilton left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two latest commits LGTM

…e per output chunk

sort_batch_stream reserves the sum of get_record_batch_memory_size over
the chunks from sort_batch_chunked. take() shares the dictionary values
across every chunk, so the dictionary is counted once per chunk. A 2.5 MB
batch with a 50k-value dictionary asks for 121.8 MB. With the disk manager
disabled there is no spill fallback and the sort fails.
…once

sort_batch_stream reserved the sum of per-chunk sizes for the output of
sort_batch_chunked. Dictionary values are shared by every chunk after
take(), so a batch with a large dictionary reserved chunks x dictionary
and failed with ResourcesExhausted or starved the pool.

Size the sorted output with one pass over all chunks so a shared buffer
is counted once. ReservationStream releases per batch, and per-batch
sizes can now add up to more than was reserved, so it releases at most
what remains; the rest is freed when the stream ends.
MinIO withdrew minio/minio from Docker Hub on 2026-09-11, so the
storage integration tests fail to start their container. Point the
image pinned by testcontainers-modules at quay.io, which serves the
same tags. Minimal backport of apache#25216.
…d batch

Clamping the per-batch release kept the reservation from underflowing
but freed shared buffers as soon as the first batch was emitted, while
the queued batches still referenced them.

get_record_batches_release_sizes charges each buffer to the last batch
that references it, so the sum is the shared-buffer total and the
stream releases exactly what each batch stops holding. ReservationStream
had no other user and is removed.
get_record_batch_memory_size and get_record_batches_memory_size count
into a scalar again; only sort_batch_stream builds the release-size
vector. Add a test that drops a partially consumed sorted stream and
checks the pool returns to zero, and share the test scaffolding.
datafusion/common/src/utils/memory.rs as of apache/datafusion main
9951f0b (2026-09-24): apache#24319 introduced the counter and
apache#24364, apache#24837, apache#25335 refined it. Counts each buffer once across a
sequence of batches; replaces the fork-local helpers.
…unter

Walk the chunks in reverse so each buffer is charged to the last chunk
that references it; the counter total is the reservation and the
per-chunk counts are what each emitted chunk releases.
…servation

fix(physical-plan): count buffers shared across sorted output chunks once
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants