You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "wangsheng (Code Review)" <ge...@cloudera.org> on 2020/12/24 09:44:52 UTC

[Impala-ASF-CR] IMPALA-10368: Support required/optional property when creating Iceberg table

wangsheng has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16904


Change subject: IMPALA-10368: Support required/optional property when creating Iceberg table
......................................................................

IMPALA-10368: Support required/optional property when creating Iceberg table

We supported create required/optional field for Iceberg table in this patch.
If we set 'NOT NULL' property for Iceberg table column in SQL, Impala will
create required field by Iceberg api, 'NULL' or default will create optional
field.
Besides, 'DESCRIBE XXX' for Iceberg table will display 'optional' property like
this:
+------+--------+---------+----------+
| name | type   | comment | optional |
+------+--------+---------+----------+
| id   | int    |         | false    |
| name | string |         | true     |
| age  | int    |         | true     |
+------+--------+---------+----------+
And 'SHOW CREATE TABLE XXX' will also display 'NULL'/'NOT NULL' property for Iceberg
table.

Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
---
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/catalog/Column.java
M fe/src/main/java/org/apache/impala/catalog/IcebergColumn.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/IcebergSchemaConverter.java
10 files changed, 92 insertions(+), 13 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/16904/1
-- 
To view, visit http://gerrit.cloudera.org:8080/16904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Gerrit-Change-Number: 16904
Gerrit-PatchSet: 1
Gerrit-Owner: wangsheng <sk...@163.com>

[Impala-ASF-CR] IMPALA-10368: Support required/optional property when creating Iceberg table

Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/16904 )

Change subject: IMPALA-10368: Support required/optional property when creating Iceberg table
......................................................................


Patch Set 3:

There some problems with IcebergOption, I'm still working on it.


-- 
To view, visit http://gerrit.cloudera.org:8080/16904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Gerrit-Change-Number: 16904
Gerrit-PatchSet: 3
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Wed, 06 Jan 2021 12:09:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10368: Support required/optional property when creating Iceberg table

Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/16904 )

Change subject: IMPALA-10368: Support required/optional property when creating Iceberg table
......................................................................

IMPALA-10368: Support required/optional property when creating Iceberg table

We supported create required/optional field for Iceberg table in this
patch. If we set 'NOT NULL' property for Iceberg table column in SQL,
Impala will create required field by Iceberg api, 'NULL' or default
will create optional field.
Besides, 'DESCRIBE XXX' for Iceberg table will display 'optional'
property like this:
+------+--------+---------+----------+
| name | type   | comment | nullable |
+------+--------+---------+----------+
| id   | int    |         | false    |
| name | string |         | true     |
| age  | int    |         | true     |
+------+--------+---------+----------+
And 'SHOW CREATE TABLE XXX' will also display 'NULL'/'NOT NULL'
property for Iceberg table.

Tests:
 * added new test in iceberg-create.test
 * added new test in iceberg-negative.test
 * added new test in show-create-table.test
 * modify 'DESCRIBE XXX' result in iceberg-create.test
 * modify 'DESCRIBE XXX' result in iceberg-alter.test
 * modify create table result in show-create-table.test

Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
---
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/catalog/Column.java
M fe/src/main/java/org/apache/impala/catalog/IcebergColumn.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/IcebergSchemaConverter.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-create.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
14 files changed, 241 insertions(+), 173 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/16904/4
-- 
To view, visit http://gerrit.cloudera.org:8080/16904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Gerrit-Change-Number: 16904
Gerrit-PatchSet: 4
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>

[Impala-ASF-CR] IMPALA-10368: Support required/optional property when creating Iceberg table

Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/16904 )

Change subject: IMPALA-10368: Support required/optional property when creating Iceberg table
......................................................................


Patch Set 3:

(6 comments)

Thanks for review, Zoltan. I found that even if insert into 'NOT NULL' column(required) a null value, the result is success. If we insert the new test table in iceberg-create.test like this:
insert into iceberg_nullable_test (null,cast('2021-01-01' as timestamp),null);
The result is success, and we can alse select this table and get this record. I think this is innormal, we cannot insert a null value to required column.

http://gerrit.cloudera.org:8080/#/c/16904/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16904/2//COMMIT_MSG@9
PS2, Line 9: 
> nit: Commit message lines should be under 72 chars
Done


http://gerrit.cloudera.org:8080/#/c/16904/2//COMMIT_MSG@13
PS2, Line 13: Besides, 'DESCRIBE XXX' for Iceberg table will display 'optional'
            : property like this:
            : +------+--------+---------+----------+
            : | name | type   | comment | nullable |
            : +------+--------+---------+----------+
            : | id   | int    |         | false    |
            : | name | string |         | true     |
            : | age  | int    |         | true     |
            : +------+--------+---------+----------+
            : And 'SHOW CREATE TABLE XXX' will also display 'NULL'/'NOT NULL'
            : proper
> Could you please add tests for these?
Done


http://gerrit.cloudera.org:8080/#/c/16904/2/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:

http://gerrit.cloudera.org:8080/#/c/16904/2/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@275
PS2, Line 275: Column
> nit: this method analyzes all the columns, so the method name could be in p
Done


http://gerrit.cloudera.org:8080/#/c/16904/2/fe/src/main/java/org/apache/impala/analysis/TableDef.java
File fe/src/main/java/org/apache/impala/analysis/TableDef.java:

http://gerrit.cloudera.org:8080/#/c/16904/2/fe/src/main/java/org/apache/impala/analysis/TableDef.java@434
PS2, Line 434: hasKuduOptions()
> nit: maybe in the long-term (in a separate jira/commit) it would be nice to
Done


http://gerrit.cloudera.org:8080/#/c/16904/2/fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java:

http://gerrit.cloudera.org:8080/#/c/16904/2/fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java@a357
PS2, Line 357: 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
> We have similar logic in IcebergSchemaConverter's convertToImpalaSchema() a
Done
I found that we don't need these methods anymore, so I removed related code.


http://gerrit.cloudera.org:8080/#/c/16904/2/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/16904/2/fe/src/main/java/org/apache/impala/service/Frontend.java@489
PS2, Line 489: nullable
> nit: maybe use Impala/SQL terminology here, i.e. "nullable", just like for 
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/16904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Gerrit-Change-Number: 16904
Gerrit-PatchSet: 3
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Tue, 05 Jan 2021 07:36:28 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10368: Support required/optional property when creating Iceberg table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16904 )

Change subject: IMPALA-10368: Support required/optional property when creating Iceberg table
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/7973/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/16904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Gerrit-Change-Number: 16904
Gerrit-PatchSet: 5
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Fri, 08 Jan 2021 02:16:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10368: Support required/optional property when creating Iceberg table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16904 )

Change subject: IMPALA-10368: Support required/optional property when creating Iceberg table
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/7950/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/16904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Gerrit-Change-Number: 16904
Gerrit-PatchSet: 3
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Tue, 05 Jan 2021 07:50:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10368: Support required/optional property when creating Iceberg table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16904 )

Change subject: IMPALA-10368: Support required/optional property when creating Iceberg table
......................................................................

IMPALA-10368: Support required/optional property when creating Iceberg table

We supported create required/optional field for Iceberg table in this
patch. If we set 'NOT NULL' property for Iceberg table column in SQL,
Impala will create required field by Iceberg api, 'NULL' or default
will create optional field.
Besides, 'DESCRIBE XXX' for Iceberg table will display 'optional'
property like this:
+------+--------+---------+----------+
| name | type   | comment | nullable |
+------+--------+---------+----------+
| id   | int    |         | false    |
| name | string |         | true     |
| age  | int    |         | true     |
+------+--------+---------+----------+
And 'SHOW CREATE TABLE XXX' will also display 'NULL'/'NOT NULL'
property for Iceberg table.

Tests:
 * added new test in iceberg-create.test
 * added new test in iceberg-negative.test
 * added new test in show-create-table.test
 * modify 'DESCRIBE XXX' result in iceberg-create.test
 * modify 'DESCRIBE XXX' result in iceberg-alter.test
 * modify create table result in show-create-table.test

Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Reviewed-on: http://gerrit.cloudera.org:8080/16904
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/catalog/Column.java
M fe/src/main/java/org/apache/impala/catalog/IcebergColumn.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/IcebergSchemaConverter.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-create.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
14 files changed, 241 insertions(+), 173 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/16904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Gerrit-Change-Number: 16904
Gerrit-PatchSet: 8
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>

[Impala-ASF-CR] IMPALA-10368: Support required/optional property when creating Iceberg table

Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/16904 )

Change subject: IMPALA-10368: Support required/optional property when creating Iceberg table
......................................................................

IMPALA-10368: Support required/optional property when creating Iceberg table

We supported create required/optional field for Iceberg table in this
patch. If we set 'NOT NULL' property for Iceberg table column in SQL,
Impala will create required field by Iceberg api, 'NULL' or default
will create optional field.
Besides, 'DESCRIBE XXX' for Iceberg table will display 'optional'
property like this:
+------+--------+---------+----------+
| name | type   | comment | nullable |
+------+--------+---------+----------+
| id   | int    |         | false    |
| name | string |         | true     |
| age  | int    |         | true     |
+------+--------+---------+----------+
And 'SHOW CREATE TABLE XXX' will also display 'NULL'/'NOT NULL'
property for Iceberg table.

Tests:
 * added new test in iceberg-create.test
 * added new test in iceberg-negative.test
 * added new test in show-create-table.test
 * modify 'DESCRIBE XXX' result in iceberg-create.test
 * modify 'DESCRIBE XXX' result in iceberg-alter.test
 * modify create table result in show-create-table.test

Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
---
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/catalog/Column.java
M fe/src/main/java/org/apache/impala/catalog/IcebergColumn.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/IcebergSchemaConverter.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-create.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
14 files changed, 241 insertions(+), 173 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/16904/5
-- 
To view, visit http://gerrit.cloudera.org:8080/16904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Gerrit-Change-Number: 16904
Gerrit-PatchSet: 5
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>

[Impala-ASF-CR] IMPALA-10368: Support required/optional property when creating Iceberg table

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16904 )

Change subject: IMPALA-10368: Support required/optional property when creating Iceberg table
......................................................................


Patch Set 2:

(6 comments)

Thanks for working on this, the change LGTM. Only found some nits, and please add some tests.

http://gerrit.cloudera.org:8080/#/c/16904/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16904/2//COMMIT_MSG@9
PS2, Line 9: tch.
nit: Commit message lines should be under 72 chars


http://gerrit.cloudera.org:8080/#/c/16904/2//COMMIT_MSG@13
PS2, Line 13: Besides, 'DESCRIBE XXX' for Iceberg table will display 'optional' property like
            : this:
            : +------+--------+---------+----------+
            : | name | type   | comment | optional |
            : +------+--------+---------+----------+
            : | id   | int    |         | false    |
            : | name | string |         | true     |
            : | age  | int    |         | true     |
            : +------+--------+---------+----------+
            : And 'SHOW CREATE TABLE XXX' will also display 'NULL'/'NOT NULL' property for Iceberg
            : table.
Could you please add tests for these?

Also please add tests for INSERTing nulls in optional/required columns. What kind of error message do we get in the latter case?


http://gerrit.cloudera.org:8080/#/c/16904/2/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:

http://gerrit.cloudera.org:8080/#/c/16904/2/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@275
PS2, Line 275: Column
nit: this method analyzes all the columns, so the method name could be in plural form, i.e. 'analyzeIcebergColumns'


http://gerrit.cloudera.org:8080/#/c/16904/2/fe/src/main/java/org/apache/impala/analysis/TableDef.java
File fe/src/main/java/org/apache/impala/analysis/TableDef.java:

http://gerrit.cloudera.org:8080/#/c/16904/2/fe/src/main/java/org/apache/impala/analysis/TableDef.java@434
PS2, Line 434: hasKuduOptions()
nit: maybe in the long-term (in a separate jira/commit) it would be nice to have IcebergOptions, so we wouldn't hijack KuduOptions.


http://gerrit.cloudera.org:8080/#/c/16904/2/fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java:

http://gerrit.cloudera.org:8080/#/c/16904/2/fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java@a357
PS2, Line 357: 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
We have similar logic in IcebergSchemaConverter's convertToImpalaSchema() and toImpalaType().

Could you refactor the code so we'd only have this logic at one place, in IcebergSchemaConverter?


http://gerrit.cloudera.org:8080/#/c/16904/2/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/16904/2/fe/src/main/java/org/apache/impala/service/Frontend.java@489
PS2, Line 489: optional
nit: maybe use Impala/SQL terminology here, i.e. "nullable", just like for Kudu.



-- 
To view, visit http://gerrit.cloudera.org:8080/16904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Gerrit-Change-Number: 16904
Gerrit-PatchSet: 2
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Mon, 04 Jan 2021 15:09:34 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10368: Support required/optional property when creating Iceberg table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16904 )

Change subject: IMPALA-10368: Support required/optional property when creating Iceberg table
......................................................................


Patch Set 6:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6835/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/16904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Gerrit-Change-Number: 16904
Gerrit-PatchSet: 6
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Fri, 08 Jan 2021 01:55:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10368: Support required/optional property when creating Iceberg table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16904 )

Change subject: IMPALA-10368: Support required/optional property when creating Iceberg table
......................................................................


Patch Set 7: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/16904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Gerrit-Change-Number: 16904
Gerrit-PatchSet: 7
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Mon, 11 Jan 2021 17:08:20 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10368: Support required/optional property when creating Iceberg table

Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/16904 )

Change subject: IMPALA-10368: Support required/optional property when creating Iceberg table
......................................................................

IMPALA-10368: Support required/optional property when creating Iceberg table

We supported create required/optional field for Iceberg table in this patch.
If we set 'NOT NULL' property for Iceberg table column in SQL, Impala will
create required field by Iceberg api, 'NULL' or default will create optional
field.
Besides, 'DESCRIBE XXX' for Iceberg table will display 'optional' property like
this:
+------+--------+---------+----------+
| name | type   | comment | optional |
+------+--------+---------+----------+
| id   | int    |         | false    |
| name | string |         | true     |
| age  | int    |         | true     |
+------+--------+---------+----------+
And 'SHOW CREATE TABLE XXX' will also display 'NULL'/'NOT NULL' property for Iceberg
table.

Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
---
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/catalog/Column.java
M fe/src/main/java/org/apache/impala/catalog/IcebergColumn.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/IcebergSchemaConverter.java
10 files changed, 92 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/16904/2
-- 
To view, visit http://gerrit.cloudera.org:8080/16904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Gerrit-Change-Number: 16904
Gerrit-PatchSet: 2
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>

[Impala-ASF-CR] IMPALA-10368: Support required/optional property when creating Iceberg table

Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/16904 )

Change subject: IMPALA-10368: Support required/optional property when creating Iceberg table
......................................................................


Patch Set 1:

Hi guys, here is my first version for this patch. I change the default behavior of 'nullable' property for Iceberg tables, which means if we don't set 'NOT_NULL'/'NULL' in SQL, we will treat this column as 'optional' (nullable is true). If you agree with this design, I will add test case lately.


-- 
To view, visit http://gerrit.cloudera.org:8080/16904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Gerrit-Change-Number: 16904
Gerrit-PatchSet: 1
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Thu, 24 Dec 2020 09:48:52 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10368: Support required/optional property when creating Iceberg table

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16904 )

Change subject: IMPALA-10368: Support required/optional property when creating Iceberg table
......................................................................


Patch Set 6: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/16904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Gerrit-Change-Number: 16904
Gerrit-PatchSet: 6
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Mon, 11 Jan 2021 11:28:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10368: Support required/optional property when creating Iceberg table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16904 )

Change subject: IMPALA-10368: Support required/optional property when creating Iceberg table
......................................................................


Patch Set 7:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6841/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/16904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Gerrit-Change-Number: 16904
Gerrit-PatchSet: 7
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Mon, 11 Jan 2021 11:29:20 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10368: Support required/optional property when creating Iceberg table

Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/16904 )

Change subject: IMPALA-10368: Support required/optional property when creating Iceberg table
......................................................................


Patch Set 6:

(1 comment)

> (1 comment)
 > 
 > Yeah, the success of inserting null values into non-NULL columns is
 > not ideal. Unfortunately Impala currently doesn't check any
 > constraints during insertion. Adding such constraints is a bigger
 > task, I think it makes reasonable to take care of it later.
 > 
 > I think the current patch is good as it is, as it helps to more
 > accurately specify the Iceberg schema. It's useful in use-cases
 > when one creates the table via Impala, then loads the data via
 > Spark or some other engine.

Thanks, Zoltan. Besides insert problem, I found that, we need to handle with 'ALTER TABLE xxx ADD COLUMN(id int NOT NULL)' situation too. Impala handle with KuduOption for single column in ColumnDef.toThrift(), it's not easy to support IcebergOption which is overlap with KuduOption, maybe we need to redesign some code. Of course, I will try to handle this in another jira.

http://gerrit.cloudera.org:8080/#/c/16904/4/fe/src/main/java/org/apache/impala/analysis/TableDef.java
File fe/src/main/java/org/apache/impala/analysis/TableDef.java:

http://gerrit.cloudera.org:8080/#/c/16904/4/fe/src/main/java/org/apache/impala/analysis/TableDef.java@454
PS4, Line 454: thei
> nit: their
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/16904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Gerrit-Change-Number: 16904
Gerrit-PatchSet: 6
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Fri, 08 Jan 2021 02:07:50 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10368: Support required/optional property when creating Iceberg table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16904 )

Change subject: IMPALA-10368: Support required/optional property when creating Iceberg table
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/7905/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/16904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Gerrit-Change-Number: 16904
Gerrit-PatchSet: 1
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Thu, 24 Dec 2020 10:07:29 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10368: Support required/optional property when creating Iceberg table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16904 )

Change subject: IMPALA-10368: Support required/optional property when creating Iceberg table
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/7906/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/16904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Gerrit-Change-Number: 16904
Gerrit-PatchSet: 2
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Thu, 24 Dec 2020 10:13:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10368: Support required/optional property when creating Iceberg table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16904 )

Change subject: IMPALA-10368: Support required/optional property when creating Iceberg table
......................................................................


Patch Set 7: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/16904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Gerrit-Change-Number: 16904
Gerrit-PatchSet: 7
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Mon, 11 Jan 2021 11:29:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10368: Support required/optional property when creating Iceberg table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16904 )

Change subject: IMPALA-10368: Support required/optional property when creating Iceberg table
......................................................................


Patch Set 6: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/16904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Gerrit-Change-Number: 16904
Gerrit-PatchSet: 6
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Fri, 08 Jan 2021 07:31:32 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10368: Support required/optional property when creating Iceberg table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16904 )

Change subject: IMPALA-10368: Support required/optional property when creating Iceberg table
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/7968/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/16904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Gerrit-Change-Number: 16904
Gerrit-PatchSet: 4
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Thu, 07 Jan 2021 07:22:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10368: Support required/optional property when creating Iceberg table

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16904 )

Change subject: IMPALA-10368: Support required/optional property when creating Iceberg table
......................................................................


Patch Set 4: Code-Review+2

(1 comment)

Yeah, the success of inserting null values into non-NULL columns is not ideal. Unfortunately Impala currently doesn't check any constraints during insertion. Adding such constraints is a bigger task, I think it makes reasonable to take care of it later.

I think the current patch is good as it is, as it helps to more accurately specify the Iceberg schema. It's useful in use-cases when one creates the table via Impala, then loads the data via Spark or some other engine.

http://gerrit.cloudera.org:8080/#/c/16904/4/fe/src/main/java/org/apache/impala/analysis/TableDef.java
File fe/src/main/java/org/apache/impala/analysis/TableDef.java:

http://gerrit.cloudera.org:8080/#/c/16904/4/fe/src/main/java/org/apache/impala/analysis/TableDef.java@454
PS4, Line 454: it's
nit: their



-- 
To view, visit http://gerrit.cloudera.org:8080/16904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Gerrit-Change-Number: 16904
Gerrit-PatchSet: 4
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Thu, 07 Jan 2021 12:40:46 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10368: Support required/optional property when creating Iceberg table

Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/16904 )

Change subject: IMPALA-10368: Support required/optional property when creating Iceberg table
......................................................................

IMPALA-10368: Support required/optional property when creating Iceberg table

We supported create required/optional field for Iceberg table in this
patch. If we set 'NOT NULL' property for Iceberg table column in SQL,
Impala will create required field by Iceberg api, 'NULL' or default
will create optional field.
Besides, 'DESCRIBE XXX' for Iceberg table will display 'optional'
property like this:
+------+--------+---------+----------+
| name | type   | comment | nullable |
+------+--------+---------+----------+
| id   | int    |         | false    |
| name | string |         | true     |
| age  | int    |         | true     |
+------+--------+---------+----------+
And 'SHOW CREATE TABLE XXX' will also display 'NULL'/'NOT NULL'
property for Iceberg table.

Tests:
 * added new test in iceberg-create.test
 * added new test in show-create-table.test
 * modify 'DESCRIBE XXX' result in iceberg-create.test
 * modify 'DESCRIBE XXX' result in iceberg-alter.test
 * modify create table result in show-create-table.test

Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
---
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/catalog/Column.java
M fe/src/main/java/org/apache/impala/catalog/IcebergColumn.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/IcebergSchemaConverter.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-create.test
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
13 files changed, 211 insertions(+), 173 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/16904/3
-- 
To view, visit http://gerrit.cloudera.org:8080/16904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I70b8014ba99f43df1b05149ff7a15cf06b6cd8d3
Gerrit-Change-Number: 16904
Gerrit-PatchSet: 3
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>