[
https://issues.apache.org/jira/browse/IMPALA-15615?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Arnab Karmakar resolved IMPALA-15615.
-------------------------------------
Fix Version/s: Impala 5.0.0
Resolution: Fixed
> Iceberg V3: ALTER TABLE ADD COLUMN(S) ... NOT NULL DEFAULT adds a nullable
> column
> ---------------------------------------------------------------------------------
>
> Key: IMPALA-15615
> URL: https://issues.apache.org/jira/browse/IMPALA-15615
> Project: IMPALA
> Issue Type: Bug
> Components: Catalog, Frontend
> Reporter: Zoltán Borók-Nagy
> Assignee: Arnab Karmakar
> Priority: Major
> Labels: impala-iceberg
> Fix For: Impala 5.0.0
>
>
> On Iceberg V3 tables, ALTER TABLE ... ADD COLUMN and ADD COLUMNS accept a NOT
> NULL column with a DEFAULT, but the new column is optional. NOT NULL is
> dropped without a warning: DESCRIBE shows the column as nullable, the Iceberg
> schema has "required":false, and NULL can be inserted into it.
> Repro on master:
> {code:sql}
> create table t (i int) stored by iceberg tblproperties('format-version'='3');
> insert into t values (1);
> alter table t add column c int not null default 7;
> alter table t add columns (d int not null default 8);
> describe t;
> {code}
> {noformat}
> name type comment nullable
> i int true
> c int true
> d int true
> {noformat}
> current-schema in DESCRIBE FORMATTED t (trimmed):
> {noformat}
> {\"id\":2,\"name\":\"c\",\"required\":false,\"type\":\"int\",\"initial-default\":7,\"write-default\":7},{\"id\":3,\"name\":\"d\",\"required\":false,\"type\":\"int\",\"initial-default\":8,\"write-default\":8}
> {noformat}
> {code:sql}
> insert into t (i, c, d) values (2, NULL, NULL);
> select i, c, d from t order by i;
> {code}
> {noformat}
> Modified 1 row(s) in 0.18s
> i c d
> 1 7 8
> 2 NULL NULL
> {noformat}
> Expected: c and d are required fields, and the INSERT fails. This is what
> happens when CREATE TABLE defines the same column:
> {code:sql}
> create table s (i int, c int not null default 7) stored by iceberg
> tblproperties('format-version'='3');
> describe s;
> insert into s (i, c) values (2, NULL);
> {code}
> {noformat}
> name type comment nullable
> i int true
> c int false
> ERROR: Query 884c335a4cc21f26:e8f205ea00000000 failed:
> Column 'c' is not nullable but during writing a Parquet page we found 1 null
> values.
> {noformat}
> (current-schema of s has {{"required":true}} for c.)
> *Analysis*
> AlterTableAddColsStmt passes NOT NULL to the catalog: ColumnDef.toThrift()
> sets TColumn.is_nullable to false. Since IMPALA-15364, analysis rejects a NOT
> NULL column without a DEFAULT ("A new non-null column must have a default
> value: ..."), and a DEFAULT needs 'format-version' >= 3. So a NOT NULL column
> reaches the catalog only on V3 tables, and always with a DEFAULT.
> CatalogOpExecutor.alterIcebergTable() then calls
> IcebergCatalogOpExecutor.addColumns(). That method ignores is_nullable and
> always calls UpdateSchema.addColumn(), with or without a default (the default
> branch came with IMPALA-14589), which adds an optional field. CREATE TABLE is
> not affected: IcebergSchemaConverter.createIcebergField() uses
> NestedField.required() when is_nullable is false.
> The IMPALA-15364 commit message suggests {{ALTER TABLE t ADD COLUMNS (j INT
> NOT NULL DEFAULT 0)}} on a V3 table as the way to add a NOT NULL column.
> Right now that statement adds a nullable column. The IMPALA-15364 tests cover
> only the case without a default, and no Iceberg test covers ADD COLUMN(S)
> with NOT NULL DEFAULT.
> *Suggested fix*
> In IcebergCatalogOpExecutor.addColumns(), call
> UpdateSchema.addRequiredColumn(name, type, doc, defaultValue) when
> column.isSetIs_nullable() && !column.isIs_nullable(), and keep addColumn()
> for the other columns. Analysis already guarantees that such a column has a
> default. With a default, Iceberg allows adding a required column without
> allowIncompatibleChanges(): in Iceberg 1.11.0, SchemaUpdate fails with
> "Incompatible change: cannot add required column without a default value"
> only when the default is null.
> *Testing*
> Add V3 end-to-end cases for ADD COLUMN and ADD COLUMNS with NOT NULL DEFAULT.
> They should check that DESCRIBE shows nullable false, old rows read the
> initial default, an INSERT that omits the column gets the write default, and
> an INSERT of NULL fails.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
