From 3ce0edb7619342b3aa6350cd2d59685c1508a2f0 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Mon, 25 Apr 2022 21:20:56 -0600 Subject: [PATCH 1/7] Add Expr::InSubquery and Expr::ScalarSubquery --- .../core/src/datasource/listing/helpers.rs | 2 + datafusion/core/src/logical_plan/builder.rs | 48 +++++++++++++++++++ .../core/src/logical_plan/expr_rewriter.rs | 12 ++++- .../core/src/logical_plan/expr_visitor.rs | 4 +- datafusion/core/src/logical_plan/mod.rs | 15 +++--- .../src/optimizer/common_subexpr_eliminate.rs | 10 +++- .../src/optimizer/simplify_expressions.rs | 4 +- datafusion/core/src/optimizer/utils.rs | 12 +++-- datafusion/core/src/physical_plan/planner.rs | 8 +++- datafusion/core/src/prelude.rs | 9 ++-- datafusion/core/src/sql/utils.rs | 12 ++++- datafusion/expr/src/expr.rs | 44 +++++++++++++++-- datafusion/expr/src/expr_fn.rs | 36 +++++++++++++- datafusion/expr/src/expr_schema.rs | 14 +++++- datafusion/expr/src/lib.rs | 13 ++--- 15 files changed, 211 insertions(+), 32 deletions(-) diff --git a/datafusion/core/src/datasource/listing/helpers.rs b/datafusion/core/src/datasource/listing/helpers.rs index e366169650de0..d066d8d9dd2ce 100644 --- a/datafusion/core/src/datasource/listing/helpers.rs +++ b/datafusion/core/src/datasource/listing/helpers.rs @@ -93,6 +93,8 @@ impl ExpressionVisitor for ApplicabilityVisitor<'_> { | Expr::Between { .. } | Expr::InList { .. } | Expr::Exists { .. } + | Expr::InSubquery { .. } + | Expr::ScalarSubquery(_) | Expr::GetIndexedField { .. } | Expr::Case { .. } => Recursion::Continue(self), diff --git a/datafusion/core/src/logical_plan/builder.rs b/datafusion/core/src/logical_plan/builder.rs index c303cdec8639f..a225c8f4853c9 100644 --- a/datafusion/core/src/logical_plan/builder.rs +++ b/datafusion/core/src/logical_plan/builder.rs @@ -1366,6 +1366,54 @@ mod tests { Ok(()) } + #[test] + fn filter_in_subquery() -> Result<()> { + let foo = test_table_scan_with_name("foo")?; + let bar = test_table_scan_with_name("bar")?; + + let subquery = LogicalPlanBuilder::from(foo) + .project(vec![col("a")])? + .filter(col("a").eq(col("bar.a")))? + .build()?; + + // SELECT a FROM bar WHERE a IN (SELECT a FROM foo WHERE a = bar.a) + let outer_query = LogicalPlanBuilder::from(bar) + .project(vec![col("a")])? + .filter(in_subquery(col("a"), Arc::new(subquery)))? + .build()?; + + let expected = "Filter: #bar.a IN (Subquery: Filter: #foo.a = #bar.a\ + \n Projection: #foo.a\ + \n TableScan: foo projection=None)\ + \n Projection: #bar.a\ + \n TableScan: bar projection=None"; + assert_eq!(expected, format!("{:?}", outer_query)); + + Ok(()) + } + + #[test] + fn select_scalar_subquery() -> Result<()> { + let foo = test_table_scan_with_name("foo")?; + let bar = test_table_scan_with_name("bar")?; + + let subquery = LogicalPlanBuilder::from(foo) + .project(vec![col("a")])? + .filter(col("a").eq(col("bar.a")))? + .build()?; + + // SELECT (SELECT a FROM foo WHERE a = bar.a) FROM bar + let outer_query = LogicalPlanBuilder::from(bar) + .project(vec![scalar_subquery(Arc::new(subquery))])? + .build()?; + + let expected = + "Projection: (ScalarSubquery TBD )\n TableScan: bar projection=None"; + assert_eq!(expected, format!("{:?}", outer_query)); + + Ok(()) + } + #[test] fn projection_non_unique_names() -> Result<()> { let plan = LogicalPlanBuilder::scan_empty( diff --git a/datafusion/core/src/logical_plan/expr_rewriter.rs b/datafusion/core/src/logical_plan/expr_rewriter.rs index e99fc7e66cf83..ef6d8dff9c8ff 100644 --- a/datafusion/core/src/logical_plan/expr_rewriter.rs +++ b/datafusion/core/src/logical_plan/expr_rewriter.rs @@ -111,7 +111,17 @@ impl ExprRewritable for Expr { let expr = match self { Expr::Alias(expr, name) => Expr::Alias(rewrite_boxed(expr, rewriter)?, name), Expr::Column(_) => self.clone(), - Expr::Exists(_) => self.clone(), + Expr::Exists { .. } => self.clone(), + Expr::InSubquery { + expr, + subquery, + negated, + } => Expr::InSubquery { + expr: rewrite_boxed(expr, rewriter)?, + subquery, + negated, + }, + Expr::ScalarSubquery(_) => self.clone(), Expr::ScalarVariable(ty, names) => Expr::ScalarVariable(ty, names), Expr::Literal(value) => Expr::Literal(value), Expr::BinaryExpr { left, op, right } => Expr::BinaryExpr { diff --git a/datafusion/core/src/logical_plan/expr_visitor.rs b/datafusion/core/src/logical_plan/expr_visitor.rs index bfab0ca04c752..7c578da19b75a 100644 --- a/datafusion/core/src/logical_plan/expr_visitor.rs +++ b/datafusion/core/src/logical_plan/expr_visitor.rs @@ -106,7 +106,9 @@ impl ExprVisitable for Expr { Expr::Column(_) | Expr::ScalarVariable(_, _) | Expr::Literal(_) - | Expr::Exists(_) + | Expr::Exists { .. } + | Expr::InSubquery { .. } + | Expr::ScalarSubquery(_) | Expr::Wildcard | Expr::QualifiedWildcard { .. } => Ok(visitor), Expr::BinaryExpr { left, right, .. } => { diff --git a/datafusion/core/src/logical_plan/mod.rs b/datafusion/core/src/logical_plan/mod.rs index d933b0229b1f2..bcf0a161447f1 100644 --- a/datafusion/core/src/logical_plan/mod.rs +++ b/datafusion/core/src/logical_plan/mod.rs @@ -41,13 +41,14 @@ pub use expr::{ avg, bit_length, btrim, call_fn, case, ceil, character_length, chr, coalesce, col, columnize_expr, combine_filters, concat, concat_expr, concat_ws, concat_ws_expr, cos, count, count_distinct, create_udaf, create_udf, date_part, date_trunc, digest, - exists, exp, exprlist_to_fields, floor, in_list, initcap, left, length, lit, - lit_timestamp_nano, ln, log10, log2, lower, lpad, ltrim, max, md5, min, now, - now_expr, nullif, octet_length, or, random, regexp_match, regexp_replace, repeat, - replace, reverse, right, round, rpad, rtrim, sha224, sha256, sha384, sha512, signum, - sin, split_part, sqrt, starts_with, strpos, substr, sum, tan, to_hex, - to_timestamp_micros, to_timestamp_millis, to_timestamp_seconds, translate, trim, - trunc, unalias, upper, when, Column, Expr, ExprSchema, Literal, + exists, exp, exprlist_to_fields, floor, in_list, in_subquery, initcap, left, length, + lit, lit_timestamp_nano, ln, log10, log2, lower, lpad, ltrim, max, md5, min, + not_exists, not_in_subquery, now, now_expr, nullif, octet_length, or, random, + regexp_match, regexp_replace, repeat, replace, reverse, right, round, rpad, rtrim, + scalar_subquery, sha224, sha256, sha384, sha512, signum, sin, split_part, sqrt, + starts_with, strpos, substr, sum, tan, to_hex, to_timestamp_micros, + to_timestamp_millis, to_timestamp_seconds, translate, trim, trunc, unalias, upper, + when, Column, Expr, ExprSchema, Literal, }; pub use expr_rewriter::{ normalize_col, normalize_cols, replace_col, rewrite_sort_cols_by_aggs, diff --git a/datafusion/core/src/optimizer/common_subexpr_eliminate.rs b/datafusion/core/src/optimizer/common_subexpr_eliminate.rs index 4a9bf8e913a0e..a9983cdf1e08e 100644 --- a/datafusion/core/src/optimizer/common_subexpr_eliminate.rs +++ b/datafusion/core/src/optimizer/common_subexpr_eliminate.rs @@ -460,8 +460,16 @@ impl ExprIdentifierVisitor<'_> { desc.push_str("InList-"); desc.push_str(&negated.to_string()); } - Expr::Exists(_) => { + Expr::Exists { negated, .. } => { desc.push_str("Exists-"); + desc.push_str(&negated.to_string()); + } + Expr::InSubquery { negated, .. } => { + desc.push_str("InSubquery-"); + desc.push_str(&negated.to_string()); + } + Expr::ScalarSubquery(_) => { + desc.push_str("ScalarSubquery-"); } Expr::Wildcard => { desc.push_str("Wildcard-"); diff --git a/datafusion/core/src/optimizer/simplify_expressions.rs b/datafusion/core/src/optimizer/simplify_expressions.rs index 93d9fb506177e..4572246190071 100644 --- a/datafusion/core/src/optimizer/simplify_expressions.rs +++ b/datafusion/core/src/optimizer/simplify_expressions.rs @@ -375,7 +375,9 @@ impl<'a> ConstEvaluator<'a> { | Expr::AggregateUDF { .. } | Expr::ScalarVariable(_, _) | Expr::Column(_) - | Expr::Exists(_) + | Expr::Exists { .. } + | Expr::InSubquery { .. } + | Expr::ScalarSubquery(_) | Expr::WindowFunction { .. } | Expr::Sort { .. } | Expr::Wildcard diff --git a/datafusion/core/src/optimizer/utils.rs b/datafusion/core/src/optimizer/utils.rs index 939af80415627..df36761fec40b 100644 --- a/datafusion/core/src/optimizer/utils.rs +++ b/datafusion/core/src/optimizer/utils.rs @@ -85,7 +85,9 @@ impl ExpressionVisitor for ColumnNameVisitor<'_> { | Expr::AggregateFunction { .. } | Expr::AggregateUDF { .. } | Expr::InList { .. } - | Expr::Exists(_) + | Expr::Exists { .. } + | Expr::InSubquery { .. } + | Expr::ScalarSubquery(_) | Expr::Wildcard | Expr::QualifiedWildcard { .. } | Expr::GetIndexedField { .. } => {} @@ -371,7 +373,9 @@ pub fn expr_sub_expressions(expr: &Expr) -> Result> { } Ok(expr_list) } - Expr::Exists(_) => Ok(vec![]), + Expr::Exists { .. } => Ok(vec![]), + Expr::InSubquery { expr, .. } => Ok(vec![expr.as_ref().to_owned()]), + Expr::ScalarSubquery(_) => Ok(vec![]), Expr::Wildcard { .. } => Err(DataFusionError::Internal( "Wildcard expressions are not valid in a logical query plan".to_owned(), )), @@ -506,7 +510,9 @@ pub fn rewrite_expression(expr: &Expr, expressions: &[Expr]) -> Result { Expr::Column(_) | Expr::Literal(_) | Expr::InList { .. } - | Expr::Exists(_) + | Expr::Exists { .. } + | Expr::InSubquery { .. } + | Expr::ScalarSubquery(_) | Expr::ScalarVariable(_, _) => Ok(expr.clone()), Expr::Sort { asc, nulls_first, .. diff --git a/datafusion/core/src/physical_plan/planner.rs b/datafusion/core/src/physical_plan/planner.rs index 84785777b016c..4cca768c7a341 100644 --- a/datafusion/core/src/physical_plan/planner.rs +++ b/datafusion/core/src/physical_plan/planner.rs @@ -186,9 +186,15 @@ fn create_physical_name(e: &Expr, is_first_expr: bool) -> Result { Ok(format!("{} IN ({:?})", expr, list)) } } - Expr::Exists(_) => Err(DataFusionError::NotImplemented( + Expr::Exists { .. } => Err(DataFusionError::NotImplemented( "EXISTS is not yet supported in the physical plan".to_string(), )), + Expr::InSubquery { .. } => Err(DataFusionError::NotImplemented( + "IN subquery is not yet supported in the physical plan".to_string(), + )), + Expr::ScalarSubquery(_) => Err(DataFusionError::NotImplemented( + "Scalar subqueries are not yet supported in the physical plan".to_string(), + )), Expr::Between { expr, negated, diff --git a/datafusion/core/src/prelude.rs b/datafusion/core/src/prelude.rs index e0c418417c5c4..11ca3332d5a5e 100644 --- a/datafusion/core/src/prelude.rs +++ b/datafusion/core/src/prelude.rs @@ -33,8 +33,9 @@ pub use crate::execution::options::{ pub use crate::logical_plan::{ approx_percentile_cont, array, ascii, avg, bit_length, btrim, character_length, chr, coalesce, col, concat, concat_ws, count, create_udf, date_part, date_trunc, digest, - exists, in_list, initcap, left, length, lit, lower, lpad, ltrim, max, md5, min, now, - octet_length, random, regexp_match, regexp_replace, repeat, replace, reverse, right, - rpad, rtrim, sha224, sha256, sha384, sha512, split_part, starts_with, strpos, substr, - sum, to_hex, translate, trim, upper, Column, JoinType, Partitioning, + exists, in_list, in_subquery, initcap, left, length, lit, lower, lpad, ltrim, max, + md5, min, not_exists, not_in_subquery, now, octet_length, random, regexp_match, + regexp_replace, repeat, replace, reverse, right, rpad, rtrim, scalar_subquery, + sha224, sha256, sha384, sha512, split_part, starts_with, strpos, substr, sum, to_hex, + translate, trim, upper, Column, JoinType, Partitioning, }; diff --git a/datafusion/core/src/sql/utils.rs b/datafusion/core/src/sql/utils.rs index f9242c6aab639..9ed7f0b66cc99 100644 --- a/datafusion/core/src/sql/utils.rs +++ b/datafusion/core/src/sql/utils.rs @@ -371,7 +371,17 @@ where Expr::Column { .. } | Expr::Literal(_) | Expr::ScalarVariable(_, _) - | Expr::Exists(_) => Ok(expr.clone()), + | Expr::Exists { .. } + | Expr::ScalarSubquery(_) => Ok(expr.clone()), + Expr::InSubquery { + expr: nested_expr, + subquery, + negated, + } => Ok(Expr::InSubquery { + expr: Box::new(clone_with_replacement(&**nested_expr, replacement_fn)?), + subquery: subquery.clone(), + negated: *negated, + }), Expr::Wildcard => Ok(Expr::Wildcard), Expr::QualifiedWildcard { .. } => Ok(expr.clone()), Expr::GetIndexedField { expr, key } => Ok(Expr::GetIndexedField { diff --git a/datafusion/expr/src/expr.rs b/datafusion/expr/src/expr.rs index 88c489670588a..1b9b2e920b436 100644 --- a/datafusion/expr/src/expr.rs +++ b/datafusion/expr/src/expr.rs @@ -228,7 +228,23 @@ pub enum Expr { negated: bool, }, /// EXISTS subquery - Exists(Subquery), + Exists { + /// subquery that will produce a single column of data + subquery: Subquery, + /// Whether the expression is negated + negated: bool, + }, + /// IN subquery + InSubquery { + /// The expression to compare + expr: Box, + /// subquery that will produce a single column of data to compare against + subquery: Subquery, + /// Whether the expression is negated + negated: bool, + }, + /// Scalar subquery + ScalarSubquery(Subquery), /// Represents a reference to all fields in a schema. Wildcard, /// Represents a reference to all fields in a specific schema. @@ -434,7 +450,25 @@ impl fmt::Debug for Expr { Expr::Negative(expr) => write!(f, "(- {:?})", expr), Expr::IsNull(expr) => write!(f, "{:?} IS NULL", expr), Expr::IsNotNull(expr) => write!(f, "{:?} IS NOT NULL", expr), - Expr::Exists(subquery) => write!(f, "EXISTS ({:?})", subquery), + Expr::Exists { subquery, negated } => { + if *negated { + write!(f, "NOT EXISTS ({:?})", subquery) + } else { + write!(f, "EXISTS ({:?})", subquery) + } + } + Expr::InSubquery { + expr, + subquery, + negated, + } => { + if *negated { + write!(f, "{:?} NOT IN ({:?})", expr, subquery) + } else { + write!(f, "{:?} IN ({:?})", expr, subquery) + } + } + Expr::ScalarSubquery(subquery) => write!(f, "({:?})", subquery), Expr::BinaryExpr { left, op, right } => { write!(f, "{:?} {} {:?}", left, op, right) } @@ -622,7 +656,11 @@ fn create_name(e: &Expr, input_schema: &DFSchema) -> Result { let expr = create_name(expr, input_schema)?; Ok(format!("{} IS NOT NULL", expr)) } - Expr::Exists(_) => Ok("EXISTS".to_string()), + Expr::Exists { .. } => Ok("EXISTS".to_string()), + Expr::InSubquery { .. } => Ok("IN".to_string()), + Expr::ScalarSubquery(subquery) => { + Ok(subquery.subquery.schema().field(0).name().clone()) + } Expr::GetIndexedField { expr, key } => { let expr = create_name(expr, input_schema)?; Ok(format!("{}[{}]", expr, key)) diff --git a/datafusion/expr/src/expr_fn.rs b/datafusion/expr/src/expr_fn.rs index 19c311f4fa45e..0a3b2ebd05fb0 100644 --- a/datafusion/expr/src/expr_fn.rs +++ b/datafusion/expr/src/expr_fn.rs @@ -184,7 +184,41 @@ pub fn approx_percentile_cont_with_weight( /// Create an EXISTS subquery expression pub fn exists(subquery: Arc) -> Expr { - Expr::Exists(Subquery { subquery }) + Expr::Exists { + subquery: Subquery { subquery }, + negated: false, + } +} + +/// Create a NOT EXISTS subquery expression +pub fn not_exists(subquery: Arc) -> Expr { + Expr::Exists { + subquery: Subquery { subquery }, + negated: true, + } +} + +/// Create an IN subquery expression +pub fn in_subquery(expr: Expr, subquery: Arc) -> Expr { + Expr::InSubquery { + expr: Box::new(expr), + subquery: Subquery { subquery }, + negated: false, + } +} + +/// Create a NOT IN subquery expression +pub fn not_in_subquery(expr: Expr, subquery: Arc) -> Expr { + Expr::InSubquery { + expr: Box::new(expr), + subquery: Subquery { subquery }, + negated: true, + } +} + +/// Create a scalar subquery expression +pub fn scalar_subquery(subquery: Arc) -> Expr { + Expr::ScalarSubquery(Subquery { subquery }) } // TODO(kszucs): this seems buggy, unary_scalar_expr! is used for many diff --git a/datafusion/expr/src/expr_schema.rs b/datafusion/expr/src/expr_schema.rs index 4c6457962fd36..2a5528507141e 100644 --- a/datafusion/expr/src/expr_schema.rs +++ b/datafusion/expr/src/expr_schema.rs @@ -100,10 +100,14 @@ impl ExprSchemable for Expr { } Expr::Not(_) | Expr::IsNull(_) - | Expr::Exists(_) + | Expr::Exists { .. } + | Expr::InSubquery { .. } | Expr::Between { .. } | Expr::InList { .. } | Expr::IsNotNull(_) => Ok(DataType::Boolean), + Expr::ScalarSubquery(subquery) => { + Ok(subquery.subquery.schema().field(0).data_type().clone()) + } Expr::BinaryExpr { ref left, ref right, @@ -173,7 +177,13 @@ impl ExprSchemable for Expr { | Expr::WindowFunction { .. } | Expr::AggregateFunction { .. } | Expr::AggregateUDF { .. } => Ok(true), - Expr::IsNull(_) | Expr::IsNotNull(_) | Expr::Exists(_) => Ok(false), + Expr::IsNull(_) + | Expr::IsNotNull(_) + | Expr::Exists { .. } + | Expr::InSubquery { .. } => Ok(false), + Expr::ScalarSubquery(subquery) => { + Ok(subquery.subquery.schema().field(0).is_nullable()) + } Expr::BinaryExpr { ref left, ref right, diff --git a/datafusion/expr/src/lib.rs b/datafusion/expr/src/lib.rs index 3dd24600a20cd..b513bf52d4a18 100644 --- a/datafusion/expr/src/lib.rs +++ b/datafusion/expr/src/lib.rs @@ -48,12 +48,13 @@ pub use expr_fn::{ abs, acos, and, approx_distinct, approx_percentile_cont, array, ascii, asin, atan, avg, bit_length, btrim, case, ceil, character_length, chr, coalesce, col, concat, concat_expr, concat_ws, concat_ws_expr, cos, count, count_distinct, date_part, - date_trunc, digest, exists, exp, floor, in_list, initcap, left, length, ln, log10, - log2, lower, lpad, ltrim, max, md5, min, now, now_expr, nullif, octet_length, or, - random, regexp_match, regexp_replace, repeat, replace, reverse, right, round, rpad, - rtrim, sha224, sha256, sha384, sha512, signum, sin, split_part, sqrt, starts_with, - strpos, substr, sum, tan, to_hex, to_timestamp_micros, to_timestamp_millis, - to_timestamp_seconds, translate, trim, trunc, upper, when, + date_trunc, digest, exists, exp, floor, in_list, in_subquery, initcap, left, length, + ln, log10, log2, lower, lpad, ltrim, max, md5, min, not_exists, not_in_subquery, now, + now_expr, nullif, octet_length, or, random, regexp_match, regexp_replace, repeat, + replace, reverse, right, round, rpad, rtrim, scalar_subquery, sha224, sha256, sha384, + sha512, signum, sin, split_part, sqrt, starts_with, strpos, substr, sum, tan, to_hex, + to_timestamp_micros, to_timestamp_millis, to_timestamp_seconds, translate, trim, + trunc, upper, when, }; pub use expr_schema::ExprSchemable; pub use function::{ From bcc4858fc25b11c6e1e8aa95c057cafdacba85c1 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 26 Apr 2022 07:47:11 -0600 Subject: [PATCH 2/7] fix test failure --- datafusion/core/src/logical_plan/builder.rs | 8 +++++--- datafusion/core/src/logical_plan/expr.rs | 1 + 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/datafusion/core/src/logical_plan/builder.rs b/datafusion/core/src/logical_plan/builder.rs index a225c8f4853c9..0b0fdebb68f74 100644 --- a/datafusion/core/src/logical_plan/builder.rs +++ b/datafusion/core/src/logical_plan/builder.rs @@ -1398,7 +1398,7 @@ mod tests { let bar = test_table_scan_with_name("bar")?; let subquery = LogicalPlanBuilder::from(foo) - .project(vec![col("a")])? + .project(vec![col("b")])? .filter(col("a").eq(col("bar.a")))? .build()?; @@ -1407,8 +1407,10 @@ mod tests { .project(vec![scalar_subquery(Arc::new(subquery))])? .build()?; - let expected = - "Projection: (ScalarSubquery TBD )\n TableScan: bar projection=None"; + let expected = "Projection: (Subquery: Filter: #foo.a = #bar.a\ + \n Projection: #foo.b\ + \n TableScan: foo projection=None)\ + \n TableScan: bar projection=None"; assert_eq!(expected, format!("{:?}", outer_query)); Ok(()) diff --git a/datafusion/core/src/logical_plan/expr.rs b/datafusion/core/src/logical_plan/expr.rs index 8935170cfc437..673345c69b61c 100644 --- a/datafusion/core/src/logical_plan/expr.rs +++ b/datafusion/core/src/logical_plan/expr.rs @@ -71,6 +71,7 @@ pub fn columnize_expr(e: Expr, input_schema: &DFSchema) -> Expr { Expr::Alias(inner_expr, name) => { Expr::Alias(Box::new(columnize_expr(*inner_expr, input_schema)), name) } + Expr::ScalarSubquery(_) => e.clone(), _ => match e.name(input_schema) { Ok(name) => match input_schema.field_with_unqualified_name(&name) { Ok(field) => Expr::Column(field.qualified_column()), From dfeb3328e42ac94fd6787ad145dd3ccf2c56a32a Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 26 Apr 2022 13:37:08 -0600 Subject: [PATCH 3/7] Update datafusion/expr/src/expr.rs Co-authored-by: Andrew Lamb --- datafusion/expr/src/expr.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/datafusion/expr/src/expr.rs b/datafusion/expr/src/expr.rs index 1b9b2e920b436..6bd8cd26f1bd7 100644 --- a/datafusion/expr/src/expr.rs +++ b/datafusion/expr/src/expr.rs @@ -656,8 +656,10 @@ fn create_name(e: &Expr, input_schema: &DFSchema) -> Result { let expr = create_name(expr, input_schema)?; Ok(format!("{} IS NOT NULL", expr)) } - Expr::Exists { .. } => Ok("EXISTS".to_string()), - Expr::InSubquery { .. } => Ok("IN".to_string()), + Expr::Exists { negated: true, .. } => Ok("NOT EXISTS".to_string()), + Expr::Exists { negated: false, .. } => Ok("EXISTS".to_string()), + Expr::InSubquery { negated: true, .. } => Ok("NOT IN".to_string()), + Expr::InSubquery { negated: false, .. } => Ok("IN".to_string()), Expr::ScalarSubquery(subquery) => { Ok(subquery.subquery.schema().field(0).name().clone()) } From fd4a878edf52ed8604341e29fad66124040e00b5 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 26 Apr 2022 13:37:18 -0600 Subject: [PATCH 4/7] Update datafusion/expr/src/expr.rs Co-authored-by: Andrew Lamb --- datafusion/expr/src/expr.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/datafusion/expr/src/expr.rs b/datafusion/expr/src/expr.rs index 6bd8cd26f1bd7..16d63ed6836a7 100644 --- a/datafusion/expr/src/expr.rs +++ b/datafusion/expr/src/expr.rs @@ -450,13 +450,8 @@ impl fmt::Debug for Expr { Expr::Negative(expr) => write!(f, "(- {:?})", expr), Expr::IsNull(expr) => write!(f, "{:?} IS NULL", expr), Expr::IsNotNull(expr) => write!(f, "{:?} IS NOT NULL", expr), - Expr::Exists { subquery, negated } => { - if *negated { - write!(f, "NOT EXISTS ({:?})", subquery) - } else { - write!(f, "EXISTS ({:?})", subquery) - } - } + Expr::Exists { subquery, negated: true } => write!(f, "NOT EXISTS ({:?})", subquery), + Expr::Exists { subquery, negated: false } => write!(f, "EXISTS ({:?})", subquery), Expr::InSubquery { expr, subquery, From ff0f88274d9052773d91d97a1c552cbc69d3536f Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 26 Apr 2022 13:47:13 -0600 Subject: [PATCH 5/7] address PR feedback --- datafusion/expr/src/expr_schema.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/datafusion/expr/src/expr_schema.rs b/datafusion/expr/src/expr_schema.rs index 2a5528507141e..b932eefa0b967 100644 --- a/datafusion/expr/src/expr_schema.rs +++ b/datafusion/expr/src/expr_schema.rs @@ -177,10 +177,8 @@ impl ExprSchemable for Expr { | Expr::WindowFunction { .. } | Expr::AggregateFunction { .. } | Expr::AggregateUDF { .. } => Ok(true), - Expr::IsNull(_) - | Expr::IsNotNull(_) - | Expr::Exists { .. } - | Expr::InSubquery { .. } => Ok(false), + Expr::IsNull(_) | Expr::IsNotNull(_) | Expr::Exists { .. } => Ok(false), + Expr::InSubquery { expr, .. } => expr.nullable(input_schema), Expr::ScalarSubquery(subquery) => { Ok(subquery.subquery.schema().field(0).is_nullable()) } From afe485935a4d3c430d4f760e07a5c4fc0a12d83e Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 26 Apr 2022 13:47:46 -0600 Subject: [PATCH 6/7] fmt --- datafusion/expr/src/expr.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/datafusion/expr/src/expr.rs b/datafusion/expr/src/expr.rs index 16d63ed6836a7..6c0a508d4c95e 100644 --- a/datafusion/expr/src/expr.rs +++ b/datafusion/expr/src/expr.rs @@ -450,8 +450,14 @@ impl fmt::Debug for Expr { Expr::Negative(expr) => write!(f, "(- {:?})", expr), Expr::IsNull(expr) => write!(f, "{:?} IS NULL", expr), Expr::IsNotNull(expr) => write!(f, "{:?} IS NOT NULL", expr), - Expr::Exists { subquery, negated: true } => write!(f, "NOT EXISTS ({:?})", subquery), - Expr::Exists { subquery, negated: false } => write!(f, "EXISTS ({:?})", subquery), + Expr::Exists { + subquery, + negated: true, + } => write!(f, "NOT EXISTS ({:?})", subquery), + Expr::Exists { + subquery, + negated: false, + } => write!(f, "EXISTS ({:?})", subquery), Expr::InSubquery { expr, subquery, From 0b39ec44b9ead1be2165002f9eb1e6b1658e4b3a Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 26 Apr 2022 16:09:04 -0600 Subject: [PATCH 7/7] address PR feedbacl --- datafusion/expr/src/expr.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/datafusion/expr/src/expr.rs b/datafusion/expr/src/expr.rs index 6c0a508d4c95e..4d88ed815b14b 100644 --- a/datafusion/expr/src/expr.rs +++ b/datafusion/expr/src/expr.rs @@ -461,14 +461,13 @@ impl fmt::Debug for Expr { Expr::InSubquery { expr, subquery, - negated, - } => { - if *negated { - write!(f, "{:?} NOT IN ({:?})", expr, subquery) - } else { - write!(f, "{:?} IN ({:?})", expr, subquery) - } - } + negated: true, + } => write!(f, "{:?} NOT IN ({:?})", expr, subquery), + Expr::InSubquery { + expr, + subquery, + negated: false, + } => write!(f, "{:?} IN ({:?})", expr, subquery), Expr::ScalarSubquery(subquery) => write!(f, "({:?})", subquery), Expr::BinaryExpr { left, op, right } => { write!(f, "{:?} {} {:?}", left, op, right)