This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new b549b6cfa57 [test](iceberg) Cover required columns on empty tables,
writes and SHOW FULL COLUMNS (#69351)
b549b6cfa57 is described below
commit b549b6cfa57e244df399834f714ea170f6ca10ab
Author: Mingyu Chen (Rayner) <[email protected]>
AuthorDate: Wed Oct 21 09:44:49 2026 +0100
[test](iceberg) Cover required columns on empty tables, writes and SHOW
FULL COLUMNS (#69351)
### What problem does this PR solve?
Issue Number: None
Related PR: #67856, #68640
Problem Summary:
**In short.** Tests only. #68640 shows required Iceberg columns as NOT
NULL on master, as #67856 does on branch-4.1 and branch-4.2. The two
implementations behave the same (master also covers the HTTP schema APIs
and the MySQL protocol column metadata), but master's tests miss several
cases that the 4.x tests check. This PR adds them; it changes no
production code.
Original author: @hubgeter (the 4.x tests these cases come from)
**Background.** An Iceberg column declared `NOT NULL` (required) is read
by Doris as nullable, so scans and expressions keep their nullable
semantics, while DESC, SHOW CREATE TABLE, SHOW [FULL] COLUMNS and
information_schema show the declared constraint. On master,
`ConnectorColumn.sourceNullable` carries the declared nullability, and
`PluginDrivenExternalTable` projects a display view next to the read
schema in the same schema-cache entry.
**The problem, and what it cost.** These behaviors had no test on
master, so a regression in any of them would go unnoticed:
| Case | 4.x test | master before this PR |
|---|---|---|
| A table without any snapshot shows its required columns | yes | no
(the table was written before the first DESC) |
| Writing NULL into a required column is still rejected | yes | no |
| Relaxing a column creates no snapshot; information_schema, SHOW CREATE
TABLE and later writes follow it | yes | DESC only |
| SHOW FULL COLUMNS with the column comment, SHOW COLUMNS LIKE, `WHERE
Null = 'YES'`, information_schema filtered by IS_NULLABLE | yes | no |
| A hidden column keeps its declared nullability and is listed only in
the full view | yes | no (only the visible view) |
**How this PR fixes it.**
- `test_iceberg_show_nullable` (REST catalog): checks the empty table (0
snapshots) before the first write; adds SHOW FULL COLUMNS, SHOW COLUMNS
LIKE, `SHOW FULL COLUMNS ... WHERE Null = 'YES'` and information_schema
filtered by IS_NULLABLE; checks the column comment in SHOW CREATE TABLE;
writes NULL into `id` and `value` and expects "Column 'id' is declared
non-nullable but contains nulls" (the message the 4.x test expects too);
relaxes `value`, checks that the snapshot count stays 1 and that
information_schema and SHOW CREATE TABLE follow, then writes NULL into
`value` and still expects the rejection for `id`.
- `PluginDrivenExternalTableTest`: a new test with a hidden required
column checks that `getBaseSchemaForDisplay(full)` lists exactly the
columns of `getBaseSchema(full)`, in the same order, for both values of
`full`, and that the hidden column is NOT NULL only in the display view.
The setup of the existing display test moves into a helper both tests
use.
**Results.** Both pass locally. The blocks already in
`test_iceberg_show_nullable.out` are unchanged; 11 blocks are added. The
new unit test fails if the display view ignores `full`, or if a copied
display column loses its visibility.
Co-authored-by: daidai <[email protected]>
---
.../plugin/PluginDrivenExternalTableTest.java | 99 ++++++++++++++--------
.../iceberg/test_iceberg_show_nullable.out | 44 ++++++++++
.../iceberg/test_iceberg_show_nullable.groovy | 41 ++++++++-
3 files changed, 147 insertions(+), 37 deletions(-)
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/datasource/plugin/PluginDrivenExternalTableTest.java
b/fe/fe-core/src/test/java/org/apache/doris/datasource/plugin/PluginDrivenExternalTableTest.java
index d57a6dd73df..f21ed9f8006 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/datasource/plugin/PluginDrivenExternalTableTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/datasource/plugin/PluginDrivenExternalTableTest.java
@@ -61,6 +61,7 @@ import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Collectors;
/**
* Pins {@link PluginDrivenExternalTable#getFullSchema()} request-scoped
synthetic-write-column injection
@@ -1027,21 +1028,74 @@ public class PluginDrivenExternalTableTest {
@Test
public void displaySchemaUsesSourceNullability() {
+ PluginDrivenExternalTable table =
tableWithCachedSchema(ImmutableList.of(
+ new ConnectorColumn("id", ConnectorType.of("INT"), "", true,
null, true)
+ .withSourceNullable(false),
+ new ConnectorColumn("name", ConnectorType.of("STRING"), "",
true, null, true),
+ // Required + WITH_TIMEZONE: the display copy must keep the
marker Column's copy
+ // constructor drops, otherwise DESC loses "Extra:
WITH_TIMEZONE" for exactly the
+ // required TZ columns this projection touches.
+ new ConnectorColumn("ts", ConnectorType.of("TIMESTAMPTZ"), "",
true, null, true)
+ .withTimeZone().withSourceNullable(false)));
+
+ List<Column> display = table.getBaseSchemaForDisplay(false);
+
+ // Index-aligned with the read schema: same size, same names, same
order.
+ List<Column> read = table.getBaseSchema(false);
+ Assertions.assertEquals(read.size(), display.size());
+ for (int i = 0; i < read.size(); i++) {
+ Assertions.assertEquals(read.get(i).getName(),
display.get(i).getName());
+ }
+ // The source-required column is shown NOT NULL; the optional one
keeps its nullable view.
+ Assertions.assertFalse(display.get(0).isAllowNull());
+ Assertions.assertTrue(display.get(1).isAllowNull());
+ Assertions.assertFalse(display.get(2).isAllowNull());
+
Assertions.assertTrue(read.get(2).getExtraInfo().contains("WITH_TIMEZONE"));
+
Assertions.assertTrue(display.get(2).getExtraInfo().contains("WITH_TIMEZONE"));
+ // The cached columns keep the nullable read semantics used by scans.
+ Assertions.assertTrue(read.stream().allMatch(Column::isAllowNull));
+ // A column the source does not diverge on is shared, not copied: no
per-call projection is left.
+ Assertions.assertSame(read.get(1), display.get(1));
+ // One schema-cache read per display call (it must not re-read for the
declared nullability, which
+ // would hit the connector twice under a bypassed cache such as a
delegated credential).
+ Mockito.verify(table, Mockito.times(1)).getSchemaCacheValue();
+ }
+
+ @Test
+ public void displaySchemaKeepsHiddenColumnsOnlyInTheFullView() {
+ // The display view mirrors getBaseSchema(full): a hidden column is
listed, with the nullability its
+ // source declares, only when hidden columns are requested, so DESC
and SHOW COLUMNS under
+ // show_hidden_columns stay index-aligned with the read schema.
MUTATION: ignoring full, or dropping
+ // the visibility of a column the projection copies -> red.
+ PluginDrivenExternalTable table =
tableWithCachedSchema(ImmutableList.of(
+ new ConnectorColumn("id", ConnectorType.of("INT"), "", true,
null, true)
+ .withSourceNullable(false),
+ new ConnectorColumn("row_lineage", ConnectorType.of("BIGINT"),
"", true, null, false)
+ .invisible().withSourceNullable(false),
+ new ConnectorColumn("name", ConnectorType.of("STRING"), "",
true, null, true)));
+
+ for (boolean full : new boolean[] {false, true}) {
+ Assertions.assertEquals(
+
table.getBaseSchema(full).stream().map(Column::getName).collect(Collectors.toList()),
+
table.getBaseSchemaForDisplay(full).stream().map(Column::getName).collect(Collectors.toList()));
+ }
+ Assertions.assertEquals(ImmutableList.of("id", "name"),
table.getBaseSchemaForDisplay(false).stream()
+ .map(Column::getName).collect(Collectors.toList()));
+ Column hidden = table.getBaseSchemaForDisplay(true).get(1);
+ Assertions.assertEquals("row_lineage", hidden.getName());
+ Assertions.assertFalse(hidden.isVisible());
+ Assertions.assertFalse(hidden.isAllowNull());
+ Assertions.assertTrue(table.getBaseSchema(true).get(1).isAllowNull());
+ }
+
+ /** A table whose schema cache holds {@code columns}, with the
schema-cache reads of this setup cleared. */
+ private static PluginDrivenExternalTable
tableWithCachedSchema(List<ConnectorColumn> columns) {
ConnectorMetadata metadata = Mockito.mock(ConnectorMetadata.class);
ConnectorTableHandle handle = Mockito.mock(ConnectorTableHandle.class);
Mockito.when(metadata.getTableHandle(Mockito.any(), Mockito.eq("db1"),
Mockito.eq("t1")))
.thenReturn(Optional.of(handle));
Mockito.when(metadata.getTableSchema(Mockito.any(),
Mockito.eq(handle)))
- .thenReturn(new ConnectorTableSchema("t1", ImmutableList.of(
- new ConnectorColumn("id", ConnectorType.of("INT"), "",
true, null, true)
- .withSourceNullable(false),
- new ConnectorColumn("name",
ConnectorType.of("STRING"), "", true, null, true),
- // Required + WITH_TIMEZONE: the display copy must
keep the marker Column's copy
- // constructor drops, otherwise DESC loses "Extra:
WITH_TIMEZONE" for exactly the
- // required TZ columns this projection touches.
- new ConnectorColumn("ts",
ConnectorType.of("TIMESTAMPTZ"), "", true, null, true)
- .withTimeZone().withSourceNullable(false)),
- "ICEBERG", Collections.emptyMap()));
+ .thenReturn(new ConnectorTableSchema("t1", columns, "ICEBERG",
Collections.emptyMap()));
Mockito.when(metadata.fromRemoteColumnName(Mockito.any(),
Mockito.any(), Mockito.any(),
Mockito.anyString())).thenAnswer(inv -> inv.getArgument(3));
ConnectorSession session = Mockito.mock(ConnectorSession.class);
@@ -1063,30 +1117,9 @@ public class PluginDrivenExternalTableTest {
SchemaCacheValue cached = table.initSchema().get();
Mockito.doReturn(Optional.of(cached)).when(table).getSchemaCacheValue();
Mockito.doReturn(cached.getSchema()).when(table).getFullSchema();
- // Count the schema-cache reads of the display call alone, not of the
setup above.
+ // Count the schema-cache reads of the calls under test alone, not of
the setup above.
Mockito.clearInvocations(table);
-
- List<Column> display = table.getBaseSchemaForDisplay(false);
-
- // Index-aligned with the read schema: same size, same names, same
order.
- List<Column> read = table.getBaseSchema(false);
- Assertions.assertEquals(read.size(), display.size());
- for (int i = 0; i < read.size(); i++) {
- Assertions.assertEquals(read.get(i).getName(),
display.get(i).getName());
- }
- // The source-required column is shown NOT NULL; the optional one
keeps its nullable view.
- Assertions.assertFalse(display.get(0).isAllowNull());
- Assertions.assertTrue(display.get(1).isAllowNull());
- Assertions.assertFalse(display.get(2).isAllowNull());
-
Assertions.assertTrue(read.get(2).getExtraInfo().contains("WITH_TIMEZONE"));
-
Assertions.assertTrue(display.get(2).getExtraInfo().contains("WITH_TIMEZONE"));
- // The cached columns keep the nullable read semantics used by scans.
- Assertions.assertTrue(read.stream().allMatch(Column::isAllowNull));
- // A column the source does not diverge on is shared, not copied: no
per-call projection is left.
- Assertions.assertSame(read.get(1), display.get(1));
- // One schema-cache read per display call (it must not re-read for the
declared nullability, which
- // would hit the connector twice under a bypassed cache such as a
delegated credential).
- Mockito.verify(table, Mockito.times(1)).getSchemaCacheValue();
+ return table;
}
@Test
diff --git
a/regression-test/data/external_table_p0/iceberg/test_iceberg_show_nullable.out
b/regression-test/data/external_table_p0/iceberg/test_iceberg_show_nullable.out
index b14cf5aacb7..7370e41078d 100644
---
a/regression-test/data/external_table_p0/iceberg/test_iceberg_show_nullable.out
+++
b/regression-test/data/external_table_p0/iceberg/test_iceberg_show_nullable.out
@@ -1,4 +1,17 @@
-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !snapshots_empty --
+0
+
+-- !desc_empty --
+id bigint No true \N
+value text No true \N
+event_time datetime(6) Yes true \N
+
+-- !information_schema_empty --
+event_time YES
+id NO
+value NO
+
-- !desc --
id bigint No true \N
value text No true \N
@@ -9,21 +22,52 @@ id bigint NO YES \N
value text NO YES \N
event_time datetime(6) YES YES \N
+-- !show_full_columns --
+id bigint NO YES \N
+value text NO YES \N required value
+event_time datetime(6) YES YES \N
+
+-- !show_columns_like --
+id bigint NO YES \N
+
-- !show_columns_where --
id bigint NO \N
value string NO \N
+-- !show_full_columns_where --
+event_time datetime \N YES \N
+
-- !information_schema --
event_time YES
id NO
value NO
+-- !information_schema_required --
+id NO
+value NO
+
-- !select --
1 a 2026-01-01T00:00
2 b \N
+-- !snapshots_before_alter --
+1
+
+-- !snapshots_after_alter --
+1
+
-- !desc_after_alter --
id bigint No true \N
value text Yes true \N
event_time datetime(6) Yes true \N
+-- !information_schema_after_alter --
+event_time YES
+id NO
+value YES
+
+-- !select_after_alter --
+1 a 2026-01-01T00:00
+2 b \N
+3 \N \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
index 59b5cade746..5f1b9eb94e0 100644
---
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
@@ -47,25 +47,58 @@ suite("test_iceberg_show_nullable", "p0,external") {
sql """
create table iceberg_show_nullable (
id bigint not null,
- value string not null,
+ value string not null comment 'required value',
event_time datetime
)"""
+ String columnsQuery = """select COLUMN_NAME, IS_NULLABLE from
information_schema.columns
+ where TABLE_SCHEMA = 'test_iceberg_show_nullable_db' and TABLE_NAME =
'iceberg_show_nullable'"""
+
+ // A new table has no snapshot yet, but its schema already declares the
required columns.
+ qt_snapshots_empty """select count(*) from
iceberg_show_nullable\$snapshots"""
+ qt_desc_empty """desc iceberg_show_nullable"""
+ order_qt_information_schema_empty(columnsQuery)
+
sql """insert into iceberg_show_nullable values (1, 'a', '2026-01-01
00:00:00'), (2, 'b', null)"""
qt_desc """desc iceberg_show_nullable"""
qt_show_columns """show columns from iceberg_show_nullable"""
+ qt_show_full_columns """show full columns from iceberg_show_nullable"""
+ qt_show_columns_like """show columns from iceberg_show_nullable like
'id'"""
// SHOW COLUMNS ... WHERE is rewritten to information_schema.columns; no
ORDER BY, so the rows are
// sorted by the framework (order_qt_) instead of being compared in an
unspecified scan order.
order_qt_show_columns_where """show columns from iceberg_show_nullable
where `Null` = 'NO'"""
- order_qt_information_schema """select COLUMN_NAME, IS_NULLABLE from
information_schema.columns
- where TABLE_SCHEMA = 'test_iceberg_show_nullable_db' and TABLE_NAME =
'iceberg_show_nullable'"""
+ order_qt_show_full_columns_where """show full columns from
iceberg_show_nullable where `Null` = 'YES'"""
+ order_qt_information_schema(columnsQuery)
+ order_qt_information_schema_required(columnsQuery + " and IS_NULLABLE =
'NO'")
String ddl = sql("""show create table iceberg_show_nullable""")[0][1]
assertTrue(ddl.contains("`id` bigint NOT NULL"), ddl)
- assertTrue(ddl.contains("`value` text NOT NULL"), ddl)
+ assertTrue(ddl.contains("`value` text NOT NULL COMMENT \"required
value\""), ddl)
assertTrue(ddl.contains("`event_time` datetimev2(6) NULL"), ddl)
order_qt_select """select * from iceberg_show_nullable"""
+ // Only the metadata view is NOT NULL; the columns are read as nullable,
and a write still enforces
+ // the declared constraint.
+ test {
+ sql """insert into iceberg_show_nullable values (null, 'bad',
'2026-01-02 00:00:00')"""
+ exception "Column 'id' is declared non-nullable but contains nulls"
+ }
+ test {
+ sql """insert into iceberg_show_nullable values (3, null, '2026-01-02
00:00:00')"""
+ exception "Column 'value' is declared non-nullable but contains nulls"
+ }
+
// Relaxing the constraint is reflected without a new data snapshot.
+ qt_snapshots_before_alter """select count(*) from
iceberg_show_nullable\$snapshots"""
sql """alter table iceberg_show_nullable modify column value string null"""
+ qt_snapshots_after_alter """select count(*) from
iceberg_show_nullable\$snapshots"""
qt_desc_after_alter """desc iceberg_show_nullable"""
+ order_qt_information_schema_after_alter(columnsQuery)
+ ddl = sql("""show create table iceberg_show_nullable""")[0][1]
+ assertTrue(ddl.contains("`value` text NULL COMMENT \"required value\""),
ddl)
+ sql """insert into iceberg_show_nullable values (3, null, null)"""
+ order_qt_select_after_alter """select * from iceberg_show_nullable"""
+ test {
+ sql """insert into iceberg_show_nullable values (null, 'still
required', null)"""
+ exception "Column 'id' is declared non-nullable but contains nulls"
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
