metesynnada commented on code in PR #5606:
URL: https://github.com/apache/arrow-datafusion/pull/5606#discussion_r1138682158


##########
datafusion/sql/src/statement.rs:
##########
@@ -127,42 +129,66 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
                 && table_properties.is_empty()
                 && with_options.is_empty() =>
             {
-                let plan = self.query_to_plan(*query, planner_context)?;
-                let input_schema = plan.schema();
-
-                let plan = if !columns.is_empty() {
-                    let schema = 
self.build_schema(columns)?.to_dfschema_ref()?;
-                    if schema.fields().len() != input_schema.fields().len() {
-                        return Err(DataFusionError::Plan(format!(
+                match query {
+                    Some(query) => {
+                        let plan = self.query_to_plan(*query, 
planner_context)?;
+                        let input_schema = plan.schema();
+
+                        let plan = if !columns.is_empty() {
+                            let schema = 
self.build_schema(columns)?.to_dfschema_ref()?;

Review Comment:
   The problem of the `NOT NULL` default is still here since you did not change 
the `self.build_schema(columns)`.
   
   This should be the default behaviour:
   ```rust
   pub fn build_schema(&self, columns: Vec<SQLColumnDef>) -> Result<Schema> {
           let mut fields = Vec::with_capacity(columns.len());
   
           for column in columns {
               let data_type = 
self.convert_simple_data_type(&column.data_type)?;
               let prevent_null = column
                   .options
                   .iter()
                   .any(|x| x.option == ColumnOption::NotNull);
               fields.push(Field::new(
                   normalize_ident(column.name, 
self.options.enable_ident_normalization),
                   data_type,
                   !prevent_null,
               ));
           }
   
           Ok(Schema::new(fields))
       }
   ```
   then, it would help if you changed the `sqllogictests`, for example, from
   ```
   describe table_with_normalization
   ----
   field1 Int64 NO
   field2 Int64 NO
   ```
   to
   ```
   describe table_with_normalization
   ----
   field1 Int64 YES
   field2 Int64 YES
   ```
   
   In this way, you can give the same defaults with PostgreSQL.



-- 
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]

Reply via email to