-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Support Substrait Round-Trip of EmptyRelation Including produce_one_row Semantics
#18842
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
kosiew
wants to merge
7
commits into
apache:main
Choose a base branch
from
kosiew:empty-relation-16271
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+160
−48
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
bf9e411
Encode EmptyRelation plans as Substrait virtual tables
kosiew f5e2174
Update deprecated values handling in read_rel.rs
kosiew 6bfb73b
Document produce_one_row usage and enhance tests
kosiew c846512
remove allow(deprecated)
kosiew 12ee2a6
Refactor from_read_rel to use convert_literal_rows for processing Vir…
kosiew dacf601
Amend comment from roundtrip_subquery_with_empty_relation test
kosiew d0bf84e
Enhance comment in from_read_rel to clarify the concept of EmptyRelat…
kosiew File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -18,9 +18,10 @@ | |||||
| use crate::logical_plan::producer::{ | ||||||
| to_substrait_literal, to_substrait_named_struct, SubstraitProducer, | ||||||
| }; | ||||||
| use datafusion::common::{not_impl_err, substrait_datafusion_err, DFSchema, ToDFSchema}; | ||||||
| use datafusion::common::{substrait_datafusion_err, DFSchema, ToDFSchema}; | ||||||
| use datafusion::logical_expr::utils::conjunction; | ||||||
| use datafusion::logical_expr::{EmptyRelation, Expr, TableScan, Values}; | ||||||
| use datafusion::scalar::ScalarValue; | ||||||
| use std::sync::Arc; | ||||||
| use substrait::proto::expression::literal::Struct; | ||||||
| use substrait::proto::expression::mask_expression::{StructItem, StructSelect}; | ||||||
|
|
@@ -83,26 +84,61 @@ pub fn from_table_scan( | |||||
| })) | ||||||
| } | ||||||
|
|
||||||
| /// Encodes an EmptyRelation as a Substrait VirtualTable. | ||||||
| /// | ||||||
| /// EmptyRelation represents a relation with no input data. When `produce_one_row` is true, | ||||||
| /// it generates a single row with all fields set to their default values (typically NULL). | ||||||
| /// This is used for queries without a FROM clause, such as "SELECT 1 AS one" or | ||||||
| /// "SELECT current_timestamp()". | ||||||
| /// | ||||||
| /// When `produce_one_row` is false, it represents a truly empty relation with no rows, | ||||||
| /// used in optimizations or as a placeholder. | ||||||
| pub fn from_empty_relation( | ||||||
| producer: &mut impl SubstraitProducer, | ||||||
| e: &EmptyRelation, | ||||||
| ) -> datafusion::common::Result<Box<Rel>> { | ||||||
| if e.produce_one_row { | ||||||
| return not_impl_err!("Producing a row from empty relation is unsupported"); | ||||||
| } | ||||||
| #[allow(deprecated)] | ||||||
| let base_schema = to_substrait_named_struct(producer, &e.schema)?; | ||||||
|
|
||||||
| let read_type = if e.produce_one_row { | ||||||
| // Create one row with default scalar values for each field in the schema. | ||||||
| // For example, an Int32 field gets Int32(NULL), a Utf8 field gets Utf8(NULL), etc. | ||||||
| // This represents the "phantom row" that provides a context for evaluating | ||||||
| // scalar expressions in queries without a FROM clause. | ||||||
| let fields = e | ||||||
| .schema | ||||||
| .fields() | ||||||
| .iter() | ||||||
| .map(|f| { | ||||||
| let scalar = ScalarValue::try_from(f.data_type())?; | ||||||
| to_substrait_literal(producer, &scalar) | ||||||
| }) | ||||||
| .collect::<datafusion::common::Result<_>>()?; | ||||||
|
|
||||||
| ReadType::VirtualTable(VirtualTable { | ||||||
| // Use deprecated 'values' field instead of 'expressions' because the consumer's | ||||||
| // nested expression support (RexType::Nested) is not yet implemented. | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| // The 'values' field uses literal::Struct which the consumer can properly | ||||||
| // deserialize with field name preservation. | ||||||
| #[allow(deprecated)] | ||||||
| values: vec![Struct { fields }], | ||||||
| expressions: vec![], | ||||||
| }) | ||||||
| } else { | ||||||
| ReadType::VirtualTable(VirtualTable { | ||||||
| #[allow(deprecated)] | ||||||
| values: vec![], | ||||||
| expressions: vec![], | ||||||
| }) | ||||||
| }; | ||||||
| Ok(Box::new(Rel { | ||||||
| rel_type: Some(RelType::Read(Box::new(ReadRel { | ||||||
| common: None, | ||||||
| base_schema: Some(to_substrait_named_struct(producer, &e.schema)?), | ||||||
| base_schema: Some(base_schema), | ||||||
| filter: None, | ||||||
| best_effort_filter: None, | ||||||
| projection: None, | ||||||
| advanced_extension: None, | ||||||
| read_type: Some(ReadType::VirtualTable(VirtualTable { | ||||||
| values: vec![], | ||||||
| expressions: vec![], | ||||||
| })), | ||||||
| read_type: Some(read_type), | ||||||
| }))), | ||||||
| })) | ||||||
| } | ||||||
|
|
@@ -134,7 +170,6 @@ pub fn from_values( | |||||
| Ok(Struct { fields }) | ||||||
| }) | ||||||
| .collect::<datafusion::common::Result<_>>()?; | ||||||
| #[allow(deprecated)] | ||||||
| Ok(Box::new(Rel { | ||||||
| rel_type: Some(RelType::Read(Box::new(ReadRel { | ||||||
| common: None, | ||||||
|
|
@@ -144,6 +179,7 @@ pub fn from_values( | |||||
| projection: None, | ||||||
| advanced_extension: None, | ||||||
| read_type: Some(ReadType::VirtualTable(VirtualTable { | ||||||
| #[allow(deprecated)] | ||||||
| values, | ||||||
| expressions: vec![], | ||||||
| })), | ||||||
|
|
||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Oracle the "one row (dummy) table" is called the "dual table", by checking the "In other database systems" section it seems to be quite a well-known name for that concept.
I am not suggesting to necessarily adopt this name here, but maybe a reference to it in the comments could be good for people with a DB background, but not necessarily familiar with Datafusion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a reference to the dual table concept - figured it might help folks coming FROM DUAL backgrounds 😉 Thanks for the nudge!