findepi commented on code in PR #12864:
URL: https://github.com/apache/datafusion/pull/12864#discussion_r1811367927
##########
datafusion/sql/src/planner.rs:
##########
@@ -154,6 +156,7 @@ impl PlannerContext {
ctes: HashMap::new(),
outer_query_schema: None,
outer_from_schema: None,
+ table_schema: None,
Review Comment:
> Maybe we can name it `create_table_schema` to reflect that it is used for
`CREATE TABLE` processing?
that would work too
##########
datafusion/sql/src/values.rs:
##########
@@ -41,6 +44,11 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
.collect::<Result<Vec<_>>>()
})
.collect::<Result<Vec<_>>>()?;
- LogicalPlanBuilder::values(values)?.build()
+
+ if schema.fields().is_empty() {
+ LogicalPlanBuilder::values(values, None)?.build()
+ } else {
+ LogicalPlanBuilder::values(values, Some(&schema))?.build()
Review Comment:
> how do we know that VALUES being processed here should actually obey the
schema of the "main table" of the query?
My point here is that doing what we're doing here is legal only for certain
query shapes involving CREATE TABLE + VALUES (eg example above), but is not
applicable to other query shapes involving CREATE TABLE and VALUES (e.g. CREATE
TABLE + SELECT .... FROM VALUES)
Where is the logic guaranteeing that table-inferred schema gets applied in
the former case, but not in the latter?
##########
datafusion/sql/src/values.rs:
##########
@@ -31,8 +33,9 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
rows,
} = values;
- // Values should not be based on any other schema
- let schema = DFSchema::empty();
+ let schema = planner_context
+ .table_schema()
+ .unwrap_or(Arc::new(DFSchema::empty()));
Review Comment:
That's good point. I think `sql_to_expr` must still get empty schema. -- The
VALUES being constructed cannot refer to columns of the table being created.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
