You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "kumarUjjawal (via GitHub)" <gi...@apache.org> on 2026/01/11 09:40:11 UTC
[PR] fix: sqllogictest cannot convert to Substrait [datafusion]
kumarUjjawal opened a new pull request, #19739:
URL: https://github.com/apache/datafusion/pull/19739
## Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
- Closes 16281
## Rationale for this change
The sqllogictest for the substrait was failing for subquery.
```
query failed: DataFusion error: This feature is not implemented: Cannot convert <subquery> to Substrait
```
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
## What changes are included in this PR?
- added support for `ScalarSubquery` and `Exists` expressions in the Substrait producer.
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
## Are these changes tested?
Yes
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
-->
## Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!--
If there are any breaking changes to public APIs, please add the `api change` label.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: sqllogictest cannot convert to Substrait [datafusion]
Posted by "kumarUjjawal (via GitHub)" <gi...@apache.org>.
kumarUjjawal commented on code in PR #19739:
URL: https://github.com/apache/datafusion/pull/19739#discussion_r2709591938
##########
datafusion/substrait/src/logical_plan/producer/expr/subquery.rs:
##########
@@ -70,3 +71,70 @@ pub fn from_in_subquery(
Ok(substrait_subquery)
}
}
+
+/// Convert DataFusion ScalarSubquery to Substrait Scalar subquery type
+pub fn from_scalar_subquery(
+ producer: &mut impl SubstraitProducer,
+ subquery: &Subquery,
+ _schema: &DFSchemaRef,
+) -> datafusion::common::Result<Expression> {
+ let subquery_plan = producer.handle_plan(subquery.subquery.as_ref())?;
+
+ Ok(Expression {
+ rex_type: Some(RexType::Subquery(Box::new(
+ substrait::proto::expression::Subquery {
+ subquery_type: Some(
+ substrait::proto::expression::subquery::SubqueryType::Scalar(
+ Box::new(Scalar {
+ input: Some(subquery_plan),
+ }),
+ ),
+ ),
+ },
+ ))),
+ })
+}
+
+/// Convert DataFusion Exists expression to Substrait SetPredicate subquery type
+pub fn from_exists(
+ producer: &mut impl SubstraitProducer,
+ exists: &Exists,
+ _schema: &DFSchemaRef,
+) -> datafusion::common::Result<Expression> {
+ let subquery_plan = producer.handle_plan(exists.subquery.subquery.as_ref())?;
+
+ let substrait_exists = Expression {
+ rex_type: Some(RexType::Subquery(Box::new(
+ substrait::proto::expression::Subquery {
+ subquery_type: Some(
+ substrait::proto::expression::subquery::SubqueryType::SetPredicate(
+ Box::new(SetPredicate {
+ predicate_op: substrait::proto::expression::subquery::set_predicate::PredicateOp::Exists as i32,
+ tuples: Some(subquery_plan),
+ }),
+ ),
+ ),
+ },
+ ))),
+ };
+
+ // Handle negated EXISTS (NOT EXISTS)
+ if exists.negated {
Review Comment:
I had look into it earlier, I was hoping to open a seperate discussion so not to clutter this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: sqllogictest cannot convert to Substrait [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on code in PR #19739:
URL: https://github.com/apache/datafusion/pull/19739#discussion_r2791893571
##########
datafusion/substrait/src/logical_plan/producer/expr/subquery.rs:
##########
@@ -70,3 +71,70 @@ pub fn from_in_subquery(
Ok(substrait_subquery)
}
}
+
+/// Convert DataFusion ScalarSubquery to Substrait Scalar subquery type
+pub fn from_scalar_subquery(
+ producer: &mut impl SubstraitProducer,
+ subquery: &Subquery,
+ _schema: &DFSchemaRef,
+) -> datafusion::common::Result<Expression> {
+ let subquery_plan = producer.handle_plan(subquery.subquery.as_ref())?;
+
+ Ok(Expression {
+ rex_type: Some(RexType::Subquery(Box::new(
+ substrait::proto::expression::Subquery {
+ subquery_type: Some(
+ substrait::proto::expression::subquery::SubqueryType::Scalar(
+ Box::new(Scalar {
+ input: Some(subquery_plan),
+ }),
+ ),
+ ),
+ },
+ ))),
+ })
+}
+
+/// Convert DataFusion Exists expression to Substrait SetPredicate subquery type
+pub fn from_exists(
+ producer: &mut impl SubstraitProducer,
+ exists: &Exists,
+ _schema: &DFSchemaRef,
+) -> datafusion::common::Result<Expression> {
+ let subquery_plan = producer.handle_plan(exists.subquery.subquery.as_ref())?;
+
+ let substrait_exists = Expression {
+ rex_type: Some(RexType::Subquery(Box::new(
+ substrait::proto::expression::Subquery {
+ subquery_type: Some(
+ substrait::proto::expression::subquery::SubqueryType::SetPredicate(
+ Box::new(SetPredicate {
+ predicate_op: substrait::proto::expression::subquery::set_predicate::PredicateOp::Exists as i32,
+ tuples: Some(subquery_plan),
+ }),
+ ),
+ ),
+ },
+ ))),
+ };
+
+ // Handle negated EXISTS (NOT EXISTS)
+ if exists.negated {
+ let function_anchor = producer.register_function("not".to_string());
+
+ #[expect(deprecated)]
+ Ok(Expression {
+ rex_type: Some(RexType::ScalarFunction(ScalarFunction {
+ function_reference: function_anchor,
+ arguments: vec![FunctionArgument {
+ arg_type: Some(ArgType::Value(substrait_exists)),
+ }],
+ output_type: None,
+ args: vec![],
+ options: vec![],
+ })),
+ })
+ } else {
+ Ok(substrait_exists)
+ }
Review Comment:
this is duplicated in
- fn from_in_list
- fn from_in_subquery
and should be extracted into a helper for consistency
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: sqllogictest cannot convert to Substrait [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #19739:
URL: https://github.com/apache/datafusion/pull/19739#issuecomment-4030622993
Thanks @kumarUjjawal @kosiew and @gabotechs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: sqllogictest cannot convert to Substrait [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #19739:
URL: https://github.com/apache/datafusion/pull/19739
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: sqllogictest cannot convert to Substrait [datafusion]
Posted by "gabotechs (via GitHub)" <gi...@apache.org>.
gabotechs commented on PR #19739:
URL: https://github.com/apache/datafusion/pull/19739#issuecomment-3772120371
I'm struggling to find the time to take a look at this one. I'm requesting back up now!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: sqllogictest cannot convert to Substrait [datafusion]
Posted by "benbellick (via GitHub)" <gi...@apache.org>.
benbellick commented on code in PR #19739:
URL: https://github.com/apache/datafusion/pull/19739#discussion_r2714688563
##########
datafusion/substrait/src/logical_plan/producer/substrait_producer.rs:
##########
@@ -361,6 +364,31 @@ pub trait SubstraitProducer: Send + Sync + Sized {
) -> datafusion::common::Result<Expression> {
from_in_subquery(self, in_subquery, schema)
}
+
+ fn handle_scalar_subquery(
+ &mut self,
+ subquery: &Subquery,
+ schema: &DFSchemaRef,
+ ) -> datafusion::common::Result<Expression> {
+ from_scalar_subquery(self, subquery, schema)
+ }
+
+ fn handle_exists(
+ &mut self,
+ exists: &Exists,
+ schema: &DFSchemaRef,
+ ) -> datafusion::common::Result<Expression> {
+ from_exists(self, exists, schema)
+ }
+
+ fn handle_outer_reference_column(
Review Comment:
Am I correct that this is dead code? Maybe was intended to be added to the match in `datafusion/substrait/src/logical_plan/producer/expr/mod.rs`, but is now being left for later.
##########
datafusion/substrait/src/logical_plan/producer/expr/field_reference.rs:
##########
@@ -76,6 +76,22 @@ pub(crate) fn try_to_substrait_field_reference(
}
}
+/// Convert an outer reference column to a Substrait field reference.
+/// Outer reference columns reference columns from an outer query scope in correlated subqueries.
+/// We convert them the same way as regular columns since the subquery plan will be
+/// reconstructed with the proper schema context during consumption.
+pub fn from_outer_reference_column(
Review Comment:
Is this also deadcode (once the single deadcode caller is deleted)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: sqllogictest cannot convert to Substrait [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on code in PR #19739:
URL: https://github.com/apache/datafusion/pull/19739#discussion_r2845104111
##########
datafusion/substrait/src/logical_plan/producer/utils.rs:
##########
@@ -85,3 +85,35 @@ pub(crate) fn to_substrait_precision(time_unit: &TimeUnit) -> i32 {
TimeUnit::Nanosecond => 9,
}
}
+
+/// Wraps an expression with a `not()` function if `negated` is true.
+/// This is used for negated IN, EXISTS, and other predicates.
+pub(crate) fn negate_if_needed(
Review Comment:
👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: sqllogictest cannot convert to Substrait [datafusion]
Posted by "kumarUjjawal (via GitHub)" <gi...@apache.org>.
kumarUjjawal commented on PR #19739:
URL: https://github.com/apache/datafusion/pull/19739#issuecomment-3950034409
Thansk @kosiew
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: sqllogictest cannot convert to Substrait [datafusion]
Posted by "kumarUjjawal (via GitHub)" <gi...@apache.org>.
kumarUjjawal commented on code in PR #19739:
URL: https://github.com/apache/datafusion/pull/19739#discussion_r2709565045
##########
datafusion/substrait/src/logical_plan/producer/expr/mod.rs:
##########
@@ -139,16 +139,16 @@ pub fn to_substrait_rex(
}
Expr::WindowFunction(expr) => producer.handle_window_function(expr, schema),
Expr::InList(expr) => producer.handle_in_list(expr, schema),
- Expr::Exists(expr) => not_impl_err!("Cannot convert {expr:?} to Substrait"),
+ Expr::Exists(expr) => producer.handle_exists(expr, schema),
Expr::InSubquery(expr) => producer.handle_in_subquery(expr, schema),
- Expr::ScalarSubquery(expr) => {
- not_impl_err!("Cannot convert {expr:?} to Substrait")
- }
+ Expr::ScalarSubquery(expr) => producer.handle_scalar_subquery(expr, schema),
#[expect(deprecated)]
Expr::Wildcard { .. } => not_impl_err!("Cannot convert {expr:?} to Substrait"),
Expr::GroupingSet(expr) => not_impl_err!("Cannot convert {expr:?} to Substrait"),
Expr::Placeholder(expr) => not_impl_err!("Cannot convert {expr:?} to Substrait"),
Expr::OuterReferenceColumn(_, _) => {
+ // OuterReferenceColumn requires tracking outer query schema context for correlated
Review Comment:
Yeah for the remaining issues I intend to open follow up PRs. My goal is to resolve all the issue related to substrait in the next few weeks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: sqllogictest cannot convert to Substrait [datafusion]
Posted by "kumarUjjawal (via GitHub)" <gi...@apache.org>.
kumarUjjawal commented on code in PR #19739:
URL: https://github.com/apache/datafusion/pull/19739#discussion_r2717943760
##########
datafusion/substrait/src/logical_plan/producer/substrait_producer.rs:
##########
@@ -361,6 +364,31 @@ pub trait SubstraitProducer: Send + Sync + Sized {
) -> datafusion::common::Result<Expression> {
from_in_subquery(self, in_subquery, schema)
}
+
+ fn handle_scalar_subquery(
+ &mut self,
+ subquery: &Subquery,
+ schema: &DFSchemaRef,
+ ) -> datafusion::common::Result<Expression> {
+ from_scalar_subquery(self, subquery, schema)
+ }
+
+ fn handle_exists(
+ &mut self,
+ exists: &Exists,
+ schema: &DFSchemaRef,
+ ) -> datafusion::common::Result<Expression> {
+ from_exists(self, exists, schema)
+ }
+
+ fn handle_outer_reference_column(
Review Comment:
Good catch! Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: sqllogictest cannot convert to Substrait [datafusion]
Posted by "benbellick (via GitHub)" <gi...@apache.org>.
benbellick commented on code in PR #19739:
URL: https://github.com/apache/datafusion/pull/19739#discussion_r2718281932
##########
datafusion/substrait/src/logical_plan/producer/expr/subquery.rs:
##########
@@ -70,3 +71,70 @@ pub fn from_in_subquery(
Ok(substrait_subquery)
}
}
+
+/// Convert DataFusion ScalarSubquery to Substrait Scalar subquery type
+pub fn from_scalar_subquery(
+ producer: &mut impl SubstraitProducer,
+ subquery: &Subquery,
+ _schema: &DFSchemaRef,
+) -> datafusion::common::Result<Expression> {
+ let subquery_plan = producer.handle_plan(subquery.subquery.as_ref())?;
+
+ Ok(Expression {
+ rex_type: Some(RexType::Subquery(Box::new(
+ substrait::proto::expression::Subquery {
+ subquery_type: Some(
+ substrait::proto::expression::subquery::SubqueryType::Scalar(
+ Box::new(Scalar {
+ input: Some(subquery_plan),
+ }),
+ ),
+ ),
+ },
+ ))),
+ })
+}
+
+/// Convert DataFusion Exists expression to Substrait SetPredicate subquery type
+pub fn from_exists(
+ producer: &mut impl SubstraitProducer,
+ exists: &Exists,
+ _schema: &DFSchemaRef,
+) -> datafusion::common::Result<Expression> {
+ let subquery_plan = producer.handle_plan(exists.subquery.subquery.as_ref())?;
+
+ let substrait_exists = Expression {
+ rex_type: Some(RexType::Subquery(Box::new(
+ substrait::proto::expression::Subquery {
+ subquery_type: Some(
+ substrait::proto::expression::subquery::SubqueryType::SetPredicate(
+ Box::new(SetPredicate {
+ predicate_op: substrait::proto::expression::subquery::set_predicate::PredicateOp::Exists as i32,
+ tuples: Some(subquery_plan),
+ }),
+ ),
+ ),
+ },
+ ))),
+ };
+
+ // Handle negated EXISTS (NOT EXISTS)
+ if exists.negated {
+ let function_anchor = producer.register_function("not".to_string());
+
+ #[expect(deprecated)]
+ Ok(Expression {
+ rex_type: Some(RexType::ScalarFunction(ScalarFunction {
+ function_reference: function_anchor,
+ arguments: vec![FunctionArgument {
+ arg_type: Some(ArgType::Value(substrait_exists)),
+ }],
+ output_type: None,
Review Comment:
I'm not sure if this is done consistently across the codebase, but this technically makes this substrait invalid. F[rom the documentation](https://github.com/substrait-io/substrait/blob/95f8f350328f63bf44be63f28eb67680983edbb1/proto/substrait/algebra.proto#L1245-L1246):
```proto
// Must be set to the return type of the function, exactly as derived
// using the declaration in the extension.
Type output_type = 3;
```
##########
datafusion/substrait/src/logical_plan/producer/expr/subquery.rs:
##########
@@ -70,3 +71,70 @@ pub fn from_in_subquery(
Ok(substrait_subquery)
}
}
+
+/// Convert DataFusion ScalarSubquery to Substrait Scalar subquery type
+pub fn from_scalar_subquery(
+ producer: &mut impl SubstraitProducer,
+ subquery: &Subquery,
+ _schema: &DFSchemaRef,
+) -> datafusion::common::Result<Expression> {
+ let subquery_plan = producer.handle_plan(subquery.subquery.as_ref())?;
+
+ Ok(Expression {
+ rex_type: Some(RexType::Subquery(Box::new(
+ substrait::proto::expression::Subquery {
+ subquery_type: Some(
+ substrait::proto::expression::subquery::SubqueryType::Scalar(
+ Box::new(Scalar {
+ input: Some(subquery_plan),
+ }),
+ ),
+ ),
+ },
+ ))),
+ })
+}
+
+/// Convert DataFusion Exists expression to Substrait SetPredicate subquery type
+pub fn from_exists(
+ producer: &mut impl SubstraitProducer,
+ exists: &Exists,
+ _schema: &DFSchemaRef,
+) -> datafusion::common::Result<Expression> {
+ let subquery_plan = producer.handle_plan(exists.subquery.subquery.as_ref())?;
+
+ let substrait_exists = Expression {
+ rex_type: Some(RexType::Subquery(Box::new(
+ substrait::proto::expression::Subquery {
+ subquery_type: Some(
+ substrait::proto::expression::subquery::SubqueryType::SetPredicate(
+ Box::new(SetPredicate {
+ predicate_op: substrait::proto::expression::subquery::set_predicate::PredicateOp::Exists as i32,
+ tuples: Some(subquery_plan),
+ }),
+ ),
+ ),
+ },
+ ))),
+ };
+
+ // Handle negated EXISTS (NOT EXISTS)
+ if exists.negated {
+ let function_anchor = producer.register_function("not".to_string());
+
+ #[expect(deprecated)]
+ Ok(Expression {
+ rex_type: Some(RexType::ScalarFunction(ScalarFunction {
+ function_reference: function_anchor,
+ arguments: vec![FunctionArgument {
+ arg_type: Some(ArgType::Value(substrait_exists)),
+ }],
+ output_type: None,
Review Comment:
(I wish that the `substrait-rs` library were at a state where it could handle this for you, but it just isn't there yet)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: sqllogictest cannot convert to Substrait [datafusion]
Posted by "kadinrabo (via GitHub)" <gi...@apache.org>.
kadinrabo commented on code in PR #19739:
URL: https://github.com/apache/datafusion/pull/19739#discussion_r2709439239
##########
datafusion/substrait/src/logical_plan/producer/expr/mod.rs:
##########
@@ -139,16 +139,16 @@ pub fn to_substrait_rex(
}
Expr::WindowFunction(expr) => producer.handle_window_function(expr, schema),
Expr::InList(expr) => producer.handle_in_list(expr, schema),
- Expr::Exists(expr) => not_impl_err!("Cannot convert {expr:?} to Substrait"),
+ Expr::Exists(expr) => producer.handle_exists(expr, schema),
Expr::InSubquery(expr) => producer.handle_in_subquery(expr, schema),
- Expr::ScalarSubquery(expr) => {
- not_impl_err!("Cannot convert {expr:?} to Substrait")
- }
+ Expr::ScalarSubquery(expr) => producer.handle_scalar_subquery(expr, schema),
#[expect(deprecated)]
Expr::Wildcard { .. } => not_impl_err!("Cannot convert {expr:?} to Substrait"),
Expr::GroupingSet(expr) => not_impl_err!("Cannot convert {expr:?} to Substrait"),
Expr::Placeholder(expr) => not_impl_err!("Cannot convert {expr:?} to Substrait"),
Expr::OuterReferenceColumn(_, _) => {
+ // OuterReferenceColumn requires tracking outer query schema context for correlated
Review Comment:
I'm still getting a lot of `This feature is not implemented: Cannot convert OuterReferenceColumn` errors when running the tests so maybe this PR can partially close the issue instead of completely? Unless you're still working on it
##########
datafusion/substrait/src/logical_plan/producer/expr/subquery.rs:
##########
@@ -70,3 +71,70 @@ pub fn from_in_subquery(
Ok(substrait_subquery)
}
}
+
+/// Convert DataFusion ScalarSubquery to Substrait Scalar subquery type
+pub fn from_scalar_subquery(
+ producer: &mut impl SubstraitProducer,
+ subquery: &Subquery,
+ _schema: &DFSchemaRef,
+) -> datafusion::common::Result<Expression> {
+ let subquery_plan = producer.handle_plan(subquery.subquery.as_ref())?;
+
+ Ok(Expression {
+ rex_type: Some(RexType::Subquery(Box::new(
+ substrait::proto::expression::Subquery {
+ subquery_type: Some(
+ substrait::proto::expression::subquery::SubqueryType::Scalar(
+ Box::new(Scalar {
+ input: Some(subquery_plan),
+ }),
+ ),
+ ),
+ },
+ ))),
+ })
+}
+
+/// Convert DataFusion Exists expression to Substrait SetPredicate subquery type
+pub fn from_exists(
+ producer: &mut impl SubstraitProducer,
+ exists: &Exists,
+ _schema: &DFSchemaRef,
+) -> datafusion::common::Result<Expression> {
+ let subquery_plan = producer.handle_plan(exists.subquery.subquery.as_ref())?;
+
+ let substrait_exists = Expression {
+ rex_type: Some(RexType::Subquery(Box::new(
+ substrait::proto::expression::Subquery {
+ subquery_type: Some(
+ substrait::proto::expression::subquery::SubqueryType::SetPredicate(
+ Box::new(SetPredicate {
+ predicate_op: substrait::proto::expression::subquery::set_predicate::PredicateOp::Exists as i32,
+ tuples: Some(subquery_plan),
+ }),
+ ),
+ ),
+ },
+ ))),
+ };
+
+ // Handle negated EXISTS (NOT EXISTS)
+ if exists.negated {
Review Comment:
There's no `PREDICATE_OP_NOT_EXISTS` in the spec so I think this a reasonable workaround. Minor note, the consumer hardcodes `negated:false` so I don't think `NOT EXISTS`/`NOT IN` will round-trip correctly ([Exists](https://github.com/apache/datafusion/blob/dededf7c78d587afc662c8a06fb949a0bf4d457f/datafusion/substrait/src/logical_plan/consumer/expr/subquery.rs#L92)/[InSubquery](https://github.com/apache/datafusion/blob/dededf7c78d587afc662c8a06fb949a0bf4d457f/datafusion/substrait/src/logical_plan/consumer/expr/subquery.rs#L57))
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: sqllogictest cannot convert to Substrait [datafusion]
Posted by "kumarUjjawal (via GitHub)" <gi...@apache.org>.
kumarUjjawal commented on PR #19739:
URL: https://github.com/apache/datafusion/pull/19739#issuecomment-3734559197
@gabotechs if you could take a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: sqllogictest cannot convert to Substrait [datafusion]
Posted by "kumarUjjawal (via GitHub)" <gi...@apache.org>.
kumarUjjawal commented on PR #19739:
URL: https://github.com/apache/datafusion/pull/19739#issuecomment-3790496299
@kosiew since you have worked with substrait, could you take a look whenever you get time. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: sqllogictest cannot convert to Substrait [datafusion]
Posted by "kumarUjjawal (via GitHub)" <gi...@apache.org>.
kumarUjjawal commented on code in PR #19739:
URL: https://github.com/apache/datafusion/pull/19739#discussion_r2717944539
##########
datafusion/substrait/src/logical_plan/producer/expr/field_reference.rs:
##########
@@ -76,6 +76,22 @@ pub(crate) fn try_to_substrait_field_reference(
}
}
+/// Convert an outer reference column to a Substrait field reference.
+/// Outer reference columns reference columns from an outer query scope in correlated subqueries.
+/// We convert them the same way as regular columns since the subquery plan will be
+/// reconstructed with the proper schema context during consumption.
+pub fn from_outer_reference_column(
Review Comment:
Yes true.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org