fix: reassign predicate column indices by name before building pruning predicates#7
Merged
radustoenescu merged 1 commit intoJul 14, 2026
Conversation
…ng pruning predicates A filter's Column can carry an index that is only valid for the schema it was originally built against (e.g. a filter inferred across a join keeps the other side's index). ParquetSource::try_pushdown_filters and other producers store such predicates as-is, so build_pruning_predicates (used both for real per-file pruning and for ParquetSource::fmt_extra's Display output) could receive a Column whose index no longer matches its own name in file_schema. In debug builds this can panic: PhysicalExprSimplifier::simplify runs a debug-only type-preservation assertion that calls Column::data_type() using the raw index, which can resolve to an unrelated column (e.g. a struct-typed one), producing a type-coercion error that gets unwrapped. This is most visible via fmt_extra/Display (EXPLAIN, logging, or any code that displays the plan), since it does not go through per-file schema adaptation the way the real execution path does. Fix build_pruning_predicates to reassign every Column's index by name against file_schema before use, via the existing reassign_expr_columns helper (already used elsewhere in this crate for the same purpose). Adds a regression test reproducing the panic with real parquet-backed ParquetSources joined via HashJoinExec, using a DynamicFilterPhysicalExpr to match the shape of the predicate that triggers PruningPredicate::try_new's snapshot/simplify path.
adragomir
approved these changes
Jul 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ParquetSource::try_pushdown_filters(and other producers ofself.predicate) can store a filter whoseColumncarries an index that is only valid for the schema it was originally built against. In particular, when a join's filter-pushdown/inference produces a filter for one side from a condition on the other side, the resultingColumnkeeps the name of the join key but can keep the other side's index.build_pruning_predicates(used both by the real per-file pruning path and byParquetSource::fmt_extra'sDisplayoutput) uses that predicate'sColumnindices as-is againstfile_schema. When the index happens to land on a differently-typed column (e.g. a leading struct-typed column), this can panic in debug builds:PhysicalExprSimplifier::simplifyruns a debug-only type-preservation assertion (node.data_type(schema).unwrap()) before/after each rewrite. Resolving the mismatchedColumnby its (wrong) index againstfile_schemaproduces a type-coercion error (e.g.Struct(..) = Float64), and the.unwrap()panics.This is most visible via
Display(EXPLAIN, logging, or any code that displays the plan) because that path does not go through per-file schema adaptation the way the real execution path does - the real path (ParquetOpener::open) is protected by whateverPhysicalExprAdapteris configured, which re-resolves columns by name before anything type-sensitive runs.fmt_extrahas no such adapter available (it doesn't have the per-file physical schema), so it's exposed to the raw, possibly-mismatched predicate.Fix
In
build_pruning_predicates, reassign everyColumn's index by name againstfile_schemabefore building the pruning/page-pruning predicates, using the existingreassign_expr_columnshelper (datafusion_physical_expr::utils::reassign_expr_columns), which is already used elsewhere in this crate for the same purpose (row_filter.rs,opener.rs). This is a 4-line change and makesbuild_pruning_predicatesrobust regardless of which producer handed it a stale-indexed predicate.Test
Added
test_display_does_not_panic_on_predicate_with_mismatched_column_indexinsource.rs: builds two real parquet-backedParquetSources (one with a leading struct column, matching the real-world shape that triggered this), joins them viaHashJoinExec, and attaches to one side aColumn-mismatched predicate wrapped in aDynamicFilterPhysicalExpr(matching the shape that makesPruningPredicate::try_newtake its snapshot/simplify path). Assertsdisplayable(...).to_string()does not panic.Verified:
Cannot infer common argument type for comparison operation Struct(..) = Float64) when the fix is reverted.cargo test -p datafusion-datasource-parquet) and full workspace test suite (cargo test --workspace) pass with the fix applied, no regressions.