This is an automated email from the ASF dual-hosted git repository. yiguolei pushed a commit to branch branch-4.2 in repository https://gitbox.apache.org/repos/asf/doris.git
commit dbb274e1502f722312839d2f17ea7dbc25a6d44d Author: daidai <[email protected]> AuthorDate: Wed Sep 16 10:54:29 2026 +0800 branch-4.1:[fix](iceberg) Preserve nullability in schema metadata (#67856) ### What problem does this PR solve? Problem Summary: Iceberg columns declared `NOT NULL` are reported as nullable by `DESC`, `SHOW CREATE TABLE`, `SHOW [FULL] COLUMNS`, and `information_schema.columns`. For example, `id BIGINT NOT NULL` appears as `Null: Yes` / `IS_NULLABLE: YES` or `bigint NULL`, even though inserting NULL is rejected. Build independent display columns from one retained Iceberg metadata generation and preserve their top-level required/optional flags. These metadata-only paths use the display schema, including `SHOW COLUMNS` with `WHERE` through `information_schema.columns`. Metadata enumeration retains per-table schema-load failure handling. Scan columns keep their existing nullable semantics for historical files and schema evolution; nested-type and write-validation behavior is preserved. ### Release note Correct top-level Iceberg column nullability in `DESC`, `SHOW CREATE TABLE`, `SHOW [FULL] COLUMNS`, and `information_schema.columns`. ### Check List (For Author) - Test - [x] Regression test - [x] Unit Test - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - [ ] This is a refactor/code format and no logic has been changed. - [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason - Behavior changed: - [ ] No. - [x] Yes. - Does this need documentation? - [x] No. - [ ] Yes. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label --- .../main/java/org/apache/doris/catalog/Env.java | 3 +- .../apache/doris/common/proc/IndexInfoProcDir.java | 3 + .../datasource/iceberg/IcebergExternalTable.java | 41 ++- .../doris/datasource/iceberg/IcebergUtils.java | 10 + .../trees/plans/commands/ShowColumnsCommand.java | 4 +- .../apache/doris/service/FrontendServiceImpl.java | 16 +- .../iceberg/IcebergSchemaDisplayTest.java | 297 +++++++++++++++++++++ .../iceberg/iceberg_schema_change_ddl.out | 72 ++--- .../iceberg/test_iceberg_show_nullable.groovy | 117 ++++++++ 9 files changed, 517 insertions(+), 46 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java index 2fc29c44c0e..43969050b50 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java @@ -4552,7 +4552,8 @@ public class Env { sb.append(" (\n"); int idx = 0; - List<Column> columns = table.getBaseSchema(false); + List<Column> columns = table instanceof IcebergExternalTable + ? ((IcebergExternalTable) table).getBaseSchemaForDisplay(false) : table.getBaseSchema(false); for (Column column : columns) { if (idx++ != 0) { sb.append(",\n"); diff --git a/fe/fe-core/src/main/java/org/apache/doris/common/proc/IndexInfoProcDir.java b/fe/fe-core/src/main/java/org/apache/doris/common/proc/IndexInfoProcDir.java index 51dd6d0dc1a..d1600b2b622 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/common/proc/IndexInfoProcDir.java +++ b/fe/fe-core/src/main/java/org/apache/doris/common/proc/IndexInfoProcDir.java @@ -23,6 +23,7 @@ import org.apache.doris.catalog.MaterializedIndexMeta; import org.apache.doris.catalog.OlapTable; import org.apache.doris.catalog.TableIf; import org.apache.doris.common.AnalysisException; +import org.apache.doris.datasource.iceberg.IcebergExternalTable; import org.apache.doris.qe.SessionVariable; import com.google.common.base.Joiner; @@ -132,6 +133,8 @@ public class IndexInfoProcDir implements ProcDirInterface { && SessionVariable.enableDescribeExtendVariantColumn()) { return new RemoteIndexSchemaProcDir(table, schema, bfColumns); } + } else if (table instanceof IcebergExternalTable) { + schema = ((IcebergExternalTable) table).getBaseSchemaForDisplay(); } else { schema = table.getBaseSchema(); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalTable.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalTable.java index 9c9ee6d53b6..aabf3195676 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalTable.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalTable.java @@ -55,6 +55,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Maps; import com.google.common.collect.Sets; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.exception.ExceptionUtils; import org.apache.iceberg.PartitionField; import org.apache.iceberg.PartitionSpec; import org.apache.iceberg.Table; @@ -303,18 +304,44 @@ public class IcebergExternalTable extends ExternalTable implements MTMVRelatedTa List<Column> schema = IcebergUtils.getIcebergSchema(this, snapshot); schema = new ArrayList<>(schema); - if (Util.showHiddenColumns() || needInternalHiddenColumns()) { - schema.add(createIcebergRowIdColumn()); - } - Optional<Table> snapshotTable = snapshot .filter(IcebergMvccSnapshot.class::isInstance) .map(IcebergMvccSnapshot.class::cast) .flatMap(value -> value.getSnapshotCacheValue().getIcebergTable()); // Row-lineage fields are part of the pinned schema generation, not the refreshable table. - schema = IcebergUtils.appendRowLineageColumnsForV3( - schema, snapshotTable.orElseGet(this::getIcebergTable)); - return schema; + return appendHiddenColumns(schema, snapshotTable.orElseGet(this::getIcebergTable)); + } + + public List<Column> getBaseSchemaForDisplay() { + return getBaseSchemaForDisplay(Util.showHiddenColumns() || needInternalHiddenColumns()); + } + + /** Schema display uses declared nullability, independently of scan nullability. */ + public List<Column> getBaseSchemaForDisplay(boolean full) { + if (isView()) { + return getBaseSchema(full); + } + try { + return catalog.getExecutionAuthenticator().execute(() -> { + // Schema-only changes need not advance the current snapshot. Resolve the current + // table schema and hidden columns from one retained metadata generation. + Table table = IcebergSnapshotCacheValue.retainTableGeneration(getIcebergTable()); + List<Column> schema = IcebergUtils.parseSchemaForDisplay(table.schema(), + catalog.getEnableMappingVarbinary(), catalog.getEnableMappingTimestampTz()); + new SchemaCacheValue(schema).validateSchema(); + schema = appendHiddenColumns(schema, table); + return full ? schema : schema.stream().filter(Column::isVisible).collect(Collectors.toList()); + }); + } catch (Exception e) { + throw new RuntimeException(ExceptionUtils.getRootCauseMessage(e), e); + } + } + + private List<Column> appendHiddenColumns(List<Column> schema, Table table) { + if (Util.showHiddenColumns() || needInternalHiddenColumns()) { + schema.add(createIcebergRowIdColumn()); + } + return IcebergUtils.appendRowLineageColumnsForV3(schema, table); } private Column createIcebergRowIdColumn() { diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergUtils.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergUtils.java index da4513ff8a1..76b731b410b 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergUtils.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergUtils.java @@ -1423,6 +1423,16 @@ public class IcebergUtils { return resSchema; } + /** Build independent display columns without changing the nullable columns used by scans. */ + public static List<Column> parseSchemaForDisplay(Schema schema, boolean enableMappingVarbinary, + boolean enableMappingTimestampTz) { + List<Column> columns = parseSchema(schema, enableMappingVarbinary, enableMappingTimestampTz); + for (Column column : columns) { + column.setIsAllowNull(schema.findField(column.getUniqueId()).isOptional()); + } + return columns; + } + /** Convert one Iceberg field to a Doris column without using the generic Doris default-value channel. */ public static Column parseField(Types.NestedField field, boolean enableMappingVarbinary, boolean enableMappingTimestampTz) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ShowColumnsCommand.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ShowColumnsCommand.java index 221482dba7d..65705b2af0c 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ShowColumnsCommand.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ShowColumnsCommand.java @@ -28,6 +28,7 @@ import org.apache.doris.common.ErrorCode; import org.apache.doris.common.ErrorReport; import org.apache.doris.common.PatternMatcher; import org.apache.doris.common.PatternMatcherWrapper; +import org.apache.doris.datasource.iceberg.IcebergExternalTable; import org.apache.doris.info.TableNameInfo; import org.apache.doris.mysql.privilege.PrivPredicate; import org.apache.doris.nereids.analyzer.UnboundSlot; @@ -184,7 +185,8 @@ public class ShowColumnsCommand extends ShowCommand { } table.readLock(); try { - List<Column> columns = table.getBaseSchema(); + List<Column> columns = table instanceof IcebergExternalTable + ? ((IcebergExternalTable) table).getBaseSchemaForDisplay() : table.getBaseSchema(); for (Column col : columns) { if (matcher != null && !matcher.match(col.getName())) { continue; diff --git a/fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java b/fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java index f05cf689b53..54101ad5640 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java +++ b/fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java @@ -93,6 +93,7 @@ import org.apache.doris.datasource.ExternalCatalog; import org.apache.doris.datasource.ExternalDatabase; import org.apache.doris.datasource.InternalCatalog; import org.apache.doris.datasource.SplitSource; +import org.apache.doris.datasource.iceberg.IcebergExternalTable; import org.apache.doris.datasource.maxcompute.MCTransaction; import org.apache.doris.encryption.EncryptionKey; import org.apache.doris.ha.FrontendNodeType; @@ -985,7 +986,7 @@ public class FrontendServiceImpl implements FrontendService.Iface { if (table != null && !table.isTemporary()) { table.readLock(); try { - List<Column> baseSchema = table.getBaseSchemaOrEmpty(); + List<Column> baseSchema = getBaseSchemaForDisplayOrEmpty(table); for (Column column : baseSchema) { final TColumnDesc desc = getColumnDesc(column); final TColumnDef colDef = new TColumnDef(desc); @@ -1017,6 +1018,19 @@ public class FrontendServiceImpl implements FrontendService.Iface { return result; } + private List<Column> getBaseSchemaForDisplayOrEmpty(TableIf table) { + if (!(table instanceof IcebergExternalTable)) { + return table.getBaseSchemaOrEmpty(); + } + try { + return ((IcebergExternalTable) table).getBaseSchemaForDisplay(); + } catch (Exception e) { + // Keep the per-table failure handling of getBaseSchemaOrEmpty for metadata enumeration. + LOG.warn("failed to get display schema for table {}", table.getName(), e); + return Lists.newArrayList(); + } + } + private TColumnDesc getColumnDesc(Column column) { final TColumnDesc desc = new TColumnDesc(column.getName(), column.getDataType().toThrift()); final Integer precision = column.getOriginType().getPrecision(); diff --git a/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/IcebergSchemaDisplayTest.java b/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/IcebergSchemaDisplayTest.java new file mode 100644 index 00000000000..d33799d361b --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/IcebergSchemaDisplayTest.java @@ -0,0 +1,297 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.datasource.iceberg; + +import org.apache.doris.analysis.UserIdentity; +import org.apache.doris.catalog.Column; +import org.apache.doris.catalog.Env; +import org.apache.doris.catalog.TableIf; +import org.apache.doris.common.proc.IndexInfoProcDir; +import org.apache.doris.common.security.authentication.ExecutionAuthenticator; +import org.apache.doris.datasource.CatalogMgr; +import org.apache.doris.info.TableNameInfo; +import org.apache.doris.mysql.privilege.AccessControllerManager; +import org.apache.doris.mysql.privilege.PrivPredicate; +import org.apache.doris.nereids.trees.expressions.SlotReference; +import org.apache.doris.nereids.trees.expressions.StatementScopeIdGenerator; +import org.apache.doris.nereids.trees.plans.commands.ShowColumnsCommand; +import org.apache.doris.qe.ConnectContext; +import org.apache.doris.service.FrontendServiceImpl; +import org.apache.doris.thrift.TDescribeTablesParams; +import org.apache.doris.thrift.TDescribeTablesResult; + +import org.apache.iceberg.BaseTable; +import org.apache.iceberg.PartitionSpec; +import org.apache.iceberg.Schema; +import org.apache.iceberg.TableMetadata; +import org.apache.iceberg.TableOperations; +import org.apache.iceberg.TableProperties; +import org.apache.iceberg.io.FileIO; +import org.apache.iceberg.io.LocationProvider; +import org.apache.iceberg.types.Types; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +class IcebergSchemaDisplayTest { + private IcebergExternalCatalog catalog; + private IcebergExternalDatabase database; + private IcebergExternalTable table; + private TableOperations operations; + private TableMetadata metadata; + private Schema schema; + private List<Column> scanColumns; + private ConnectContext context; + + @BeforeEach + void setUp() { + context = new ConnectContext(); + context.setThreadLocalInfo(); + schema = new Schema( + Types.NestedField.required(1, "id", Types.LongType.get()), + Types.NestedField.required(2, "value", Types.StringType.get(), "value doc"), + Types.NestedField.optional(3, "event_time", Types.TimestampType.withoutZone()), + Types.NestedField.required(4, "payload", Types.StructType.of( + Types.NestedField.required(5, "child", Types.IntegerType.get(), "nested doc"))), + Types.NestedField.optional(6, "zoned_time", Types.TimestampType.withZone()), + Types.NestedField.optional(7, "bytes", Types.BinaryType.get())); + metadata = TableMetadata.newTableMetadata(schema, PartitionSpec.unpartitioned(), + "file:/tmp/iceberg-schema-display", Collections.singletonMap(TableProperties.FORMAT_VERSION, "2")); + operations = Mockito.mock(TableOperations.class); + Mockito.when(operations.current()).thenReturn(metadata); + Mockito.when(operations.io()).thenReturn(Mockito.mock(FileIO.class)); + Mockito.when(operations.locationProvider()).thenReturn(Mockito.mock(LocationProvider.class)); + + catalog = Mockito.mock(IcebergExternalCatalog.class); + Mockito.when(catalog.getExecutionAuthenticator()).thenReturn(new ExecutionAuthenticator() { }); + Mockito.when(catalog.getEnableMappingVarbinary()).thenReturn(true); + database = Mockito.spy(new IcebergExternalDatabase(catalog, 2L, "db", "db")); + table = Mockito.spy(new IcebergExternalTable(3, "required_tbl", "required_tbl", catalog, database)); + Mockito.doNothing().when(table).makeSureInitialized(); + Mockito.doReturn(new BaseTable(operations, "required_tbl")).when(table).getIcebergTable(); + // Model the cached scan schema. Displaying a table must never mutate these shared columns. + scanColumns = IcebergUtils.parseSchema(schema, true, false); + Mockito.doReturn(scanColumns).when(table).getFullSchema(); + } + + @AfterEach + void tearDown() { + ConnectContext.remove(); + } + + @Test + void testDescribeAndDdlPreserveDeclaredNullabilityWithoutChangingScanColumns() throws Exception { + context.getSessionVariable().showColumnCommentInDescribe = true; + List<List<String>> rows = describe(); + Assertions.assertEquals("No", rows.get(0).get(2)); + Assertions.assertEquals("No", rows.get(1).get(2)); + Assertions.assertEquals("Yes", rows.get(2).get(2)); + Assertions.assertEquals("No", rows.get(3).get(2)); + Assertions.assertEquals("value doc", rows.get(1).get(6)); + Assertions.assertTrue(rows.get(3).get(1).contains("child:int not null comment 'nested doc'")); + Assertions.assertEquals("WITH_TIMEZONE", rows.get(4).get(5)); + Assertions.assertTrue(rows.get(5).get(1).startsWith("varbinary")); + + String ddl = showCreate(); + Assertions.assertTrue(ddl.contains("`id` bigint NOT NULL")); + Assertions.assertTrue(ddl.contains("`value` text NOT NULL")); + Assertions.assertTrue(ddl.contains("`event_time` datetimev2(6) NULL")); + Assertions.assertTrue(ddl.contains("struct<child:int not null comment 'nested doc'> NOT NULL")); + Assertions.assertEquals(rows, describe()); + Assertions.assertEquals(ddl, showCreate()); + Assertions.assertTrue(scanColumns.stream().allMatch(Column::isAllowNull)); + Assertions.assertTrue(scanColumns.get(3).getChildren().get(0).isAllowNull()); + Assertions.assertTrue(SlotReference.fromColumn(StatementScopeIdGenerator.newExprId(), + table, scanColumns.get(0), Collections.emptyList()).nullable()); + } + + @Test + void testSchemaOnlyChangeUsesCurrentTableSchema() throws Exception { + Assertions.assertNull(metadata.currentSnapshot()); + Assertions.assertEquals("No", describe().get(1).get(2)); + List<Types.NestedField> fields = new ArrayList<>(schema.columns()); + fields.set(1, Types.NestedField.optional(2, "value", Types.StringType.get(), "value doc")); + TableMetadata evolved = metadata.updateSchema(new Schema(fields)); + Mockito.when(operations.current()).thenReturn(evolved); + + Assertions.assertNull(evolved.currentSnapshot()); + Assertions.assertNotEquals(metadata.currentSchemaId(), evolved.currentSchemaId()); + Assertions.assertEquals("Yes", describe().get(1).get(2)); + Assertions.assertTrue(showCreate().contains("`value` text NULL")); + Assertions.assertTrue(showCreate().contains("`id` bigint NOT NULL")); + } + + @Test + void testDisplayRetainsOneMetadataGeneration() { + List<Types.NestedField> fields = new ArrayList<>(schema.columns()); + fields.set(1, Types.NestedField.optional(2, "renamed_value", Types.StringType.get())); + TableMetadata evolved = metadata.updateSchema(new Schema(fields)) + .upgradeToFormatVersion(3); + Mockito.when(operations.current()).thenReturn(metadata, evolved); + Mockito.clearInvocations(operations); + + List<Column> displayed = table.getBaseSchemaForDisplay(true); + Assertions.assertEquals(schema.columns().size(), displayed.size()); + Assertions.assertEquals("value", displayed.get(1).getName()); + Assertions.assertFalse(displayed.get(1).isAllowNull()); + Mockito.verify(operations, Mockito.times(1)).current(); + } + + @Test + void testHiddenColumnsFollowExistingVisibilityRules() throws Exception { + Mockito.when(operations.current()).thenReturn(metadata.upgradeToFormatVersion(3)); + Assertions.assertEquals(schema.columns().size(), describe().size()); + context.getSessionVariable().setShowHiddenColumns(true); + List<List<String>> rows = describe(); + Assertions.assertEquals(schema.columns().size() + 3, rows.size()); + Assertions.assertTrue(rows.stream().anyMatch(row -> row.get(0).equals(IcebergUtils.ICEBERG_ROW_ID_COL))); + Assertions.assertTrue(rows.stream().anyMatch(row -> row.get(0).equals( + IcebergUtils.ICEBERG_LAST_UPDATED_SEQUENCE_NUMBER_COL))); + Assertions.assertEquals(schema.columns().size(), table.getBaseSchemaForDisplay(false).size()); + Assertions.assertFalse(showCreate().contains("`" + IcebergUtils.ICEBERG_ROW_ID_COL + "`")); + Assertions.assertFalse(showCreate().contains("`" + IcebergRowId.createHiddenColumn().getName() + "`")); + } + + @Test + void testViewsKeepTheirExistingSchemaPath() throws Exception { + Mockito.doReturn(true).when(table).isView(); + Assertions.assertEquals("Yes", describe().get(0).get(2)); + Mockito.verify(table, Mockito.never()).getIcebergTable(); + } + + @Test + void testOtherTablesKeepTheirExistingSchemaPath() throws Exception { + TableIf otherTable = Mockito.mock(TableIf.class); + Mockito.when(otherTable.getBaseSchema()).thenReturn(scanColumns); + List<List<String>> rows = new IndexInfoProcDir(database, otherTable).lookup("4").fetchResult().getRows(); + Assertions.assertEquals("Yes", rows.get(0).get(2)); + Mockito.verify(otherTable).getBaseSchema(); + } + + @Test + void testShowColumnsUsesDeclaredNullability() throws Exception { + try (MockedStatic<Env> ignored = mockMetadataEnv()) { + for (boolean full : new boolean[] {false, true}) { + int nullIndex = full ? 3 : 2; + ShowColumnsCommand command = new ShowColumnsCommand(full, + new TableNameInfo("iceberg", "db", "required_tbl"), null, null, null); + List<List<String>> rows = command.doRun(context, null).getResultRows(); + Assertions.assertEquals("NO", rows.get(0).get(nullIndex)); + Assertions.assertEquals("NO", rows.get(1).get(nullIndex)); + Assertions.assertEquals("YES", rows.get(2).get(nullIndex)); + ShowColumnsCommand filtered = new ShowColumnsCommand(full, + new TableNameInfo("iceberg", "db", "required_tbl"), null, "id", null); + List<List<String>> filteredRows = filtered.doRun(context, null).getResultRows(); + Assertions.assertEquals(1, filteredRows.size()); + Assertions.assertEquals("id", filteredRows.get(0).get(0)); + Assertions.assertEquals("NO", filteredRows.get(0).get(nullIndex)); + } + } + Assertions.assertTrue(scanColumns.stream().allMatch(Column::isAllowNull)); + } + + @Test + void testDescribeTablesUsesDeclaredNullabilityWithoutSessionContext() throws Exception { + try (MockedStatic<Env> ignored = mockMetadataEnv()) { + // information_schema requests arrive on an RPC thread, without a SQL session context. + ConnectContext.remove(); + TDescribeTablesResult result = describeTables("required_tbl"); + Assertions.assertEquals(Collections.singletonList(scanColumns.size()), result.getTablesOffset()); + Assertions.assertFalse(result.getColumns().get(0).getColumnDesc().isIsAllowNull()); + Assertions.assertFalse(result.getColumns().get(1).getColumnDesc().isIsAllowNull()); + Assertions.assertTrue(result.getColumns().get(2).getColumnDesc().isIsAllowNull()); + Assertions.assertEquals("value doc", result.getColumns().get(1).getComment()); + } + Assertions.assertTrue(scanColumns.stream().allMatch(Column::isAllowNull)); + Assertions.assertTrue(scanColumns.get(3).getChildren().get(0).isAllowNull()); + } + + @Test + void testDescribeTablesKeepsOffsetsWhenDisplaySchemaFails() throws Exception { + try (MockedStatic<Env> ignored = mockMetadataEnv()) { + IcebergExternalTable unavailable = Mockito.mock(IcebergExternalTable.class); + Mockito.when(unavailable.getBaseSchemaForDisplay()).thenThrow(new RuntimeException("schema unavailable")); + Mockito.doReturn(unavailable).when(database).getTableNullableIfException("unavailable"); + TDescribeTablesResult result = describeTables("unavailable", "required_tbl"); + Assertions.assertEquals(Arrays.asList(0, scanColumns.size()), result.getTablesOffset()); + Assertions.assertEquals(scanColumns.size(), result.getColumns().size()); + Assertions.assertFalse(result.getColumns().get(0).getColumnDesc().isIsAllowNull()); + } + } + + @Test + void testDescribeTablesChecksPrivilegesBeforeLoadingSchema() throws Exception { + try (MockedStatic<Env> ignored = mockMetadataEnv()) { + Mockito.when(Env.getCurrentEnv().getAccessManager().checkTblPriv(Mockito.any(UserIdentity.class), + Mockito.eq("iceberg"), Mockito.eq("db"), Mockito.eq("required_tbl"), + Mockito.eq(PrivPredicate.SHOW))).thenReturn(false); + TDescribeTablesResult result = describeTables("required_tbl"); + Assertions.assertTrue(result.getColumns().isEmpty()); + Mockito.verify(table, Mockito.never()).getBaseSchemaForDisplay(); + } + } + + private MockedStatic<Env> mockMetadataEnv() throws Exception { + Env env = Mockito.mock(Env.class); + CatalogMgr catalogMgr = Mockito.mock(CatalogMgr.class); + AccessControllerManager accessManager = Mockito.mock(AccessControllerManager.class); + Mockito.when(env.getCatalogMgr()).thenReturn(catalogMgr); + Mockito.when(env.getAccessManager()).thenReturn(accessManager); + Mockito.doReturn(catalog).when(catalogMgr).getCatalogOrAnalysisException("iceberg"); + Mockito.doReturn(catalog).when(catalogMgr).getCatalogOrException(Mockito.eq("iceberg"), Mockito.any()); + Mockito.doReturn(database).when(catalog).getDbOrAnalysisException("db"); + Mockito.doReturn(database).when(catalog).getDbNullable("db"); + Mockito.doReturn(table).when(database).getTableOrAnalysisException("required_tbl"); + Mockito.doReturn(table).when(database).getTableNullableIfException("required_tbl"); + Mockito.when(accessManager.checkTblPriv(Mockito.any(ConnectContext.class), Mockito.anyString(), + Mockito.anyString(), Mockito.anyString(), Mockito.eq(PrivPredicate.SHOW))).thenReturn(true); + Mockito.when(accessManager.checkTblPriv(Mockito.any(UserIdentity.class), Mockito.anyString(), + Mockito.anyString(), Mockito.anyString(), Mockito.eq(PrivPredicate.SHOW))).thenReturn(true); + MockedStatic<Env> mocked = Mockito.mockStatic(Env.class); + mocked.when(Env::getCurrentEnv).thenReturn(env); + return mocked; + } + + private TDescribeTablesResult describeTables(String... names) throws Exception { + TDescribeTablesParams params = new TDescribeTablesParams(); + params.setCatalog("iceberg"); + params.setDb("db"); + params.setTablesName(Arrays.asList(names)); + params.setCurrentUserIdent(UserIdentity.ROOT.toThrift()); + // The RPC handler does not need the background report thread started by its constructor. + return Mockito.mock(FrontendServiceImpl.class, Mockito.CALLS_REAL_METHODS).describeTables(params); + } + + private List<List<String>> describe() throws Exception { + return new IndexInfoProcDir(database, table).lookup("3").fetchResult().getRows(); + } + + private String showCreate() { + List<String> statements = new ArrayList<>(); + Env.getDdlStmt(table, statements, null, null, false, true, -1L); + return statements.get(0); + } +} diff --git a/regression-test/data/external_table_p0/iceberg/iceberg_schema_change_ddl.out b/regression-test/data/external_table_p0/iceberg/iceberg_schema_change_ddl.out index 4565871aa8a..a67ef7f8a16 100644 --- a/regression-test/data/external_table_p0/iceberg/iceberg_schema_change_ddl.out +++ b/regression-test/data/external_table_p0/iceberg/iceberg_schema_change_ddl.out @@ -1,9 +1,9 @@ -- This file is automatically generated. You should know what you did if you want to edit this -- !init_1 -- -id int Yes true \N -name text Yes true \N -age int Yes true \N -score double Yes true \N +id int No true \N +name text No true \N +age int No true \N +score double No true \N -- !init_2 -- 1 Alice 25 95.5 @@ -11,11 +11,11 @@ score double Yes true \N 3 Charlie 22 92.8 -- !add_1 -- -id int Yes true \N -name text Yes true \N -age int Yes true \N +id int No true \N +name text No true \N +age int No true \N phone text Yes true \N User phone number -score double Yes true \N +score double No true \N email text Yes true \N -- !add_2 -- @@ -33,11 +33,11 @@ email text Yes true \N 3 \N -- !add_multi_1 -- -id int Yes true \N -name text Yes true \N -age int Yes true \N +id int No true \N +name text No true \N +age int No true \N phone text Yes true \N User phone number -score double Yes true \N +score double No true \N email text Yes true \N address struct<city:text,country:text> Yes true \N @@ -52,11 +52,11 @@ address struct<city:text,country:text> Yes true \N 5 {"city":"New York", "country":"USA"} -- !rename_1 -- -id int Yes true \N -name text Yes true \N -age int Yes true \N +id int No true \N +name text No true \N +age int No true \N phone text Yes true \N User phone number -grade double Yes true \N +grade double No true \N email text Yes true \N address struct<city:text,country:text> Yes true \N @@ -73,10 +73,10 @@ address struct<city:text,country:text> Yes true \N 5 Eve 26 223-345-132 91.3 [email protected] {"city":"New York", "country":"USA"} -- !drop_1 -- -id int Yes true \N -age int Yes true \N +id int No true \N +age int No true \N phone text Yes true \N User phone number -grade double Yes true \N +grade double No true \N email text Yes true \N address struct<city:text,country:text> Yes true \N @@ -93,10 +93,10 @@ address struct<city:text,country:text> Yes true \N 5 26 -- !add_columns_1 -- -id int Yes true \N -age int Yes true \N +id int No true \N +age int No true \N phone text Yes true \N User phone number -grade double Yes true \N +grade double No true \N email text Yes true \N address struct<city:text,country:text> Yes true \N col1 float Yes true \N User defined column1 @@ -111,9 +111,9 @@ col2 text Yes true \N User defined column2 -- !modify_1 -- age bigint Yes true \N -id int Yes true \N +id int No true \N phone text Yes true \N User phone number -grade double Yes true \N +grade double No true \N email text Yes true \N address struct<city:text,country:text> Yes true \N col1 double Yes true \N Updated column1 type @@ -134,9 +134,9 @@ col2 text Yes true \N User defined column2 -- !before_no_comment -- age bigint Yes true \N -id int Yes true \N +id int No true \N phone text Yes true \N User phone number -grade double Yes true \N +grade double No true \N email text Yes true \N address struct<city:text,country:text> Yes true \N col1 double Yes true \N Updated column1 type @@ -144,9 +144,9 @@ col2 text Yes true \N User defined column2 -- !after_no_comment -- age bigint Yes true \N -id int Yes true \N +id int No true \N phone text Yes true \N User phone number -grade double Yes true \N +grade double No true \N email text Yes true \N address struct<city:text,country:text> Yes true \N col1 double Yes true \N Updated column1 type @@ -154,9 +154,9 @@ col2 text Yes true \N User defined column2 -- !after_no_comment -- age bigint Yes true \N -id int Yes true \N +id int No true \N phone text Yes true \N User phone number -grade double Yes true \N +grade double No true \N email text Yes true \N address struct<city:text,country:text> Yes true \N col1 double Yes true \N Updated column1 type @@ -164,9 +164,9 @@ col2 text Yes true \N User defined column2 -- !modify_positive_1 -- age bigint Yes true \N -id int Yes true \N +id int No true \N phone text Yes true \N User phone number -grade double Yes true \N +grade double No true \N email text Yes true \N address struct<city:text,country:text> Yes true \N col1 double Yes true \N Updated column1 type @@ -178,11 +178,11 @@ test_decimal decimal(10,2) Yes true \N 7 3.140000104904175 123.45 -- !reorder_1 -- -id int Yes true \N +id int No true \N age bigint Yes true \N col1 double Yes true \N Updated column1 type col2 text Yes true \N User defined column2 -grade double Yes true \N +grade double No true \N phone text Yes true \N User phone number email text Yes true \N address struct<city:text,country:text> Yes true \N @@ -209,11 +209,11 @@ iceberg_ddl_test iceberg_ddl_test_renamed -- !rename_table_1 -- -id int Yes true \N +id int No true \N age bigint Yes true \N col1 double Yes true \N Updated column1 type col2 text Yes true \N User defined column2 -grade double Yes true \N +grade double No true \N phone text Yes true \N User phone number email text Yes true \N address struct<city:text,country:text> Yes true \N diff --git a/regression-test/suites/external_table_p0/iceberg/test_iceberg_show_nullable.groovy b/regression-test/suites/external_table_p0/iceberg/test_iceberg_show_nullable.groovy new file mode 100644 index 00000000000..c92787a4d98 --- /dev/null +++ b/regression-test/suites/external_table_p0/iceberg/test_iceberg_show_nullable.groovy @@ -0,0 +1,117 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +suite("test_iceberg_show_nullable", "p0,external,doris,external_docker,external_docker_doris") { + String enabled = context.config.otherConfigs.get("enableIcebergTest") + if (enabled == null || !enabled.equalsIgnoreCase("true")) { + logger.info("disable iceberg test.") + return + } + + String catalogName = "test_iceberg_show_nullable" + String suffix = UUID.randomUUID().toString().replace("-", "") + String dbName = "iceberg_show_nullable_db_" + suffix + String tblName = "required_tbl_" + suffix + String externalEnvIp = context.config.otherConfigs.get("externalEnvIp") + String hmsPort = context.config.otherConfigs.get("hive2HmsPort") + String hdfsPort = context.config.otherConfigs.get("hive2HdfsPort") + String defaultFs = "hdfs://${externalEnvIp}:${hdfsPort}" + String tableName = "${catalogName}.${dbName}.${tblName}" + + sql """drop catalog if exists ${catalogName}""" + sql """create catalog ${catalogName} properties ( + 'type' = 'iceberg', + 'iceberg.catalog.type' = 'hms', + 'hive.metastore.uris' = 'thrift://${externalEnvIp}:${hmsPort}', + 'fs.defaultFS' = '${defaultFs}', + 'warehouse' = '${defaultFs}/warehouse' + )""" + + try { + sql """create database ${catalogName}.${dbName}""" + sql """create table ${tableName} ( + id bigint not null, + value string not null comment 'required value', + event_time datetime + ) properties ('format-version' = '2', 'write.format.default' = 'parquet')""" + + def checkSchema = { boolean valueIsRequired -> + def nullable = sql("desc ${tableName}").collectEntries { row -> [(row[0]): row[2]] } + assertEquals("No", nullable["id"]) + assertEquals(valueIsRequired ? "No" : "Yes", nullable["value"]) + assertEquals("Yes", nullable["event_time"]) + String ddl = sql("show create table ${tableName}")[0][1] + assertTrue(ddl.contains("`id` bigint NOT NULL")) + assertTrue(ddl.contains("`value` text " + (valueIsRequired ? "NOT NULL" : "NULL"))) + assertTrue(ddl.contains("`event_time` datetimev2(6) NULL")) + assertTrue(ddl.contains("required value")) + + def expected = [id: "NO", value: valueIsRequired ? "NO" : "YES", event_time: "YES"] + [false, true].each { full -> + String command = full ? "show full columns" : "show columns" + int nullIndex = full ? 3 : 2 + def columns = sql("${command} from ${tableName}") + assertEquals(expected, columns.collectEntries { row -> [(row[0]): row[nullIndex]] }) + def likeColumns = sql("${command} from ${tableName} like 'id'") + assertEquals([id: "NO"], likeColumns.collectEntries { row -> [(row[0]): row[nullIndex]] }) + // WHERE is rewritten to information_schema.columns, unlike the direct and LIKE paths. + ["NO", "YES"].each { nullableFlag -> + def filtered = sql("${command} from ${tableName} where `Null` = '${nullableFlag}'") + assertEquals(expected.findAll { name, flag -> flag == nullableFlag }, + filtered.collectEntries { row -> [(row[0]): row[nullIndex]] }) + } + } + + String metadataQuery = """select COLUMN_NAME, IS_NULLABLE + from ${catalogName}.information_schema.columns + where TABLE_SCHEMA in ('${dbName}', '${catalogName}.${dbName}') and TABLE_NAME = '${tblName}'""" + assertEquals(expected, sql(metadataQuery).collectEntries { row -> [(row[0]): row[1]] }) + assertEquals(expected.findAll { name, flag -> flag == "NO" }, + sql(metadataQuery + " and IS_NULLABLE = 'NO'").collectEntries { row -> [(row[0]): row[1]] }) + } + + // Empty tables have no snapshot yet, but still have a declared schema. + checkSchema(true) + sql """insert into ${tableName} values (1, 'ok', '2026-01-01 00:00:00'), (2, 'optional time', null)""" + checkSchema(true) + assertEquals(1L, sql("select count(*) from ${tableName} where event_time is null")[0][0]) + + test { + sql """insert into ${tableName} values (null, 'bad', '2026-01-02 00:00:00')""" + exception "Column 'id' is declared non-nullable but contains nulls" + } + test { + sql """insert into ${tableName} values (3, null, '2026-01-02 00:00:00')""" + exception "Column 'value' is declared non-nullable but contains nulls" + } + assertEquals(2L, sql("select count(*) from ${tableName}")[0][0]) + + // A schema-only change must be visible even though it creates no new data snapshot. + def snapshotBefore = sql("select snapshot_id from ${tableName}\$snapshots order by snapshot_id") + sql """alter table ${tableName} modify column value string null""" + sql """refresh table ${tableName}""" + assertEquals(snapshotBefore, sql("select snapshot_id from ${tableName}\$snapshots order by snapshot_id")) + checkSchema(false) + sql """insert into ${tableName} values (3, null, null)""" + assertEquals(1L, sql("select count(*) from ${tableName} where value is null")[0][0]) + assertEquals(0L, sql("select count(*) from ${tableName} where id is null")[0][0]) + checkSchema(false) + } finally { + sql """drop database if exists ${catalogName}.${dbName} force""" + sql """drop catalog if exists ${catalogName}""" + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
