Skip to content

fix: rewrite ordered timestamp widening predicates - #23

Merged
WenyXu merged 1 commit into
GreptimeTeam:greptimedb-53.1.0-function-signature-exec-errorfrom
discord9:fix/timestamp-widening-ordered-preimage
Jul 29, 2026
Merged

fix: rewrite ordered timestamp widening predicates#23
WenyXu merged 1 commit into
GreptimeTeam:greptimedb-53.1.0-function-signature-exec-errorfrom
discord9:fix/timestamp-widening-ordered-preimage

Conversation

@discord9

Copy link
Copy Markdown

Which issue does this PR close?

  • N/A — issues are disabled in this fork.

Rationale for this change

GreptimeDB metric timestamps use millisecond precision while now() returns a nanosecond timestamp. After mixed-timestamp coercion began preserving the finer precision, predicates such as:

CAST(ts_ms AS Timestamp(ns)) >= TimestampNanosecond(1784895098272960878)

could no longer be unwrapped when the target literal was not aligned to the source timestamp unit. The residual cast prevents native timestamp predicate pushdown.

Although a non-aligned target literal has no singleton value preimage, ordered comparisons have exact source-domain bounds. For example, the predicate above is equivalent under the fork's existing timestamp-widening overflow policy to:

ts_ms >= TimestampMillisecond(1784895098273)

What changes are included in this PR?

  • Add an ordered timestamp-widening fallback after the existing exact cast-preimage path.
  • Compute source bounds with Euclidean floor/ceil arithmetic using i128:
    • >= L and < L use ceil(L / q).
    • > L and <= L use floor(L / q).
  • Support all timestamp precision-widening unit pairs with matching timezones.
  • Keep non-aligned equality, distinctness, and IN predicates unchanged.
  • Cover logical and physical rewrites, CAST/TRY_CAST, literal-left operator swaps, negative timestamps, and i64 boundaries.

This change deliberately follows the fork's existing timestamp-widening overflow policy. It does not claim full-domain equivalence for extreme source values whose regular CAST would overflow or whose TRY_CAST would produce NULL.

Are these changes tested?

Yes:

cargo test -p datafusion-expr-common test_cast_predicate_preimage_timestamp_widening
cargo test -p datafusion-optimizer test_cast_preimage_timestamp_widening
cargo test -p datafusion-physical-expr timestamp_widening
cargo fmt --all -- --check
cargo clippy -p datafusion-expr-common -p datafusion-optimizer -p datafusion-physical-expr --all-targets -- -D warnings
git diff --check

Are there any user-facing changes?

Yes. Ordered comparisons over timestamp precision-widening casts can now remove the column-side cast for non-aligned literals, enabling predicate pushdown while preserving the comparison boundary within the existing overflow policy. There is no SQL syntax or public API change.

Signed-off-by: discord9 <discord9@163.com>
@WenyXu

WenyXu commented Jul 29, 2026

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f2a0b22e49

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread datafusion/expr-common/src/casts.rs
Comment thread datafusion/expr-common/src/casts.rs
@WenyXu
WenyXu merged commit 6d6ae9a into GreptimeTeam:greptimedb-53.1.0-function-signature-exec-error Jul 29, 2026
discord9 added a commit that referenced this pull request Aug 3, 2026
Ports GreptimeTeam/datafusion PR #23
(source commit f2a0b22) onto the 54.1
thin fork.

Ordered comparisons (>=, <, >, <=) over a coarse-to-fine timestamp
widening cast now rewrite to a source-domain bound: >= and < use ceil,
> and <= use floor, computed with i128 div_euclid/rem_euclid for both
aligned and non-aligned target literals (aligned values collapse to the
common bound). The rewrite is limited to coarse->fine timestamp units
with identical timezone metadata and a target-typed non-NULL literal;
it does not extend to equality/IN, timezone changes, or other cast
families.

Overflow policy: the ordered source bound is equivalent only for source
values whose widening is representable. In line with the accepted DF53
fork PR #23 policy, this fork also applies the rewrite outside that
domain, where regular CAST can error and TRY_CAST can return NULL. No
full-domain equivalence is claimed.

Preserved protections: equality/IN, typed NULL, timezone mismatch,
dictionary, and non-default physical cast options / target field
metadata all retain the cast.

Signed-off-by: discord9 <discord9@163.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants