AST Filter Pushdown and timestamp_type Conversion Interaction#1
Open
dan13bauer wants to merge 3 commits into
Open
AST Filter Pushdown and timestamp_type Conversion Interaction#1dan13bauer wants to merge 3 commits into
timestamp_type Conversion Interaction#1dan13bauer wants to merge 3 commits into
Conversation
Demonstrates that cuDF's AST filter evaluates against raw Parquet values before timestamp_type conversion, causing cross-precision filters to silently return zero rows.
Cover all combinations of native precision (us/ms/ns), filter scalar precision (us/ms/ns), and timestamp_type (us/ms/ns/unset). Add per-claim verification section. Update FINDINGS.md with summary table.
Both readers produce identical results across all combinations, confirming the behavior is in the shared filter evaluation path. 72 total tests (36 x 2 readers), all pass.
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.
Summary
cuDF's Parquet reader evaluates AST filters against raw, unconverted column
values even when
timestamp_typerequests a precision conversion. The filter'sscalar type is validated against the output type (post-conversion), but the
comparison itself runs against the raw Parquet data (pre-conversion). This is
a consistent design choice within libcudf -- the filter operates at the storage
layer before type coercion -- but it creates a catch-22 for callers like Velox
that construct filter scalars based on the output type.
Mechanism
When
parquet_reader_options::timestamp_typeis set (e.g., toTIMESTAMP_MILLISECONDS) and the Parquet file stores timestamps at a differentnative precision (e.g., microseconds), cuDF:
A millisecond scalar like
1,773,536,400,000is compared against a rawmicrosecond value like
1,773,536,400,000,000-- off by 1000x -- so the rangecheck fails and no rows survive.
The same issue applies in both directions and to both readers:
timestamp_typeWhen no
timestamp_typeis set, the reader preserves native precision andfilters work correctly.
The Catch-22
A caller cannot work around this by constructing scalars at the Parquet native
precision either. cuDF validates scalar type_ids against the output column
type (after
timestamp_typeconversion). Native-precision scalars (e.g., us)on an ms output column throw
non-matching operand types. So:raw us data -- 0 rows.
non-matching operand types.There is no scalar precision that produces correct results when
timestamp_typediffers from the Parquet native precision.
Reproducing
The
timestamp_readerbinary in this directory exercises the issue directlyagainst libcudf with no external dependencies:
cmake -S . -B build -Dcudf_ROOT=../../build cmake --build build -j4 ./build/timestamp_readerThe test Parquet file (
timestamps_multi_precision.parquet) contains 5 rowswith identical instants stored at us, ms, and ns precision. The filter selects a
3-row window. Regenerate with
python generate_timestamp_test.pyif needed.Impact on Velox
Velox's
CudfHiveDataSourcesetstimestamp_type = TIMESTAMP_MILLISECONDS(via
CudfHiveConfig) because Velox represents timestamps at millisecondprecision internally. It then constructs AST filter scalars at millisecond
precision to match the declared output type. cuDF accepts this without error,
but the filter silently drops all rows when the Parquet file stores timestamps
at a different native precision (the common case -- most Parquet writers default
to microseconds).
This manifests as missing data in query results: tables filtered on timestamp
ranges return empty, while non-timestamp filters (on string or integer columns)
work correctly.
Mitigation
Because of the catch-22, AST filter pushdown cannot be used for timestamp
columns when
timestamp_typetriggers a precision conversion. The practicalmitigation in Velox is to skip
kTimestampRangefilters from the Parquetreader AST and redirect them to the post-read GPU filter path
(
remainingFilterevaluated bycudfExpressionEvaluator_).Non-timestamp filters (
kBytesRange,kBytesValuesfor string/integer columns)still push down to the Parquet reader and handle the bulk of row elimination.
The timestamp predicate runs on the already-filtered, smaller result set --
evaluated on GPU, not on CPU. See
timestamp-filter-pushdown-fix-plan.mdin theVelox tree for the full implementation plan.
The correct long-term fix would be in libcudf: making the filter layer aware of
the
timestamp_typecoercion so that either the filter literals or the columndata are converted to a common precision before evaluation.