metesynnada commented on code in PR #5606:
URL: https://github.com/apache/arrow-datafusion/pull/5606#discussion_r1138717193
##########
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()?;
+ if schema.fields().len() !=
input_schema.fields().len() {
+ return Err(DataFusionError::Plan(format!(
"Mismatch: {} columns specified, but result has {}
columns",
schema.fields().len(),
input_schema.fields().len()
)));
+ }
+ let input_fields = input_schema.fields();
+ let project_exprs = schema
+ .fields()
+ .iter()
+ .zip(input_fields)
+ .map(|(field, input_field)| {
+ cast(
+ col(input_field.name()),
+ field.data_type().clone(),
+ )
+ .alias(field.name())
+ })
+ .collect::<Vec<_>>();
+ LogicalPlanBuilder::from(plan.clone())
+ .project(project_exprs)?
+ .build()?
+ } else {
+ plan
+ };
+
+ Ok(LogicalPlan::CreateMemoryTable(CreateMemoryTable {
+ name: self.object_name_to_table_reference(name)?,
+ input: Arc::new(plan),
+ if_not_exists,
+ or_replace,
+ }))
}
- let input_fields = input_schema.fields();
- let project_exprs = schema
- .fields()
- .iter()
- .zip(input_fields)
- .map(|(field, input_field)| {
- cast(col(input_field.name()),
field.data_type().clone())
- .alias(field.name())
- })
- .collect::<Vec<_>>();
- LogicalPlanBuilder::from(plan.clone())
- .project(project_exprs)?
- .build()?
- } else {
- plan
- };
-
- Ok(LogicalPlan::CreateMemoryTable(CreateMemoryTable {
- name: self.object_name_to_table_reference(name)?,
- input: Arc::new(plan),
- if_not_exists,
- or_replace,
- }))
+
+ None => {
+ let schema =
self.build_schema(columns)?.to_dfschema_ref()?;
+ let plan = EmptyRelation {
+ produce_one_row: false,
+ schema,
+ };
+ let plan = LogicalPlan::EmptyRelation(plan);
Review Comment:
Cool idea 👍🏻
##########
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()?;
+ if schema.fields().len() !=
input_schema.fields().len() {
+ return Err(DataFusionError::Plan(format!(
"Mismatch: {} columns specified, but result has {}
columns",
schema.fields().len(),
input_schema.fields().len()
)));
+ }
+ let input_fields = input_schema.fields();
+ let project_exprs = schema
+ .fields()
+ .iter()
+ .zip(input_fields)
+ .map(|(field, input_field)| {
+ cast(
+ col(input_field.name()),
+ field.data_type().clone(),
+ )
+ .alias(field.name())
+ })
+ .collect::<Vec<_>>();
+ LogicalPlanBuilder::from(plan.clone())
+ .project(project_exprs)?
+ .build()?
+ } else {
+ plan
+ };
+
+ Ok(LogicalPlan::CreateMemoryTable(CreateMemoryTable {
+ name: self.object_name_to_table_reference(name)?,
+ input: Arc::new(plan),
+ if_not_exists,
+ or_replace,
+ }))
}
- let input_fields = input_schema.fields();
- let project_exprs = schema
- .fields()
- .iter()
- .zip(input_fields)
- .map(|(field, input_field)| {
- cast(col(input_field.name()),
field.data_type().clone())
- .alias(field.name())
- })
- .collect::<Vec<_>>();
- LogicalPlanBuilder::from(plan.clone())
- .project(project_exprs)?
- .build()?
- } else {
- plan
- };
-
- Ok(LogicalPlan::CreateMemoryTable(CreateMemoryTable {
- name: self.object_name_to_table_reference(name)?,
- input: Arc::new(plan),
- if_not_exists,
- or_replace,
- }))
+
+ None => {
+ 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]
