vbarua commented on code in PR #13772:
URL: https://github.com/apache/datafusion/pull/13772#discussion_r1896987187
##########
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:
> I chose to pass the schema and projection to this method (instead of
keeping the TableScan postprocessing in from_read_rel) to allow custom
implementations to use that information for fully restoring their custom tables
if needed.
I like where your head is at with this, but I almost want to go further. You
already called out:
> leverage the available ReadRel information that is currently unused (e.g.
filtering, advanced extensions etc.)
Maybe the interface for this should just be:
```rust
fn consume_extension_table(
&self,
read_rel: &ReadRel
extension_table: &ExtensionTable) -> Result<LogicalPlan>
```
which will be future proofed for if fields are ever added to the ReadRel,
and also provides access to common fields on the ReadRel.
We could even go further and add
```rust
fn consume_named_table(
&self,
read_rel: &ReadRel
named_table: &NamedTable) -> Result<LogicalPlan>
fn consume_virtual_table(
&self,
read_rel: &ReadRel
named_table: &VirtualTable) -> Result<LogicalPlan>
```
to make it easier to customize behaviour for specific read_types. This last
idea might be better as it's own PR, as we would need to factor out some of the
code in `from_read_rel` into functions to be re-used across the new helpers.
--
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]
