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..b0cbcbf7c4128 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,32 @@ 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())?; + let Some(offset) = offset else { + return Ok(WindowFrameBound::CurrentRow); + }; + 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())?; + let Some(offset) = offset else { + return Ok(WindowFrameBound::CurrentRow); + }; + 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 +189,76 @@ 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`, 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> { + match offset_expr { + Some(Expression { + rex_type: + Some(RexType::Literal(Literal { + literal_type: Some(LiteralType::I64(value)), + .. + })), + }) => Ok((*value != 0).then_some(*value)), + Some(_) => not_impl_err!( + "Window frame bound offsets other than int64 literals are not supported" + ), + 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/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..ec82fbc642d92 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(), @@ -155,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/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..4b3121e777501 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,56 @@ 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)?; + 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. + 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: Some(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 +228,25 @@ mod tests { 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() { for value in [ ScalarValue::UInt8(Some(1)), @@ -207,9 +266,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(())