damokelis opened a new pull request, #67462:
URL: https://github.com/apache/doris/pull/67462
### What problem does this PR solve?
`RowLevelDmlRegistry` dispatches a row-level `DELETE`/`UPDATE`/`MERGE` to
the first registered `RowLevelDmlTransform` whose `handles()` claims the table.
The single entry today, `IcebergRowLevelDmlTransform`, claims a table on a
**pure capability probe** with no identity check:
```java
public boolean handles(TableIf table) {
return table instanceof PluginDrivenExternalTable
&&
pluginConnectorSupportsRowLevelDml((PluginDrivenExternalTable) table);
}
```
`pluginConnectorSupportsRowLevelDml` only asks whether the connector
declares `DELETE` or `MERGE`. That is safe **only while iceberg is the sole
connector declaring those capabilities** — which is the case on master right
now, so this is not currently a live bug.
It stops being safe the moment a second connector declares them. The
capability probe alone then lets **registry order** decide which transform
claims the table, and the iceberg transform would synthesize its own
connector-specific plan shape (a position-delete stream over
`ICEBERG_ROWID_COL`) for a table whose connector expects a different one. The
failure mode is not a clean error: the plan is built, bound, and executed
against the wrong write protocol.
The registry javadoc already acknowledges the hazard — "order only matters
if two transforms could claim the same table" — but nothing enforces it.
### Fix
Check catalog identity first, capability second, so a transform only ever
claims tables it can actually synthesize for:
```java
return table instanceof PluginDrivenExternalTable
&& "iceberg".equalsIgnoreCase(
((PluginDrivenExternalTable) table).getCatalog().getType())
&& pluginConnectorSupportsRowLevelDml((PluginDrivenExternalTable)
table);
```
This is a defensive fix that makes the registry safe to extend. It changes
nothing for iceberg tables, which continue to be claimed exactly as before.
### Release note
None
### Check List
- [x] Test <!-- At least one of them must be included. -->
- [x] Unit Test
- `IcebergRowLevelDmlTransformTest` now stubs the catalog type its
`handles()` reads, and asserts that a capability-declaring **non-iceberg**
table is not claimed. Existing assertions (iceberg tables with each capability
combination are still claimed; no-capability, non-plugin and null tables are
still rejected) are unchanged and still pass.
- [x] Behavior changed:
- [x] No — iceberg tables are claimed exactly as before. Only a
hypothetical non-iceberg plugin table declaring row-level capabilities is newly
rejected, and no such connector exists on master today.
- [x] Does this need documentation?
- [x] No.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
