You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "eliot1480 (via GitHub)" <gi...@apache.org> on 2026/08/13 21:04:51 UTC
[PR] change: fully deprecated the acceptance of value field in VirtualTables [datafusion]
eliot1480 opened a new pull request, #24346:
URL: https://github.com/apache/datafusion/pull/24346
## Which issue does this PR close?
https://github.com/apache/datafusion/issues/23608
- Closes #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.values` consumer path, including the old literal-row conversion helper and the broad deprecation allowance around `from_read_rel`. The consumer now handles empty virtual tables, one-row empty relations, and normal virtual table rows using `VirtualTable.expressions` only.
## Are these changes tested?
Yes.
## Are there any user-facing changes?
No.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] change: fully deprecated the acceptance of value field in VirtualTables [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew merged PR #24346:
URL: https://github.com/apache/datafusion/pull/24346
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] change: fully deprecated the acceptance of value field in VirtualTables [datafusion]
Posted by "vbarua (via GitHub)" <gi...@apache.org>.
vbarua commented on PR #24346:
URL: https://github.com/apache/datafusion/pull/24346#issuecomment-5358172823
There is actually a user-facing change in this PR. If DataFusion consumes Substrait plans that set only the deprecated `VirtualTable.values` fields, those fields will be ignored.
Elliot's prior changes in https://github.com/apache/datafusion/pull/23672, now released in v55.0.0, make it so DataFusion produces plans using the `VirtualTable.expressions` fields. Any plans produced by DataFusion v55.0.0 onwards should be consumed correctly, which will allow users to update any stored plans they may have. Externally produced plans will also be affected by this change. Users sending plans to DataFusion will need to make sure they are setting `VirtualTable.expressions` in order for DataFusion to see them.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] change: fully deprecated the acceptance of value field in VirtualTables [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #24346:
URL: https://github.com/apache/datafusion/pull/24346#issuecomment-5441521435
@vbarua is this one ready to merge from your perspective?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] change: fully deprecated the acceptance of value field in VirtualTables [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on PR #24346:
URL: https://github.com/apache/datafusion/pull/24346#issuecomment-5570311088
🚀
@eliot1480
Thank you for your contribution.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] change: fully deprecated the acceptance of value field in VirtualTables [datafusion]
Posted by "vbarua (via GitHub)" <gi...@apache.org>.
vbarua commented on code in PR #24346:
URL: https://github.com/apache/datafusion/pull/24346#discussion_r3823060507
##########
datafusion/substrait/src/logical_plan/consumer/rel/read_rel.rs:
##########
@@ -114,29 +113,24 @@ pub async fn from_read_rel(
.await
}
Some(ReadType::VirtualTable(vt)) => {
- if vt.values.is_empty() && vt.expressions.is_empty() {
+ if vt.expressions.is_empty() {
return Ok(LogicalPlan::EmptyRelation(EmptyRelation {
produce_one_row: false,
schema: DFSchemaRef::new(substrait_schema),
}));
}
- // Check for produce_one_row pattern in both old (values) and new (expressions) formats.
+ // Check for produce_one_row pattern in the new expressions format.
Review Comment:
minor: it's not particularly useful, IMO, to indicate that this is the "new" expression format here.
```suggestion
// Check for produce_one_row pattern
```
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] change: fully deprecated the acceptance of value field in VirtualTables [datafusion]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #24346:
URL: https://github.com/apache/datafusion/pull/24346#issuecomment-5286596394
## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/24346?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `78.12500%` with `7 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 81.18%. Comparing base ([`f171587`](https://app.codecov.io/gh/apache/datafusion/commit/f17158738b74afbe71b009f197e0c84272062615?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`a03ba1f`](https://app.codecov.io/gh/apache/datafusion/commit/a03ba1f2416fe5aff4e7a42271e23ad27afcf136?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on main.
| [Files with missing lines](https://app.codecov.io/gh/apache/datafusion/pull/24346?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ubstrait/src/logical\_plan/consumer/rel/read\_rel.rs](https://app.codecov.io/gh/apache/datafusion/pull/24346?src=pr&el=tree&filepath=datafusion%2Fsubstrait%2Fsrc%2Flogical_plan%2Fconsumer%2Frel%2Fread_rel.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9zdWJzdHJhaXQvc3JjL2xvZ2ljYWxfcGxhbi9jb25zdW1lci9yZWwvcmVhZF9yZWwucnM=) | 78.12% | [6 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/24346?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #24346 +/- ##
=======================================
Coverage 81.17% 81.18%
=======================================
Files 1109 1109
Lines 388033 387995 -38
Branches 388033 387995 -38
=======================================
- Hits 314985 314975 -10
+ Misses 54509 54478 -31
- Partials 18539 18542 +3
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/24346?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org