Skip to content

fix(ir): distinguish SQL current time from PromQL evaluation time - #275

Merged
zzylol merged 4 commits into
mainfrom
issue-184
Aug 25, 2026
Merged

zzylol merged 4 commits into
mainfrom
issue-184

Conversation

@milindsrivastava1997

@milindsrivastava1997 milindsrivastava1997 commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

Closes #184.

SQL NOW() and CURRENT_TIMESTAMP previously lowered to the generic FunctionCall catch-all, so the shared IR could not identify them as runtime-dependent clock expressions. This PR gives SQL current-time expressions an explicit, timestamp-typed representation while preserving PromQL's different time() contract.

The final IR has two distinct leaves:

  • EvalTimestamp: PromQL time(), represented as Unix seconds (Float64). It is the timestamp at which the expression is evaluated—not inherently the current wall-clock time. For an instant Prometheus HTTP query only, omitting the optional time request parameter makes the API use the server's current time.
  • CurrentTimestamp: SQL NOW() / CURRENT_TIMESTAMP, represented as DataType::Timestamp.

Keeping these leaves separate prevents SQL timestamp comparisons such as ts < NOW() from being incorrectly typed as comparisons against a Float64. QueryTimestamp was renamed to the more precise EvalTimestamp throughout the IR, frontends, traversals, CSE hashing, DAG export, devtools, tests, and design documentation.

The codebase uses one recursive QueryExpr<C> enum for relational operators and scalar subexpressions, so both leaves can appear directly inside predicates and projections without introducing a separate scalar IR.

Before / after

SQL query:

SELECT * FROM metrics WHERE ts < NOW()

Before:

Compare {
    left: Column(0),
    op: Lt,
    right: FunctionCall {
        name: "now",
        args: [],
    },
}

After:

Compare {
    left: Column(0),
    op: Lt,
    right: CurrentTimestamp, // DataType::Timestamp
}

PromQL remains semantically distinct:

time()
EvalTimestamp // Unix seconds as DataType::Float64

Implementation

  • Lower zero-argument, case-insensitive SQL now and current_timestamp to CurrentTimestamp.
  • Add timestamp-aware schema inference for CurrentTimestamp.
  • Rename the PromQL clock leaf from QueryTimestamp to EvalTimestamp.
  • Thread both leaf variants through column resolution, binding, canonicalization, CSE, DAG export, explanation/replacement traversal, and variant coverage.
  • Clarify evaluation-timestamp semantics in the pre-ASAP IR documentation.

Bare FROM-less SELECT NOW() still encounters the pre-existing EmptyRelation lowering gap and remains outside this PR. SELECT CURRENT_TIMESTAMP FROM <table> and predicate forms are covered.

Test plan

  • DataFusion SQL: NOW() in a timestamp predicate lowers to CurrentTimestamp.
  • ClickHouse SQL: now() in a timestamp predicate lowers to CurrentTimestamp.
  • SQL projection: CURRENT_TIMESTAMP lowers to a Timestamp-typed leaf.
  • Cross-frontend integration test: PromQL time() lowers to EvalTimestamp / Float64, while SQL CURRENT_TIMESTAMP lowers to CurrentTimestamp / Timestamp.
  • cargo check --workspace.
  • cargo test -p asap-types -p asap-frontend-promql -p asap-frontend-sql.

SQL's NOW()/CURRENT_TIMESTAMP fell into the generic FunctionCall
catch-all, giving the IR no signal that a predicate using it is
evaluation-time-dependent. Map zero-arg now()/current_timestamp to
the same QueryTimestamp leaf PromQL's time() already uses.

Exposed two latent gaps in code that assumed every scalar-position
QueryExpr was one of a fixed list of variants (never hit before
since PromQL's time() never appears inside a scan predicate):
columns_referenced() and resolve_expr() both lacked a QueryTimestamp
arm.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@zzylol

zzylol commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

If promql doesn't have timestamp set, it should be NOW.
SQL may have implicit Eval Time
change QueryTimestamp to EvalTimestamp.

@zzylol

zzylol commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

TODO: add a test for differentiating these two timestamps for two languages.

@zzylol

zzylol commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Addressed both follow-up comments in 93ff568.

  • Renamed QueryTimestamp to EvalTimestamp across the IR, frontends, traversals, DAG export, devtools, tests, and design docs. Its documentation now states that PromQL uses the request evaluation time and defaults to the current time when one is not supplied.
  • Kept SQL NOW() / CURRENT_TIMESTAMP as the distinct CurrentTimestamp leaf because SQL exposes a timestamp value, while PromQL time() exposes Unix seconds as Float64.
  • Added a cross-frontend integration test (frontend_timestamps.rs) which lowers both real languages and verifies time()EvalTimestamp/Float64 versus CURRENT_TIMESTAMPCurrentTimestamp/Timestamp.

Validation: cargo check --workspace; the new integration test; and all asap-types, asap-frontend-promql, and asap-frontend-sql tests pass.

@zzylol zzylol changed the title fix(frontend-sql): map NOW()/CURRENT_TIMESTAMP to QueryTimestamp fix(ir): distinguish SQL current time from PromQL evaluation time Aug 25, 2026
@zzylol
zzylol merged commit 3aba6f8 into main Aug 25, 2026
4 checks passed
@milindsrivastava1997
milindsrivastava1997 deleted the issue-184 branch August 25, 2026 14:48
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.

Consider mapping SQL now()/CURRENT_TIMESTAMP (incl. ClickHouse now()) to EvalTime

2 participants