Blizzara commented on code in PR #13772:
URL: https://github.com/apache/datafusion/pull/13772#discussion_r1899123986
##########
datafusion/substrait/src/logical_plan/consumer.rs:
##########
@@ -438,6 +439,22 @@ pub trait SubstraitConsumer: Send + Sync + Sized {
user_defined_literal.type_reference
)
}
+
+ fn consume_extension_table(
+ &self,
+ extension_table: &ExtensionTable,
+ _schema: &DFSchema,
+ _projection: &Option<MaskExpression>,
+ ) -> Result<LogicalPlan> {
Review Comment:
How about something like:
```
from_read_rel(consumer: &SubstraitConsumer, read: &ReadRel, ..) -> .. {
let plan = match read.type {
Some(NamedTable(nt)) => consumer.consume_named_table(nt)
Some(VirtualTable(vt)) => consumer.consume_virtual_table(vt),
Some(ExtensionTable(et)) => consumer.consume_extension_table(et),
...
}
ensure_schema_compatibility(plan.schema(), schema.clone())?;
let schema = apply_masking(schema, projection)?;
apply_projection(plan, schema)
}
```
That way the ReadRel handling doesn't need to happen in multiple places, its
projection and schema are by default handled the same way for all relations
(which I'd think they should?), but if a user wants they can easily override
the whole from_read_rel (or more specifically, SubstraitConsumer::consume_read)
and compose their desired result.
(Actually, dunno if there's reason to have `consumer.consume_named_table()`
and `.consume_virtual_table()`, given probably nothing else than `consume_read`
calls those, their default impl should be good enough and if not they can
always be overridden by implementing consume_read. But having
consumer.consume_extension_table makes sense as an easy way to specify that
behavior.)
--
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]
