alexandrefimov opened a new pull request, #24861: URL: https://github.com/apache/datafusion/pull/24861
## Which issue does this PR close? None. Following #20750 and #20876, which carried the previous substrait bumps together with the code changes, this supersedes the two dependabot bumps: #24255 (0.64.1, open since 11 August) and #24842 (0.65.0), both red with no diagnosis. ## Rationale for this change `datafusion-substrait` does not compile against `substrait` 0.64.1 or 0.65.0, so the dependency is stuck at 0.63.0. The two bumps fail for different reasons. 0.64.1 still carries spec 0.87.0 and costs six errors, three of which are not about Substrait at all — `pbjson_types::Any` moved when the crate was restructured. 0.65.0 is the real step: it moves the embedded spec from 0.87.0 to 0.102.0 in one release (substrait-io/substrait-rs#531), so fifteen spec releases of removals arrive together, and `cargo check -p datafusion-substrait` reports 65 errors. ## What changes are included in this PR? Grouped by what the spec did rather than by file: - **Fetch.** `offset_mode` and `count_mode` are gone; `offset_expr` and `count_expr` are the only form. The `count == -1` sentinel disappears with them — an unset count already means ALL. - **Window bounds.** `Bound` and its `Preceding` / `Following` are boxed, and a bound now carries `offset_expr` beside the deprecated `offset`. The producer writes both, which the field docs allow, except for a zero distance: `offset` cannot represent zero and the spec asks producers not to write a zero `offset_expr`. The consumer reads `offset_expr` when it is set, as the spec requires, and supports only an int64 literal there: a DataFusion frame bound holds a `ScalarValue`, not an expression. - **Interval literals.** The `precision_mode` oneof became a plain `precision` field, and the deprecated `microseconds` form is gone. The producer already emitted `Precision(9)` and `Precision(3)`, so the arms map across unchanged. - **Grouping.** A grouping set references relation-level expressions by index. `parse_flat_grouping_exprs` already filled `expression_references` next to the deprecated field, so the producer only stops writing the latter; `from_substrait_grouping` loses the branch that preferred it, and with it the `consumer` and `input_schema` parameters and its `async`. - **Virtual tables, function arguments, types.** `values`, the deprecated `args`, `Kind::Timestamp` / `TimestampTz` / `Time`, `LiteralType::Timestamp` and the enum expression form are all removed; `UserDefinedTypeReference` became `UserDefined`. - **New surface.** `Plan` requires `execution_behavior`, `RelCommon` requires `rel_anchor`, `RelType` gained `LateralJoin` and `TopN`, and `RexType` gained `ExecutionContextVariable`. `OuterReference.steps_out` moved into a oneof next to a relation reference. The consumer branches that read the removed fields are deleted rather than kept. Those fields are `reserved` in spec 0.102 — `read_rel::VirtualTable` carries `reserved 1; reserved "values";` — so they cannot appear on a decoded plan and there is nothing left to stay compatible with. The checked-in fixtures had to move with them: 24 files used `groupings[].groupingExpressions` and five TPC-H plans used `fetch.count`, so a `LIMIT` read back as `fetch=None`. Both groups are rewritten, with the grouping keys moved to the relation and referenced by index. `pbjson-types` moves to 0.9.0 to match the version `substrait` uses; the pin exists to track it. `Plan.execution_behavior` is documented as required, so it is set, with `VARIABLE_EVALUATION_MODE_UNSPECIFIED`: DataFusion emits no execution context variables, and `PER_PLAN` would claim semantics it does not implement. Two hand-written fixtures, `test_plans/multiple_joins.json` and `test_plans/multilayer_aggregate.substrait.json`, are reformatted rather than edited in place: they mix printer styles within one file, so no serializer reproduces them and their diffs are noisier than the rest. ## What is the testing strategy for this PR? `cargo test -p datafusion-substrait` passes 207 integration and 51 unit tests. `cargo clippy -p datafusion-substrait --all-targets -- -D warnings` and `cargo fmt --all -- --check` are clean, as is clippy for `datafusion-sqllogictest` with the `substrait` feature. The fixture rewrite was checked for meaning, not just for parsing: for every edited file the number of grouping keys per aggregate is the same before and after, and the referenced indices resolve to the expressions that were previously inline. `cargo test --test sqllogictests --features substrait -- --substrait-round-trip` reports 120 failures, the same count as `main`, with an identical distribution across failure classes — so the migration moves neither producer nor consumer behaviour on the plans DataFusion builds itself. ## Are there any user-facing changes? A consumer no longer reads plans that use the removed fields: grouping sets with inline expressions, virtual tables with `values`, function calls with `args`, interval literals in the `microseconds` form, the old `Timestamp` / `TimestampTz` / `Time` types and literals, and enum expressions. This is not a choice the crate can make differently — the fields are reserved in the spec version the crate now targets. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
