From c33e2aaf69a0680c2e122f7f2ea97b50f2fbf5ba Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Mon, 1 Apr 2024 07:19:45 -0400 Subject: [PATCH 1/4] Revert "use alias (#9894)" This reverts commit 9487ca057353370aa75895453c92bb40b9f33ac6. --- datafusion/sqllogictest/test_files/expr.slt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/datafusion/sqllogictest/test_files/expr.slt b/datafusion/sqllogictest/test_files/expr.slt index 60ab4777883e..2e0cbf50cab9 100644 --- a/datafusion/sqllogictest/test_files/expr.slt +++ b/datafusion/sqllogictest/test_files/expr.slt @@ -2288,7 +2288,7 @@ select struct(time,load1,load2,host) from t1; # can have an aggregate function with an inner coalesce query TR -select t2.info['c3'] as host, sum(coalesce(t2.info)['c1']) from (select struct(time,load1,load2,host) as info from t1) t2 where t2.info['c3'] IS NOT NULL group by t2.info['c3'] order by host; +select t2."struct(t1.time,t1.load1,t1.load2,t1.host)"['c3'] as host, sum(coalesce(t2."struct(t1.time,t1.load1,t1.load2,t1.host)")['c1']) from (select struct(time,load1,load2,host) from t1) t2 where t2."struct(t1.time,t1.load1,t1.load2,t1.host)"['c3'] IS NOT NULL group by t2."struct(t1.time,t1.load1,t1.load2,t1.host)"['c3'] order by host; ---- host1 1.1 host2 2.2 @@ -2296,7 +2296,7 @@ host3 3.3 # can have an aggregate function with an inner CASE WHEN query TR -select t2.info['c3'] as host, sum((case when t2.info['c3'] is not null then t2.info end)['c2']) from (select struct(time,load1,load2,host) as info from t1) t2 where t2.info['c3'] IS NOT NULL group by t2.info['c3'] order by host; +select t2."struct(t1.time,t1.load1,t1.load2,t1.host)"['c3'] as host, sum((case when t2."struct(t1.time,t1.load1,t1.load2,t1.host)"['c3'] is not null then t2."struct(t1.time,t1.load1,t1.load2,t1.host)" end)['c2']) from (select struct(time,load1,load2,host) from t1) t2 where t2."struct(t1.time,t1.load1,t1.load2,t1.host)"['c3'] IS NOT NULL group by t2."struct(t1.time,t1.load1,t1.load2,t1.host)"['c3'] order by host; ---- host1 101 host2 202 @@ -2304,7 +2304,7 @@ host3 303 # can have 2 projections with aggr(short_circuited), with different short-circuited expr query TRR -select t2.info['c3'] as host, sum(coalesce(t2.info)['c1']), sum((case when t2.info['c3'] is not null then t2.info end)['c2']) from (select struct(time,load1,load2,host) as info from t1) t2 where t2.info['c3'] IS NOT NULL group by t2.info['c3'] order by host; +select t2."struct(t1.time,t1.load1,t1.load2,t1.host)"['c3'] as host, sum(coalesce(t2."struct(t1.time,t1.load1,t1.load2,t1.host)")['c1']), sum((case when t2."struct(t1.time,t1.load1,t1.load2,t1.host)"['c3'] is not null then t2."struct(t1.time,t1.load1,t1.load2,t1.host)" end)['c2']) from (select struct(time,load1,load2,host) from t1) t2 where t2."struct(t1.time,t1.load1,t1.load2,t1.host)"['c3'] IS NOT NULL group by t2."struct(t1.time,t1.load1,t1.load2,t1.host)"['c3'] order by host; ---- host1 1.1 101 host2 2.2 202 @@ -2312,7 +2312,7 @@ host3 3.3 303 # can have 2 projections with aggr(short_circuited), with the same short-circuited expr (e.g. CASE WHEN) query TRR -select t2.info['c3'] as host, sum((case when t2.info['c3'] is not null then t2.info end)['c1']), sum((case when t2.info['c3'] is not null then t2.info end)['c2']) from (select struct(time,load1,load2,host) as info from t1) t2 where t2.info['c3'] IS NOT NULL group by t2.info['c3'] order by host; +select t2."struct(t1.time,t1.load1,t1.load2,t1.host)"['c3'] as host, sum((case when t2."struct(t1.time,t1.load1,t1.load2,t1.host)"['c3'] is not null then t2."struct(t1.time,t1.load1,t1.load2,t1.host)" end)['c1']), sum((case when t2."struct(t1.time,t1.load1,t1.load2,t1.host)"['c3'] is not null then t2."struct(t1.time,t1.load1,t1.load2,t1.host)" end)['c2']) from (select struct(time,load1,load2,host) from t1) t2 where t2."struct(t1.time,t1.load1,t1.load2,t1.host)"['c3'] IS NOT NULL group by t2."struct(t1.time,t1.load1,t1.load2,t1.host)"['c3'] order by host; ---- host1 1.1 101 host2 2.2 202 @@ -2320,7 +2320,7 @@ host3 3.3 303 # can have 2 projections with aggr(short_circuited), with the same short-circuited expr (e.g. coalesce) query TRR -select t2.info['c3'] as host, sum(coalesce(t2.info)['c1']), sum(coalesce(t2.info)['c2']) from (select struct(time,load1,load2,host) as info from t1) t2 where t2.info['c3'] IS NOT NULL group by t2.info['c3'] order by host; +select t2."struct(t1.time,t1.load1,t1.load2,t1.host)"['c3'] as host, sum(coalesce(t2."struct(t1.time,t1.load1,t1.load2,t1.host)")['c1']), sum(coalesce(t2."struct(t1.time,t1.load1,t1.load2,t1.host)")['c2']) from (select struct(time,load1,load2,host) from t1) t2 where t2."struct(t1.time,t1.load1,t1.load2,t1.host)"['c3'] IS NOT NULL group by t2."struct(t1.time,t1.load1,t1.load2,t1.host)"['c3'] order by host; ---- host1 1.1 101 host2 2.2 202 From c0895b5975333642f733907d4d0185e874394a22 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Mon, 1 Apr 2024 07:36:27 -0400 Subject: [PATCH 2/4] Use `struct` instead of `named_struct` when there are no aliases --- datafusion/sql/src/expr/mod.rs | 47 +++++++++++++++++++ .../sqllogictest/test_files/explain.slt | 4 +- datafusion/sqllogictest/test_files/struct.slt | 6 +-- 3 files changed, 52 insertions(+), 5 deletions(-) diff --git a/datafusion/sql/src/expr/mod.rs b/datafusion/sql/src/expr/mod.rs index 064578ad51d6..4150e3145025 100644 --- a/datafusion/sql/src/expr/mod.rs +++ b/datafusion/sql/src/expr/mod.rs @@ -593,6 +593,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { } } + /// Parses a struct(..) expression fn parse_struct( &self, values: Vec, @@ -603,6 +604,23 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { if !fields.is_empty() { return not_impl_err!("Struct fields are not supported yet"); } + + if values.iter().any(|value| matches!(value, SQLExpr::Named { .. })) { + self.create_named_struct(values, input_schema, planner_context) + } + else { + self.create_struct(values, input_schema, planner_context) + } + } + + // Handles a call to struct(...) where the arguments are named. For example + // `struct (v as foo, v2 as bar)` by creating a call to the `named_struct` function + fn create_named_struct( + &self, + values: Vec, + input_schema: &DFSchema, + planner_context: &mut PlannerContext, + ) -> Result { let args = values .into_iter() .enumerate() @@ -647,6 +665,35 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { ))) } + // Handles a call to struct(...) where the arguments are not named. For example + // `struct (v, v2)` by creating a call to the `struct` function + // which will create a struct with fields named `c0`, `c1`, etc. + fn create_struct( + &self, + values: Vec, + input_schema: &DFSchema, + planner_context: &mut PlannerContext, + ) -> Result { + let args = values + .into_iter() + .map(|value| { + self.sql_expr_to_logical_expr(value, input_schema, planner_context) + }) + .collect::>>()?; + let struct_func = self + .context_provider + .get_function_meta("struct") + .ok_or_else(|| { + internal_datafusion_err!("Unable to find expected 'struct' function") + })?; + + Ok(Expr::ScalarFunction(ScalarFunction::new_udf( + struct_func, + args, + ))) + } + + fn parse_array_agg( &self, array_agg: ArrayAgg, diff --git a/datafusion/sqllogictest/test_files/explain.slt b/datafusion/sqllogictest/test_files/explain.slt index 4653250cf93f..b7ad36dace16 100644 --- a/datafusion/sqllogictest/test_files/explain.slt +++ b/datafusion/sqllogictest/test_files/explain.slt @@ -390,8 +390,8 @@ query TT explain select struct(1, 2.3, 'abc'); ---- logical_plan -Projection: Struct({c0:1,c1:2.3,c2:abc}) AS named_struct(Utf8("c0"),Int64(1),Utf8("c1"),Float64(2.3),Utf8("c2"),Utf8("abc")) +Projection: Struct({c0:1,c1:2.3,c2:abc}) AS struct(Int64(1),Float64(2.3),Utf8("abc")) --EmptyRelation physical_plan -ProjectionExec: expr=[{c0:1,c1:2.3,c2:abc} as named_struct(Utf8("c0"),Int64(1),Utf8("c1"),Float64(2.3),Utf8("c2"),Utf8("abc"))] +ProjectionExec: expr=[{c0:1,c1:2.3,c2:abc} as struct(Int64(1),Float64(2.3),Utf8("abc"))] --PlaceholderRowExec diff --git a/datafusion/sqllogictest/test_files/struct.slt b/datafusion/sqllogictest/test_files/struct.slt index 2e0b699f6dd6..8a6256add6ac 100644 --- a/datafusion/sqllogictest/test_files/struct.slt +++ b/datafusion/sqllogictest/test_files/struct.slt @@ -85,10 +85,10 @@ query TT explain select struct(a, b, c) from values; ---- logical_plan -Projection: named_struct(Utf8("c0"), values.a, Utf8("c1"), values.b, Utf8("c2"), values.c) +Projection: struct(values.a, values.b, values.c) --TableScan: values projection=[a, b, c] physical_plan -ProjectionExec: expr=[named_struct(c0, a@0, c1, b@1, c2, c@2) as named_struct(Utf8("c0"),values.a,Utf8("c1"),values.b,Utf8("c2"),values.c)] +ProjectionExec: expr=[struct(a@0, b@1, c@2) as struct(values.a,values.b,values.c)] --MemoryExec: partitions=1, partition_sizes=[1] # error on 0 arguments @@ -179,4 +179,4 @@ drop table values; query T select arrow_typeof(named_struct('first', 1, 'second', 2, 'third', 3)); ---- -Struct([Field { name: "first", data_type: Int64, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: {} }, Field { name: "second", data_type: Int64, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: {} }, Field { name: "third", data_type: Int64, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: {} }]) \ No newline at end of file +Struct([Field { name: "first", data_type: Int64, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: {} }, Field { name: "second", data_type: Int64, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: {} }, Field { name: "third", data_type: Int64, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: {} }]) From 10a40dd89cc436d36739ed1bff84bafec7610dd5 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Mon, 1 Apr 2024 08:47:07 -0400 Subject: [PATCH 3/4] Update docs --- docs/source/user-guide/sql/scalar_functions.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/source/user-guide/sql/scalar_functions.md b/docs/source/user-guide/sql/scalar_functions.md index e2e129a2e2d1..62b81ea7ea4b 100644 --- a/docs/source/user-guide/sql/scalar_functions.md +++ b/docs/source/user-guide/sql/scalar_functions.md @@ -3336,6 +3336,16 @@ select * from t; | 3 | 4 | +---+---+ +-- use default names `c0`, `c1` +❯ select struct(a, b) from t; ++-----------------+ +| struct(t.a,t.b) | ++-----------------+ +| {c0: 1, c1: 2} | +| {c0: 3, c1: 4} | ++-----------------+ + +-- name the first field `field_a` select struct(a as field_a, b) from t; +--------------------------------------------------+ | named_struct(Utf8("field_a"),t.a,Utf8("c1"),t.b) | From 792212cd216a062741239c6087e95cae90d78ceb Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Mon, 1 Apr 2024 08:51:56 -0400 Subject: [PATCH 4/4] fmt --- datafusion/sql/src/expr/mod.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/datafusion/sql/src/expr/mod.rs b/datafusion/sql/src/expr/mod.rs index 4150e3145025..43aea1f2b75a 100644 --- a/datafusion/sql/src/expr/mod.rs +++ b/datafusion/sql/src/expr/mod.rs @@ -605,10 +605,12 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { return not_impl_err!("Struct fields are not supported yet"); } - if values.iter().any(|value| matches!(value, SQLExpr::Named { .. })) { + if values + .iter() + .any(|value| matches!(value, SQLExpr::Named { .. })) + { self.create_named_struct(values, input_schema, planner_context) - } - else { + } else { self.create_struct(values, input_schema, planner_context) } } @@ -693,7 +695,6 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { ))) } - fn parse_array_agg( &self, array_agg: ArrayAgg,