Skip to content

[fix](nereids) gate IcebergRowLevelDmlTransform on catalog identity, not capability alone#67462

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
damokelis wants to merge 1 commit into
base:master
Choose a base branch
from
+17 −1
Open

[fix](nereids) gate IcebergRowLevelDmlTransform on catalog identity, not capability alone #67462

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .../main/java/org/apache/doris/nereids/trees/plans/commands/IcebergRowLevelDmlTransform.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -83,7 +83,13 @@ public class IcebergRowLevelDmlTransform implements RowLevelDmlTransform {

@Override
public boolean handles(TableIf table) {
// Identity FIRST, capability second: several connectors now declare row-level write
// capabilities, so the capability probe alone would let registry order decide which
// transform claims a table. The synthesized plan shape is connector-specific (iceberg's
// position-delete stream vs paimon's full-row keyed delete), so the claim must be too.
return table instanceof PluginDrivenExternalTable
&& "iceberg".equalsIgnoreCase(
((PluginDrivenExternalTable) table).getCatalog().getType())
&& pluginConnectorSupportsRowLevelDml((PluginDrivenExternalTable) table);
}

Expand Down
12 changes: 11 additions & 1 deletion ...t/java/org/apache/doris/nereids/trees/plans/commands/IcebergRowLevelDmlTransformTest.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -96,8 +96,14 @@ private Plan filterOver(TableIf table, String columnName) {
* {@code getConnector().getWritePlanProvider(handle).supportedOperations()} probe.
*/
private static PluginDrivenExternalTable pluginTable(boolean supportsDelete, boolean supportsMerge) {
return pluginTable("iceberg", supportsDelete, supportsMerge);
}

private static PluginDrivenExternalTable pluginTable(
String catalogType, boolean supportsDelete, boolean supportsMerge) {
PluginDrivenExternalTable table = Mockito.mock(PluginDrivenExternalTable.class);
PluginDrivenExternalCatalog catalog = Mockito.mock(PluginDrivenExternalCatalog.class);
Mockito.when(catalog.getType()).thenReturn(catalogType);
Connector connector = Mockito.mock(Connector.class);
Set<WriteOperation> ops = EnumSet.noneOf(WriteOperation.class);
if (supportsDelete) {
Expand All@@ -121,9 +127,13 @@ public void handlesPluginDrivenTableByRowLevelDmlCapability() {
Assertions.assertTrue(transform.handles(pluginTable(true, false)));
Assertions.assertTrue(transform.handles(pluginTable(false, true)));
Assertions.assertTrue(transform.handles(pluginTable(true, true)));
// A plugin connector with neither capability (e.g. jdbc/es/paimon today) must NOT be admitted,
// A plugin connector with neither capability must NOT be admitted,
// else its row-level DML would route through the iceberg synthesis path.
Assertions.assertFalse(transform.handles(pluginTable(false, false)));
// Identity gate: a NON-iceberg connector that DOES declare row-level capabilities must not be
// claimed either — with several connectors declaring capabilities, registry order must not
// decide whose connector-specific plan shape a table gets.
Assertions.assertFalse(transform.handles(pluginTable("paimon", true, true)));
// Non-plugin table types and null are never admitted.
Assertions.assertFalse(transform.handles(Mockito.mock(TableIf.class)));
Assertions.assertFalse(transform.handles(null));
Expand Down