feat!: upgrade DataFusion to 55 - #8555
Conversation
There was a problem hiding this comment.
Code Review
This pull request upgrades the DataFusion dependency from version 53.1.0 to 54.0.0, which involves extensive refactoring to adapt to API changes such as the removal of explicit as_any() downcasting, updates to the Cast expression structure, and changes to PruningStatistics and partition_statistics signatures. It also introduces a frozen hash_v1 module to preserve compatibility with legacy count_hash states. The review identified critical compilation errors: a missing import for as_largestring_array in the new hash_v1.rs file, and incorrect iterations over the return value of hash_utils::create_hashes in union_distinct_on.rs since it now returns Result<()> instead of the hashes.
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.
|
orc and pg-catalog are our managed libraries, make sure it merges to upstream before merging this. |
|
I'm working on the pg-catalog upgrade for datafusion 54. will commit to this branch. |
|
Thanks — I see |
|
DCO is a non-blocking check, just set it passed. |
|
Thanks, DCO is green now. One remaining managed-dependency setup: |
|
datafusion-orc is now on 0.9 |
|
The pg-catalog 0.18 integration uncovered one remaining upstream compatibility gap during real PostgreSQL-protocol SQLness validation: OID aliases are represented as |
|
Updated locally to the managed |
|
Could you confirm the intended ownership for the DataFusion thin-fork pin itself? The existing dependency used |
|
The managed DataFusion 54 path is now available. I created |
|
datafusion 54.1 is out. |
ff94fb3 to
fd09114
Compare
fd09114 to
cf9188e
Compare
Address review feedback on PR GreptimeTeam#8555: extract duplicated coordinate x resolution signature construction, coordinate/resolution argument parsing, and per-row resolution conversion into shared helpers (signature_of_coordinates_and_resolution, extract_coordinate_resolution_args, resolution_at). Drop redundant alias statics (RESOLUTION_TYPES, DISTANCE_TYPES, POSITION_TYPES) in favor of INTEGER_TYPES. Behavior, signatures, error messages, and visibility are unchanged. Signed-off-by: discord9 <discord9@163.com>
|
Addressed: |
|
We can use this commit for arrow-pg and datafusion-pg-catalog: |
5f62a1b to
b1943eb
Compare
|
Marking this PR as draft for now. We will merge it after the 1.2 release. |
|
hold the merge for easier 1.2 backport plz |
|
This upgrade also fixes #9038 (projection pushdown into a recursive CTE prunes columns the recursive term still reads, causing |
1d8e1aa to
c488399
Compare
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
apache/datafusion@d5552342LIMITacross multiple partitions: apache/datafusion#23800GreptimeTeam/datafusion-postgres@3c77e6c3GreptimeTeam/sqlparser-rs@9e9019bbWhat's changed and what's your intention?
This PR upgrades GreptimeDB from the DataFusion 53.1 dependency line to DataFusion
55.0.0, Arrow59.2, and Parquet59.2. It rebases the work on307fe0a692848422f2eb0d1c51990160437ebd55; the GreptimeDB head is37830649eadce7d8028550c483fd45614913f42c.The upgrade adapts GreptimeDB planning and execution integrations to DataFusion 55 while retaining the behaviors that are material to GreptimeDB: distributed ordering via
EnforceSorting, custom physical plans and expression traversal, recursiveMergeScanfallback handling, global Top-K throughMergeSort, table-function planning, PostgreSQL compatibility, generated range-query validation,count_hashcompatibility, JSON field ordering, and topology-specific SQLness plan expectations.Dependency versions and pins
55.0.0; fork7ac19906c191e51433c0c3d8810d4fd47ab1529a59.2.0datafusion-orc0.10.0orc-rust0.9.0datafusion-pg-catalog/arrow-pg3c77e6c32b8dsqlparser0.62.0at9e9019bbAll 15 DataFusion entries in
[patch.crates-io]use the same immutable fork revision,7ac19906c191e51433c0c3d8810d4fd47ab1529a; the corresponding 35Cargo.locksource blocks use that same revision. The released ORC packages replace the temporary ORC git patches, anddatafusion-pg-catalogandarrow-pgremain pinned together at their DataFusion 55-compatible commit.Thin-fork history
The fork starts at official DataFusion
55.0.0commitd555234201. The six original retained commits remain for these purposes:LIMIT96625f97aad1dd7b9ad6da6529d542e47063d1e46922ddce3dec6af133dcount_hashstate compatibilityThe scalar-planner hook restoration
aab310aac36is retained for JSON2. Four additional fork commits are now pushed:72872aedb(lint-only timestamp conversion cleanup),a17899ec6(recursive-CTE schema metadata),85fc18b51(Date32 predicate widening), and7ac19906c(cross-timezone regression correction).Targeted repairs
JSON2 scalar-function planning
ExprPlannerhook GreptimeDB uses to supply JSON2 extraction types. Removing the obsolete GreptimeDB method would compile but makes expressions such asabs(j.metrics.value)fail during planning before analyzer rules can repair them.RawScalarExpr/plan_scalarextension point after scalar argument resolution. ModifiedOriginalarguments continue through later planners and normal function construction. GreptimeDB keeps its production JSON2 type injection; this does not introduce a test-only path or a new rewrite framework.Recursive CTE schema metadata
information_schema.tablesalso exposes a distinct DataFusion 55 physical-planning failure:Cannot project plan to expected schema: schema metadata differ. A literal seed and table-backed recursive branch may have different top-level metadata even though the declared recursive output takes the seed metadata; the query must continue to return depths0, 1, 2.a17899ec6makes the existingproject_plan_to_schemahelper constructProjectionExecwith the declared output schema metadata rather than reject only a top-level metadata difference. The exact-schema fast path remains. Column-count, data-type, nullability-narrowing, and field-metadata mismatches remain rejected. It neither strips scan metadata nor changes fix: re-scan stream-backed tables in recursive CTEs #9039's stream-reopening behavior.Date32 predicate widening
Date32 → Date64, so existingsimplify_expr.sltEXPLAIN cases could not eliminate those casts. Everyi32day value scaled by 86,400,000 fits ini64.85fc18b51allows onlyDate32 → Date64. Inverse-literal whole-day alignment and Date32-domain checks remain, andDate64 → Date32remains rejected. Coverage includes boundaries, ordered comparisons, nonaligned and out-of-range literals, and reverse-direction rejection.Cross-timezone equality regression
2024-02-01 00:00:01 +01and Los Angeles2024-01-31 16:00:01 -08as equal, although they differ by one hour. Because the view explicitly casts the input totimestampbefore assigning Brussels, its matching Los Angeles wall clock is15:00:01.7ac19906cchanges the positive literal to 15:00, retains the expected positive row, and preserves 16:00 as a no-match regression. It changes neither timezone implementation nor the positive result semantics.The lint-only commit
72872aedbremoves four redundanti128::from(timestamp_unit_scale(...))conversions: the helper already returnsi128, so strict clippy rejects these identity conversions. Timestamp arithmetic and guards are unchanged.EnforceSortingremains intentionally preserved: distributed plan-shape regression coverage depends on it to retain ordering behavior.Validation and PR state
Current head
37830649eadce7d8028550c483fd45614913f42c: locked Cargo metadata,cargo check --locked -p cmd --bin greptime --features vector_index, builds ofgreptimeandsqlness-runner, Rust formatting, and manifest formatting pass. The dependency update changes only 15 manifest pins and 35 lock-file sources, with no unrelated version changes. All 32 standalone/distributed SQLness executions pass, including the recursive CTE overinformation_schema.tablesreturning depths 0, 1, 2, all 11 JSON2 cases in each deployment, and four related controls per deployment. Existing SQLness expected files are unchanged.Fork revision
7ac19906c: all-target/all-feature strict clippy passes; schema-alignment and recursive-plan focused tests pass; the completedatafusion-expr-commoncrate passes (175 unit tests and 11 doctests).simplify_expr.sltanddatetime/timestamps.sltpass. No completed full extended-suite result is claimed.Earlier migration validation (
c488399dc82): 692 query tests passed / 2 skipped; 2519 affected-crate tests passed (one index-build synchronization test passed on its third attempt); seven-crate strict clippy and 30 standalone/distributed SQLness executions passed. These broader results predate the final rebase and pin update and are not a current-head full-suite result.Current-head CI: builds, Rust tests, Clippy, formatting, and dependency checks passed. Integration CI is not green: all five SQLness variants fail the
delta_temporalityPromQL regression with an array-type comparison error; four Fuzz jobs also failed and are being diagnosed. These failures remain unresolved; the 32 targeted local SQLness passes above do not cover the failing temporality case.Cross-version compatibility CI is pending; a normal push does not trigger that workflow. This PR remains a Draft. Transitioning it to Ready for review requires separate approval; no compatibility or current CI-green result is claimed.
PR Checklist