github-actions[bot] commented on code in PR #69806:
URL: https://github.com/apache/doris/pull/69806#discussion_r4408850310
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/plugin/PluginDrivenExternalTable.java:
##########
@@ -139,14 +154,31 @@ public Optional<ConnectorTableHandle>
resolveConnectorTableHandle(
}
}
});
- String dbName = db != null ? db.getRemoteName() : "";
- Optional<ConnectorTableHandle> handle =
metadata.getTableHandle(session, dbName, getRemoteName());
- // A view has no table handle. Check live view existence before
treating an empty handle as a
- // dropped table; the cached isView flag can outlive a remote view
deletion.
- if (!handle.isPresent() && !metadata.viewExists(session, dbName,
getRemoteName())) {
- observation.ifPresent(Runnable::run);
Review Comment:
[P1] `readConnectorMetadata` does not cover provider reads after handle
resolution in MVCC and scan paths. `PluginDrivenMvccExternalTable` and
`PluginDrivenScanNode.buildColumnHandles` call connector metadata directly
after resolving a handle; Iceberg now turns a concurrent `NoSuchTableException`
there into `ConnectorTableNotFoundException`, but no observation is retained to
retire the stale source identity and caches. Keep the lookup observation
through these post-handle reads (or wrap them through a shared funnel) and add
a concurrent-drop test for a no-table-cache Iceberg path.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/plugin/PluginDrivenExternalTable.java:
##########
@@ -894,7 +932,8 @@ public Optional<String> getShowCreateTableDdl() {
if (!handleOpt.isPresent()) {
return Optional.empty();
}
- return metadata.renderShowCreateTableDdl(session, handleOpt.get());
+ return readConnectorMetadata(observeConnectorTableLookup(connector),
session, metadata,
+ () -> metadata.renderShowCreateTableDdl(session,
handleOpt.get()));
}
Review Comment:
[P1] Comment cleanup is defeated when connector-level caches return stale
metadata. Hive `getTableComment` reads `CachingHmsClient.getTable`, which can
return a cached `HmsTableInfo` after the remote table is dropped; enabled
Iceberg `IcebergCommentCache` likewise returns the old comment. No
`ConnectorTableNotFoundException` is raised, so this wrapper returns stale
metadata and leaves the FE table/ID route resident until cache expiry or
REFRESH. Make the comment path freshness/existence-aware or invalidate the
connector comment/table cache before relying on typed absence, and add
cache-enabled drop coverage.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/plugin/PluginDrivenExternalTable.java:
##########
@@ -119,11 +121,24 @@ public PluginDrivenExternalTable(long id, String name,
String remoteName,
*/
public Optional<ConnectorTableHandle> resolveConnectorTableHandle(
Review Comment:
[P1] System-table resolution still bypasses the guarded lookup cleanup.
`PluginDrivenSysExternalTable.resolveConnectorTableHandle` directly asks for
`sourceTable.getRemoteName()` and returns `Optional.empty()` before the new
base resolver can observe the source. If a `tbl$...` relation is discovered and
the source is dropped before sys schema/scan resolution, the sys path fails but
the source names, ID/object cache, and connector metadata cache remain live.
Route the source lookup through the same identity-checked observation while
still returning the sys handle, and add a drop-between-discovery-and-resolution
test.
--
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]
