Skip to content

carnot: push the ClickHouse time window down in the timestamp column's own units - #119

Closed
ConstanzeTU wants to merge 1 commit into
fix/ae-protocol-export-pxexportfrom
fix/clickhouse-time-pushdown-units
Closed

ConstanzeTU wants to merge 1 commit into
fix/ae-protocol-export-pxexportfrom
fix/clickhouse-time-pushdown-units

Conversation

@ConstanzeTU

@ConstanzeTU ConstanzeTU commented Sep 16, 2026

Copy link
Copy Markdown

Problem

A PxL script that reads a ClickHouse-backed table with start_time=-30m fails:

Failed to execute ClickHouse batch query: DB::Exception: Timeout exceeded:
elapsed 90.29 seconds, maximum: 90

The time window was not being pushed down. ClickHouseSourceNode divided the
start/end bounds from nanoseconds to seconds before rendering them into the
WHERE clause. That is correct for a DateTime/DateTime64 column, which
ClickHouse compares against a plain integer as a unix-seconds timestamp — but
wrong for an integer timestamp column holding unix-epoch nanoseconds, a
convention several of our retention tables use. A seconds bound is below every
stored value, so the predicate was trivially true, no partition or primary-key
range was pruned, and the table was scanned whole, once per pagination page.

Measured against a 20M-row table with a UInt64 nanosecond event_time, from
the server's own query_log: 7.4M rows read per page across ~2,500 pages, and
the predicate matched all 20,000,600 rows where the correct one matches 600.

Fix

Resolve the timestamp column's type in ClickHouseSourceIR::ResolveType
before PruneOutputColumnsTo can drop the column from the projection — carry it
in the plan proto as timestamp_column_type, and let the exec node render the
bound in that column's own units:

  • INT64 column → nanoseconds, as stored
  • DateTime / DateTime64 → seconds, unchanged from today
  • unresolved → seconds, the previous behaviour

Also copies timestamp_column_ and its type in CopyFromNodeImpl, which
dropped them and left copied source nodes with no pushdown at all.

Verification

Same script, same cluster, same 20M-row seed, before and after:

predicate read_rows pages
before seconds bound 7,837,026 per page ~2,500
after nanosecond bound 1,000 1, in 10 ms

A probe over the nanosecond-column table went from never returning (killed at
400s) to 6.8s with the correct rows. A join of a DateTime64 table against a
UInt64-nanosecond table returns in 12.8s with each side pushed down in its own
units — the DateTime64 side still receives the seconds bound and reads 50 rows.

Three unit tests cover the three unit cases. Planner, plan and logical-planner
suites pass.

Rollout note

The planner runs inside the query broker, so it is the query broker that writes
timestamp_column_type into the plan. With an old query broker the field is
unset and the exec node keeps the seconds bound: PEM, Kelvin and query broker
must roll together
or the fix is inert.

@ConstanzeTU
ConstanzeTU force-pushed the fix/clickhouse-time-pushdown-units branch from 111d1c6 to 3243d5a Compare September 16, 2026 07:28
@ConstanzeTU
ConstanzeTU changed the base branch from main to feat/pixie-native-sbob September 16, 2026 07:28
@ConstanzeTU
ConstanzeTU force-pushed the fix/clickhouse-time-pushdown-units branch from 3243d5a to 1ee5a65 Compare September 16, 2026 07:58
@ConstanzeTU
ConstanzeTU changed the base branch from feat/pixie-native-sbob to fix/ae-protocol-export-pxexport September 16, 2026 07:58
The ClickHouse source node divided the start/end bounds from nanoseconds to
seconds before rendering them into the WHERE clause. That is right for a
DateTime/DateTime64 column, which ClickHouse compares against a plain integer as
a unix-seconds timestamp, but wrong for an integer timestamp column holding
unix-epoch NANOSECONDS, the convention several of our retention tables use.
Against those, a seconds bound is smaller than every stored value, so the
predicate was trivially true, no partition or primary-key range was pruned, and
the table was scanned whole once per pagination page -- surfacing as "Failed to
execute ClickHouse batch query: Timeout exceeded: elapsed 90.2s, maximum: 90".

Resolve the timestamp column's type in ClickHouseSourceIR::ResolveType, before
column pruning can drop the column from the projection, carry it in the plan
proto, and let the exec node render the bound as nanoseconds for an INT64 column
and as seconds for DateTime/DateTime64. An unresolved type keeps the DateTime
reading.

Also copy timestamp_column_ (and its type) in CopyFromNodeImpl, which dropped
them and left copied source nodes with no pushdown at all.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017S6W3BHCmX5kCwGps9vevw
@ConstanzeTU
ConstanzeTU force-pushed the fix/clickhouse-time-pushdown-units branch from 1ee5a65 to 1412500 Compare September 16, 2026 10:51
@ConstanzeTU

Copy link
Copy Markdown
Author

Closing — the connector already does time pushdown; the established mechanism is schema-side (dx_ord__* keep event_time as DateTime64 so the forwarded seconds filter windows the pull). No engine change needed.

@ConstanzeTU
ConstanzeTU deleted the fix/clickhouse-time-pushdown-units branch September 18, 2026 12:25
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.

1 participant