Skip to content

fix: prune statistics only through casts that keep order - #26982

Merged
jayzhan211 merged 9 commits into
apache:mainfrom
jayzhan211:fix-pruning-cast-order-rule
Nov 7, 2026
Merged

jayzhan211 merged 9 commits into
apache:mainfrom
jayzhan211:fix-pruning-cast-order-rule

Conversation

@jayzhan211

@jayzhan211 jayzhan211 commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

Parquet statistics pruning can drop row groups that contain matching rows when the predicate casts the column. With i in (-1, 0, 1, 255, 256) and naive timestamps ts at 00:00, 01:00, 02:30, 02:45 and 03:00 on 2024-03-10, all in one row group:

Query In-memory table Parquet
SELECT count(*) FROM t WHERE CAST(i AS BOOLEAN) <= false 1 0
SELECT count(*) FROM t WHERE CAST(i AS BYTEA) < X'05' 3 0
SELECT count(*) FROM t WHERE ts > TIMESTAMPTZ '2024-03-10 07:15:00+00:00' with time_zone = 'America/New_York' 2 0

Pruning casts the min/max statistics and compares them with the literal, which is only correct when the cast keeps the order of the values. Pruning decides that with its own check: any cast is allowed unless exactly one side is a string. So int → boolean, int → binary (little-endian bytes) and named-timezone timestamp → date all pass, although none of them keep order.

The last row is new with arrow 60.1.0 (#26867). A naive timestamp compared with a named timezone is read as a local time, and arrow now moves a local time that a transition skips forward instead of rejecting it: 02:30, which New York skipped, becomes 07:30 UTC, after 03:00 (07:00 UTC). When such a value is the minimum of a row group, the cast bounds are inverted and the query fails with Internal error: Interval's lower bound ... is greater than the upper bound.

Sort elimination already asks the same question through is_order_preserving_cast. The two checks disagree in both directions: pruning accepts the casts above, and is_order_preserving_cast rejects casts that do keep order and that pruning relies on today (integer ↔ date/timestamp/duration, string and binary re-encodings, date → timestamp with a timezone). Arrow 60.1.0 also made one of its own entries wrong: a named-timezone timestamp → Date64 now takes the local date, as Date32 does, so a sort on that cast can be removed although the output is out of order.

What changes are included in this PR?

  1. is_order_preserving_cast (now public in datafusion_physical_expr::expressions) accepts the casts that keep order and that only pruning knew about:
    • string ↔ string and binary ↔ binary re-encodings (including FixedSizeBinary)
    • numbers ↔ temporal types stored as a count (date, timestamp, time, duration)
    • Date32 → timestamp with any timezone (a skipped midnight moves forward by at most a day, so it never passes the next one), and Date64 → timestamp with a fixed offset
    • Time64 → Time32 and Time64(µs) → Time64(ns), which Arrow 60.1.0 checks for overflow
  2. It rejects named-timezone timestamp → Date64, as it already did for Date32.
  3. Pruning (verify_support_type_for_prune) asks is_order_preserving_cast instead of its own string check.
  4. A test that checks is_order_preserving_cast against the strict cast that CastExpr runs (ColumnarValue::cast_to): every pair it accepts must keep the order of sample values that convert. The samples include values around midnight, a DST gap and overlap, a timezone that moved the date backwards, and a day that Pacific/Apia skipped. A positive control asserts the samples expose the casts the rule rejects.

Pruning keeps every other cast it used before (compared over all castable pairs of the numeric, string, binary and temporal types) and stops using:

Cast Why
integer/float → boolean -1 and 1 are true, 0 is false
integer → binary little-endian bytes do not sort like the integers
named-timezone timestamp → Date32/Date64 a transition can move the local date backwards
naive timestamp or Date64 → named-timezone timestamp arrow moves a skipped local time forward, past the local times just after the gap
casts from or to Null, to list/struct, interval casts the rule has no entry for them

What is the testing strategy for this PR?

  • test_order_preserving_cast_matches_cast_kernel (physical-expr): checks the rule against the strict cast, with a positive control. Making named-timezone timestamp → date return true fails it with 562129259 -> 1987-10-25, but 562129260 -> 1987-10-24.
  • row_group_predicate_cast_must_preserve_order (pruning): casts that must not prune and casts that must keep pruning, for both CAST and TRY_CAST.
  • push_down_filter_parquet.slt: the three queries above against an in-memory table and Parquet, plus an EXPLAIN showing that the same timestamp comparison under a fixed-offset session timezone still prunes.
  • cast.slt: the sort on arrow_cast(i, 'Timestamp(Second, None)') is removed when the input is ordered by i (EXPLAIN and the row order), and the sort on a named-timezone timestamp cast to Date64 is kept.
  • The new pruning unit test cases and Parquet queries fail with the old pruning check. The Date64 sort case and the New York Parquet query return wrong results on main.
  • bloom_filter_pruning_error_retains_row_group_without_match relied on the int → FixedSizeBinary cast failing at evaluation. Pruning now rejects that cast up front, so the test compares c1 with CAST('abc' AS Int32) instead, which still fails at evaluation.

Are there any user-facing changes?

  • Queries that filter Parquet through the casts in the table above return the correct rows; those predicates no longer prune row groups. With a named session timezone, comparing a naive timestamp column with a timezone-aware timestamp no longer prunes; with a fixed offset it still does.
  • Sort elimination treats the newly accepted casts as order-preserving under CAST (not TRY_CAST), so a sort on, for example, CAST(int_col AS TIMESTAMP) over input ordered by int_col can be removed. A sort on a named-timezone timestamp cast to Date64 is no longer removed.
  • New public function datafusion_physical_expr::expressions::is_order_preserving_cast (addition only).

@github-actions github-actions Bot added physical-expr Changes to the physical-expr crates sqllogictest SQL Logic Tests (.slt) labels Nov 1, 2026
// then we can compare them.
// PruningPredicate does not support casting of strings to numbers and such.
if from_type.is_string() == to_type.is_string() {
if phys_expr::is_order_preserving_cast(from_type, to_type) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Call is_order_preserving_cast

@kosiew kosiew left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@jayzhan211,

Thanks for the fix. Reusing the order-preserving cast rule for statistics pruning is the right approach. It prevents unsafe min/max reasoning while retaining valid pruning and sort-elimination opportunities.

The architectural boundary is sound: pruning must reject non-monotonic casts before deriving bounds from statistics, while successful-value ordering, strict/lossless ordering, and NULL-on-failure handling remain separate contracts. Consolidating this policy avoids conflicting decisions between pruning and physical expression ordering analysis.

I'm requesting changes because the new restriction invalidates an existing Bloom-filter error-path fixture, causing the required Rust test workflow to fail. Please restore coverage of genuine evaluation failures without relaxing the cast restriction. The additional sampling and SQL ordering suggestions are non-blocking.

// then we can compare them.
// PruningPredicate does not support casting of strings to numbers and such.
if from_type.is_string() == to_type.is_string() {
if phys_expr::is_order_preserving_cast(from_type, to_type) {

@kosiew kosiew Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The new rule correctly rejects CAST(Int32 AS FixedSizeBinary(4)), but bloom_filter_pruning_error_retains_row_group_without_match relies on that cast to trigger an evaluation error, so CI now fails at row_group_filter.rs:1686. Please replace the fixture with a genuine evaluation failure while preserving row-group retention, an incremented error counter, and unchanged Bloom matched/pruned counters. Do not relax the cast restriction. Failed CI job.

// A panic is an unchecked overflow in a debug build, which
// wraps in a release build, so it is not evidence of order
// either. These pairs are listed separately in the output.
Some(Err(e)) => println!("{source} -> {target}: {e}"),

@kosiew kosiew Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The test prints Date64 -> Timestamp(ns): cast kernel panicked but still passes, skipping successful-value ordering comparisons for that pair. Could we use DataFusion's strict ColumnarValue::cast_to path, skip conversion errors, and compare successful outputs? The existing cast_date64_array_to_timestamp_overflow test already covers overflow rejection, so no duplicate overflow test is needed.

# An integer read as a count of seconds keeps its order, so the outer sort is
# removed.
query TT
EXPLAIN SELECT arrow_cast(i, 'Timestamp(Second, None)') AS ts

@kosiew kosiew Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The EXPLAIN verifies that the outer sort disappears, but does not check the resulting row order. Could you also execute the query with query P (required for timestamp columns by strict_column_validator, without rowsort or valuesort) and assert 1970-01-01T00:00:01, 1970-01-01T00:00:02, and 1970-01-01T00:00:03 in that order? Please retain the EXPLAIN to cover both optimization and SQL-visible ordering.

@jayzhan211
jayzhan211 force-pushed the fix-pruning-cast-order-rule branch from 07410bb to 87c2447 Compare November 3, 2026 12:41
@jayzhan211

Copy link
Copy Markdown
Contributor Author

@kosiew thanks for the review. Rebasing onto arrow 60.1.0 (#26867) made two of the rule's timezone entries wrong, so the last commit changes the rule itself. A naive timestamp read in a named timezone no longer counts as order-preserving, because arrow now moves skipped local times forward instead of rejecting them. A named-timezone timestamp cast to Date64 is now rejected the same way as Date32. The description has the queries that return wrong results on main. Could you take another look at that commit?

@kosiew kosiew left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@jayzhan211, thanks for the follow-up changes. The revised Bloom-filter fixture now exercises a genuine evaluation failure while preserving conservative row-group retention and the associated metrics. The strict cast sampling and order-sensitive SQL regression also address my earlier suggestions. I found no new issues in the follow-up.

Using is_order_preserving_cast as the shared authority for pruning and sort elimination is the right architectural boundary. Both optimizations depend on monotonic conversions, while successful casting alone cannot justify transforming min/max statistics or preserving sort order. The Arrow 60.1.0 timezone corrections and regression coverage support that distinction.

My previous blocking concern is resolved, and the relevant tests pass.
Approving.

@jayzhan211
jayzhan211 added this pull request to the merge queue Nov 7, 2026
@jayzhan211

Copy link
Copy Markdown
Contributor Author

Thanks @kosiew !

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 7, 2026
@github-actions github-actions Bot added the datasource Changes to the datasource crate label Nov 7, 2026
@jayzhan211
jayzhan211 added this pull request to the merge queue Nov 7, 2026
Merged via the queue into apache:main with commit 4bf32cb Nov 7, 2026
44 of 45 checks passed
@jayzhan211
jayzhan211 deleted the fix-pruning-cast-order-rule branch November 7, 2026 04:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

datasource Changes to the datasource crate physical-expr Changes to the physical-expr crates sqllogictest SQL Logic Tests (.slt) v57.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Parquet pruning drops rows for CAST(int AS BOOLEAN) and CAST(int AS BYTEA) predicates

2 participants