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
ARROW-9889: [Rust][DataFusion] Implement physical plan for EmptyRelation #8083
Conversation
@@ -44,7 +44,7 @@ pub struct ExplainExec { | |||
} | |||
|
|||
impl ExplainExec { | |||
/// Create a new MergeExec | |||
/// Create a new ExplainExec |
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.
I snuck this comment fix in -- I can remove it if people prefer.
@@ -278,6 +278,9 @@ impl DefaultPhysicalPlanner { | |||
ctx_state.config.concurrency, | |||
)?)) | |||
} | |||
LogicalPlan::EmptyRelation { schema } => { |
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.
this is the actual fix
@@ -318,12 +330,9 @@ impl DefaultPhysicalPlanner { | |||
format!("{:#?}", input), | |||
)); | |||
} | |||
let schema_ref = Arc::new((**schema).clone()); | |||
let schema_ref = Arc::new(schema.as_ref().clone()); |
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.
this **
bothered me, so I switched to the different syntax. It has no intended behavior change
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.
I prefer to use idiomatic [language being written]. These changes are very welcoming from my side.
|
||
// Mimic the CLI and execute the resulting plan -- even though it | ||
// is effectively a no-op (returns zero rows) | ||
let results = df.collect().expect("Executing CREATE EXTERNAL TABLE"); |
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.
Prior to the code change, the tests now fail in the same way as reported in ARROW-9889:
---- csv_query_create_external_table stdout ----
thread 'csv_query_create_external_table' panicked at 'Executing CREATE EXTERNAL TABLE: General("Unsupported logical plan variant")', datafusion/tests/sql.rs:526:19
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
f6bc35f
to
e2da41d
Compare
Rebased |
This fixes an error I saw in the Datafusion CLI. Running `CREATE EXTERNAL TABLE` errors with `"Unsupported logical plan variant"` Before this PR, you can't make an external table via the CLI: ``` > create external table test(c1 boolean) stored as CSV location '/tmp/foo'; General("Unsupported logical plan variant") > ``` After this PR: ``` > create external table test(c1 boolean) stored as CSV location '/tmp/foo'; 0 rows in set. Query took 0 seconds. > ``` Other changes: * Made the list of unsupported plan nodes explicit in the planer * Added a test for this behavior. I think this error was introduced in apache#8027 which started (rightly) to treat all execution plans in a uniform way and do less special casing. FYI @andygrove Closes apache#8083 from alamb/alamb/ARROW-9889-create-table Authored-by: alamb <andrew@nerdnetworks.org> Signed-off-by: Andy Grove <andygrove73@gmail.com>
This fixes an error I saw in the Datafusion CLI. Running `CREATE EXTERNAL TABLE` errors with `"Unsupported logical plan variant"` Before this PR, you can't make an external table via the CLI: ``` > create external table test(c1 boolean) stored as CSV location '/tmp/foo'; General("Unsupported logical plan variant") > ``` After this PR: ``` > create external table test(c1 boolean) stored as CSV location '/tmp/foo'; 0 rows in set. Query took 0 seconds. > ``` Other changes: * Made the list of unsupported plan nodes explicit in the planer * Added a test for this behavior. I think this error was introduced in apache#8027 which started (rightly) to treat all execution plans in a uniform way and do less special casing. FYI @andygrove Closes apache#8083 from alamb/alamb/ARROW-9889-create-table Authored-by: alamb <andrew@nerdnetworks.org> Signed-off-by: Andy Grove <andygrove73@gmail.com>
This fixes an error I saw in the Datafusion CLI. Running
CREATE EXTERNAL TABLE
errors with"Unsupported logical plan variant"
Before this PR, you can't make an external table via the CLI:
After this PR:
Other changes:
I think this error was introduced in #8027 which started (rightly) to treat all execution plans in a uniform way and do less special casing. FYI @andygrove