datafusion icon indicating copy to clipboard operation
datafusion copied to clipboard

Support Substrait's VirtualTables

Open Blizzara opened this issue 1 year ago • 3 comments

Which issue does this PR close?

Closes #10530

This is my first (but hopefully not last!) contribution to DataFusion, and I'm pretty new with Rust as well - so all feedback on both style and details is very much welcome!

Rationale for this change

What changes are included in this PR?

Adds support for Substrait's VirtualTables, ie. tables with data baked-in into the Substrait plan instead of being read from a source.

Adds conversion in both ways (Substrait -> DataFusion and DataFusion -> Substrait)

Are these changes tested?

Tested through round-trip tests, similar to other Substrait support.

Are there any user-facing changes?

  • More support through Substrait

Blizzara avatar May 15 '24 16:05 Blizzara

Thanks @Blizzara -- I started the CI on this PR

alamb avatar May 17 '24 00:05 alamb

@jonahgao @alamb I think this is ready by me :)

Blizzara avatar May 21 '24 10:05 Blizzara

@jonahgao Thanks for the review - I pushed a new version but it builds on top of https://github.com/apache/datafusion/pull/10622 and https://github.com/apache/datafusion/pull/10640 so will need those merged and this rebased first before this PRs looks clean.

This is mostly what it used to be with one more major change: I removed passing the datatype into from_substrait_literal and instead do the same thing with names as I did for from_substrait_type. That maybe seems a bit cleaner (even though I dislike the name handling, but I don't see a better way to deal with it based on how it works in Substrait :/)

Blizzara avatar May 23 '24 16:05 Blizzara

@jonahgao @alamb this last one is ready now too :)

Blizzara avatar May 24 '24 14:05 Blizzara