You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "jonahgao (via GitHub)" <gi...@apache.org> on 2023/11/20 16:18:38 UTC
[PR] feat: support customizing column default values for inserting [arrow-datafusion]
jonahgao opened a new pull request, #8283:
URL: https://github.com/apache/arrow-datafusion/pull/8283
## Which issue does this PR close?
This is a continuation of #8146.
## Rationale for this change
Currently, we only support NULL as the default value for columns.
This pr will enable users to customize the default value while creating a table.
For example:
```sh
DataFusion CLI v33.0.0
❯ create table t(a int, b int default 100);
0 rows in set. Query took 0.004 seconds.
❯ insert into t(a) values(1);
+-------+
| count |
+-------+
| 1 |
+-------+
1 row in set. Query took 0.007 seconds.
❯ select * from t;
+---+-----+
| a | b |
+---+-----+
| 1 | 100 |
+---+-----+
1 row in set. Query took 0.006 seconds.
```
## What changes are included in this PR?
- Parsing column default values when constructing the `CreateMemory` plan.
- The default value of a column can be obtained from the `TableSource`.
- Fill default value if a column value is not specified during inserting.
- Implement it on `MemTable` for verification.
## Are these changes tested?
Yes
## Are there any user-facing changes?
Yes.
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!--
If there are any breaking changes to public APIs, please add the `api change` label.
-->
- New methods for `TableSource` and `TableProvider`, but provided default implementations.
- New field `column_defaults` in the struct `CreateMemoryTable`.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support customizing column default values for inserting [arrow-datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #8283:
URL: https://github.com/apache/arrow-datafusion/pull/8283#discussion_r1400080866
##########
datafusion/sql/src/planner.rs:
##########
@@ -230,6 +231,41 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
Ok(Schema::new(fields))
}
+ pub(super) fn build_column_defaults(
Review Comment:
would be nice to explain in comments what exactly gets returned in the tuple for future readers, like `<column name, expr>`
I'm even thinking should be that be a map instead of `Vec<>`
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support customizing column default values for inserting [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #8283:
URL: https://github.com/apache/arrow-datafusion/pull/8283
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support customizing column default values for inserting [arrow-datafusion]
Posted by "jonahgao (via GitHub)" <gi...@apache.org>.
jonahgao commented on code in PR #8283:
URL: https://github.com/apache/arrow-datafusion/pull/8283#discussion_r1400092066
##########
datafusion/sql/src/planner.rs:
##########
@@ -230,6 +231,41 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
Ok(Schema::new(fields))
}
+ pub(super) fn build_column_defaults(
+ &self,
+ columns: &Vec<SQLColumnDef>,
+ planner_context: &mut PlannerContext,
+ ) -> Result<Vec<(String, Expr)>> {
+ let mut column_defaults = vec![];
+ // Default expressions are restricted, column references are not allowed
+ let empty_schema = DFSchema::empty();
+ let human_readable_error = |e: DataFusionError| match e {
Review Comment:
Applied, thanks!
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support customizing column default values for inserting [arrow-datafusion]
Posted by "jonahgao (via GitHub)" <gi...@apache.org>.
jonahgao commented on code in PR #8283:
URL: https://github.com/apache/arrow-datafusion/pull/8283#discussion_r1400094450
##########
datafusion/sql/src/planner.rs:
##########
@@ -230,6 +231,41 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
Ok(Schema::new(fields))
}
+ pub(super) fn build_column_defaults(
Review Comment:
A comment has been added for this.
The main reason is that `Hashmap` has not implemented the `Hash` trait, which is required by `CreateMemoryTable`.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support customizing column default values for inserting [arrow-datafusion]
Posted by "jonahgao (via GitHub)" <gi...@apache.org>.
jonahgao commented on code in PR #8283:
URL: https://github.com/apache/arrow-datafusion/pull/8283#discussion_r1399937631
##########
datafusion/sql/src/planner.rs:
##########
@@ -230,6 +229,41 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
Ok(Schema::new(fields))
}
+ pub(super) fn build_column_defaults(
+ &self,
+ columns: &Vec<SQLColumnDef>,
+ planner_context: &mut PlannerContext,
+ ) -> Result<Vec<(String, Expr)>> {
+ let mut column_defaults = vec![];
+ // Default expressions are restricted, column references are not allowed
+ let empty_schema = DFSchema::empty();
+ let human_readable_error = |e: DataFusionError| match e {
+ DataFusionError::SchemaError(SchemaError::FieldNotFound { .. }) => {
+ DataFusionError::Plan(format!(
Review Comment:
I’ve tried using it once, but it returned a `Result`, what is needed here is an `Error`.
##########
datafusion/sql/src/planner.rs:
##########
@@ -230,6 +229,41 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
Ok(Schema::new(fields))
}
+ pub(super) fn build_column_defaults(
+ &self,
+ columns: &Vec<SQLColumnDef>,
+ planner_context: &mut PlannerContext,
+ ) -> Result<Vec<(String, Expr)>> {
+ let mut column_defaults = vec![];
+ // Default expressions are restricted, column references are not allowed
+ let empty_schema = DFSchema::empty();
+ let human_readable_error = |e: DataFusionError| match e {
+ DataFusionError::SchemaError(SchemaError::FieldNotFound { .. }) => {
+ DataFusionError::Plan(format!(
Review Comment:
I’ve tried using it once, but it returned a `Result`, what is needed here is an `Error`.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support customizing column default values for inserting [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #8283:
URL: https://github.com/apache/arrow-datafusion/pull/8283#issuecomment-1822623580
Thank you @jonahgao and @comphead for the review!
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support customizing column default values for inserting [arrow-datafusion]
Posted by "jonahgao (via GitHub)" <gi...@apache.org>.
jonahgao commented on code in PR #8283:
URL: https://github.com/apache/arrow-datafusion/pull/8283#discussion_r1399960021
##########
datafusion/sql/src/planner.rs:
##########
@@ -230,6 +229,41 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
Ok(Schema::new(fields))
}
+ pub(super) fn build_column_defaults(
+ &self,
+ columns: &Vec<SQLColumnDef>,
+ planner_context: &mut PlannerContext,
+ ) -> Result<Vec<(String, Expr)>> {
+ let mut column_defaults = vec![];
+ // Default expressions are restricted, column references are not allowed
+ let empty_schema = DFSchema::empty();
+ let human_readable_error = |e: DataFusionError| match e {
+ DataFusionError::SchemaError(SchemaError::FieldNotFound { .. }) => {
+ DataFusionError::Plan(format!(
Review Comment:
I have changed it to `plan_datafusion_err`.
Thank you @comphead .The code looks cleaner now.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support customizing column default values for inserting [arrow-datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #8283:
URL: https://github.com/apache/arrow-datafusion/pull/8283#discussion_r1399622169
##########
datafusion/sqllogictest/test_files/insert.slt:
##########
@@ -350,3 +350,65 @@ insert into bad_new_empty_table values (1);
statement ok
drop table bad_new_empty_table;
+
+
+### Test for specifying column's default value
+
+statement ok
+create table test_column_defaults(
+ a int,
+ b int not null default null,
Review Comment:
I'm wondering if we can prevent such case....
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support customizing column default values for inserting [arrow-datafusion]
Posted by "jonahgao (via GitHub)" <gi...@apache.org>.
jonahgao commented on PR #8283:
URL: https://github.com/apache/arrow-datafusion/pull/8283#issuecomment-1821955925
Thank you for reviewing @comphead ♥️.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support customizing column default values for inserting [arrow-datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on PR #8283:
URL: https://github.com/apache/arrow-datafusion/pull/8283#issuecomment-1820315820
Thanks @jonahgao I think we're very close
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support customizing column default values for inserting [arrow-datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #8283:
URL: https://github.com/apache/arrow-datafusion/pull/8283#discussion_r1400081741
##########
datafusion/sql/src/planner.rs:
##########
@@ -230,6 +231,41 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
Ok(Schema::new(fields))
}
+ pub(super) fn build_column_defaults(
+ &self,
+ columns: &Vec<SQLColumnDef>,
+ planner_context: &mut PlannerContext,
+ ) -> Result<Vec<(String, Expr)>> {
+ let mut column_defaults = vec![];
+ // Default expressions are restricted, column references are not allowed
+ let empty_schema = DFSchema::empty();
+ let human_readable_error = |e: DataFusionError| match e {
Review Comment:
```suggestion
let error_desc = |e: DataFusionError| match e {
```
or something
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support customizing column default values for inserting [arrow-datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #8283:
URL: https://github.com/apache/arrow-datafusion/pull/8283#discussion_r1399623703
##########
datafusion/sql/src/planner.rs:
##########
@@ -230,6 +229,41 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
Ok(Schema::new(fields))
}
+ pub(super) fn build_column_defaults(
+ &self,
+ columns: &Vec<SQLColumnDef>,
+ planner_context: &mut PlannerContext,
+ ) -> Result<Vec<(String, Expr)>> {
+ let mut column_defaults = vec![];
+ // Default expressions are restricted, column references are not allowed
+ let empty_schema = DFSchema::empty();
+ let human_readable_error = |e: DataFusionError| match e {
+ DataFusionError::SchemaError(SchemaError::FieldNotFound { .. }) => {
+ DataFusionError::Plan(format!(
Review Comment:
please use `plan_err!` macro
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support customizing column default values for inserting [arrow-datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #8283:
URL: https://github.com/apache/arrow-datafusion/pull/8283#discussion_r1399955409
##########
datafusion/sql/src/planner.rs:
##########
@@ -230,6 +229,41 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
Ok(Schema::new(fields))
}
+ pub(super) fn build_column_defaults(
+ &self,
+ columns: &Vec<SQLColumnDef>,
+ planner_context: &mut PlannerContext,
+ ) -> Result<Vec<(String, Expr)>> {
+ let mut column_defaults = vec![];
+ // Default expressions are restricted, column references are not allowed
+ let empty_schema = DFSchema::empty();
+ let human_readable_error = |e: DataFusionError| match e {
+ DataFusionError::SchemaError(SchemaError::FieldNotFound { .. }) => {
+ DataFusionError::Plan(format!(
Review Comment:
> I’ve tried using it once, but it returned a `Result`, what is needed here is an `Error`.
You are right, in this specific case `plan_datafusion_err!` will work
##########
datafusion/sql/src/planner.rs:
##########
@@ -230,6 +229,41 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
Ok(Schema::new(fields))
}
+ pub(super) fn build_column_defaults(
+ &self,
+ columns: &Vec<SQLColumnDef>,
+ planner_context: &mut PlannerContext,
+ ) -> Result<Vec<(String, Expr)>> {
+ let mut column_defaults = vec![];
+ // Default expressions are restricted, column references are not allowed
+ let empty_schema = DFSchema::empty();
+ let human_readable_error = |e: DataFusionError| match e {
+ DataFusionError::SchemaError(SchemaError::FieldNotFound { .. }) => {
+ DataFusionError::Plan(format!(
Review Comment:
> I’ve tried using it once, but it returned a `Result`, what is needed here is an `Error`.
You are right, in this specific case `plan_datafusion_err!` will work
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support customizing column default values for inserting [arrow-datafusion]
Posted by "jonahgao (via GitHub)" <gi...@apache.org>.
jonahgao commented on code in PR #8283:
URL: https://github.com/apache/arrow-datafusion/pull/8283#discussion_r1399935262
##########
datafusion/sqllogictest/test_files/insert.slt:
##########
@@ -350,3 +350,65 @@ insert into bad_new_empty_table values (1);
statement ok
drop table bad_new_empty_table;
+
+
+### Test for specifying column's default value
+
+statement ok
+create table test_column_defaults(
+ a int,
+ b int not null default null,
Review Comment:
I have tested on PostgreSQL, it does not prevent this case.
```sh
psql (14.9 (Ubuntu 14.9-0ubuntu0.22.04.1))
Type "help" for help.
psql=> create table t(a int, b int not null default null);
CREATE TABLE
```
I guess there might be two reasons.
1. Expressions like `nullif(current_date(), '2023-11-21')` require us to check during execution, so checking here might be redundant.
2. The user may define a default value but never use it.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org