Skip to content

Commit df697f2

Browse files
morningmanhubgeterclaude
committed
[test](iceberg) Cover required columns on empty tables, writes and SHOW FULL COLUMNS
### What problem does this PR solve? Issue Number: None Related PR: #67856, #68640 Problem Summary: #68640 shows required Iceberg columns as NOT NULL in DESC, SHOW CREATE TABLE, SHOW COLUMNS and information_schema on master, as #67856 does on branch-4.1 and branch-4.2. The implementation is equivalent and covers more statements, but master's tests miss cases that the 4.x tests check: - a table without any snapshot already shows its required columns as NOT NULL; - only the metadata view is NOT NULL: writing NULL into a required column is still rejected ("Column 'id' is declared non-nullable but contains nulls"), and once a column is relaxed it accepts NULL while id still rejects it; - relaxing a column creates no snapshot, and information_schema and SHOW CREATE TABLE follow it; - SHOW FULL COLUMNS with the column comment, SHOW COLUMNS LIKE, SHOW FULL COLUMNS ... WHERE `Null` = 'YES', and information_schema filtered by IS_NULLABLE; - a hidden column keeps its declared nullability and is listed only in the full display view. This adds them to test_iceberg_show_nullable and PluginDrivenExternalTableTest. No production code changes. ### Release note None ### Check List (For Author) - Test: Regression test / Unit Test - Regression test: test_iceberg_show_nullable against the iceberg docker REST catalog; the blocks already in its .out are unchanged - Unit Test: PluginDrivenExternalTableTest, which fails if the display view ignores `full` or a copied display column loses its visibility - Behavior changed: No - Does this need documentation: No Co-authored-by: daidai <[email protected]> Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
1 parent cefe023 commit df697f2

3 files changed

Lines changed: 147 additions & 37 deletions

File tree

‎fe/fe-core/src/test/java/org/apache/doris/datasource/plugin/PluginDrivenExternalTableTest.java‎

Lines changed: 66 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
import java.util.Optional;
6262
import java.util.Set;
6363
import java.util.concurrent.atomic.AtomicReference;
64+
import java.util.stream.Collectors;
6465

6566
/**
6667
* Pins {@link PluginDrivenExternalTable#getFullSchema()} request-scoped synthetic-write-column injection
@@ -1027,21 +1028,74 @@ public void initSchemaUsesTableHandlePathForNonView() {
10271028

10281029
@Test
10291030
public void displaySchemaUsesSourceNullability() {
1031+
PluginDrivenExternalTable table = tableWithCachedSchema(ImmutableList.of(
1032+
new ConnectorColumn("id", ConnectorType.of("INT"), "", true, null, true)
1033+
.withSourceNullable(false),
1034+
new ConnectorColumn("name", ConnectorType.of("STRING"), "", true, null, true),
1035+
// Required + WITH_TIMEZONE: the display copy must keep the marker Column's copy
1036+
// constructor drops, otherwise DESC loses "Extra: WITH_TIMEZONE" for exactly the
1037+
// required TZ columns this projection touches.
1038+
new ConnectorColumn("ts", ConnectorType.of("TIMESTAMPTZ"), "", true, null, true)
1039+
.withTimeZone().withSourceNullable(false)));
1040+
1041+
List<Column> display = table.getBaseSchemaForDisplay(false);
1042+
1043+
// Index-aligned with the read schema: same size, same names, same order.
1044+
List<Column> read = table.getBaseSchema(false);
1045+
Assertions.assertEquals(read.size(), display.size());
1046+
for (int i = 0; i < read.size(); i++) {
1047+
Assertions.assertEquals(read.get(i).getName(), display.get(i).getName());
1048+
}
1049+
// The source-required column is shown NOT NULL; the optional one keeps its nullable view.
1050+
Assertions.assertFalse(display.get(0).isAllowNull());
1051+
Assertions.assertTrue(display.get(1).isAllowNull());
1052+
Assertions.assertFalse(display.get(2).isAllowNull());
1053+
Assertions.assertTrue(read.get(2).getExtraInfo().contains("WITH_TIMEZONE"));
1054+
Assertions.assertTrue(display.get(2).getExtraInfo().contains("WITH_TIMEZONE"));
1055+
// The cached columns keep the nullable read semantics used by scans.
1056+
Assertions.assertTrue(read.stream().allMatch(Column::isAllowNull));
1057+
// A column the source does not diverge on is shared, not copied: no per-call projection is left.
1058+
Assertions.assertSame(read.get(1), display.get(1));
1059+
// One schema-cache read per display call (it must not re-read for the declared nullability, which
1060+
// would hit the connector twice under a bypassed cache such as a delegated credential).
1061+
Mockito.verify(table, Mockito.times(1)).getSchemaCacheValue();
1062+
}
1063+
1064+
@Test
1065+
public void displaySchemaKeepsHiddenColumnsOnlyInTheFullView() {
1066+
// The display view mirrors getBaseSchema(full): a hidden column is listed, with the nullability its
1067+
// source declares, only when hidden columns are requested, so DESC and SHOW COLUMNS under
1068+
// show_hidden_columns stay index-aligned with the read schema. MUTATION: ignoring full, or dropping
1069+
// the visibility of a column the projection copies -> red.
1070+
PluginDrivenExternalTable table = tableWithCachedSchema(ImmutableList.of(
1071+
new ConnectorColumn("id", ConnectorType.of("INT"), "", true, null, true)
1072+
.withSourceNullable(false),
1073+
new ConnectorColumn("row_lineage", ConnectorType.of("BIGINT"), "", true, null, false)
1074+
.invisible().withSourceNullable(false),
1075+
new ConnectorColumn("name", ConnectorType.of("STRING"), "", true, null, true)));
1076+
1077+
for (boolean full : new boolean[] {false, true}) {
1078+
Assertions.assertEquals(
1079+
table.getBaseSchema(full).stream().map(Column::getName).collect(Collectors.toList()),
1080+
table.getBaseSchemaForDisplay(full).stream().map(Column::getName).collect(Collectors.toList()));
1081+
}
1082+
Assertions.assertEquals(ImmutableList.of("id", "name"), table.getBaseSchemaForDisplay(false).stream()
1083+
.map(Column::getName).collect(Collectors.toList()));
1084+
Column hidden = table.getBaseSchemaForDisplay(true).get(1);
1085+
Assertions.assertEquals("row_lineage", hidden.getName());
1086+
Assertions.assertFalse(hidden.isVisible());
1087+
Assertions.assertFalse(hidden.isAllowNull());
1088+
Assertions.assertTrue(table.getBaseSchema(true).get(1).isAllowNull());
1089+
}
1090+
1091+
/** A table whose schema cache holds {@code columns}, with the schema-cache reads of this setup cleared. */
1092+
private static PluginDrivenExternalTable tableWithCachedSchema(List<ConnectorColumn> columns) {
10301093
ConnectorMetadata metadata = Mockito.mock(ConnectorMetadata.class);
10311094
ConnectorTableHandle handle = Mockito.mock(ConnectorTableHandle.class);
10321095
Mockito.when(metadata.getTableHandle(Mockito.any(), Mockito.eq("db1"), Mockito.eq("t1")))
10331096
.thenReturn(Optional.of(handle));
10341097
Mockito.when(metadata.getTableSchema(Mockito.any(), Mockito.eq(handle)))
1035-
.thenReturn(new ConnectorTableSchema("t1", ImmutableList.of(
1036-
new ConnectorColumn("id", ConnectorType.of("INT"), "", true, null, true)
1037-
.withSourceNullable(false),
1038-
new ConnectorColumn("name", ConnectorType.of("STRING"), "", true, null, true),
1039-
// Required + WITH_TIMEZONE: the display copy must keep the marker Column's copy
1040-
// constructor drops, otherwise DESC loses "Extra: WITH_TIMEZONE" for exactly the
1041-
// required TZ columns this projection touches.
1042-
new ConnectorColumn("ts", ConnectorType.of("TIMESTAMPTZ"), "", true, null, true)
1043-
.withTimeZone().withSourceNullable(false)),
1044-
"ICEBERG", Collections.emptyMap()));
1098+
.thenReturn(new ConnectorTableSchema("t1", columns, "ICEBERG", Collections.emptyMap()));
10451099
Mockito.when(metadata.fromRemoteColumnName(Mockito.any(), Mockito.any(), Mockito.any(),
10461100
Mockito.anyString())).thenAnswer(inv -> inv.getArgument(3));
10471101
ConnectorSession session = Mockito.mock(ConnectorSession.class);
@@ -1063,30 +1117,9 @@ public void displaySchemaUsesSourceNullability() {
10631117
SchemaCacheValue cached = table.initSchema().get();
10641118
Mockito.doReturn(Optional.of(cached)).when(table).getSchemaCacheValue();
10651119
Mockito.doReturn(cached.getSchema()).when(table).getFullSchema();
1066-
// Count the schema-cache reads of the display call alone, not of the setup above.
1120+
// Count the schema-cache reads of the calls under test alone, not of the setup above.
10671121
Mockito.clearInvocations(table);
1068-
1069-
List<Column> display = table.getBaseSchemaForDisplay(false);
1070-
1071-
// Index-aligned with the read schema: same size, same names, same order.
1072-
List<Column> read = table.getBaseSchema(false);
1073-
Assertions.assertEquals(read.size(), display.size());
1074-
for (int i = 0; i < read.size(); i++) {
1075-
Assertions.assertEquals(read.get(i).getName(), display.get(i).getName());
1076-
}
1077-
// The source-required column is shown NOT NULL; the optional one keeps its nullable view.
1078-
Assertions.assertFalse(display.get(0).isAllowNull());
1079-
Assertions.assertTrue(display.get(1).isAllowNull());
1080-
Assertions.assertFalse(display.get(2).isAllowNull());
1081-
Assertions.assertTrue(read.get(2).getExtraInfo().contains("WITH_TIMEZONE"));
1082-
Assertions.assertTrue(display.get(2).getExtraInfo().contains("WITH_TIMEZONE"));
1083-
// The cached columns keep the nullable read semantics used by scans.
1084-
Assertions.assertTrue(read.stream().allMatch(Column::isAllowNull));
1085-
// A column the source does not diverge on is shared, not copied: no per-call projection is left.
1086-
Assertions.assertSame(read.get(1), display.get(1));
1087-
// One schema-cache read per display call (it must not re-read for the declared nullability, which
1088-
// would hit the connector twice under a bypassed cache such as a delegated credential).
1089-
Mockito.verify(table, Mockito.times(1)).getSchemaCacheValue();
1122+
return table;
10901123
}
10911124

10921125
@Test

‎regression-test/data/external_table_p0/iceberg/test_iceberg_show_nullable.out‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,17 @@
11
-- This file is automatically generated. You should know what you did if you want to edit this
2+
-- !snapshots_empty --
3+
0
4+
5+
-- !desc_empty --
6+
id bigint No true \N
7+
value text No true \N
8+
event_time datetime(6) Yes true \N
9+
10+
-- !information_schema_empty --
11+
event_time YES
12+
id NO
13+
value NO
14+
215
-- !desc --
316
id bigint No true \N
417
value text No true \N
@@ -9,21 +22,52 @@ id bigint NO YES \N
922
value text NO YES \N
1023
event_time datetime(6) YES YES \N
1124

25+
-- !show_full_columns --
26+
id bigint NO YES \N
27+
value text NO YES \N required value
28+
event_time datetime(6) YES YES \N
29+
30+
-- !show_columns_like --
31+
id bigint NO YES \N
32+
1233
-- !show_columns_where --
1334
id bigint NO \N
1435
value string NO \N
1536

37+
-- !show_full_columns_where --
38+
event_time datetime \N YES \N
39+
1640
-- !information_schema --
1741
event_time YES
1842
id NO
1943
value NO
2044

45+
-- !information_schema_required --
46+
id NO
47+
value NO
48+
2149
-- !select --
2250
1 a 2026-01-01T00:00
2351
2 b \N
2452

53+
-- !snapshots_before_alter --
54+
1
55+
56+
-- !snapshots_after_alter --
57+
1
58+
2559
-- !desc_after_alter --
2660
id bigint No true \N
2761
value text Yes true \N
2862
event_time datetime(6) Yes true \N
2963

64+
-- !information_schema_after_alter --
65+
event_time YES
66+
id NO
67+
value YES
68+
69+
-- !select_after_alter --
70+
1 a 2026-01-01T00:00
71+
2 b \N
72+
3 \N \N
73+

‎regression-test/suites/external_table_p0/iceberg/test_iceberg_show_nullable.groovy‎

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,25 +47,58 @@ suite("test_iceberg_show_nullable", "p0,external") {
4747
sql """
4848
create table iceberg_show_nullable (
4949
id bigint not null,
50-
value string not null,
50+
value string not null comment 'required value',
5151
event_time datetime
5252
)"""
53+
String columnsQuery = """select COLUMN_NAME, IS_NULLABLE from information_schema.columns
54+
where TABLE_SCHEMA = 'test_iceberg_show_nullable_db' and TABLE_NAME = 'iceberg_show_nullable'"""
55+
56+
// A new table has no snapshot yet, but its schema already declares the required columns.
57+
qt_snapshots_empty """select count(*) from iceberg_show_nullable\$snapshots"""
58+
qt_desc_empty """desc iceberg_show_nullable"""
59+
order_qt_information_schema_empty(columnsQuery)
60+
5361
sql """insert into iceberg_show_nullable values (1, 'a', '2026-01-01 00:00:00'), (2, 'b', null)"""
5462

5563
qt_desc """desc iceberg_show_nullable"""
5664
qt_show_columns """show columns from iceberg_show_nullable"""
65+
qt_show_full_columns """show full columns from iceberg_show_nullable"""
66+
qt_show_columns_like """show columns from iceberg_show_nullable like 'id'"""
5767
// SHOW COLUMNS ... WHERE is rewritten to information_schema.columns; no ORDER BY, so the rows are
5868
// sorted by the framework (order_qt_) instead of being compared in an unspecified scan order.
5969
order_qt_show_columns_where """show columns from iceberg_show_nullable where `Null` = 'NO'"""
60-
order_qt_information_schema """select COLUMN_NAME, IS_NULLABLE from information_schema.columns
61-
where TABLE_SCHEMA = 'test_iceberg_show_nullable_db' and TABLE_NAME = 'iceberg_show_nullable'"""
70+
order_qt_show_full_columns_where """show full columns from iceberg_show_nullable where `Null` = 'YES'"""
71+
order_qt_information_schema(columnsQuery)
72+
order_qt_information_schema_required(columnsQuery + " and IS_NULLABLE = 'NO'")
6273
String ddl = sql("""show create table iceberg_show_nullable""")[0][1]
6374
assertTrue(ddl.contains("`id` bigint NOT NULL"), ddl)
64-
assertTrue(ddl.contains("`value` text NOT NULL"), ddl)
75+
assertTrue(ddl.contains("`value` text NOT NULL COMMENT \"required value\""), ddl)
6576
assertTrue(ddl.contains("`event_time` datetimev2(6) NULL"), ddl)
6677
order_qt_select """select * from iceberg_show_nullable"""
6778

79+
// Only the metadata view is NOT NULL; the columns are read as nullable, and a write still enforces
80+
// the declared constraint.
81+
test {
82+
sql """insert into iceberg_show_nullable values (null, 'bad', '2026-01-02 00:00:00')"""
83+
exception "Column 'id' is declared non-nullable but contains nulls"
84+
}
85+
test {
86+
sql """insert into iceberg_show_nullable values (3, null, '2026-01-02 00:00:00')"""
87+
exception "Column 'value' is declared non-nullable but contains nulls"
88+
}
89+
6890
// Relaxing the constraint is reflected without a new data snapshot.
91+
qt_snapshots_before_alter """select count(*) from iceberg_show_nullable\$snapshots"""
6992
sql """alter table iceberg_show_nullable modify column value string null"""
93+
qt_snapshots_after_alter """select count(*) from iceberg_show_nullable\$snapshots"""
7094
qt_desc_after_alter """desc iceberg_show_nullable"""
95+
order_qt_information_schema_after_alter(columnsQuery)
96+
ddl = sql("""show create table iceberg_show_nullable""")[0][1]
97+
assertTrue(ddl.contains("`value` text NULL COMMENT \"required value\""), ddl)
98+
sql """insert into iceberg_show_nullable values (3, null, null)"""
99+
order_qt_select_after_alter """select * from iceberg_show_nullable"""
100+
test {
101+
sql """insert into iceberg_show_nullable values (null, 'still required', null)"""
102+
exception "Column 'id' is declared non-nullable but contains nulls"
103+
}
71104
}

0 commit comments

Comments
 (0)