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: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to