change: fully deprecated the acceptance of value field in VirtualTables - #24346
change: fully deprecated the acceptance of value field in VirtualTables#24346eliot1480 wants to merge 2 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #24346 +/- ##
==========================================
+ Coverage 81.17% 81.31% +0.13%
==========================================
Files 1109 1117 +8
Lines 388033 395873 +7840
Branches 388033 395873 +7840
==========================================
+ Hits 314985 321903 +6918
- Misses 54509 55148 +639
- Partials 18539 18822 +283 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
kosiew
left a comment
There was a problem hiding this comment.
@eliot1480, thanks for working on this. This looks good to me.
The change cleanly removes support for consuming the deprecated VirtualTable.values field and the associated literal-row conversion helper, while consolidating virtual-table planning on VirtualTable.expressions. The empty-table and one-row EmptyRelation cases are also preserved using the expressions representation.
I don't have any blocking concerns or additional suggestions. 👍
|
There is actually a user-facing change in this PR. If DataFusion consumes Substrait plans that set only the deprecated Elliot's prior changes in #23672, now released in v55.0.0, make it so DataFusion produces plans using the |
Co-authored-by: Victor Barua <victor.barua@datadoghq.com>
Which issue does this PR close?
#23608
Rationale for this change
This PR is the final step in fully deprecating the
VirtualTables.values, as we now only allow the consumption of expressions instead of the value field as well.What changes are included in this PR?
These changes remove the deprecated
VirtualTable.valuesconsumer path, including the old literal-row conversion helper and the broad deprecation allowance aroundfrom_read_rel. The consumer now handles empty virtual tables, one-row empty relations, and normal virtual table rows usingVirtualTable.expressionsonly.Are these changes tested?
Yes.
Are there any user-facing changes?
No.