vbarua commented on code in PR #23724:
URL: https://github.com/apache/datafusion/pull/23724#discussion_r3617488555
##########
datafusion/substrait/src/logical_plan/consumer/rel/read_rel.rs:
##########
@@ -216,6 +216,11 @@ pub async fn from_read_rel(
)
.await
}
+ Some(ReadType::ExtensionTable(extension_table)) => {
+ consumer
+ .consume_extension_table(read, extension_table)
+ .await
+ }
Review Comment:
One bit of API that might be worth thinking about if you haven't already.
The NamedTable and LocalFiles `read_types` call the `read_with_schema`
function at the end to also translate the `projection`, and `filter` fields of
the
[ReadRel](https://github.com/substrait-io/substrait/blob/b322d463804660674e43c9d2b659730375e3026e/proto/substrait/algebra.proto#L114-L116)
message. `read_with_schema` assumes that the translated `read_type` is vaguely
`TableReference` shaped
With the addition of `ExtensionTable`, we have two `read_types` that aren't
TableReference shaped (the other is VirtualTable) and don't correctly handle
the projection and filter fields. Now might be a good time to try and figure
out a better abstraction to handle those fields consistently for all read types.
--
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]
