benbellick commented on code in PR #21193:
URL: https://github.com/apache/datafusion/pull/21193#discussion_r3209970320
##########
datafusion/substrait/src/logical_plan/consumer/substrait_consumer.rs:
##########
@@ -594,6 +662,124 @@ impl SubstraitConsumer for DefaultSubstraitConsumer<'_> {
let plan = plan.with_exprs_and_inputs(plan.expressions(), inputs)?;
Ok(LogicalPlan::Extension(Extension { node: plan }))
}
+
+ fn with_lambda_parameters(
+ &self,
+ lambda_parameters: &[Type],
+ input_schema: &DFSchema,
+ ) -> datafusion::common::Result<(Vec<String>, Self)> {
+ let (names, lambda_consumer) =
self.lambda_consumer.with_lambda_parameters(
+ self,
+ lambda_parameters,
+ input_schema,
+ )?;
+
+ Ok((
+ names,
+ Self {
+ extensions: self.extensions,
+ state: self.state,
+ outer_schemas:
RwLock::new(self.outer_schemas.read().unwrap().clone()),
+ lambda_consumer,
+ },
+ ))
+ }
+
+ fn lambda_variable(
+ &self,
+ steps_out: usize,
+ field_idx: usize,
+ ) -> datafusion::common::Result<Expr> {
+ self.lambda_consumer.lambda_variable(steps_out, field_idx)
+ }
+}
+
+/// Default implementation of lambda related methods of the
[SubstraitConsumer] trait
+///
+/// Can be embedded into a custom [SubstraitConsumer] to implement them
+pub struct DefaultSubstraitLambdaConsumer {
Review Comment:
Ah I see now, I appreciate the explanation. That makes a lot of sense then!
AFAICT the strategy for outer schemas is to provide default impls so that
consumer implementors who don't care about the behavior can ignore their
existence, but then will encounter a runtime error if they are used:
https://github.com/apache/datafusion/blob/6cdfc056d90b81f4b85785df53c14344d48413ae/datafusion/substrait/src/logical_plan/consumer/substrait_consumer.rs#L437-L449
I'm wondering if we should do the same thing for lambdas? Rather than
enforce that implementors _must_ implement the lambda-handling fns, they could
instead optionally ignore them, resulting in an error if lambda expressions are
encountered.
What do you think? I am not so particular here TBH. Ultimately my goal is to
validate that the translation itself is correct, and I am happy to leave API
concerns to the project maintainers :)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
