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 c6fd85b28b0 [test](iceberg) Port the branch-4.1 schema matrix
assertions for joins, branch reads and DESC comments (#69552)
c6fd85b28b0 is described below
commit c6fd85b28b0dff0c762b55307318269aadbe8eee
Author: Mingyu Chen (Rayner) <[email protected]>
AuthorDate: Sun Oct 25 11:01:00 2026 +0000
[test](iceberg) Port the branch-4.1 schema matrix assertions for joins,
branch reads and DESC comments (#69552)
### What problem does this PR solve?
Issue Number: None
Related PR: #66246, #66007, #66112
Problem Summary:
**In short.** This ports test assertions from branch-4.1 #66246 that
master never got. #66246 backported #66112, #66223 and #66007 to
branch-4.1 and added checks for historical joins, branch reads and DESC
comments on Iceberg tables. Master already behaves as those checks
expect, but its suites either lack them or still assert the old,
negative contract. No production code changes.
**Background.** An audit of every block of #66246 that branch-4.2 still
has, against master, found these test gaps. Everything else in #66246 is
on master in another form (mostly in the connector plugins) or was
replaced on master. One behavior differs on purpose: a plain Hudi read
on master is not pinned to the instant seen at analysis (#64304). This
PR leaves that alone.
**The problem, and what it cost.** Without the checks, a regression in
these paths would go unnoticed, and two suites still describe the
current behavior as a defect:
| Suite | Master today | After this PR |
|---|---|---|
| `test_iceberg_schema_dual_relation_matrix` | no check that a
historical relation and the latest relation of the same table keep their
own schemas in one join | both join orders checked: `old_name` on the
snapshot, `new_name` on the latest table |
| `test_iceberg_schema_time_travel_matrix` | branch reads checked only
as "negative contracts": the old name is unknown, nested types are
promoted | a branch read uses the table's current schema; renamed and
re-added fields read NULL from the branch head (`qt_`, same rows as
branch-4.2) |
| `test_iceberg_schema_metadata_atomicity_matrix` | asserts that DESC
omits Iceberg field comments, which only holds because
`show_column_comment_in_describe` is off by default | enables the
variable and asserts that DESC shows the top-level and nested comments |
| `CreateTableCommandTest` | #67545 replaced the "existing CTAS target
fails before any sink is built" case with the configuration preflight
case | the existing-target case is back, next to the preflight case |
**How this PR fixes it.**
- `test_iceberg_schema_dual_relation_matrix` gets the two joins from
branch-4.2, written with `assertEquals` like the rest of the master
suite.
- `test_iceberg_schema_time_travel_matrix` keeps master's checks,
rewords their comments, and adds `qt_doris_nested_branch_current_schema`
and `qt_top_branch_current_schema` from branch-4.2, with a new `.out`.
- `test_iceberg_schema_metadata_atomicity_matrix` sets
`show_column_comment_in_describe=true` and asserts that the comments are
shown, as on branch-4.2.
-
`CreateTableCommandTest#existingCtasTargetPrecedesUnsupportedSinkValidation`
comes from branch-4.2. On master the configuration preflight runs first,
so the test stubs it to pass.
**Results.** On a local cluster built from master with the Iceberg
docker environment, the three suites pass. The `.out` is generated by
the regression script and has the same rows as branch-4.2. All 4 cases
of `CreateTableCommandTest` pass, and checkstyle passes.
Co-authored-by: Gabriel <[email protected]>
---
.../plans/commands/CreateTableCommandTest.java | 34 ++++++++++++++++++++++
.../test_iceberg_schema_time_travel_matrix.out | 7 +++++
...test_iceberg_schema_dual_relation_matrix.groovy | 16 ++++++++++
...iceberg_schema_metadata_atomicity_matrix.groovy | 7 +++--
.../test_iceberg_schema_time_travel_matrix.groovy | 19 ++++++++++--
5 files changed, 78 insertions(+), 5 deletions(-)
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/CreateTableCommandTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/CreateTableCommandTest.java
index 5e502220dcd..0f8113bfc3a 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/CreateTableCommandTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/CreateTableCommandTest.java
@@ -100,6 +100,40 @@ class CreateTableCommandTest {
}
}
+ @Test
+ void existingCtasTargetPrecedesUnsupportedSinkValidation() throws
Exception {
+ LogicalPlan query = Mockito.mock(LogicalPlan.class);
+ CreateTableInfo createTableInfo = Mockito.mock(CreateTableInfo.class);
+ ConnectContext context = Mockito.mock(ConnectContext.class);
+ StmtExecutor executor = Mockito.mock(StmtExecutor.class);
+ Env env = Mockito.mock(Env.class);
+ List<String> tableNameParts = ImmutableList.of("catalog", "database",
"target");
+
+
Mockito.when(createTableInfo.getTableNameParts()).thenReturn(tableNameParts);
+ Mockito.when(createTableInfo.getTableName()).thenReturn("target");
+ CreateTableCommand command = Mockito.spy(
+ new CreateTableCommand(Optional.of(query), createTableInfo));
+ Mockito.doNothing().when(command).validateCreateTableAsSelect(context,
query);
+
Mockito.doNothing().when(command).validateCreateTableProperties(context);
+ Mockito.doReturn(true).when(command).targetTableExists(context);
+
+ try (MockedStatic<Env> envMock = Mockito.mockStatic(Env.class);
+ MockedStatic<UnboundTableSinkCreator> sinkMock =
+ Mockito.mockStatic(UnboundTableSinkCreator.class)) {
+ envMock.when(Env::getCurrentEnv).thenReturn(env);
+ Exception exception =
org.junit.jupiter.api.Assertions.assertThrows(
+ Exception.class, () -> command.run(context, executor));
+
+
org.junit.jupiter.api.Assertions.assertTrue(exception.getMessage().contains("already
exists"));
+ sinkMock.verifyNoInteractions();
+ Mockito.verify(env, Mockito.never()).createTable(createTableInfo);
+ Mockito.verify(env, Mockito.never()).dropTable(
+ Mockito.anyString(), Mockito.anyString(),
Mockito.anyString(),
+ Mockito.anyBoolean(), Mockito.anyBoolean(),
Mockito.anyBoolean(),
+ Mockito.anyBoolean(), Mockito.anyBoolean(),
Mockito.anyBoolean());
+ }
+ }
+
@Test
void ctasConfigurationPreflightPrecedesTargetLookup() throws Exception {
LogicalPlan query = Mockito.mock(LogicalPlan.class);
diff --git
a/regression-test/data/external_table_p0/iceberg/test_iceberg_schema_time_travel_matrix.out
b/regression-test/data/external_table_p0/iceberg/test_iceberg_schema_time_travel_matrix.out
new file mode 100644
index 00000000000..bf4a957a123
--- /dev/null
+++
b/regression-test/data/external_table_p0/iceberg/test_iceberg_schema_time_travel_matrix.out
@@ -0,0 +1,7 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !doris_nested_branch_current_schema --
+1 10 \N 100 \N 1000 \N
+
+-- !top_branch_current_schema --
+1 alpha \N 10
+
diff --git
a/regression-test/suites/external_table_p0/iceberg/test_iceberg_schema_dual_relation_matrix.groovy
b/regression-test/suites/external_table_p0/iceberg/test_iceberg_schema_dual_relation_matrix.groovy
index 3dddfe69b87..db60c79d1d3 100644
---
a/regression-test/suites/external_table_p0/iceberg/test_iceberg_schema_dual_relation_matrix.groovy
+++
b/regression-test/suites/external_table_p0/iceberg/test_iceberg_schema_dual_relation_matrix.groovy
@@ -93,6 +93,22 @@ suite("test_iceberg_schema_dual_relation_matrix",
order by id
"""))
+ // Latest metadata must be restored even when a historical relation is
bound first.
+ assertEquals([[1, "old-1", "old-1"]], sql("""
+ select h.id, h.old_name, l.new_name
+ from ${tableName} for version as of ${oldSnapshot} h
+ join ${tableName} l on h.id = l.id
+ order by h.id
+ """))
+
+ // Binding the same pair in reverse order must preserve both
relation-local schemas.
+ assertEquals([[1, "old-1", "old-1"]], sql("""
+ select l.id, l.new_name, h.old_name
+ from ${tableName} l
+ join ${tableName} for version as of ${oldSnapshot} h on l.id = h.id
+ order by l.id
+ """))
+
// Scenario TC07-join: each historical relation binds and scans its
OWN snapshot schema, so a
// self-join across the rename resolves old_name (old snapshot) and
new_name (new snapshot).
assertEquals([[1, "old-1", "old-1"]], sql("""
diff --git
a/regression-test/suites/external_table_p0/iceberg/test_iceberg_schema_metadata_atomicity_matrix.groovy
b/regression-test/suites/external_table_p0/iceberg/test_iceberg_schema_metadata_atomicity_matrix.groovy
index 14fc7771089..365d4a58b1a 100644
---
a/regression-test/suites/external_table_p0/iceberg/test_iceberg_schema_metadata_atomicity_matrix.groovy
+++
b/regression-test/suites/external_table_p0/iceberg/test_iceberg_schema_metadata_atomicity_matrix.groovy
@@ -58,6 +58,8 @@ suite("test_iceberg_schema_metadata_atomicity_matrix",
sql """switch ${catalogName}"""
sql """create database if not exists ${dbName}"""
sql """use ${dbName}"""
+ // DESC hides comments by default, so enable them before validating
Iceberg field docs.
+ sql """set show_column_comment_in_describe=true"""
try {
sql """drop table if exists ${tableName}"""
@@ -89,9 +91,8 @@ suite("test_iceberg_schema_metadata_atomicity_matrix",
it == null ? "" : it.toString()
}.join(" ")
assertTrue(sparkDescriptionText.contains("top-level-comment"))
- // Negative contract: Doris DESC currently omits Iceberg field
comments.
- assertFalse(descAfterComment.contains("top-level-comment"))
- assertFalse(descAfterComment.contains("nested-comment"))
+ assertTrue(descAfterComment.contains("top-level-comment"))
+ assertTrue(descAfterComment.contains("nested-comment"))
assertEquals(initialSnapshots, snapshotCount())
// Scenario S19-nullability: relaxing required to optional preserves
data and historical refs.
diff --git
a/regression-test/suites/external_table_p0/iceberg/test_iceberg_schema_time_travel_matrix.groovy
b/regression-test/suites/external_table_p0/iceberg/test_iceberg_schema_time_travel_matrix.groovy
index dffbc236ea7..67e944ff3df 100644
---
a/regression-test/suites/external_table_p0/iceberg/test_iceberg_schema_time_travel_matrix.groovy
+++
b/regression-test/suites/external_table_p0/iceberg/test_iceberg_schema_time_travel_matrix.groovy
@@ -443,13 +443,22 @@ suite("test_iceberg_schema_time_travel_matrix",
from ${dorisNestedTable}@tag(doris_nested_cp0)
order by id
"""))
- // Negative contract: an old branch currently leaks the latest BIGINT
nested types.
+ // A branch read uses the table's current schema, so an old branch
reads the promoted BIGINT
+ // nested types.
assertEquals([[1, 10L, 100L, 1000L]],
sql("""
select id, info.metric, events[1].score, attrs['k'].code
from ${dorisNestedTable}@branch(doris_nested_cp0_branch)
order by id
"""))
+ // Named branches keep their head rows but expose renamed fields from
the current schema.
+ qt_doris_nested_branch_current_schema """
+ select id, info.metric, info.renamed,
+ events[1].score, events[1].renamed,
+ attrs['k'].code, attrs['k'].renamed
+ from ${dorisNestedTable}@branch(doris_nested_cp0_branch)
+ order by id
+ """
assertUnknownColumn("""
select info.renamed
from ${dorisNestedTable} for version as of ${dorisNestedCp0}
@@ -546,7 +555,8 @@ suite("test_iceberg_schema_time_travel_matrix",
from ${topTable}@tag(top_cp0)
order by id
"""))
- // Negative contract: an old branch is currently analyzed with the
latest rename schema.
+ // A branch read uses the table's current schema, so the old name is
unknown on an old branch
+ // and the current names read the branch head rows.
test {
sql """
select id, old_name, victim, metric
@@ -555,6 +565,11 @@ suite("test_iceberg_schema_time_travel_matrix",
"""
exception "Unknown column 'old_name'"
}
+ qt_top_branch_current_schema """
+ select id, MixedName, victim, metric
+ from ${topTable}@branch(top_cp0_branch)
+ order by id
+ """
assertUnknownColumn("""
select MixedName from ${topTable} for version as of ${topCp0}
""", "MixedName")
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
