From 2dc13406fb19d7d4e64782237b32a5e0294a3b0a Mon Sep 17 00:00:00 2001 From: Aleksandr Efimov Date: Tue, 1 Sep 2026 22:25:37 +0300 Subject: [PATCH 1/2] chore(deps): bump substrait from 0.63.0 to 0.65.0 The 0.65.0 release of the substrait crate moves its embedded spec from 0.87.0 to 0.102.0, so fifteen spec releases of removals arrive at once. API changes adapted: - FetchRel: the offset/count oneofs are gone, the expression fields are the only form left - window function: Bound and its Preceding/Following are boxed, and a bound carries offset_expr next to the deprecated offset - interval literals: the precision_mode oneof became a plain precision field, and the deprecated microseconds form is gone - aggregates: a grouping set references relation-level expressions by index rather than embedding them - virtual tables: rows are expressions, values was removed - function calls: the deprecated args field was removed in favour of arguments - types and literals: Timestamp, TimestampTz and Time gave way to the precision-carrying variants, and UserDefinedTypeReference to UserDefined - OuterReference: steps_out moved into a oneof alongside a relation reference - Plan requires execution_behavior, RelCommon requires rel_anchor, and RelType gained LateralJoin and TopN - the enum expression form was removed The consumer paths that read the removed fields are deleted rather than kept: those fields are reserved in the spec, so they cannot appear on a decoded plan. The checked-in JSON fixtures that used the old grouping and fetch shapes are rewritten accordingly. The pbjson-types pin moves to 0.9.0 to match the version substrait uses. --- Cargo.lock | 231 +++++++------ datafusion/substrait/Cargo.toml | 4 +- .../consumer/expr/field_reference.rs | 14 +- .../src/logical_plan/consumer/expr/literal.rs | 62 +--- .../src/logical_plan/consumer/expr/mod.rs | 17 +- .../consumer/expr/window_function.rs | 51 ++- .../consumer/rel/aggregate_rel.rs | 56 ++-- .../logical_plan/consumer/rel/fetch_rel.rs | 25 +- .../src/logical_plan/consumer/rel/mod.rs | 2 + .../src/logical_plan/consumer/rel/read_rel.rs | 57 +--- .../consumer/substrait_consumer.rs | 12 +- .../src/logical_plan/consumer/types.rs | 49 +-- .../producer/expr/aggregate_function.rs | 2 - .../producer/expr/field_reference.rs | 6 +- .../src/logical_plan/producer/expr/literal.rs | 5 +- .../producer/expr/scalar_function.rs | 11 - .../producer/expr/window_function.rs | 71 ++-- .../src/logical_plan/producer/plan.rs | 5 +- .../producer/rel/aggregate_rel.rs | 13 +- .../logical_plan/producer/rel/fetch_rel.rs | 16 +- .../logical_plan/producer/rel/project_rel.rs | 2 + .../src/logical_plan/producer/rel/read_rel.rs | 6 +- .../src/logical_plan/producer/rel/sort_rel.rs | 21 +- .../src/logical_plan/producer/utils.rs | 2 - ...ntical_grouping_expressions.substrait.json | 25 +- ...ggregate_identical_measures.substrait.json | 3 +- .../aggregate_no_project.substrait.json | 21 +- ...aggregate_sorted_no_project.substrait.json | 21 +- .../test_plans/join_with_expression_key.json | 105 +++--- .../multilayer_aggregate.substrait.json | 71 ++-- .../testdata/test_plans/multiple_joins.json | 306 ++++++++---------- .../select_count_from_select_1.substrait.json | 1 - .../tpch_substrait_plans/query_01_plan.json | 41 +-- .../tpch_substrait_plans/query_02_plan.json | 6 +- .../tpch_substrait_plans/query_03_plan.json | 67 ++-- .../tpch_substrait_plans/query_04_plan.json | 21 +- .../tpch_substrait_plans/query_05_plan.json | 21 +- .../tpch_substrait_plans/query_07_plan.json | 61 ++-- .../tpch_substrait_plans/query_08_plan.json | 21 +- .../tpch_substrait_plans/query_09_plan.json | 41 +-- .../tpch_substrait_plans/query_10_plan.json | 147 +++++---- .../tpch_substrait_plans/query_11_plan.json | 21 +- .../tpch_substrait_plans/query_12_plan.json | 21 +- .../tpch_substrait_plans/query_13_plan.json | 42 +-- .../tpch_substrait_plans/query_16_plan.json | 61 ++-- .../tpch_substrait_plans/query_18_plan.json | 128 ++++---- .../tpch_substrait_plans/query_21_plan.json | 27 +- .../tpch_substrait_plans/query_22_plan.json | 21 +- datafusion/substrait/tests/utils.rs | 11 +- 49 files changed, 993 insertions(+), 1058 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3077074e862fa..2303e74e52301 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -226,7 +226,7 @@ dependencies = [ "bzip2", "crc", "flate2", - "indexmap 2.14.1", + "indexmap 2.14.0", "liblzma", "rand 0.9.4", "serde", @@ -356,7 +356,7 @@ dependencies = [ "arrow-select", "chrono", "half", - "indexmap 2.14.1", + "indexmap 2.14.0", "itoa", "lexical-core", "memchr", @@ -511,7 +511,7 @@ checksum = "82f6aeea286b8eb4dd3431a1be1b59d290ace00f5bfd8e2a159bc2a05e2c1667" dependencies = [ "proc-macro2", "quote", - "syn 3.0.4", + "syn 3.0.3", ] [[package]] @@ -1335,7 +1335,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn 3.0.4", + "syn 3.0.3", ] [[package]] @@ -1510,9 +1510,9 @@ checksum = "217698eaf96b4a3f0bc4f3662aaa55bdf913cd54d7204591faa790070c6d0853" [[package]] name = "crc32fast" -version = "1.5.1" +version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8498c871161e1742aaa9d52551b2d6ebdd4c3d45a3be423e3728f33b955be550" +checksum = "9481c1c90cbf2ac953f07c8d4a58aa3945c425b7185c9154d67a65e4230da511" dependencies = [ "cfg-if", ] @@ -1746,7 +1746,7 @@ dependencies = [ "futures", "glob", "half", - "indexmap 2.14.1", + "indexmap 2.14.0", "insta", "itertools 0.15.0", "liblzma", @@ -1897,7 +1897,7 @@ dependencies = [ "half", "hashbrown 0.17.1", "hex", - "indexmap 2.14.1", + "indexmap 2.14.0", "insta", "itertools 0.15.0", "libc", @@ -2173,7 +2173,7 @@ dependencies = [ "datafusion-proto-common", "datafusion-proto-models", "env_logger", - "indexmap 2.14.1", + "indexmap 2.14.0", "insta", "itertools 0.15.0", "recursive", @@ -2187,7 +2187,7 @@ version = "55.0.0" dependencies = [ "arrow", "datafusion-common", - "indexmap 2.14.1", + "indexmap 2.14.0", "insta", "itertools 0.15.0", "rstest", @@ -2367,7 +2367,7 @@ version = "55.0.0" dependencies = [ "datafusion-doc", "quote", - "syn 3.0.4", + "syn 3.0.3", ] [[package]] @@ -2388,7 +2388,7 @@ dependencies = [ "datafusion-physical-expr", "datafusion-sql", "env_logger", - "indexmap 2.14.1", + "indexmap 2.14.0", "insta", "itertools 0.15.0", "log", @@ -2413,7 +2413,7 @@ dependencies = [ "datafusion-proto-models", "half", "hashbrown 0.17.1", - "indexmap 2.14.1", + "indexmap 2.14.0", "insta", "itertools 0.15.0", "parking_lot", @@ -2448,7 +2448,7 @@ dependencies = [ "datafusion-expr-common", "datafusion-proto-models", "hashbrown 0.17.1", - "indexmap 2.14.1", + "indexmap 2.14.0", "itertools 0.15.0", "parking_lot", "pin-project", @@ -2506,7 +2506,7 @@ dependencies = [ "futures", "half", "hashbrown 0.17.1", - "indexmap 2.14.1", + "indexmap 2.14.0", "insta", "itertools 0.15.0", "log", @@ -2565,7 +2565,7 @@ dependencies = [ "arrow", "datafusion-common", "doc-comment", - "pbjson 0.9.0", + "pbjson", "prost", "serde", ] @@ -2576,7 +2576,7 @@ version = "55.0.0" dependencies = [ "datafusion-common", "datafusion-proto-common", - "pbjson 0.9.0", + "pbjson", "prost", "serde", ] @@ -2659,7 +2659,7 @@ dependencies = [ "datafusion-functions-nested", "datafusion-functions-window", "env_logger", - "indexmap 2.14.1", + "indexmap 2.14.0", "insta", "itertools 0.15.0", "log", @@ -3059,9 +3059,9 @@ dependencies = [ [[package]] name = "flate2" -version = "1.1.10" +version = "1.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6e634e2e0ebac1ee034020da1ca582e17ffe4e0f5e985823721e168928136dcb" +checksum = "843fba2746e448b37e26a819579957415c8cef339bf08564fe8b7ddbd959573c" dependencies = [ "crc32fast", "miniz_oxide", @@ -3208,7 +3208,7 @@ dependencies = [ name = "gen" version = "0.1.0" dependencies = [ - "pbjson-build 0.9.0", + "pbjson-build", "prost-build", ] @@ -3216,7 +3216,7 @@ dependencies = [ name = "gen-common" version = "0.1.0" dependencies = [ - "pbjson-build 0.9.0", + "pbjson-build", "prost-build", ] @@ -3302,7 +3302,7 @@ dependencies = [ "futures-core", "futures-sink", "http 1.4.0", - "indexmap 2.14.1", + "indexmap 2.14.0", "slab", "tokio", "tokio-util", @@ -3721,6 +3721,26 @@ dependencies = [ "icu_properties", ] +[[package]] +name = "include_dir" +version = "0.7.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "923d117408f1e49d914f1a379a309cffe4f18c05cf4e3d12e613a15fc81bd0dd" +dependencies = [ + "glob", + "include_dir_macros", +] + +[[package]] +name = "include_dir_macros" +version = "0.7.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7cab85a7ed0bd5f0e76d93846e0147172bed2e2d3f859bcc33a8d9699cad1a75" +dependencies = [ + "proc-macro2", + "quote", +] + [[package]] name = "indexmap" version = "1.9.3" @@ -3734,9 +3754,9 @@ dependencies = [ [[package]] name = "indexmap" -version = "2.14.1" +version = "2.14.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "07aa2048142242915a31d35844fb311e0e53fcca590c3a0a40dcf1b841fa09eb" +checksum = "d466e9454f08e4a911e14806c24e16fba1b4c121d1ea474396f396069cf949d9" dependencies = [ "equivalent", "hashbrown 0.17.1", @@ -4072,9 +4092,9 @@ dependencies = [ [[package]] name = "log" -version = "0.4.34" +version = "0.4.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f9f8bd3e56ce4dfc153cf470fffbfa98c7620958b312ca5c3a4b8d5181fd13c6" +checksum = "0ceec5bc11778974d1bcb055b18002eba7f4b3518b6a0081b3af5f21666da9ad" [[package]] name = "lru-slab" @@ -4150,9 +4170,9 @@ dependencies = [ [[package]] name = "miniz_oxide" -version = "0.9.1" +version = "0.8.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b63fbc4a50860e98e7b2aa7804ded1db5cbc3aff9193adaff57a6931bf7c4b4c" +checksum = "1fa76a2c86f704bdb222d66965fb3d63269ce38518b83cb0575fca855ebb6316" dependencies = [ "adler2", "simd-adler32", @@ -4562,16 +4582,6 @@ dependencies = [ "syn 2.0.119", ] -[[package]] -name = "pbjson" -version = "0.8.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "898bac3fa00d0ba57a4e8289837e965baa2dee8c3749f3b11d45a64b4223d9c3" -dependencies = [ - "base64 0.22.1", - "serde", -] - [[package]] name = "pbjson" version = "0.9.0" @@ -4582,18 +4592,6 @@ dependencies = [ "serde", ] -[[package]] -name = "pbjson-build" -version = "0.8.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "af22d08a625a2213a78dbb0ffa253318c5c79ce3133d32d296655a7bdfb02095" -dependencies = [ - "heck", - "itertools 0.14.0", - "prost", - "prost-types", -] - [[package]] name = "pbjson-build" version = "0.9.0" @@ -4608,14 +4606,14 @@ dependencies = [ [[package]] name = "pbjson-types" -version = "0.8.0" +version = "0.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8e748e28374f10a330ee3bb9f29b828c0ac79831a32bab65015ad9b661ead526" +checksum = "a14e2757d877c0f607a82ce1b8560e224370f159d66c5d52eb55ea187ef0350e" dependencies = [ "bytes", "chrono", - "pbjson 0.8.0", - "pbjson-build 0.8.0", + "pbjson", + "pbjson-build", "prost", "prost-build", "serde", @@ -4635,7 +4633,7 @@ checksum = "8701b58ea97060d5e5b155d383a69952a60943f0e6dfe30b04c287beb0b27455" dependencies = [ "fixedbitset", "hashbrown 0.15.5", - "indexmap 2.14.1", + "indexmap 2.14.0", "serde", ] @@ -4850,6 +4848,16 @@ dependencies = [ "syn 2.0.119", ] +[[package]] +name = "prettyplease" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2bfe0f4c752e450fc2faf62654f1c134747922825d5b04ca717b8874f41a40c0" +dependencies = [ + "proc-macro2", + "syn 3.0.3", +] + [[package]] name = "proc-macro-crate" version = "3.5.0" @@ -4889,7 +4897,7 @@ dependencies = [ "log", "multimap", "petgraph", - "prettyplease", + "prettyplease 0.2.37", "prost", "prost-types", "regex", @@ -5246,9 +5254,9 @@ checksum = "d6f6ff9a378485b298a5286656da665ba74413d36db0979633275d2e708145d4" [[package]] name = "regress" -version = "0.10.5" +version = "0.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2057b2325e68a893284d1538021ab90279adac1139957ca2a74426c6f118fb48" +checksum = "158a764437582235e3501f683b93a0a6f8d825d04a789dbe5ed30b8799b8908a" dependencies = [ "hashbrown 0.16.1", "memchr", @@ -5605,7 +5613,7 @@ checksum = "e7a5d71263a5a7d47b41f6b3f06ba276f10cc18b0931f1799f710578e2309348" dependencies = [ "proc-macro2", "quote", - "syn 3.0.4", + "syn 3.0.3", ] [[package]] @@ -5625,7 +5633,7 @@ version = "1.0.151" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c841b55ecdae098c80dcae9cf767f6f8a0c2cdb3416bbef72181df4d0fe73f14" dependencies = [ - "indexmap 2.14.1", + "indexmap 2.14.0", "itoa", "memchr", "serde", @@ -5688,7 +5696,7 @@ dependencies = [ "chrono", "hex", "indexmap 1.9.3", - "indexmap 2.14.1", + "indexmap 2.14.0", "schemars 0.9.0", "schemars 1.2.1", "serde_core", @@ -5715,7 +5723,7 @@ version = "0.9.34+deprecated" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6a8b1a1a2ebf674015cc02edccce75287f1a0130d394307b36743c2f5d504b47" dependencies = [ - "indexmap 2.14.1", + "indexmap 2.14.0", "itoa", "ryu", "serde", @@ -6035,28 +6043,55 @@ dependencies = [ [[package]] name = "substrait" -version = "0.63.0" +version = "0.65.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "47f793c23baf7aa21919e0c2253ac3a47a306ced93359f036629a2d959dfe284" +dependencies = [ + "indexmap 2.14.0", + "prost", + "semver", + "serde", + "serde_json", + "substrait-extensions", + "substrait-prost", + "toml", +] + +[[package]] +name = "substrait-extensions" +version = "0.102.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e620ff4d5c02fd6f7752931aa74b16a26af66a63022cc1ad412c77edbe0bab47" +checksum = "d33afcbcf85005f788199c2de7983caef24d858c6909391208f8aad2516db46d" dependencies = [ "heck", - "indexmap 2.14.1", - "pbjson 0.8.0", - "pbjson-build 0.8.0", + "include_dir", + "indexmap 2.14.0", + "prettyplease 0.3.0", + "regress", + "schemars 0.8.22", + "serde", + "serde_json", + "serde_yaml", + "syn 3.0.3", + "typify", + "walkdir", +] + +[[package]] +name = "substrait-prost" +version = "0.102.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ee6b74c488901400ff424a98b3505a105dd041de1aa9c355dad9f7f6115b4521" +dependencies = [ + "pbjson", + "pbjson-build", "pbjson-types", - "prettyplease", "prost", "prost-build", "prost-types", "protobuf-src", - "regress", - "schemars 0.8.22", - "semver", "serde", "serde_json", - "serde_yaml", - "syn 2.0.119", - "typify", "walkdir", ] @@ -6079,9 +6114,9 @@ dependencies = [ [[package]] name = "syn" -version = "3.0.4" +version = "3.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e6275cddf4610d1775e6d1fe9469b2e77d0f39fd98fb7450901b821e0c53649f" +checksum = "53e9bae58849f64dfa4f5d5ae372c8341f7305f82a3868709269343628b659a3" dependencies = [ "proc-macro2", "quote", @@ -6204,7 +6239,7 @@ checksum = "bc04cd3e1236dd4a98afca4569f2deb3f120e5422a4023be2cb683f8486292af" dependencies = [ "proc-macro2", "quote", - "syn 3.0.4", + "syn 3.0.3", ] [[package]] @@ -6387,7 +6422,7 @@ version = "1.1.4+spec-1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3aace63f4bbcdfc2c965b059de67119c89c4017a70d633be6c104910f67056f5" dependencies = [ - "indexmap 2.14.1", + "indexmap 2.14.0", "serde_core", "serde_spanned", "toml_datetime", @@ -6411,7 +6446,7 @@ version = "0.25.11+spec-1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0b59c4d22ed448339746c59b905d24568fcbb3ab65a500494f7b8c3e97739f2b" dependencies = [ - "indexmap 2.14.1", + "indexmap 2.14.0", "toml_datetime", "toml_parser", "winnow", @@ -6480,7 +6515,7 @@ checksum = "ebe5ef63511595f1344e2d5cfa636d973292adc0eec1f0ad45fae9f0851ab1d4" dependencies = [ "futures-core", "futures-util", - "indexmap 2.14.1", + "indexmap 2.14.0", "pin-project-lite", "slab", "sync_wrapper", @@ -6586,9 +6621,9 @@ checksum = "e421abadd41a4225275504ea4d6566923418b7f05506fbc9c0fe86ba7396114b" [[package]] name = "twox-hash" -version = "2.1.4" +version = "2.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5283634e518fe9e82c7b20520bb4bc209009fd16c82077c802f8111ecbb0117a" +checksum = "8464ec13c3691491391d9fce00f6416c9a48e46972f72d7865688be2080192c9" dependencies = [ "rand 0.10.1", ] @@ -6601,9 +6636,9 @@ checksum = "40ce102ab67701b8526c123c1bab5cbe42d7040ccfd0f64af1a385808d2f43de" [[package]] name = "typify" -version = "0.5.0" +version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e6d5bcc6f62eb1fa8aa4098f39b29f93dcb914e17158b76c50360911257aa629" +checksum = "8cdc2e612ea322c6e232d46a0b34607c8eb28978fd6060ecfb139f2a50db8d5f" dependencies = [ "typify-impl", "typify-macro", @@ -6611,9 +6646,9 @@ dependencies = [ [[package]] name = "typify-impl" -version = "0.5.0" +version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a1eb359f7ffa4f9ebe947fa11a1b2da054564502968db5f317b7e37693cb2240" +checksum = "691591f49550c0d371bc441d019c30ce241d4116aee7d68df7a9840d6c70bf8f" dependencies = [ "heck", "log", @@ -6631,9 +6666,9 @@ dependencies = [ [[package]] name = "typify-macro" -version = "0.5.0" +version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "911c32f3c8514b048c1b228361bebb5e6d73aeec01696e8cc0e82e2ffef8ab7a" +checksum = "d41aea893c49cf95661389207b8af0c6254ff48b2c3ae1e4f3704777dbdfcf03" dependencies = [ "proc-macro2", "quote", @@ -6781,9 +6816,9 @@ checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" [[package]] name = "uuid" -version = "1.26.0" +version = "1.24.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b5772d71c9be8a8a6ac2117d949c5b224c1b72241bb611d9a3012edcf8af7812" +checksum = "2cefc03fd367c0c6d4305de1b312cf00248c4114f4a0418ce6a6af769e3b0bd9" dependencies = [ "getrandom 0.4.2", "js-sys", @@ -6989,7 +7024,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bb0e353e6a2fbdc176932bbaab493762eb1255a7900fe0fea1a2f96c296cc909" dependencies = [ "anyhow", - "indexmap 2.14.1", + "indexmap 2.14.0", "wasm-encoder", "wasmparser", ] @@ -7015,7 +7050,7 @@ checksum = "47b807c72e1bac69382b3a6fb3dbe8ea4c0ed87ff5629b8685ae6b9a611028fe" dependencies = [ "bitflags", "hashbrown 0.15.5", - "indexmap 2.14.1", + "indexmap 2.14.0", "semver", ] @@ -7392,8 +7427,8 @@ checksum = "b7c566e0f4b284dd6561c786d9cb0142da491f46a9fbed79ea69cdad5db17f21" dependencies = [ "anyhow", "heck", - "indexmap 2.14.1", - "prettyplease", + "indexmap 2.14.0", + "prettyplease 0.2.37", "syn 2.0.119", "wasm-metadata", "wit-bindgen-core", @@ -7407,7 +7442,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0c0f9bfd77e6a48eccf51359e3ae77140a7f50b1e2ebfe62422d8afdaffab17a" dependencies = [ "anyhow", - "prettyplease", + "prettyplease 0.2.37", "proc-macro2", "quote", "syn 2.0.119", @@ -7423,7 +7458,7 @@ checksum = "9d66ea20e9553b30172b5e831994e35fbde2d165325bec84fc43dbf6f4eb9cb2" dependencies = [ "anyhow", "bitflags", - "indexmap 2.14.1", + "indexmap 2.14.0", "log", "serde", "serde_derive", @@ -7442,7 +7477,7 @@ checksum = "ecc8ac4bc1dc3381b7f59c34f00b67e18f910c2c0f50015669dde7def656a736" dependencies = [ "anyhow", "id-arena", - "indexmap 2.14.1", + "indexmap 2.14.0", "log", "semver", "serde", diff --git a/datafusion/substrait/Cargo.toml b/datafusion/substrait/Cargo.toml index a0f203cec8db6..62aa8bdf573ea 100644 --- a/datafusion/substrait/Cargo.toml +++ b/datafusion/substrait/Cargo.toml @@ -42,9 +42,9 @@ half = { workspace = true } itertools = { workspace = true } object_store = { workspace = true } # We need to match the version in substrait, so we don't use the workspace version here -pbjson-types = { version = "0.8.0" } +pbjson-types = { version = "0.9.0" } prost = { workspace = true } -substrait = { version = "0.63.0", features = ["serde"] } +substrait = { version = "0.65.0", features = ["serde"] } url = { workspace = true } tokio = { workspace = true, features = ["fs"] } diff --git a/datafusion/substrait/src/logical_plan/consumer/expr/field_reference.rs b/datafusion/substrait/src/logical_plan/consumer/expr/field_reference.rs index be084f360358a..993c9ca91e4b5 100644 --- a/datafusion/substrait/src/logical_plan/consumer/expr/field_reference.rs +++ b/datafusion/substrait/src/logical_plan/consumer/expr/field_reference.rs @@ -74,7 +74,19 @@ fn resolve_outer_reference( outer_ref: &substrait::proto::expression::field_reference::OuterReference, field_idx: usize, ) -> datafusion::common::Result { - let steps_out = outer_ref.steps_out as usize; + use substrait::proto::expression::field_reference::outer_reference::OuterReferenceType; + // `StepsOut` is deprecated in favour of `RelReference`, but a relation + // reference needs anchors that DataFusion does not assign. + #[expect(deprecated)] + let steps_out = match outer_ref.outer_reference_type { + Some(OuterReferenceType::StepsOut(steps_out)) => steps_out as usize, + Some(OuterReferenceType::RelReference(_)) => { + return not_impl_err!( + "OuterReference by relation reference is not supported" + ); + } + None => return substrait_err!("OuterReference without a reference type"), + }; let Some(outer_schema) = consumer.get_outer_schema(steps_out) else { return substrait_err!( "OuterReference with steps_out={steps_out} \ diff --git a/datafusion/substrait/src/logical_plan/consumer/expr/literal.rs b/datafusion/substrait/src/logical_plan/consumer/expr/literal.rs index d7d7a69581f05..3222aef521309 100644 --- a/datafusion/substrait/src/logical_plan/consumer/expr/literal.rs +++ b/datafusion/substrait/src/logical_plan/consumer/expr/literal.rs @@ -25,9 +25,7 @@ use crate::variation_const::{ INTERVAL_DAY_TIME_TYPE_REF, INTERVAL_MONTH_DAY_NANO_TYPE_NAME, INTERVAL_MONTH_DAY_NANO_TYPE_REF, INTERVAL_YEAR_MONTH_TYPE_REF, LARGE_CONTAINER_TYPE_VARIATION_REF, TIME_32_TYPE_VARIATION_REF, - TIME_64_TYPE_VARIATION_REF, TIMESTAMP_MICRO_TYPE_VARIATION_REF, - TIMESTAMP_MILLI_TYPE_VARIATION_REF, TIMESTAMP_NANO_TYPE_VARIATION_REF, - TIMESTAMP_SECOND_TYPE_VARIATION_REF, UNSIGNED_INTEGER_TYPE_VARIATION_REF, + TIME_64_TYPE_VARIATION_REF, UNSIGNED_INTEGER_TYPE_VARIATION_REF, VIEW_CONTAINER_TYPE_VARIATION_REF, }; use datafusion::arrow::array::{AsArray, MapArray, new_empty_array}; @@ -46,7 +44,6 @@ use substrait::proto::expression::Literal; use substrait::proto::expression::literal::user_defined::{TypeAnchorType, Val}; use substrait::proto::expression::literal::{ IntervalCompound, IntervalDayToSecond, IntervalYearToMonth, LiteralType, - interval_day_to_second, }; pub async fn from_literal( @@ -102,28 +99,6 @@ pub(crate) fn from_substrait_literal( }, Some(LiteralType::Fp32(f)) => ScalarValue::Float32(Some(*f)), Some(LiteralType::Fp64(f)) => ScalarValue::Float64(Some(*f)), - #[expect(deprecated)] - Some(LiteralType::Timestamp(t)) => { - // Kept for backwards compatibility, new plans should use PrecisionTimestamp(Tz) instead - #[expect(deprecated)] - match lit.type_variation_reference { - TIMESTAMP_SECOND_TYPE_VARIATION_REF => { - ScalarValue::TimestampSecond(Some(*t), None) - } - TIMESTAMP_MILLI_TYPE_VARIATION_REF => { - ScalarValue::TimestampMillisecond(Some(*t), None) - } - TIMESTAMP_MICRO_TYPE_VARIATION_REF => { - ScalarValue::TimestampMicrosecond(Some(*t), None) - } - TIMESTAMP_NANO_TYPE_VARIATION_REF => { - ScalarValue::TimestampNanosecond(Some(*t), None) - } - others => { - return substrait_err!("Unknown type variation reference {others}"); - } - } - } Some(LiteralType::PrecisionTimestamp(pt)) => match pt.precision { 0 => ScalarValue::TimestampSecond(Some(pt.value), None), 3 => ScalarValue::TimestampMillisecond(Some(pt.value), None), @@ -381,29 +356,17 @@ pub(crate) fn from_substrait_literal( days, seconds, subseconds, - precision_mode, + precision, })) => { - use interval_day_to_second::PrecisionMode; // DF only supports millisecond precision, so for any more granular type we lose precision - let milliseconds = match precision_mode { - #[expect(deprecated)] - Some(PrecisionMode::Microseconds(ms)) => ms / 1000, - None => { - if *subseconds != 0 { - return substrait_err!( - "Cannot set subseconds field of IntervalDayToSecond without setting precision" - ); - } else { - 0_i32 - } - } - Some(PrecisionMode::Precision(0)) => *subseconds as i32 * 1000, - Some(PrecisionMode::Precision(3)) => *subseconds as i32, - Some(PrecisionMode::Precision(6)) => (subseconds / 1000) as i32, - Some(PrecisionMode::Precision(9)) => (subseconds / 1000 / 1000) as i32, - _ => { + let milliseconds = match precision { + 0 => *subseconds as i32 * 1000, + 3 => *subseconds as i32, + 6 => (subseconds / 1000) as i32, + 9 => (subseconds / 1000 / 1000) as i32, + p => { return not_impl_err!( - "Unsupported Substrait interval day to second precision mode: {precision_mode:?}" + "Unsupported Substrait interval day to second precision: {p}" ); } }; @@ -423,8 +386,7 @@ pub(crate) fn from_substrait_literal( days, seconds, subseconds, - precision_mode: - Some(interval_day_to_second::PrecisionMode::Precision(p)), + precision: p, }), ) => { if *p < 0 || *p > 9 { @@ -607,9 +569,7 @@ mod tests { days: 3, seconds: 4, subseconds: 5, - precision_mode: Some( - interval_day_to_second::PrecisionMode::Precision(6), - ), + precision: 6, }), })), }; diff --git a/datafusion/substrait/src/logical_plan/consumer/expr/mod.rs b/datafusion/substrait/src/logical_plan/consumer/expr/mod.rs index 623bb715cd838..5b8ca8396f114 100644 --- a/datafusion/substrait/src/logical_plan/consumer/expr/mod.rs +++ b/datafusion/substrait/src/logical_plan/consumer/expr/mod.rs @@ -62,6 +62,9 @@ pub async fn from_substrait_rex( ) -> datafusion::common::Result { match &expression.rex_type { Some(t) => match t { + RexType::ExecutionContextVariable(_) => { + not_impl_err!("Execution context variables are not supported") + } RexType::Literal(expr) => consumer.consume_literal(expr).await, RexType::Selection(expr) => { consumer.consume_field_reference(expr, input_schema).await @@ -92,8 +95,6 @@ pub async fn from_substrait_rex( consumer.consume_subquery(expr.as_ref(), input_schema).await } RexType::Nested(expr) => consumer.consume_nested(expr, input_schema).await, - #[expect(deprecated)] - RexType::Enum(expr) => consumer.consume_enum(expr, input_schema).await, RexType::DynamicParameter(expr) => { consumer.consume_dynamic_parameter(expr, input_schema).await } @@ -216,14 +217,14 @@ mod tests { async fn window_function_with_range_unit_and_no_order_by() -> datafusion::common::Result<()> { let substrait = Expression { - rex_type: Some(RexType::WindowFunction( + rex_type: Some(RexType::WindowFunction(Box::new( substrait::proto::expression::WindowFunction { function_reference: 0, bounds_type: BoundsType::Range as i32, sorts: vec![], ..Default::default() }, - )), + ))), }; let mut consumer = test_consumer(); @@ -247,12 +248,12 @@ mod tests { #[tokio::test] async fn window_function_with_count() -> datafusion::common::Result<()> { let substrait = Expression { - rex_type: Some(RexType::WindowFunction( + rex_type: Some(RexType::WindowFunction(Box::new( substrait::proto::expression::WindowFunction { function_reference: 0, ..Default::default() }, - )), + ))), }; let mut consumer = test_consumer(); @@ -274,13 +275,13 @@ mod tests { #[tokio::test] async fn window_function_with_invalid_invocation() { let substrait = Expression { - rex_type: Some(RexType::WindowFunction( + rex_type: Some(RexType::WindowFunction(Box::new( substrait::proto::expression::WindowFunction { function_reference: 0, invocation: 3, ..Default::default() }, - )), + ))), }; let mut consumer = test_consumer(); diff --git a/datafusion/substrait/src/logical_plan/consumer/expr/window_function.rs b/datafusion/substrait/src/logical_plan/consumer/expr/window_function.rs index d39b325a54827..e366599e0770b 100644 --- a/datafusion/substrait/src/logical_plan/consumer/expr/window_function.rs +++ b/datafusion/substrait/src/logical_plan/consumer/expr/window_function.rs @@ -27,11 +27,14 @@ use datafusion::logical_expr::expr::WindowFunctionParams; use datafusion::logical_expr::{ Expr, WindowFrameBound, WindowFrameUnits, WindowFunctionDefinition, expr, }; +use substrait::proto::Expression; use substrait::proto::aggregate_function::AggregationInvocation; use substrait::proto::expression::WindowFunction; +use substrait::proto::expression::literal::LiteralType; use substrait::proto::expression::window_function::{Bound, BoundsType}; use substrait::proto::expression::{ - window_function::bound as SubstraitBound, window_function::bound::Kind as BoundKind, + Literal, RexType, window_function::bound as SubstraitBound, + window_function::bound::Kind as BoundKind, }; pub async fn from_window_function( @@ -85,8 +88,8 @@ pub async fn from_window_function( }; let window_frame = datafusion::logical_expr::WindowFrame::new_bounds( bound_units, - from_substrait_bound(window.lower_bound.as_ref(), true)?, - from_substrait_bound(window.upper_bound.as_ref(), false)?, + from_substrait_bound(window.lower_bound.as_deref(), true)?, + from_substrait_bound(window.upper_bound.as_deref(), false)?, ); window_frame.regularize_order_bys(&mut order_by)?; @@ -139,20 +142,26 @@ fn from_substrait_bound( BoundKind::CurrentRow(SubstraitBound::CurrentRow {}) => { Ok(WindowFrameBound::CurrentRow) } - BoundKind::Preceding(SubstraitBound::Preceding { offset }) => { - if *offset <= 0 { + BoundKind::Preceding(bound) => { + #[expect(deprecated)] + let offset = + bound_offset(bound.offset, bound.offset_expr.as_deref())?; + if offset <= 0 { return plan_err!("Preceding bound must be positive"); } Ok(WindowFrameBound::Preceding(ScalarValue::UInt64(Some( - *offset as u64, + offset as u64, )))) } - BoundKind::Following(SubstraitBound::Following { offset }) => { - if *offset <= 0 { + BoundKind::Following(bound) => { + #[expect(deprecated)] + let offset = + bound_offset(bound.offset, bound.offset_expr.as_deref())?; + if offset <= 0 { return plan_err!("Following bound must be positive"); } Ok(WindowFrameBound::Following(ScalarValue::UInt64(Some( - *offset as u64, + offset as u64, )))) } BoundKind::Unbounded(SubstraitBound::Unbounded {}) => { @@ -174,3 +183,27 @@ fn from_substrait_bound( } } } + +/// Reads the distance of a window frame bound. +/// +/// The specification requires a consumer to use `offset_expr` when it is set and +/// to ignore `offset`. DataFusion frame bounds hold a literal, so an expression +/// that is not an int64 literal cannot be represented. +fn bound_offset( + offset: i64, + offset_expr: Option<&Expression>, +) -> datafusion::common::Result { + match offset_expr { + Some(Expression { + rex_type: + Some(RexType::Literal(Literal { + literal_type: Some(LiteralType::I64(value)), + .. + })), + }) => Ok(*value), + Some(_) => not_impl_err!( + "Window frame bound offsets other than int64 literals are not supported" + ), + None => Ok(offset), + } +} diff --git a/datafusion/substrait/src/logical_plan/consumer/rel/aggregate_rel.rs b/datafusion/substrait/src/logical_plan/consumer/rel/aggregate_rel.rs index 8c0114b90ee13..acb08d3afb129 100644 --- a/datafusion/substrait/src/logical_plan/consumer/rel/aggregate_rel.rs +++ b/datafusion/substrait/src/logical_plan/consumer/rel/aggregate_rel.rs @@ -17,7 +17,7 @@ use crate::logical_plan::consumer::{NameTracker, SubstraitConsumer}; use crate::logical_plan::consumer::{from_substrait_agg_func, from_substrait_sorts}; -use datafusion::common::{DFSchemaRef, not_impl_err}; +use datafusion::common::{not_impl_err, substrait_datafusion_err}; use datafusion::logical_expr::{Expr, GroupingSet, LogicalPlan, LogicalPlanBuilder}; use substrait::proto::AggregateRel; use substrait::proto::aggregate_function::AggregationInvocation; @@ -42,26 +42,16 @@ pub async fn from_aggregate_rel( match agg.groupings.len() { 0 => {} 1 => { - group_exprs.extend_from_slice( - &from_substrait_grouping( - consumer, - &agg.groupings[0], - &ref_group_exprs, - input.schema(), - ) - .await?, - ); + group_exprs.extend_from_slice(&from_substrait_grouping( + &agg.groupings[0], + &ref_group_exprs, + )?); } _ => { let mut grouping_sets = vec![]; for grouping in &agg.groupings { - let grouping_set = from_substrait_grouping( - consumer, - grouping, - &ref_group_exprs, - input.schema(), - ) - .await?; + let grouping_set = + from_substrait_grouping(grouping, &ref_group_exprs)?; grouping_sets.push(grouping_set); } // Single-element grouping expression of type Expr::GroupingSet. @@ -128,24 +118,22 @@ pub async fn from_aggregate_rel( } } -#[expect(deprecated)] -async fn from_substrait_grouping( - consumer: &impl SubstraitConsumer, +/// A grouping set names the expressions it groups by index into the +/// relation-level `grouping_expressions`. +fn from_substrait_grouping( grouping: &Grouping, expressions: &[Expr], - input_schema: &DFSchemaRef, ) -> datafusion::common::Result> { - let mut group_exprs = vec![]; - if !grouping.grouping_expressions.is_empty() { - for e in &grouping.grouping_expressions { - let expr = consumer.consume_expression(e, input_schema).await?; - group_exprs.push(expr); - } - return Ok(group_exprs); - } - for idx in &grouping.expression_references { - let e = &expressions[*idx as usize]; - group_exprs.push(e.clone()); - } - Ok(group_exprs) + grouping + .expression_references + .iter() + .map(|idx| { + expressions.get(*idx as usize).cloned().ok_or_else(|| { + substrait_datafusion_err!( + "Grouping references expression {idx} but the aggregate declares {}", + expressions.len() + ) + }) + }) + .collect() } diff --git a/datafusion/substrait/src/logical_plan/consumer/rel/fetch_rel.rs b/datafusion/substrait/src/logical_plan/consumer/rel/fetch_rel.rs index 12a8a77199b1a..6f96d5b9d307f 100644 --- a/datafusion/substrait/src/logical_plan/consumer/rel/fetch_rel.rs +++ b/datafusion/substrait/src/logical_plan/consumer/rel/fetch_rel.rs @@ -18,8 +18,8 @@ use crate::logical_plan::consumer::SubstraitConsumer; use async_recursion::async_recursion; use datafusion::common::{DFSchema, DFSchemaRef, not_impl_err}; -use datafusion::logical_expr::{LogicalPlan, LogicalPlanBuilder, lit}; -use substrait::proto::{FetchRel, fetch_rel}; +use datafusion::logical_expr::{LogicalPlan, LogicalPlanBuilder}; +use substrait::proto::FetchRel; #[async_recursion] pub async fn from_fetch_rel( @@ -29,23 +29,14 @@ pub async fn from_fetch_rel( if let Some(input) = fetch.input.as_ref() { let input = LogicalPlanBuilder::from(consumer.consume_rel(input).await?); let empty_schema = DFSchemaRef::new(DFSchema::empty()); - let offset = match &fetch.offset_mode { - #[expect(deprecated)] - Some(fetch_rel::OffsetMode::Offset(offset)) => Some(lit(*offset)), - Some(fetch_rel::OffsetMode::OffsetExpr(expr)) => { - Some(consumer.consume_expression(expr, &empty_schema).await?) - } + // Unset offset is treated as 0 and unset count signals that ALL records + // should be returned, so an absent expression maps to None either way. + let offset = match &fetch.offset_expr { + Some(expr) => Some(consumer.consume_expression(expr, &empty_schema).await?), None => None, }; - let count = match &fetch.count_mode { - #[expect(deprecated)] - Some(fetch_rel::CountMode::Count(count)) => { - // -1 means that ALL records should be returned, equivalent to None - (*count != -1).then(|| lit(*count)) - } - Some(fetch_rel::CountMode::CountExpr(expr)) => { - Some(consumer.consume_expression(expr, &empty_schema).await?) - } + let count = match &fetch.count_expr { + Some(expr) => Some(consumer.consume_expression(expr, &empty_schema).await?), None => None, }; input.limit_by_expr(offset, count)?.build() diff --git a/datafusion/substrait/src/logical_plan/consumer/rel/mod.rs b/datafusion/substrait/src/logical_plan/consumer/rel/mod.rs index 038ada115b9d8..1c5e280eb487a 100644 --- a/datafusion/substrait/src/logical_plan/consumer/rel/mod.rs +++ b/datafusion/substrait/src/logical_plan/consumer/rel/mod.rs @@ -134,6 +134,8 @@ fn retrieve_rel_common(rel: &Rel) -> Option<&RelCommon> { match rel.rel_type.as_ref() { None => None, Some(rt) => match rt { + RelType::LateralJoin(l) => l.common.as_ref(), + RelType::TopN(t) => t.common.as_ref(), RelType::Read(r) => r.common.as_ref(), RelType::Filter(f) => f.common.as_ref(), RelType::Fetch(f) => f.common.as_ref(), diff --git a/datafusion/substrait/src/logical_plan/consumer/rel/read_rel.rs b/datafusion/substrait/src/logical_plan/consumer/rel/read_rel.rs index 2cad1440807a5..9457c155ed9e4 100644 --- a/datafusion/substrait/src/logical_plan/consumer/rel/read_rel.rs +++ b/datafusion/substrait/src/logical_plan/consumer/rel/read_rel.rs @@ -35,7 +35,6 @@ use substrait::proto::read_rel::local_files::file_or_files::PathType::UriFile; use substrait::proto::{Expression, ReadRel}; use url::Url; -#[expect(deprecated)] pub async fn from_read_rel( consumer: &impl SubstraitConsumer, read: &ReadRel, @@ -114,14 +113,13 @@ pub async fn from_read_rel( .await } Some(ReadType::VirtualTable(vt)) => { - if vt.values.is_empty() && vt.expressions.is_empty() { + if vt.expressions.is_empty() { return Ok(LogicalPlan::EmptyRelation(EmptyRelation { produce_one_row: false, schema: DFSchemaRef::new(substrait_schema), })); } - // Check for produce_one_row pattern in both old (values) and new (expressions) formats. // A VirtualTable with exactly one row containing only empty/default fields represents // an EmptyRelation with produce_one_row=true. This pattern is used for queries without // a FROM clause (e.g., "SELECT 1 AS one") where a single phantom row is needed to @@ -129,14 +127,9 @@ pub async fn from_read_rel( // the SQL "DUAL" table (see: https://en.wikipedia.org/wiki/DUAL_table) which some // databases provide as a single-row source for selecting constant expressions when no // real table is present. - let is_produce_one_row = (vt.values.len() == 1 - && vt.expressions.is_empty() + let is_produce_one_row = vt.expressions.len() == 1 && substrait_schema.fields().is_empty() - && vt.values[0].fields.is_empty()) - || (vt.expressions.len() == 1 - && vt.values.is_empty() - && substrait_schema.fields().is_empty() - && vt.expressions[0].fields.is_empty()); + && vt.expressions[0].fields.is_empty(); if is_produce_one_row { return Ok(LogicalPlan::EmptyRelation(EmptyRelation { @@ -145,7 +138,7 @@ pub async fn from_read_rel( })); } - let values = if !vt.expressions.is_empty() { + let values = { let mut exprs = vec![]; for row in &vt.expressions { if row.fields.len() != substrait_schema.fields().len() { @@ -195,8 +188,6 @@ pub async fn from_read_rel( exprs.push(row_exprs); } exprs - } else { - convert_literal_rows(consumer, vt, named_struct)? }; Ok(LogicalPlan::Values(Values { @@ -251,46 +242,6 @@ pub async fn from_read_rel( } } -/// Converts Substrait literal rows from a VirtualTable into DataFusion expressions. -/// -/// This function processes the deprecated `values` field of VirtualTable, converting -/// each literal value into a `Expr::Literal` while tracking and validating the name -/// indices against the provided named struct schema. -fn convert_literal_rows( - consumer: &impl SubstraitConsumer, - vt: &substrait::proto::read_rel::VirtualTable, - named_struct: &substrait::proto::NamedStruct, -) -> datafusion::common::Result>> { - #[expect(deprecated)] - vt.values - .iter() - .map(|row| { - let mut name_idx = 0; - let lits = row - .fields - .iter() - .map(|lit| { - name_idx += 1; // top-level names are provided through schema - Ok(Expr::Literal(from_substrait_literal( - consumer, - lit, - &named_struct.names, - &mut name_idx, - )?, None)) - }) - .collect::>()?; - if name_idx != named_struct.names.len() { - return substrait_err!( - "Names list must match exactly to nested schema, but found {} uses for {} names", - name_idx, - named_struct.names.len() - ); - } - Ok(lits) - }) - .collect::>() -} - pub fn apply_masking( schema: DFSchema, mask_expression: Option<&MaskExpression>, diff --git a/datafusion/substrait/src/logical_plan/consumer/substrait_consumer.rs b/datafusion/substrait/src/logical_plan/consumer/substrait_consumer.rs index bbd80b4cff001..4de2a4503024e 100644 --- a/datafusion/substrait/src/logical_plan/consumer/substrait_consumer.rs +++ b/datafusion/substrait/src/logical_plan/consumer/substrait_consumer.rs @@ -40,8 +40,8 @@ use std::collections::VecDeque; use std::sync::{Arc, RwLock}; use substrait::proto::expression as substrait_expression; use substrait::proto::expression::{ - Enum, FieldReference, IfThen, Literal, MultiOrList, Nested, ScalarFunction, - SingularOrList, SwitchExpression, WindowFunction, + FieldReference, IfThen, Literal, MultiOrList, Nested, ScalarFunction, SingularOrList, + SwitchExpression, WindowFunction, }; use substrait::proto::{self, Type}; use substrait::proto::{ @@ -388,14 +388,6 @@ pub trait SubstraitConsumer: Send + Sync + Sized { from_nested(self, expr, input_schema).await } - async fn consume_enum( - &self, - _expr: &Enum, - _input_schema: &DFSchema, - ) -> datafusion::common::Result { - not_impl_err!("Enum expression not supported") - } - async fn consume_dynamic_parameter( &self, expr: &DynamicParameter, diff --git a/datafusion/substrait/src/logical_plan/consumer/types.rs b/datafusion/substrait/src/logical_plan/consumer/types.rs index 6ca9ced2fcb06..c8e35ec12b965 100644 --- a/datafusion/substrait/src/logical_plan/consumer/types.rs +++ b/datafusion/substrait/src/logical_plan/consumer/types.rs @@ -27,14 +27,12 @@ use crate::variation_const::{ INTERVAL_DAY_TIME_TYPE_REF, INTERVAL_MONTH_DAY_NANO_TYPE_NAME, INTERVAL_MONTH_DAY_NANO_TYPE_REF, INTERVAL_YEAR_MONTH_TYPE_REF, LARGE_CONTAINER_TYPE_VARIATION_REF, TIME_32_TYPE_VARIATION_REF, - TIME_64_TYPE_VARIATION_REF, TIMESTAMP_MICRO_TYPE_VARIATION_REF, - TIMESTAMP_MILLI_TYPE_VARIATION_REF, TIMESTAMP_NANO_TYPE_VARIATION_REF, - TIMESTAMP_SECOND_TYPE_VARIATION_REF, UNSIGNED_INTEGER_TYPE_VARIATION_REF, + TIME_64_TYPE_VARIATION_REF, UNSIGNED_INTEGER_TYPE_VARIATION_REF, VIEW_CONTAINER_TYPE_VARIATION_REF, }; use crate::variation_const::{FLOAT_16_TYPE_NAME, NULL_TYPE_NAME}; use datafusion::arrow::datatypes::{ - DataType, Field, FieldRef, Fields, IntervalUnit, Schema, TimeUnit, + DataType, Field, FieldRef, Fields, IntervalUnit, Schema, }; use datafusion::common::datatype::DataTypeExt; use datafusion::common::{ @@ -106,28 +104,6 @@ pub fn from_substrait_type( }, r#type::Kind::Fp32(_) => Ok(DataType::Float32), r#type::Kind::Fp64(_) => Ok(DataType::Float64), - #[expect(deprecated)] - r#type::Kind::Timestamp(ts) => { - // Kept for backwards compatibility, new plans should use PrecisionTimestamp(Tz) instead - #[expect(deprecated)] - match ts.type_variation_reference { - TIMESTAMP_SECOND_TYPE_VARIATION_REF => { - Ok(DataType::Timestamp(TimeUnit::Second, None)) - } - TIMESTAMP_MILLI_TYPE_VARIATION_REF => { - Ok(DataType::Timestamp(TimeUnit::Millisecond, None)) - } - TIMESTAMP_MICRO_TYPE_VARIATION_REF => { - Ok(DataType::Timestamp(TimeUnit::Microsecond, None)) - } - TIMESTAMP_NANO_TYPE_VARIATION_REF => { - Ok(DataType::Timestamp(TimeUnit::Nanosecond, None)) - } - v => not_impl_err!( - "Unsupported Substrait type variation {v} of type {s_kind:?}" - ), - } - } r#type::Kind::PrecisionTimestamp(pts) => { let unit = from_substrait_precision(pts.precision, "PrecisionTimestamp")?; Ok(DataType::Timestamp(unit, None)) @@ -368,13 +344,7 @@ fn type_is_nullable(dt: &Type) -> datafusion::common::Result { r#type::Kind::I64(integer) => integer.nullability, r#type::Kind::Fp32(float) => float.nullability, r#type::Kind::Fp64(float) => float.nullability, - #[expect(deprecated)] - r#type::Kind::Timestamp(timestamp) => timestamp.nullability, r#type::Kind::Date(date) => date.nullability, - #[expect(deprecated)] - r#type::Kind::Time(time) => time.nullability, - #[expect(deprecated)] - r#type::Kind::TimestampTz(timestamp) => timestamp.nullability, r#type::Kind::IntervalYear(interval) => interval.nullability, r#type::Kind::IntervalDay(interval) => interval.nullability, r#type::Kind::IntervalCompound(interval) => interval.nullability, @@ -393,8 +363,7 @@ fn type_is_nullable(dt: &Type) -> datafusion::common::Result { r#type::Kind::Map(map) => map.nullability, r#type::Kind::Func(func) => func.nullability, r#type::Kind::UserDefined(user_defined) => user_defined.nullability, - #[expect(deprecated)] - r#type::Kind::UserDefinedTypeReference(_) => r#type::Nullability::Required as i32, + r#type::Kind::Unbound(_) => r#type::Nullability::Nullable as i32, r#type::Kind::Alias(alias) => alias.nullability, }; @@ -412,18 +381,6 @@ fn is_nullable(nullability: i32) -> datafusion::common::Result { #[cfg(test)] mod tests { use super::*; - use substrait::proto::r#type::Kind; - - #[test] - fn type_is_nullable_user_defined_type_reference_is_required() { - // The deprecated `UserDefinedTypeReference` variant doesn't carry a - // nullability field; the consumer hardcodes Required (non-null). - #[expect(deprecated)] - let dt = Type { - kind: Some(Kind::UserDefinedTypeReference(0)), - }; - assert!(!type_is_nullable(&dt).unwrap()); - } #[test] fn type_is_nullable_missing_kind_defaults_to_nullable() { diff --git a/datafusion/substrait/src/logical_plan/producer/expr/aggregate_function.rs b/datafusion/substrait/src/logical_plan/producer/expr/aggregate_function.rs index 3713f8934f19f..ad8f57812f461 100644 --- a/datafusion/substrait/src/logical_plan/producer/expr/aggregate_function.rs +++ b/datafusion/substrait/src/logical_plan/producer/expr/aggregate_function.rs @@ -54,7 +54,6 @@ pub fn from_aggregate_function( }); } let function_anchor = producer.register_function(func.name().to_string()); - #[expect(deprecated)] Ok(Measure { measure: Some(AggregateFunction { function_reference: function_anchor, @@ -66,7 +65,6 @@ pub fn from_aggregate_function( false => AggregationInvocation::All as i32, }, phase: AggregationPhase::Unspecified as i32, - args: vec![], options: vec![], }), filter: match filter { diff --git a/datafusion/substrait/src/logical_plan/producer/expr/field_reference.rs b/datafusion/substrait/src/logical_plan/producer/expr/field_reference.rs index 12ed92c64fe57..d12d825b9c206 100644 --- a/datafusion/substrait/src/logical_plan/producer/expr/field_reference.rs +++ b/datafusion/substrait/src/logical_plan/producer/expr/field_reference.rs @@ -19,6 +19,7 @@ use crate::logical_plan::producer::SubstraitProducer; use datafusion::common::{Column, DFSchemaRef, substrait_err}; use datafusion::logical_expr::Expr; use substrait::proto::Expression; +use substrait::proto::expression::field_reference::outer_reference::OuterReferenceType; use substrait::proto::expression::field_reference::{ OuterReference, ReferenceType, RootReference, RootType, }; @@ -101,8 +102,11 @@ pub fn from_outer_reference_column( if let Some(index) = outer_schema.maybe_index_of_column(col) { return substrait_field_ref_with_root( index, + #[expect(deprecated)] RootType::OuterReference(OuterReference { - steps_out: steps_out as u32, + outer_reference_type: Some(OuterReferenceType::StepsOut( + steps_out as u32, + )), }), ); } diff --git a/datafusion/substrait/src/logical_plan/producer/expr/literal.rs b/datafusion/substrait/src/logical_plan/producer/expr/literal.rs index f26984ad4a394..2f808146778b1 100644 --- a/datafusion/substrait/src/logical_plan/producer/expr/literal.rs +++ b/datafusion/substrait/src/logical_plan/producer/expr/literal.rs @@ -26,7 +26,6 @@ use crate::variation_const::{ use datafusion::arrow::array::{Array, GenericListArray, OffsetSizeTrait}; use datafusion::arrow::temporal_conversions::NANOSECONDS; use datafusion::common::{ScalarValue, exec_err, not_impl_err}; -use substrait::proto::expression::literal::interval_day_to_second::PrecisionMode; use substrait::proto::expression::literal::map::KeyValue; use substrait::proto::expression::literal::{ Decimal, IntervalCompound, IntervalDayToSecond, IntervalYearToMonth, List, @@ -217,7 +216,7 @@ pub(crate) fn to_substrait_literal( days: i.days, seconds: (i.nanoseconds / NANOSECONDS) as i32, subseconds: i.nanoseconds % NANOSECONDS, - precision_mode: Some(PrecisionMode::Precision(9)), // nanoseconds + precision: 9, // nanoseconds }), }), DEFAULT_TYPE_VARIATION_REF, @@ -227,7 +226,7 @@ pub(crate) fn to_substrait_literal( days: i.days, seconds: i.milliseconds / 1000, subseconds: (i.milliseconds % 1000) as i64, - precision_mode: Some(PrecisionMode::Precision(3)), // 3 for milliseconds + precision: 3, // 3 for milliseconds }), DEFAULT_TYPE_VARIATION_REF, ), diff --git a/datafusion/substrait/src/logical_plan/producer/expr/scalar_function.rs b/datafusion/substrait/src/logical_plan/producer/expr/scalar_function.rs index 75720395aae7c..5736d1cc2b849 100644 --- a/datafusion/substrait/src/logical_plan/producer/expr/scalar_function.rs +++ b/datafusion/substrait/src/logical_plan/producer/expr/scalar_function.rs @@ -121,14 +121,12 @@ pub fn from_higher_order_function( output_field.is_nullable(), )?; - #[expect(deprecated)] Ok(Expression { rex_type: Some(RexType::ScalarFunction(ScalarFunction { function_reference: function_anchor, arguments, output_type: Some(output_type), options: vec![], - args: vec![], })), }) } @@ -152,14 +150,12 @@ fn from_function( let output_type = to_substrait_type(producer, output_type, output_nullability)?; let function_anchor = producer.register_function(name.to_string()); - #[expect(deprecated)] Ok(Expression { rex_type: Some(RexType::ScalarFunction(ScalarFunction { function_reference: function_anchor, arguments, output_type: Some(output_type), options: vec![], - args: vec![], })), }) } @@ -288,13 +284,11 @@ fn make_substrait_like_expr( }, ]; - #[expect(deprecated)] let substrait_like = Expression { rex_type: Some(RexType::ScalarFunction(ScalarFunction { function_reference: function_anchor, arguments, output_type: None, - args: vec![], options: vec![], })), }; @@ -302,7 +296,6 @@ fn make_substrait_like_expr( if negated { let function_anchor = producer.register_function("not".to_string()); - #[expect(deprecated)] Ok(Expression { rex_type: Some(RexType::ScalarFunction(ScalarFunction { function_reference: function_anchor, @@ -310,7 +303,6 @@ fn make_substrait_like_expr( arg_type: Some(ArgType::Value(substrait_like)), }], output_type: None, - args: vec![], options: vec![], })), }) @@ -338,7 +330,6 @@ fn to_substrait_unary_scalar_fn( }], output_type: Some(output_type.clone()), options: vec![], - ..Default::default() })), }) } @@ -352,7 +343,6 @@ pub fn make_binary_op_scalar_func( output_type: &Type, ) -> Expression { let function_anchor = producer.register_function(operator_to_name(op).to_string()); - #[expect(deprecated)] Expression { rex_type: Some(RexType::ScalarFunction(ScalarFunction { function_reference: function_anchor, @@ -365,7 +355,6 @@ pub fn make_binary_op_scalar_func( }, ], output_type: Some(output_type.clone()), - args: vec![], options: vec![], })), } diff --git a/datafusion/substrait/src/logical_plan/producer/expr/window_function.rs b/datafusion/substrait/src/logical_plan/producer/expr/window_function.rs index d35771bf099d3..c711f695fca02 100644 --- a/datafusion/substrait/src/logical_plan/producer/expr/window_function.rs +++ b/datafusion/substrait/src/logical_plan/producer/expr/window_function.rs @@ -17,15 +17,17 @@ use crate::logical_plan::producer::SubstraitProducer; use crate::logical_plan::producer::utils::substrait_sort_field; +use crate::variation_const::DEFAULT_TYPE_VARIATION_REF; use datafusion::common::{DFSchemaRef, ScalarValue, not_impl_err}; use datafusion::logical_expr::expr::{WindowFunction, WindowFunctionParams}; use datafusion::logical_expr::{WindowFrame, WindowFrameBound, WindowFrameUnits}; use substrait::proto::aggregate_function::AggregationInvocation; -use substrait::proto::expression::RexType; use substrait::proto::expression::WindowFunction as SubstraitWindowFunction; +use substrait::proto::expression::literal::LiteralType; use substrait::proto::expression::window_function::bound as SubstraitBound; use substrait::proto::expression::window_function::bound::Kind as BoundKind; use substrait::proto::expression::window_function::{Bound, BoundsType}; +use substrait::proto::expression::{Literal, RexType}; use substrait::proto::function_argument::ArgType; use substrait::proto::{Expression, FunctionArgument, SortField}; @@ -99,9 +101,8 @@ fn make_substrait_window_function( bounds_type: BoundsType, distinct: bool, ) -> Expression { - #[expect(deprecated)] Expression { - rex_type: Some(RexType::WindowFunction(SubstraitWindowFunction { + rex_type: Some(RexType::WindowFunction(Box::new(SubstraitWindowFunction { function_reference, arguments, partitions, @@ -114,11 +115,10 @@ fn make_substrait_window_function( } else { AggregationInvocation::All as i32 }, - lower_bound: Some(bounds.0), - upper_bound: Some(bounds.1), - args: vec![], + lower_bound: Some(Box::new(bounds.0)), + upper_bound: Some(Box::new(bounds.1)), bounds_type: bounds_type as i32, - })), + }))), } } @@ -152,16 +152,45 @@ fn to_substrait_bound(bound: &WindowFrameBound) -> datafusion::common::Result Ok(Bound { kind: Some(BoundKind::CurrentRow(SubstraitBound::CurrentRow {})), }), - WindowFrameBound::Preceding(s) => Ok(Bound { - kind: Some(BoundKind::Preceding(SubstraitBound::Preceding { - offset: to_substrait_bound_offset(s)?, - })), - }), - WindowFrameBound::Following(s) => Ok(Bound { - kind: Some(BoundKind::Following(SubstraitBound::Following { - offset: to_substrait_bound_offset(s)?, - })), - }), + WindowFrameBound::Preceding(s) => { + let offset = to_substrait_bound_offset(s)?; + #[expect(deprecated)] + Ok(Bound { + kind: Some(BoundKind::Preceding(Box::new(SubstraitBound::Preceding { + // `offset` carries the int64-literal equivalent for consumers + // that do not read `offset_expr` yet. A zero distance is + // equivalent to CurrentRow and `offset` cannot represent it, + // so the specification asks producers not to write a zero + // `offset_expr`. + offset, + offset_expr: (offset != 0) + .then(|| Box::new(bound_offset_expr(offset))), + }))), + }) + } + WindowFrameBound::Following(s) => { + let offset = to_substrait_bound_offset(s)?; + #[expect(deprecated)] + Ok(Bound { + kind: Some(BoundKind::Following(Box::new(SubstraitBound::Following { + offset, + offset_expr: (offset != 0) + .then(|| Box::new(bound_offset_expr(offset))), + }))), + }) + } + } +} + +/// Builds the int64 literal expression that a window frame bound offset carries +/// in `offset_expr`. +fn bound_offset_expr(offset: i64) -> Expression { + Expression { + rex_type: Some(RexType::Literal(Literal { + nullable: false, + type_variation_reference: DEFAULT_TYPE_VARIATION_REF, + literal_type: Some(LiteralType::I64(offset)), + })), } } @@ -188,6 +217,7 @@ mod tests { use datafusion::common::assert_contains; #[test] + #[expect(deprecated)] fn window_frame_offsets() { for value in [ ScalarValue::UInt8(Some(1)), @@ -207,9 +237,10 @@ mod tests { let (_, bound) = to_substrait_bounds(&frame).unwrap(); assert_eq!( bound.kind, - Some(BoundKind::Following(SubstraitBound::Following { - offset: 1 - })) + Some(BoundKind::Following(Box::new(SubstraitBound::Following { + offset: 1, + offset_expr: Some(Box::new(bound_offset_expr(1))), + }))) ); } diff --git a/datafusion/substrait/src/logical_plan/producer/plan.rs b/datafusion/substrait/src/logical_plan/producer/plan.rs index 3b58720dba832..f6bdaa3e69fa7 100644 --- a/datafusion/substrait/src/logical_plan/producer/plan.rs +++ b/datafusion/substrait/src/logical_plan/producer/plan.rs @@ -20,7 +20,7 @@ use crate::logical_plan::producer::{ }; use datafusion::execution::SessionState; use datafusion::logical_expr::{LogicalPlan, SubqueryAlias}; -use substrait::proto::{Plan, PlanRel, Rel, RelRoot, plan_rel}; +use substrait::proto::{ExecutionBehavior, Plan, PlanRel, Rel, RelRoot, plan_rel}; use substrait::version; /// Convert DataFusion LogicalPlan to Substrait Plan @@ -51,6 +51,9 @@ pub fn to_substrait_plan( expected_type_urls: vec![], parameter_bindings: vec![], type_aliases: vec![], + // DataFusion does not emit execution context variables, so the + // evaluation mode does not govern anything in the plans it produces. + execution_behavior: Some(ExecutionBehavior::default()), })) } diff --git a/datafusion/substrait/src/logical_plan/producer/rel/aggregate_rel.rs b/datafusion/substrait/src/logical_plan/producer/rel/aggregate_rel.rs index dec94b0422257..d31a0a9809eda 100644 --- a/datafusion/substrait/src/logical_plan/producer/rel/aggregate_rel.rs +++ b/datafusion/substrait/src/logical_plan/producer/rel/aggregate_rel.rs @@ -64,15 +64,16 @@ pub fn from_distinct( .map(substrait_field_ref) .collect::>>()?; - #[expect(deprecated)] + // The keys are declared once on the relation and the single grouping + // set references them by index. + let expression_references = (0..grouping.len() as u32).collect(); Ok(Box::new(Rel { rel_type: Some(RelType::Aggregate(Box::new(AggregateRel { common: None, input: Some(input), - grouping_expressions: vec![], + grouping_expressions: grouping, groupings: vec![Grouping { - grouping_expressions: grouping, - expression_references: vec![], + expression_references, }], measures: vec![], advanced_extension: None, @@ -160,17 +161,13 @@ pub fn parse_flat_grouping_exprs( ref_group_exprs: &mut Vec, ) -> datafusion::common::Result { let mut expression_references = vec![]; - let mut grouping_expressions = vec![]; for e in exprs { let rex = producer.handle_expr(e, schema)?; - grouping_expressions.push(rex.clone()); ref_group_exprs.push(rex); expression_references.push((ref_group_exprs.len() - 1) as u32); } - #[expect(deprecated)] Ok(Grouping { - grouping_expressions, expression_references, }) } diff --git a/datafusion/substrait/src/logical_plan/producer/rel/fetch_rel.rs b/datafusion/substrait/src/logical_plan/producer/rel/fetch_rel.rs index e878b3816ff42..76418af562c6a 100644 --- a/datafusion/substrait/src/logical_plan/producer/rel/fetch_rel.rs +++ b/datafusion/substrait/src/logical_plan/producer/rel/fetch_rel.rs @@ -20,7 +20,7 @@ use datafusion::common::DFSchema; use datafusion::logical_expr::Limit; use std::sync::Arc; use substrait::proto::rel::RelType; -use substrait::proto::{FetchRel, Rel, fetch_rel}; +use substrait::proto::{FetchRel, Rel}; pub fn from_limit( producer: &mut impl SubstraitProducer, @@ -28,26 +28,24 @@ pub fn from_limit( ) -> datafusion::common::Result> { let input = producer.handle_plan(limit.input.as_ref())?; let empty_schema = Arc::new(DFSchema::empty()); - let offset_mode = limit + let offset_expr = limit .skip .as_ref() .map(|expr| producer.handle_expr(expr.as_ref(), &empty_schema)) .transpose()? - .map(Box::new) - .map(fetch_rel::OffsetMode::OffsetExpr); - let count_mode = limit + .map(Box::new); + let count_expr = limit .fetch .as_ref() .map(|expr| producer.handle_expr(expr.as_ref(), &empty_schema)) .transpose()? - .map(Box::new) - .map(fetch_rel::CountMode::CountExpr); + .map(Box::new); Ok(Box::new(Rel { rel_type: Some(RelType::Fetch(Box::new(FetchRel { common: None, input: Some(input), - offset_mode, - count_mode, + offset_expr, + count_expr, advanced_extension: None, }))), })) diff --git a/datafusion/substrait/src/logical_plan/producer/rel/project_rel.rs b/datafusion/substrait/src/logical_plan/producer/rel/project_rel.rs index 33920cdf86f7a..d610704e5bb82 100644 --- a/datafusion/substrait/src/logical_plan/producer/rel/project_rel.rs +++ b/datafusion/substrait/src/logical_plan/producer/rel/project_rel.rs @@ -39,6 +39,7 @@ pub fn from_projection( let common = RelCommon { emit_kind: Some(emit_kind), hint: None, + rel_anchor: None, advanced_extension: None, }; @@ -73,6 +74,7 @@ pub fn from_window( let common = RelCommon { emit_kind: Some(emit_kind), hint: None, + rel_anchor: None, advanced_extension: None, }; let project_rel = Box::new(ProjectRel { diff --git a/datafusion/substrait/src/logical_plan/producer/rel/read_rel.rs b/datafusion/substrait/src/logical_plan/producer/rel/read_rel.rs index 900273bf8e6d7..a12460f785a46 100644 --- a/datafusion/substrait/src/logical_plan/producer/rel/read_rel.rs +++ b/datafusion/substrait/src/logical_plan/producer/rel/read_rel.rs @@ -144,7 +144,6 @@ pub fn from_empty_relation( ReadType::VirtualTable(VirtualTable { expressions: vec![NestedStruct { fields }], - ..Default::default() }) } else { ReadType::VirtualTable(VirtualTable::default()) @@ -178,10 +177,7 @@ pub fn from_values( best_effort_filter: None, projection: None, advanced_extension: None, - read_type: Some(ReadType::VirtualTable(VirtualTable { - expressions, - ..Default::default() - })), + read_type: Some(ReadType::VirtualTable(VirtualTable { expressions })), }))), })) } diff --git a/datafusion/substrait/src/logical_plan/producer/rel/sort_rel.rs b/datafusion/substrait/src/logical_plan/producer/rel/sort_rel.rs index d4520a4c37b14..43d27ea792e69 100644 --- a/datafusion/substrait/src/logical_plan/producer/rel/sort_rel.rs +++ b/datafusion/substrait/src/logical_plan/producer/rel/sort_rel.rs @@ -21,7 +21,7 @@ use datafusion::logical_expr::Sort; use substrait::proto::expression::literal::LiteralType; use substrait::proto::expression::{Literal, RexType}; use substrait::proto::rel::RelType; -use substrait::proto::{Expression, FetchRel, Rel, SortRel, fetch_rel}; +use substrait::proto::{Expression, FetchRel, Rel, SortRel}; pub fn from_sort( producer: &mut impl SubstraitProducer, @@ -46,20 +46,19 @@ pub fn from_sort( match fetch { Some(amount) => { - let count_mode = - Some(fetch_rel::CountMode::CountExpr(Box::new(Expression { - rex_type: Some(RexType::Literal(Literal { - nullable: false, - type_variation_reference: DEFAULT_TYPE_VARIATION_REF, - literal_type: Some(LiteralType::I64(*amount as i64)), - })), - }))); + let count_expr = Some(Box::new(Expression { + rex_type: Some(RexType::Literal(Literal { + nullable: false, + type_variation_reference: DEFAULT_TYPE_VARIATION_REF, + literal_type: Some(LiteralType::I64(*amount as i64)), + })), + })); Ok(Box::new(Rel { rel_type: Some(RelType::Fetch(Box::new(FetchRel { common: None, input: Some(sort_rel), - offset_mode: None, - count_mode, + offset_expr: None, + count_expr, advanced_extension: None, }))), })) diff --git a/datafusion/substrait/src/logical_plan/producer/utils.rs b/datafusion/substrait/src/logical_plan/producer/utils.rs index e8310f4acd31e..13984eb6b5d5a 100644 --- a/datafusion/substrait/src/logical_plan/producer/utils.rs +++ b/datafusion/substrait/src/logical_plan/producer/utils.rs @@ -93,7 +93,6 @@ pub(crate) fn negate( ) -> Expression { let function_anchor = producer.register_function("not".to_string()); - #[expect(deprecated)] Expression { rex_type: Some(substrait::proto::expression::RexType::ScalarFunction( substrait::proto::expression::ScalarFunction { @@ -104,7 +103,6 @@ pub(crate) fn negate( )), }], output_type: None, - args: vec![], options: vec![], }, )), diff --git a/datafusion/substrait/tests/testdata/test_plans/aggregate_identical_grouping_expressions.substrait.json b/datafusion/substrait/tests/testdata/test_plans/aggregate_identical_grouping_expressions.substrait.json index 15c0b0505fa68..2901f7e537ed7 100644 --- a/datafusion/substrait/tests/testdata/test_plans/aggregate_identical_grouping_expressions.substrait.json +++ b/datafusion/substrait/tests/testdata/test_plans/aggregate_identical_grouping_expressions.substrait.json @@ -25,18 +25,19 @@ }, "groupings": [ { - "groupingExpressions": [ - { - "literal": { - "i32": 1 - } - }, - { - "literal": { - "i32": 1 - } - } - ] + "expressionReferences": [0, 1] + } + ], + "groupingExpressions": [ + { + "literal": { + "i32": 1 + } + }, + { + "literal": { + "i32": 1 + } } ], "measures": [] diff --git a/datafusion/substrait/tests/testdata/test_plans/aggregate_identical_measures.substrait.json b/datafusion/substrait/tests/testdata/test_plans/aggregate_identical_measures.substrait.json index 620d55e93ee1e..5186ace7f1e03 100644 --- a/datafusion/substrait/tests/testdata/test_plans/aggregate_identical_measures.substrait.json +++ b/datafusion/substrait/tests/testdata/test_plans/aggregate_identical_measures.substrait.json @@ -39,8 +39,9 @@ } }, "groupings": [{ - "groupingExpressions": [] + "expressionReferences": [] }], + "groupingExpressions": [], "measures": [ { "measure": { diff --git a/datafusion/substrait/tests/testdata/test_plans/aggregate_no_project.substrait.json b/datafusion/substrait/tests/testdata/test_plans/aggregate_no_project.substrait.json index ed8675b968269..1394e3ca2bbf9 100644 --- a/datafusion/substrait/tests/testdata/test_plans/aggregate_no_project.substrait.json +++ b/datafusion/substrait/tests/testdata/test_plans/aggregate_no_project.substrait.json @@ -46,15 +46,8 @@ }, "groupings": [ { - "groupingExpressions": [ - { - "selection": { - "directReference": { - "structField": {} - }, - "rootReference": {} - } - } + "expressionReferences": [ + 0 ] } ], @@ -80,6 +73,16 @@ ] } } + ], + "groupingExpressions": [ + { + "selection": { + "directReference": { + "structField": {} + }, + "rootReference": {} + } + } ] } }, diff --git a/datafusion/substrait/tests/testdata/test_plans/aggregate_sorted_no_project.substrait.json b/datafusion/substrait/tests/testdata/test_plans/aggregate_sorted_no_project.substrait.json index d5170223cd65b..2ef9c86eee2c0 100644 --- a/datafusion/substrait/tests/testdata/test_plans/aggregate_sorted_no_project.substrait.json +++ b/datafusion/substrait/tests/testdata/test_plans/aggregate_sorted_no_project.substrait.json @@ -46,16 +46,17 @@ }, "groupings": [ { - "groupingExpressions": [ - { - "selection": { - "directReference": { - "structField": {} - }, - "rootReference": {} - } - } - ] + "expressionReferences": [0] + } + ], + "groupingExpressions": [ + { + "selection": { + "directReference": { + "structField": {} + }, + "rootReference": {} + } } ], "measures": [ diff --git a/datafusion/substrait/tests/testdata/test_plans/join_with_expression_key.json b/datafusion/substrait/tests/testdata/test_plans/join_with_expression_key.json index 8a81a9a0c780f..f1cda34c7715a 100644 --- a/datafusion/substrait/tests/testdata/test_plans/join_with_expression_key.json +++ b/datafusion/substrait/tests/testdata/test_plans/join_with_expression_key.json @@ -226,28 +226,29 @@ } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - "field": 1 - } - }, - "rootReference": { - } + "expressionReferences": [0, 1] + }], + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + "field": 1 } - }, { - "selection": { - "directReference": { - "structField": { - "field": 2 - } - }, - "rootReference": { - } + }, + "rootReference": { + } + } + }, { + "selection": { + "directReference": { + "structField": { + "field": 2 } - }] - }], + }, + "rootReference": { + } + } + }], "measures": [{ "measure": { "functionReference": 2, @@ -531,28 +532,29 @@ } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - "field": 1 - } - }, - "rootReference": { - } + "expressionReferences": [0, 1] + }], + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + "field": 1 } - }, { - "selection": { - "directReference": { - "structField": { - "field": 2 - } - }, - "rootReference": { - } + }, + "rootReference": { + } + } + }, { + "selection": { + "directReference": { + "structField": { + "field": 2 } - }] - }], + }, + "rootReference": { + } + } + }], "measures": [{ "measure": { "functionReference": 2, @@ -679,18 +681,19 @@ } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - "field": 1 - } - }, - "rootReference": { - } - } - }] + "expressionReferences": [0] }], + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + "field": 1 + } + }, + "rootReference": { + } + } + }], "measures": [{ "measure": { "functionReference": 2, diff --git a/datafusion/substrait/tests/testdata/test_plans/multilayer_aggregate.substrait.json b/datafusion/substrait/tests/testdata/test_plans/multilayer_aggregate.substrait.json index 1f47b916daf0f..3e5f9f4aedc6f 100644 --- a/datafusion/substrait/tests/testdata/test_plans/multilayer_aggregate.substrait.json +++ b/datafusion/substrait/tests/testdata/test_plans/multilayer_aggregate.substrait.json @@ -52,43 +52,27 @@ "input": { "read": { "common": { - "direct": {} + "direct": { + } }, "baseSchema": { - "names": [ - "product" - ], + "names": ["product"], "struct": { - "types": [ - { - "string": { - "nullability": "NULLABILITY_REQUIRED" - } + "types": [{ + "string": { + "nullability": "NULLABILITY_REQUIRED" } - ], + }], "nullability": "NULLABILITY_REQUIRED" } }, "namedTable": { - "names": [ - "sales" - ] + "names": ["sales"] } } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - "field": 0 - } - }, - "rootReference": { - } - } - }], - "expressionReferences": [] + "expressionReferences": [0] }], "measures": [{ "measure": { @@ -119,22 +103,21 @@ "options": [] } }], - "groupingExpressions": [] + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + "field": 0 + } + }, + "rootReference": { + } + } + }] } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - "field": 0 - } - }, - "rootReference": { - } - } - }], - "expressionReferences": [] + "expressionReferences": [0] }], "measures": [{ "measure": { @@ -165,7 +148,17 @@ "options": [] } }], - "groupingExpressions": [] + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + "field": 0 + } + }, + "rootReference": { + } + } + }] } }, "expressions": [{ diff --git a/datafusion/substrait/tests/testdata/test_plans/multiple_joins.json b/datafusion/substrait/tests/testdata/test_plans/multiple_joins.json index 15c0313b43b54..7de3d1d042562 100644 --- a/datafusion/substrait/tests/testdata/test_plans/multiple_joins.json +++ b/datafusion/substrait/tests/testdata/test_plans/multiple_joins.json @@ -72,46 +72,28 @@ } }, "virtualTable": { - "expressions": [ - { - "fields": [ - { - "literal": { - "i64": "1", - "nullable": true, - "typeVariationReference": 0 - } - } - ] - }, - { - "fields": [ - { - "literal": { - "i64": "2", - "nullable": true, - "typeVariationReference": 0 - } - } - ] - } - ] + "expressions": [{ + "fields": [{ + "literal": { + "i64": "1", + "nullable": true, + "typeVariationReference": 0 + } + }] + }, { + "fields": [{ + "literal": { + "i64": "2", + "nullable": true, + "typeVariationReference": 0 + } + }] + }] } } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - "field": 0 - } - }, - "rootReference": { - } - } - }], - "expressionReferences": [] + "expressionReferences": [0] }], "measures": [{ "measure": { @@ -130,7 +112,17 @@ "options": [] } }], - "groupingExpressions": [] + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + "field": 0 + } + }, + "rootReference": { + } + } + }] } }, "right": { @@ -164,73 +156,63 @@ } }, "virtualTable": { - "expressions": [ - { - "fields": [ - { - "literal": { - "i64": "1", - "nullable": true, - "typeVariationReference": 0 - } - }, - { - "literal": { - "string": "info", - "nullable": true, - "typeVariationReference": 0 - } - } - ] - }, - { - "fields": [ - { - "literal": { - "i64": "2", - "nullable": true, - "typeVariationReference": 0 - } - }, - { - "literal": { - "string": "low", - "nullable": true, - "typeVariationReference": 0 - } - } - ] - } - ] + "expressions": [{ + "fields": [{ + "literal": { + "i64": "1", + "nullable": true, + "typeVariationReference": 0 + } + }, { + "literal": { + "string": "info", + "nullable": true, + "typeVariationReference": 0 + } + }] + }, { + "fields": [{ + "literal": { + "i64": "2", + "nullable": true, + "typeVariationReference": 0 + } + }, { + "literal": { + "string": "low", + "nullable": true, + "typeVariationReference": 0 + } + }] + }] } } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - "field": 0 - } - }, - "rootReference": { + "expressionReferences": [0, 1] + }], + "measures": [], + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + "field": 0 } + }, + "rootReference": { } - }, { - "selection": { - "directReference": { - "structField": { - "field": 1 - } - }, - "rootReference": { + } + }, { + "selection": { + "directReference": { + "structField": { + "field": 1 } + }, + "rootReference": { } - }], - "expressionReferences": [] - }], - "measures": [], - "groupingExpressions": [] + } + }] } }, "expression": { @@ -300,46 +282,28 @@ } }, "virtualTable": { - "expressions": [ - { - "fields": [ - { - "literal": { - "i64": "1", - "nullable": true, - "typeVariationReference": 0 - } - } - ] - }, - { - "fields": [ - { - "literal": { - "i64": "2", - "nullable": true, - "typeVariationReference": 0 - } - } - ] - } - ] + "expressions": [{ + "fields": [{ + "literal": { + "i64": "1", + "nullable": true, + "typeVariationReference": 0 + } + }] + }, { + "fields": [{ + "literal": { + "i64": "2", + "nullable": true, + "typeVariationReference": 0 + } + }] + }] } } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - "field": 0 - } - }, - "rootReference": { - } - } - }], - "expressionReferences": [] + "expressionReferences": [0] }], "measures": [{ "measure": { @@ -358,7 +322,17 @@ "options": [] } }], - "groupingExpressions": [] + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + "field": 0 + } + }, + "rootReference": { + } + } + }] } }, "expression": { @@ -428,46 +402,28 @@ } }, "virtualTable": { - "expressions": [ - { - "fields": [ - { - "literal": { - "i64": "1", - "nullable": true, - "typeVariationReference": 0 - } - } - ] - }, - { - "fields": [ - { - "literal": { - "i64": "2", - "nullable": true, - "typeVariationReference": 0 - } - } - ] - } - ] + "expressions": [{ + "fields": [{ + "literal": { + "i64": "1", + "nullable": true, + "typeVariationReference": 0 + } + }] + }, { + "fields": [{ + "literal": { + "i64": "2", + "nullable": true, + "typeVariationReference": 0 + } + }] + }] } } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - "field": 0 - } - }, - "rootReference": { - } - } - }], - "expressionReferences": [] + "expressionReferences": [0] }], "measures": [{ "measure": { @@ -486,7 +442,17 @@ "options": [] } }], - "groupingExpressions": [] + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + "field": 0 + } + }, + "rootReference": { + } + } + }] } }, "expression": { diff --git a/datafusion/substrait/tests/testdata/test_plans/select_count_from_select_1.substrait.json b/datafusion/substrait/tests/testdata/test_plans/select_count_from_select_1.substrait.json index b0d4ba4813bcf..9e223e16e38f1 100644 --- a/datafusion/substrait/tests/testdata/test_plans/select_count_from_select_1.substrait.json +++ b/datafusion/substrait/tests/testdata/test_plans/select_count_from_select_1.substrait.json @@ -60,7 +60,6 @@ }, "groupings": [ { - "groupingExpressions": [], "expressionReferences": [] } ], diff --git a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_01_plan.json b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_01_plan.json index 3738a50a62382..b51e4996f657e 100644 --- a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_01_plan.json +++ b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_01_plan.json @@ -472,26 +472,7 @@ } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - } - }, - "rootReference": { - } - } - }, { - "selection": { - "directReference": { - "structField": { - "field": 1 - } - }, - "rootReference": { - } - } - }] + "expressionReferences": [0, 1] }], "measures": [{ "measure": { @@ -686,6 +667,26 @@ }, "invocation": "AGGREGATION_INVOCATION_ALL" } + }], + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + } + }, + "rootReference": { + } + } + }, { + "selection": { + "directReference": { + "structField": { + "field": 1 + } + }, + "rootReference": { + } + } }] } }, diff --git a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_02_plan.json b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_02_plan.json index f6c5e802a5e36..20ce5529da83f 100644 --- a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_02_plan.json +++ b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_02_plan.json @@ -1148,7 +1148,11 @@ }] } }, - "count": "100" + "countExpr": { + "literal": { + "i64": "100" + } + } } }, "names": ["S_ACCTBAL", "S_NAME", "N_NAME", "P_PARTKEY", "P_MFGR", "S_ADDRESS", "S_PHONE", "S_COMMENT"] diff --git a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_03_plan.json b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_03_plan.json index d4dea1d03c466..e90059e784353 100644 --- a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_03_plan.json +++ b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_03_plan.json @@ -602,36 +602,7 @@ } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - } - }, - "rootReference": { - } - } - }, { - "selection": { - "directReference": { - "structField": { - "field": 1 - } - }, - "rootReference": { - } - } - }, { - "selection": { - "directReference": { - "structField": { - "field": 2 - } - }, - "rootReference": { - } - } - }] + "expressionReferences": [0, 1, 2] }], "measures": [{ "measure": { @@ -659,6 +630,36 @@ } }] } + }], + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + } + }, + "rootReference": { + } + } + }, { + "selection": { + "directReference": { + "structField": { + "field": 1 + } + }, + "rootReference": { + } + } + }, { + "selection": { + "directReference": { + "structField": { + "field": 2 + } + }, + "rootReference": { + } + } }] } }, @@ -733,7 +734,11 @@ }] } }, - "count": "10" + "countExpr": { + "literal": { + "i64": "10" + } + } } }, "names": ["L_ORDERKEY", "REVENUE", "O_ORDERDATE", "O_SHIPPRIORITY"] diff --git a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_04_plan.json b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_04_plan.json index 3e665f50f3207..b25b47a86193e 100644 --- a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_04_plan.json +++ b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_04_plan.json @@ -418,16 +418,7 @@ } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - } - }, - "rootReference": { - } - } - }] + "expressionReferences": [0] }], "measures": [{ "measure": { @@ -440,6 +431,16 @@ }, "invocation": "AGGREGATION_INVOCATION_ALL" } + }], + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + } + }, + "rootReference": { + } + } }] } }, diff --git a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_05_plan.json b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_05_plan.json index d42975d3326d8..791cdfd367ff0 100644 --- a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_05_plan.json +++ b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_05_plan.json @@ -850,16 +850,7 @@ } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - } - }, - "rootReference": { - } - } - }] + "expressionReferences": [0] }], "measures": [{ "measure": { @@ -887,6 +878,16 @@ } }] } + }], + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + } + }, + "rootReference": { + } + } }] } }, diff --git a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_07_plan.json b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_07_plan.json index 82740fb3d87bd..62e385af3878f 100644 --- a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_07_plan.json +++ b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_07_plan.json @@ -988,36 +988,7 @@ } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - } - }, - "rootReference": { - } - } - }, { - "selection": { - "directReference": { - "structField": { - "field": 1 - } - }, - "rootReference": { - } - } - }, { - "selection": { - "directReference": { - "structField": { - "field": 2 - } - }, - "rootReference": { - } - } - }] + "expressionReferences": [0, 1, 2] }], "measures": [{ "measure": { @@ -1045,6 +1016,36 @@ } }] } + }], + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + } + }, + "rootReference": { + } + } + }, { + "selection": { + "directReference": { + "structField": { + "field": 1 + } + }, + "rootReference": { + } + } + }, { + "selection": { + "directReference": { + "structField": { + "field": 2 + } + }, + "rootReference": { + } + } }] } }, diff --git a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_08_plan.json b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_08_plan.json index 8c886f84ed166..cd53224b51159 100644 --- a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_08_plan.json +++ b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_08_plan.json @@ -1167,16 +1167,7 @@ } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - } - }, - "rootReference": { - } - } - }] + "expressionReferences": [0] }], "measures": [{ "measure": { @@ -1230,6 +1221,16 @@ } }] } + }], + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + } + }, + "rootReference": { + } + } }] } }, diff --git a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_09_plan.json b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_09_plan.json index 04b367a0b5bfe..6d54d90ae2a17 100644 --- a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_09_plan.json +++ b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_09_plan.json @@ -873,26 +873,7 @@ } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - } - }, - "rootReference": { - } - } - }, { - "selection": { - "directReference": { - "structField": { - "field": 1 - } - }, - "rootReference": { - } - } - }] + "expressionReferences": [0, 1] }], "measures": [{ "measure": { @@ -920,6 +901,26 @@ } }] } + }], + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + } + }, + "rootReference": { + } + } + }, { + "selection": { + "directReference": { + "structField": { + "field": 1 + } + }, + "rootReference": { + } + } }] } }, diff --git a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_10_plan.json b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_10_plan.json index 2daa1dabb4238..a9265f981f263 100644 --- a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_10_plan.json +++ b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_10_plan.json @@ -720,76 +720,7 @@ } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - } - }, - "rootReference": { - } - } - }, { - "selection": { - "directReference": { - "structField": { - "field": 1 - } - }, - "rootReference": { - } - } - }, { - "selection": { - "directReference": { - "structField": { - "field": 2 - } - }, - "rootReference": { - } - } - }, { - "selection": { - "directReference": { - "structField": { - "field": 3 - } - }, - "rootReference": { - } - } - }, { - "selection": { - "directReference": { - "structField": { - "field": 4 - } - }, - "rootReference": { - } - } - }, { - "selection": { - "directReference": { - "structField": { - "field": 5 - } - }, - "rootReference": { - } - } - }, { - "selection": { - "directReference": { - "structField": { - "field": 6 - } - }, - "rootReference": { - } - } - }] + "expressionReferences": [0, 1, 2, 3, 4, 5, 6] }], "measures": [{ "measure": { @@ -817,6 +748,76 @@ } }] } + }], + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + } + }, + "rootReference": { + } + } + }, { + "selection": { + "directReference": { + "structField": { + "field": 1 + } + }, + "rootReference": { + } + } + }, { + "selection": { + "directReference": { + "structField": { + "field": 2 + } + }, + "rootReference": { + } + } + }, { + "selection": { + "directReference": { + "structField": { + "field": 3 + } + }, + "rootReference": { + } + } + }, { + "selection": { + "directReference": { + "structField": { + "field": 4 + } + }, + "rootReference": { + } + } + }, { + "selection": { + "directReference": { + "structField": { + "field": 5 + } + }, + "rootReference": { + } + } + }, { + "selection": { + "directReference": { + "structField": { + "field": 6 + } + }, + "rootReference": { + } + } }] } }, @@ -918,7 +919,11 @@ }] } }, - "count": "20" + "countExpr": { + "literal": { + "i64": "20" + } + } } }, "names": ["C_CUSTKEY", "C_NAME", "REVENUE", "C_ACCTBAL", "N_NAME", "C_ADDRESS", "C_PHONE", "C_COMMENT"] diff --git a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_11_plan.json b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_11_plan.json index d79b065403d54..8434543e26563 100644 --- a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_11_plan.json +++ b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_11_plan.json @@ -384,16 +384,7 @@ } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - } - }, - "rootReference": { - } - } - }] + "expressionReferences": [0] }], "measures": [{ "measure": { @@ -421,6 +412,16 @@ } }] } + }], + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + } + }, + "rootReference": { + } + } }] } }, diff --git a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_12_plan.json b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_12_plan.json index db3100052704c..abd112f405794 100644 --- a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_12_plan.json +++ b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_12_plan.json @@ -711,16 +711,7 @@ } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - } - }, - "rootReference": { - } - } - }] + "expressionReferences": [0] }], "measures": [{ "measure": { @@ -770,6 +761,16 @@ } }] } + }], + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + } + }, + "rootReference": { + } + } }] } }, diff --git a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_13_plan.json b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_13_plan.json index 19b80b0aac73a..e73087b69cee2 100644 --- a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_13_plan.json +++ b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_13_plan.json @@ -326,16 +326,7 @@ } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - } - }, - "rootReference": { - } - } - }] + "expressionReferences": [0] }], "measures": [{ "measure": { @@ -361,6 +352,16 @@ } }] } + }], + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + } + }, + "rootReference": { + } + } }] } }, @@ -378,16 +379,7 @@ } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - } - }, - "rootReference": { - } - } - }] + "expressionReferences": [0] }], "measures": [{ "measure": { @@ -400,6 +392,16 @@ }, "invocation": "AGGREGATION_INVOCATION_ALL" } + }], + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + } + }, + "rootReference": { + } + } }] } }, diff --git a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_16_plan.json b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_16_plan.json index bf97fb9185711..138bc91097c75 100644 --- a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_16_plan.json +++ b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_16_plan.json @@ -754,36 +754,7 @@ } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - } - }, - "rootReference": { - } - } - }, { - "selection": { - "directReference": { - "structField": { - "field": 1 - } - }, - "rootReference": { - } - } - }, { - "selection": { - "directReference": { - "structField": { - "field": 2 - } - }, - "rootReference": { - } - } - }] + "expressionReferences": [0, 1, 2] }], "measures": [{ "measure": { @@ -809,6 +780,36 @@ } }] } + }], + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + } + }, + "rootReference": { + } + } + }, { + "selection": { + "directReference": { + "structField": { + "field": 1 + } + }, + "rootReference": { + } + } + }, { + "selection": { + "directReference": { + "structField": { + "field": 2 + } + }, + "rootReference": { + } + } }] } }, diff --git a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_18_plan.json b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_18_plan.json index 7f0ff438db78e..a6455b458ee29 100644 --- a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_18_plan.json +++ b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_18_plan.json @@ -441,16 +441,7 @@ } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - } - }, - "rootReference": { - } - } - }] + "expressionReferences": [0] }], "measures": [{ "measure": { @@ -478,6 +469,16 @@ } }] } + }], + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + } + }, + "rootReference": { + } + } }] } }, @@ -678,56 +679,7 @@ } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - } - }, - "rootReference": { - } - } - }, { - "selection": { - "directReference": { - "structField": { - "field": 1 - } - }, - "rootReference": { - } - } - }, { - "selection": { - "directReference": { - "structField": { - "field": 2 - } - }, - "rootReference": { - } - } - }, { - "selection": { - "directReference": { - "structField": { - "field": 3 - } - }, - "rootReference": { - } - } - }, { - "selection": { - "directReference": { - "structField": { - "field": 4 - } - }, - "rootReference": { - } - } - }] + "expressionReferences": [0, 1, 2, 3, 4] }], "measures": [{ "measure": { @@ -755,6 +707,56 @@ } }] } + }], + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + } + }, + "rootReference": { + } + } + }, { + "selection": { + "directReference": { + "structField": { + "field": 1 + } + }, + "rootReference": { + } + } + }, { + "selection": { + "directReference": { + "structField": { + "field": 2 + } + }, + "rootReference": { + } + } + }, { + "selection": { + "directReference": { + "structField": { + "field": 3 + } + }, + "rootReference": { + } + } + }, { + "selection": { + "directReference": { + "structField": { + "field": 4 + } + }, + "rootReference": { + } + } }] } }, @@ -787,7 +789,11 @@ }] } }, - "count": "100" + "countExpr": { + "literal": { + "i64": "100" + } + } } }, "names": ["C_NAME", "C_CUSTKEY", "O_ORDERKEY", "O_ORDERDATE", "O_TOTALPRICE", "EXPR$5"] diff --git a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_21_plan.json b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_21_plan.json index c3d4fc3bcb873..9d4b1dab24c54 100644 --- a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_21_plan.json +++ b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_21_plan.json @@ -988,16 +988,7 @@ } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - } - }, - "rootReference": { - } - } - }] + "expressionReferences": [0] }], "measures": [{ "measure": { @@ -1010,6 +1001,16 @@ }, "invocation": "AGGREGATION_INVOCATION_ALL" } + }], + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + } + }, + "rootReference": { + } + } }] } }, @@ -1041,7 +1042,11 @@ }] } }, - "count": "100" + "countExpr": { + "literal": { + "i64": "100" + } + } } }, "names": ["S_NAME", "NUMWAIT"] diff --git a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_22_plan.json b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_22_plan.json index fcd61b23ae2de..e1bcc23bfdba6 100644 --- a/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_22_plan.json +++ b/datafusion/substrait/tests/testdata/tpch_substrait_plans/query_22_plan.json @@ -1438,16 +1438,7 @@ } }, "groupings": [{ - "groupingExpressions": [{ - "selection": { - "directReference": { - "structField": { - } - }, - "rootReference": { - } - } - }] + "expressionReferences": [0] }], "measures": [{ "measure": { @@ -1486,6 +1477,16 @@ } }] } + }], + "groupingExpressions": [{ + "selection": { + "directReference": { + "structField": { + } + }, + "rootReference": { + } + } }] } }, diff --git a/datafusion/substrait/tests/utils.rs b/datafusion/substrait/tests/utils.rs index 89363931f1594..1e1f3fec1b569 100644 --- a/datafusion/substrait/tests/utils.rs +++ b/datafusion/substrait/tests/utils.rs @@ -157,7 +157,6 @@ pub mod test { Ok(()) } - #[expect(deprecated)] fn collect_schemas_from_rel(&mut self, rel: &Rel) -> Result<()> { let rel_type = rel .rel_type @@ -195,10 +194,8 @@ pub mod test { } RelType::Aggregate(a) => { self.apply(a.input.as_ref().map(|b| b.as_ref()))?; - for grouping in a.groupings.iter() { - for expr in grouping.grouping_expressions.iter() { - self.collect_schemas_from_expr(expr)? - } + for expr in a.grouping_expressions.iter() { + self.collect_schemas_from_expr(expr)? } for measure in a.measures.iter() { if let Some(agg_fn) = measure.measure.as_ref() { @@ -483,9 +480,7 @@ pub mod test { } } RexType::DynamicParameter(_) => {} - // Enum is deprecated - #[expect(deprecated)] - RexType::Enum(_) => {} + RexType::ExecutionContextVariable(_) => {} RexType::Lambda(_) | RexType::LambdaInvocation(_) => {} } Ok(()) From 53a853706608f9e95946fda5a6db10e1d27dfbb8 Mon Sep 17 00:00:00 2001 From: Aleksandr Efimov Date: Tue, 1 Sep 2026 22:49:26 +0300 Subject: [PATCH 2/2] Emit CurrentRow for a zero window frame distance, read a zero offset_expr as CurrentRow, and read RelCommon from UpdateRel --- .../consumer/expr/window_function.rs | 65 +++++++++++++++++-- .../src/logical_plan/consumer/rel/mod.rs | 2 +- .../producer/expr/window_function.rs | 47 +++++++++++--- 3 files changed, 99 insertions(+), 15 deletions(-) diff --git a/datafusion/substrait/src/logical_plan/consumer/expr/window_function.rs b/datafusion/substrait/src/logical_plan/consumer/expr/window_function.rs index e366599e0770b..b0cbcbf7c4128 100644 --- a/datafusion/substrait/src/logical_plan/consumer/expr/window_function.rs +++ b/datafusion/substrait/src/logical_plan/consumer/expr/window_function.rs @@ -146,6 +146,9 @@ fn from_substrait_bound( #[expect(deprecated)] let offset = bound_offset(bound.offset, bound.offset_expr.as_deref())?; + let Some(offset) = offset else { + return Ok(WindowFrameBound::CurrentRow); + }; if offset <= 0 { return plan_err!("Preceding bound must be positive"); } @@ -157,6 +160,9 @@ fn from_substrait_bound( #[expect(deprecated)] let offset = bound_offset(bound.offset, bound.offset_expr.as_deref())?; + let Some(offset) = offset else { + return Ok(WindowFrameBound::CurrentRow); + }; if offset <= 0 { return plan_err!("Following bound must be positive"); } @@ -187,12 +193,14 @@ fn from_substrait_bound( /// Reads the distance of a window frame bound. /// /// The specification requires a consumer to use `offset_expr` when it is set and -/// to ignore `offset`. DataFusion frame bounds hold a literal, so an expression -/// that is not an int64 literal cannot be represented. +/// to ignore `offset`, and defines a zero `offset_expr` as equivalent to +/// CurrentRow, which `None` reports here. DataFusion frame bounds hold a +/// literal, so an expression that is not an int64 literal cannot be +/// represented. fn bound_offset( offset: i64, offset_expr: Option<&Expression>, -) -> datafusion::common::Result { +) -> datafusion::common::Result> { match offset_expr { Some(Expression { rex_type: @@ -200,10 +208,57 @@ fn bound_offset( literal_type: Some(LiteralType::I64(value)), .. })), - }) => Ok(*value), + }) => Ok((*value != 0).then_some(*value)), Some(_) => not_impl_err!( "Window frame bound offsets other than int64 literals are not supported" ), - None => Ok(offset), + None => Ok(Some(offset)), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn i64_literal(value: i64) -> Expression { + Expression { + rex_type: Some(RexType::Literal(Literal { + literal_type: Some(LiteralType::I64(value)), + ..Default::default() + })), + } + } + + /// A zero `offset_expr` is defined as equivalent to CurrentRow, so it is + /// read as such rather than rejected as a non-positive distance. + #[test] + fn zero_offset_expression_reads_as_current_row() { + #[expect(deprecated)] + let bound = Bound { + kind: Some(BoundKind::Preceding(Box::new(SubstraitBound::Preceding { + offset: 0, + offset_expr: Some(Box::new(i64_literal(0))), + }))), + }; + assert_eq!( + from_substrait_bound(Some(&bound), true).unwrap(), + WindowFrameBound::CurrentRow + ); + } + + /// When `offset_expr` is set the consumer must use it and ignore `offset`. + #[test] + fn offset_expression_wins_over_the_deprecated_offset() { + #[expect(deprecated)] + let bound = Bound { + kind: Some(BoundKind::Preceding(Box::new(SubstraitBound::Preceding { + offset: 7, + offset_expr: Some(Box::new(i64_literal(3))), + }))), + }; + assert_eq!( + from_substrait_bound(Some(&bound), true).unwrap(), + WindowFrameBound::Preceding(ScalarValue::UInt64(Some(3))) + ); } } diff --git a/datafusion/substrait/src/logical_plan/consumer/rel/mod.rs b/datafusion/substrait/src/logical_plan/consumer/rel/mod.rs index 1c5e280eb487a..ec82fbc642d92 100644 --- a/datafusion/substrait/src/logical_plan/consumer/rel/mod.rs +++ b/datafusion/substrait/src/logical_plan/consumer/rel/mod.rs @@ -157,7 +157,7 @@ fn retrieve_rel_common(rel: &Rel) -> Option<&RelCommon> { RelType::Window(w) => w.common.as_ref(), RelType::Exchange(e) => e.common.as_ref(), RelType::Expand(e) => e.common.as_ref(), - RelType::Update(_) => None, + RelType::Update(u) => u.common.as_ref(), }, } } diff --git a/datafusion/substrait/src/logical_plan/producer/expr/window_function.rs b/datafusion/substrait/src/logical_plan/producer/expr/window_function.rs index c711f695fca02..4b3121e777501 100644 --- a/datafusion/substrait/src/logical_plan/producer/expr/window_function.rs +++ b/datafusion/substrait/src/logical_plan/producer/expr/window_function.rs @@ -154,28 +154,39 @@ fn to_substrait_bound(bound: &WindowFrameBound) -> datafusion::common::Result { let offset = to_substrait_bound_offset(s)?; + if offset == 0 { + // A zero distance is equivalent to CurrentRow, and `offset` + // cannot represent zero, so the specification asks producers to + // emit CurrentRow instead of a zero bound. + return Ok(Bound { + kind: Some(BoundKind::CurrentRow(SubstraitBound::CurrentRow {})), + }); + } #[expect(deprecated)] Ok(Bound { kind: Some(BoundKind::Preceding(Box::new(SubstraitBound::Preceding { - // `offset` carries the int64-literal equivalent for consumers - // that do not read `offset_expr` yet. A zero distance is - // equivalent to CurrentRow and `offset` cannot represent it, - // so the specification asks producers not to write a zero - // `offset_expr`. + // `offset` carries the int64-literal equivalent for + // consumers that do not read `offset_expr` yet. offset, - offset_expr: (offset != 0) - .then(|| Box::new(bound_offset_expr(offset))), + offset_expr: Some(Box::new(bound_offset_expr(offset))), }))), }) } WindowFrameBound::Following(s) => { let offset = to_substrait_bound_offset(s)?; + if offset == 0 { + // A zero distance is equivalent to CurrentRow, and `offset` + // cannot represent zero, so the specification asks producers to + // emit CurrentRow instead of a zero bound. + return Ok(Bound { + kind: Some(BoundKind::CurrentRow(SubstraitBound::CurrentRow {})), + }); + } #[expect(deprecated)] Ok(Bound { kind: Some(BoundKind::Following(Box::new(SubstraitBound::Following { offset, - offset_expr: (offset != 0) - .then(|| Box::new(bound_offset_expr(offset))), + offset_expr: Some(Box::new(bound_offset_expr(offset))), }))), }) } @@ -216,6 +227,24 @@ mod tests { use super::*; use datafusion::common::assert_contains; + #[test] + fn zero_distance_bounds_become_current_row() { + // `offset` cannot represent zero and the specification asks producers to + // emit CurrentRow rather than a zero bound, so neither field is written. + let frame = WindowFrame::new_bounds( + WindowFrameUnits::Rows, + WindowFrameBound::Preceding(ScalarValue::UInt64(Some(0))), + WindowFrameBound::Following(ScalarValue::UInt64(Some(0))), + ); + let (lower, upper) = to_substrait_bounds(&frame).unwrap(); + for bound in [lower, upper] { + assert_eq!( + bound.kind, + Some(BoundKind::CurrentRow(SubstraitBound::CurrentRow {})) + ); + } + } + #[test] #[expect(deprecated)] fn window_frame_offsets() {