Uh oh!
There was an error while loading. Please reload this page.
fix: Handle input_file_name() pushdown into ParquetSource with filter pushdown enabled - #23638
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@## main #23638 +/- ##
==========================================
- Coverage 80.75% 80.75% -0.01%
==========================================
Files 1089 1089 Lines 368809 368811 +2 Branches 368809 368811 +2 ==========================================
- Hits 297837 297830 -7 - Misses 53217 53224 +7 - Partials 17755 17757 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: Adam Gutglick <adamgsal@gmail.com>
AdamGS
commented
Jul 16, 2026
I think this is a reasonable fix especially with the target date for the release getting closer. |
| } | ||
| if ScalarFunctionExpr::try_downcast_func::<FileRowIndexFunc>(node.as_ref()) | ||
| if ScalarFunctionExpr::try_downcast_func::<InputFileNameFunc>(node.as_ref()) |
There was a problem hiding this comment.
As a follow on, it might be nice to hoist this into a method on ScalarUDFImpl to avoid having the special cases here
Uh oh!
There was an error while loading. Please reload this page.
…t_file_name handling lost in rebase Three fixes on top of the rebase onto latest main: 1. row_filter.rs carried a stale pre-apache#23638 copy of the column-resolution machinery (ParquetReadPlan, PushdownChecker, StructFieldAccess, build_filter_schema, etc.) that main has since moved into projection_read_plan.rs. Rebuilt the file on main's shared-module version, keeping only this PR's real additions (PrebuiltRowFilterCandidate, prebuild_row_filter_candidates, row_filter_from_prebuilt, and the RowFilterGenerator removal): 2194 → 1483 lines, -798 net. The stray "strinrg" typo lived in the deleted copy. 2. The rebase silently dropped main's input_file_name() handling in ParquetMorselizer (import, the per-file literal rewrite in open(), and the test_input_file_name_projection test) because this PR's restructure of open() overlapped the region apache#23638 later touched. Restored all three; the test passes again. 3. metrics: the row_filter_skipped_fully_matched doc claimed to count row groups, but the counter increments once per suppression event (a run of consecutive fully-matched RGs shares a single toggle). Doc now states the actual semantics. Verified: 158 datasource-parquet tests pass (with PARQUET_TEST_DATA), cargo doc clean, clippy clean on touched files.
…t_file_name handling lost in rebase Three fixes on top of the rebase onto latest main: 1. row_filter.rs carried a stale pre-apache#23638 copy of the column-resolution machinery (ParquetReadPlan, PushdownChecker, StructFieldAccess, build_filter_schema, etc.) that main has since moved into projection_read_plan.rs. Rebuilt the file on main's shared-module version, keeping only this PR's real additions (PrebuiltRowFilterCandidate, prebuild_row_filter_candidates, row_filter_from_prebuilt, and the RowFilterGenerator removal): 2194 → 1483 lines, -798 net. The stray "strinrg" typo lived in the deleted copy. 2. The rebase silently dropped main's input_file_name() handling in ParquetMorselizer (import, the per-file literal rewrite in open(), and the test_input_file_name_projection test) because this PR's restructure of open() overlapped the region apache#23638 later touched. Restored all three; the test passes again. 3. metrics: the row_filter_skipped_fully_matched doc claimed to count row groups, but the counter increments once per suppression event (a run of consecutive fully-matched RGs shares a single toggle). Doc now states the actual semantics. Verified: 158 datasource-parquet tests pass (with PARQUET_TEST_DATA), cargo doc clean, clippy clean on touched files.
…lter pushdown enabled (apache#23638) ## Which issue does this PR close? - Closesapache#23531. ## Rationale for this change `input_file_name()` is `FileSource` dependent like `file_row_index()`, and therefore shouldn't be pushed down into a filter. ## What changes are included in this PR? `PushdownChecker` now handles both UDFs consistently. If we keep adding this sort of metadata functions, we might want a better API to detect them, but for now I think this is a reasonable approach that isn't very invasive. ## Are these changes tested? Additional SLT test that verifies that both function behave correctly with pushdown either enabled or disabled. ## Are there any user-facing changes? None --------- Signed-off-by: Adam Gutglick <adamgsal@gmail.com>
Which issue does this PR close?
Rationale for this change
input_file_name()isFileSourcedependent likefile_row_index(), and therefore shouldn't be pushed down into a filter.What changes are included in this PR?
PushdownCheckernow handles both UDFs consistently.If we keep adding this sort of metadata functions, we might want a better API to detect them, but for now I think this is a reasonable approach that isn't very invasive.
Are these changes tested?
Additional SLT test that verifies that both function behave correctly with pushdown either enabled or disabled.
Are there any user-facing changes?
None