[branch-54] Avoid sort pushdown panic on incomplete file statistics#152
Closed
geoffreyclaude wants to merge 1 commit into
Closed
[branch-54] Avoid sort pushdown panic on incomplete file statistics#152geoffreyclaude wants to merge 1 commit into
geoffreyclaude wants to merge 1 commit into
Conversation
Author
|
Duplicate of upstream PR apache#23277 |
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
DF54 added a sort-pushdown fallback that tries to reorder files within each file group by min/max statistics. This is an optional optimization: if the files do not have usable stats, planning should keep the original file order and leave the normal
SortExecin place.One path did not handle incomplete file stats safely.
MinMaxStatistics::new_from_files()first looks for min/max infile.statistics.column_statistics. If the requested column index is past that vector, it assumes the value must be a partition column and readsfile.partition_values[...].That assumption is not always true. A file can have a statistics object with no column statistics and also have no partition values.
Simplified failure:
Observed production stack:
This surfaced on
df-executor-canarywhile planning BlobStore parquet scans foridp-shadowListEntitiesrequests. The failing query shape sorted service names byLOWER(name)with pagination. This is not response-cache related: the trace hadcache_aware_planning=falseandconnectors_used=[blob].Patch
MinMaxStatistics::new_from_files().statistics not foundplanning signal when neither column stats nor partition values contain the requested stat.sort_files_within_groups_by_statistics()already catches missing-stat errors, keeps the original file group order, and skips the optional pushdown.In other words, this does not turn the panic into a user-visible planning error. It makes the optional optimization gracefully decline:
Why this belongs in DataFusion
The invalid assumption is in the shared file-statistics helper, not in a Datadog connector. A dd-source-only mitigation, such as forcing BlobStore scans to collect parquet footer stats, would add planning I/O and only cover one caller. The helper should be robust whenever file statistics are incomplete.
Tests
cargo test -p datafusion-datasource min_max_statistics_errors_when_column_and_partition_stats_are_missingcargo test -p datafusion-datasource sort_pushdown_unsupported_source_empty_column_statisticsReferences
cdfade51a feat: sort file groups by statistics during sort pushdown (Sort pushdown phase 2) (#21182)