Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the DataFusion dependency revision and enhances the distributed query planner to support pushing down cast and try-cast expressions. Specifically, it updates the commutativity analysis to evaluate Cast and TryCast expressions based on their inner expressions, and integrates the SimplifyExpressions optimizer rule into the pre-merge scan optimizer. Additionally, new unit and integration tests (using a nanosecond-precision table host_nano) have been added to verify that PromQL time predicates on non-millisecond time indexes are correctly simplified and pushed down as native timestamp range filters. No review comments were provided, so there is no feedback to address.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Code Review
This pull request updates the DataFusion dependency to a newer revision and enhances the query planner to support pushing down and simplifying cast expressions. Specifically, it adds the SimplifyExpressions optimizer rule to the pre-merge scan optimizer and updates the commutativity categorizer to recursively check Cast and TryCast expressions. Additionally, new unit tests and integration tests with nanosecond-precision timestamps have been added to verify that PromQL time predicates on non-millisecond time indexes are correctly simplified and pushed down as native timestamp range filters. I have no feedback to provide as there are no review comments.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Signed-off-by: discord9 <discord9@163.com>
Signed-off-by: discord9 <discord9@163.com>
32b0ea3 to
d478f24
Compare
Signed-off-by: discord9 <discord9@163.com>
Signed-off-by: discord9 <discord9@163.com>
Signed-off-by: discord9 <discord9@163.com>
* fix(query): push down PromQL cast filters Signed-off-by: discord9 <discord9@163.com> * test(query): cover cast preimage pushdown Signed-off-by: discord9 <discord9@163.com> * test(query): expand cast preimage coverage Signed-off-by: discord9 <discord9@163.com> * test(query): compare cast normalization rules Signed-off-by: discord9 <discord9@163.com> * test(query): update cast pushdown sqlness plans Signed-off-by: discord9 <discord9@163.com> --------- Signed-off-by: discord9 <discord9@163.com> (cherry picked from commit 4f5dccf)
* fix(query): push down PromQL cast filters (#8407) * fix(query): push down PromQL cast filters Signed-off-by: discord9 <discord9@163.com> * test(query): cover cast preimage pushdown Signed-off-by: discord9 <discord9@163.com> * test(query): expand cast preimage coverage Signed-off-by: discord9 <discord9@163.com> * test(query): compare cast normalization rules Signed-off-by: discord9 <discord9@163.com> * test(query): update cast pushdown sqlness plans Signed-off-by: discord9 <discord9@163.com> --------- Signed-off-by: discord9 <discord9@163.com> (cherry picked from commit 4f5dccf) * fix: update datafusion-pg-catalog (#8417) (cherry picked from commit 434e0c7) * fix: global limit for distributed inspect streams (#8412) * fix: global limit for distributed inspect streams Signed-off-by: jeremyhi <fengjiachun@gmail.com> * fix: honor tighter filter fetch under global limit Signed-off-by: jeremyhi <fengjiachun@gmail.com> * fix: restore distribution after global limit Signed-off-by: jeremyhi <fengjiachun@gmail.com> * fix: preserve inherited distribution for global limit Signed-off-by: jeremyhi <fengjiachun@gmail.com> * fix: propagate inherited hash partitioning Signed-off-by: jeremyhi <fengjiachun@gmail.com> --------- Signed-off-by: jeremyhi <fengjiachun@gmail.com> (cherry picked from commit 5bcc2e9) * feat: update dashboard to v0.13.7 (#8431) (cherry picked from commit ebfe4f6) * fix: preserve repartitioned file refs during GC (#8445) * fix: preserve repartitioned file refs during GC Signed-off-by: discord9 <discord9@163.com> * refactor: clarify file refs target region Signed-off-by: discord9 <discord9@163.com> --------- Signed-off-by: discord9 <discord9@163.com> (cherry picked from commit 149b687) * fix: repartition subset partition key joins (#8460) Signed-off-by: discord9 <discord9@163.com> (cherry picked from commit ff2fa71) * fix: honor exclusive end in log queries (#8495) * fix: exclude logs end boundary Signed-off-by: discord9 <discord9@163.com> * fix: normalize log query date bounds Signed-off-by: discord9 <discord9@163.com> --------- Signed-off-by: discord9 <discord9@163.com> (cherry picked from commit 99235e4) * feat: skip oversized compaction tasks (#8466) fix: skip oversized compaction tasks Signed-off-by: jeremyhi <fengjiachun@gmail.com> (cherry picked from commit 0fcf61c) * feat: use scan output bytes for read costing (#8276) * feat: track scan output bytes and use them for read costing Track the actual byte count of each record batch produced by `RegionScanExec` and use it in place of the aggregated per-plan `memory_usage` as the `table_scan` cost input. This avoids double counting bytes that flow through multiple operators. A named constant `REGION_SCAN_EXEC_NAME` exposes the plan node name so downstream metric parsers remain correct if the struct is renamed. Affected files: - `src/table/src/table/metrics.rs` -- add `output_bytes` counter - `src/table/src/table/scan.rs` -- record bytes, export name constant - `src/query/src/dist_plan/merge_scan.rs` -- consume scan bytes Signed-off-by: Lei, HUANG <mrsatangel@gmail.com> * refactor: remove misleading `mem_used` gauge from scan metrics The `mem_used` gauge was used with `add()` on every output batch, but a RegionScan does not hold memory across polls — each batch is yielded immediately to the upstream operator. The gauge semantics were incorrect (cumulative `add()` on a gauge) and the value duplicated `output_bytes` anyway. Affected files: - `src/table/src/table/metrics.rs` — drop `mem_used` field and methods - `src/table/src/table/scan.rs` — remove `record_mem_usage` call Signed-off-by: Lei, HUANG <mrsatangel@gmail.com> * fix: use stable plan names for scan byte metrics Expose `plan_name` alongside rendered plan text so scan byte extraction no longer depends on EXPLAIN formatting. Keep old serialized metrics compatible by defaulting missing `plan_name`. Affected files: - `src/common/recordbatch/src/adapter.rs` -- add `plan_name` to `PlanMetrics` and populate it from `ExecutionPlan::name` - `src/query/src/dist_plan/merge_scan.rs` -- match `REGION_SCAN_EXEC_NAME` through `plan_name` Signed-off-by: Lei, HUANG <mrsatangel@gmail.com> * fix: preserve merge scan CPU read cost Record read cost whenever stream metrics are available, default scan bytes to zero, and aggregate scan output bytes across region scan nodes. Files: `src/query/src/dist_plan/merge_scan.rs` Signed-off-by: Lei, HUANG <mrsatangel@gmail.com> * fix: correct doc comment in `StreamMetrics` Fix doc comment on `StreamMetrics::new()` to reference the correct struct name (`StreamMetrics`) instead of the old `MemoryUsageMetrics`. - `src/table/src/table/metrics.rs` — fix struct name in doc comment Signed-off-by: Lei, HUANG <mrsatangel@gmail.com> --------- Signed-off-by: Lei, HUANG <mrsatangel@gmail.com> (cherry picked from commit 4373c77) * fix: account logical record batch slice memory (#8480) * fix: account logical record batch slice memory Signed-off-by: jeremyhi <fengjiachun@gmail.com> * fix: account nested view payload memory Signed-off-by: jeremyhi <fengjiachun@gmail.com> * test: align scan memory expectation with logical size Signed-off-by: jeremyhi <fengjiachun@gmail.com> * refactor: name Arrow inline view limit Signed-off-by: jeremyhi <fengjiachun@gmail.com> * fix: account visible list child memory Signed-off-by: jeremyhi <fengjiachun@gmail.com> * fix: exclude null list child memory Signed-off-by: jeremyhi <fengjiachun@gmail.com> * fix: exclude null struct child memory Signed-off-by: jeremyhi <fengjiachun@gmail.com> * fix: account fixed-size list slices Signed-off-by: jeremyhi <fengjiachun@gmail.com> * fix: limit scan accounting to flat slices Signed-off-by: jeremyhi <fengjiachun@gmail.com> * docs: clarify nested view accounting scope Signed-off-by: jeremyhi <fengjiachun@gmail.com> * perf: reduce view memory accounting overhead Signed-off-by: jeremyhi <fengjiachun@gmail.com> * chore: fix benchmark license header Signed-off-by: jeremyhi <fengjiachun@gmail.com> * perf: optimize view slice accounting Signed-off-by: jeremyhi <fengjiachun@gmail.com> * test: cover sliced mixed-null view accounting Signed-off-by: jeremyhi <fengjiachun@gmail.com> * refactor: narrow slice accounting to arrow buffers Signed-off-by: jeremyhi <fengjiachun@gmail.com> * docs: note view accounting assumption Signed-off-by: jeremyhi <fengjiachun@gmail.com> --------- Signed-off-by: jeremyhi <fengjiachun@gmail.com> (cherry picked from commit 9cde31a) * fix: preserve distributed topk merge ordering (#8432) * fix: preserve distributed topk for scalar latest Signed-off-by: discord9 <discord9@163.com> * fix: preserve scalar latest ordering across merge scan Signed-off-by: discord9 <discord9@163.com> * fix: carry merge scan ordering from planner rewrite Signed-off-by: discord9 <discord9@163.com> * test: cover merge scan ordering metadata Signed-off-by: discord9 <discord9@163.com> * test: cover merge scan partition ordering gate Signed-off-by: discord9 <discord9@163.com> * fix: require adjacent merge sort for ordering metadata Signed-off-by: discord9 <discord9@163.com> * test: update ordering sqlness plans Signed-off-by: discord9 <discord9@163.com> * fix: preserve distributed topk merge ordering Signed-off-by: discord9 <discord9@163.com> * test: cover latest per series queries Signed-off-by: discord9 <discord9@163.com> * test: update distributed merge sort sqlness Signed-off-by: discord9 <discord9@163.com> * test: cover merge scan ordering partition gate Signed-off-by: discord9 <discord9@163.com> * test: cover merge scan over-partition ordering Signed-off-by: discord9 <discord9@163.com> * test: cover distributed latest with low parallelism Signed-off-by: discord9 <discord9@163.com> * fix: keep distributed merge sort opaque Signed-off-by: discord9 <discord9@163.com> * test: assert distributed scalar latest merge sort Signed-off-by: discord9 <discord9@163.com> * test: clarify merge scan ordering helper Signed-off-by: discord9 <discord9@163.com> * fix: complete merge sort exec delegation Signed-off-by: discord9 <discord9@163.com> * docs: explain merge sort limit pushdown Signed-off-by: discord9 <discord9@163.com> * test: cover merge sort optimizer opacity Signed-off-by: discord9 <discord9@163.com> * docs: explain merge sort optimizer hooks Signed-off-by: discord9 <discord9@163.com> * test: cover merge sort optimizer hooks Signed-off-by: discord9 <discord9@163.com> * fix: preserve merge sort child topk Signed-off-by: discord9 <discord9@163.com> * test: update order by topk plan Signed-off-by: discord9 <discord9@163.com> * fix: recognize merge sort global fetch Signed-off-by: discord9 <discord9@163.com> --------- Signed-off-by: discord9 <discord9@163.com> (cherry picked from commit b30765e) * fix: reject out-of-range PostgreSQL numeric UInt64 (#8517) Signed-off-by: discord9 <discord9@163.com> (cherry picked from commit d48f296) * fix(log-store): deduplicate Kafka WAL multipart records (#8514) * fix(log-store): deduplicate Kafka WAL multipart records Signed-off-by: WenyXu <wenymedia@gmail.com> * fix(log-store): preserve delayed Kafka WAL entry offsets Signed-off-by: WenyXu <wenymedia@gmail.com> * fix(log-store): reject conflicting Kafka WAL last records Signed-off-by: WenyXu <wenymedia@gmail.com> * fix(log-store): emit Kafka WAL entries on last record Signed-off-by: WenyXu <wenymedia@gmail.com> * fix(log-store): discard duplicate Kafka WAL first records Signed-off-by: WenyXu <wenymedia@gmail.com> --------- Signed-off-by: WenyXu <wenymedia@gmail.com> (cherry picked from commit 912db22) * fix: apply log query limit after expressions (#8496) Signed-off-by: discord9 <discord9@163.com> (cherry picked from commit 1122d1e) * fix(query): delegate merge sort cardinality effect (#8532) Signed-off-by: discord9 <discord9@163.com> (cherry picked from commit 887fdf7) * fix: upgrade datafusion fork (#8438) Signed-off-by: jeremyhi <fengjiachun@gmail.com> (cherry picked from commit f12a1da) * chore: bump version Signed-off-by: WenyXu <wenymedia@gmail.com> --------- Signed-off-by: discord9 <discord9@163.com> Signed-off-by: jeremyhi <fengjiachun@gmail.com> Signed-off-by: Lei, HUANG <mrsatangel@gmail.com> Signed-off-by: WenyXu <wenymedia@gmail.com> Co-authored-by: discord9 <discord9@163.com> Co-authored-by: Ning Sun <sunng@protonmail.com> Co-authored-by: jeremyhi <jiachun_feng@proton.me> Co-authored-by: sun <sunchang_long@163.com> Co-authored-by: Lei, HUANG <6406592+v0y4g3r@users.noreply.github.com>
* fix(query): push down PromQL cast filters Signed-off-by: discord9 <discord9@163.com> * test(query): cover cast preimage pushdown Signed-off-by: discord9 <discord9@163.com> * test(query): expand cast preimage coverage Signed-off-by: discord9 <discord9@163.com> * test(query): compare cast normalization rules Signed-off-by: discord9 <discord9@163.com> * test(query): update cast pushdown sqlness plans Signed-off-by: discord9 <discord9@163.com> --------- Signed-off-by: discord9 <discord9@163.com> Signed-off-by: onepizzateam <palakjha916@gmail.com>
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
What's changed and what's your intention?
This PR updates GreptimeDB's patched DataFusion revision to the fork commit that contains the cast predicate preimage fix, and wires the distributed planner so cast-based timestamp filters/projections can reach the remote datanode plan.
The original PromQL issue was a composite planner bug:
Timestamp(ms)for PromQL execution semantics.Expr::Cast/Expr::TryCastasUnimplemented, so cast projections could stop at the frontend side aboveMergeScan.MergeScanLogicalPlanintentionally hides itsremote_input, so any missing filter/projection rewrite after wrapping cannot be recovered by later optimizer passes.This PR fixes the path by:
fe8422ed266dc8a527c06699057c20875a465173.Cast/TryCastas commutative when their inner expression is commutative.SimplifyExpressionsafter the focused pre-MergeScanPushDownFilterpass, so cast-preimage can rewrite pushed scan filters such asCAST(ts_ns AS Timestamp(ms)) <= literalinto native timestamp bounds.DistPlannerAnalyzerdirectly, bypassing the globally registeredConstNormalizationRule, so the native timestamp rewrite is covered by the pre-MergeScanSimplifyExpressionspath.filter/cast_preimagethat is not PromQL-specific and covers timestamp downcast comparison/equality/literal-left/BETWEEN, timestamp widening exact/non-exact behavior, integer widening comparison/swapped comparison/BETWEEN/NOT BETWEEN/IN, plain/casted constants, andTRY_CASTpushdown.timestampprecision sqlness coverage for both instant selectors and range selectors overtimestamp(6)andtimestamp(9)time indexes.ConstNormalizationRuleand DataFusionSimplifyExpressionscase-by-case for overlapping and non-overlapping cast-normalization behavior.MergeScanpushdown changes plan shape.ConstNormalizationRule vs DataFusion cast-preimage overlap
GreptimeDB still keeps its existing
ConstNormalizationRule. The new DataFusion revision overlaps with part of it, but it is not a full replacement. The concrete behavior is:ConstNormalizationRuleSimplifyExpressions/ cast-preimageCAST(v AS BIGINT) >= 42v >= Int16(42)v >= Int16(42)42 <= CAST(v AS BIGINT)v >= Int16(42)v >= Int16(42)TRY_CAST(v AS BIGINT) >= 42v >= Int16(42)v >= Int16(42)TRY_CASTwidening can become a scan-local typed literal.CAST(v AS BIGINT) IN (1, 2)v IN ([Int16(1), Int16(2)])v = Int16(1) OR v = Int16(2)CAST(v AS BIGINT) BETWEEN 3 AND 5v BETWEEN Int16(3) AND Int16(5)v >= Int16(3) AND v <= Int16(5)BETWEENto binary comparisons.CAST(v AS BIGINT) NOT BETWEEN 3 AND 5v NOT BETWEEN Int16(3) AND Int16(5)v < Int16(3) OR v > Int16(5)v >= 42_i64v >= Int16(42)v >= Int64(42)v >= CAST(42_i8 AS BIGINT)v >= Int16(42)v >= Int64(42)CAST(ts_ns AS TIMESTAMP(3)) = ts_ms(5000)ts_ns >= 5000000000ns AND ts_ns < 5001000000nsCAST(ts_ms AS TIMESTAMP(9)) = ts_ns(5000000000)ts_ms = TimestampMillisecond(5000)The design implication is that the focused pre-
MergeScanpass can rely on DataFusion for cast-preimage rewrites afterPushDownFilter, but this PR should not removeConstNormalizationRule: it still covers Greptime-specific plain literal, casted constant, and string-pattern normalization behavior.Compatibility:
MergeScanpass; it does not add broad plan-shaping rules such as projection optimization or limit pushdown.Validation run locally:
PR Checklist
Please convert it to a draft if some of the following conditions are not met.