github-actions[bot] commented on code in PR #66530:
URL: https://github.com/apache/doris/pull/66530#discussion_r4023299611
##########
fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ConnectorSession.java:
##########
@@ -103,6 +103,17 @@ default Map<String, String> getSessionProperties() {
return java.util.Collections.emptyMap();
}
+ /**
+ * Returns whether equivalent external scan plans may be reused within the
current statement.
+ *
+ * <p>The property must be explicitly present and {@code true}. Sessions
created by an older engine or by
+ * offline tooling do not carry this setting and therefore keep the
optimization disabled.</p>
+ */
+ default boolean isExternalScanTaskReuseEnabled() {
Review Comment:
[P1] Advance the connector API major for this new shared method. Both the
exact base and this head still advertise API 8.0, while API admission is
exact-major and the SPI is parent-first. A provider built from this head is
therefore accepted by a pre-PR API-8 FE, then its direct call to this method
resolves against the old `ConnectorSession` and can fail with
`NoSuchMethodError` on the first scan. Please bump
`connector.plugin.api.version` (and the pinned assertion) to 9.0 so the
incompatible pair is rejected at load time.
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonScanPlanProvider.java:
##########
@@ -538,8 +540,79 @@ public void releaseReadTransaction(String queryId) {
*/
@Override
public List<ConnectorScanRange> planScan(ConnectorSession session,
ConnectorScanRequest request) {
- return planScanInternal(session, request.getTableHandle(),
request.getColumns(),
- request.getFilter(), request.getLimit(),
request.isCountPushdown());
+ PaimonTableHandle paimonHandle = (PaimonTableHandle)
request.getTableHandle();
+ if (session == null || !session.isExternalScanTaskReuseEnabled()) {
+ return planScanInternal(session, request.getTableHandle(),
request.getColumns(),
+ request.getFilter(), request.getLimit(),
request.isCountPushdown());
+ }
+ if (paimonHandle.isSystemTable()) {
+ // System tables resolve their snapshot on the BE and carry
deferred side effects
+ // (authorized file enumeration); never reuse their planned ranges.
+ return planScanInternal(session, request.getTableHandle(),
request.getColumns(),
+ request.getFilter(), request.getLimit(),
request.isCountPushdown());
+ }
+ // Resolve the table ONCE at the statement scope so both the
scan-planning path (here) and
+ // the properties path (getScanNodeProperties) observe the SAME table
generation. Without
+ // this, a no-cache catalog or a schema change between two aliases can
give alias A's ranges
+ // to alias B's generation-B serialized table. The resolved table is
cached at the scope;
+ // its latestSnapshotId fences the reuse key so different generations
never share ranges.
+ long generation = resolvePaimonGeneration(session, paimonHandle);
+ // Statement-scoped reuse: within one statement the identical scan
(same table, same
+ // branch/options pin, same generation, same projection, same filter,
same limit, same COUNT
+ // pushdown) plans once and every duplicated relation shares the
result. Session variables
+ // are constant within a statement and deliberately absent from the
key.
+ String memoKey = SCAN_REUSE_NAMESPACE + ":" + session.getCatalogId() +
":" + session.getQueryId();
+ Map<PaimonScanReuseKey, List<ConnectorScanRange>> scanReuse =
session.getStatementScope().computeIfAbsent(
+ memoKey, () -> new ConcurrentHashMap<>());
+ PaimonScanReuseKey reuseKey = new PaimonScanReuseKey(paimonHandle,
request, generation);
+ return scanReuse.computeIfAbsent(reuseKey,
+ key -> Collections.unmodifiableList(planScanInternal(session,
+ request.getTableHandle(), request.getColumns(),
request.getFilter(),
+ request.getLimit(), request.isCountPushdown())));
+ }
+
+ /**
+ * Resolve the Paimon table at the statement scope and return its
generation token
+ * (latestSnapshotId, or -1 when unresolvable). The resolved table is
cached at the scope so
+ * {@link #getScanNodeProperties} observes the same generation as the
scan-planning path.
+ */
+ private long resolvePaimonGeneration(ConnectorSession session,
PaimonTableHandle paimonHandle) {
+ if (session == null || session.getStatementScope() == null
+ || session.getStatementScope() ==
ConnectorStatementScope.NONE) {
+ return -1L;
+ }
+ String tableKey = SCAN_REUSE_NAMESPACE + ":table:" +
session.getCatalogId() + ":"
+ + paimonHandle.getDatabaseName() + "." +
paimonHandle.getTableName();
+ try {
+ Table table = session.getStatementScope().computeIfAbsent(tableKey,
+ () -> resolveScanTable(paimonHandle));
+ if (catalogOps != null) {
+ return catalogOps.latestSnapshotId(table).orElse(-1L);
+ }
+ return -1L;
+ } catch (Exception e) {
+ return -1L;
+ }
+ }
+
+ /**
+ * Resolve the scan table consistently with {@link
#resolvePaimonGeneration}: when the statement
+ * scope already holds a cached table (set by the reuse path in planScan),
return that SAME
+ * instance so getScanNodeProperties observes the same generation.
Otherwise resolve fresh.
+ */
+ private Table resolveScanTableConsistent(ConnectorSession session,
PaimonTableHandle paimonHandle) {
+ if (session != null && session.isExternalScanTaskReuseEnabled()
+ && session.getStatementScope() != null
+ && session.getStatementScope() !=
ConnectorStatementScope.NONE) {
+ String tableKey = SCAN_REUSE_NAMESPACE + ":table:" +
session.getCatalogId() + ":"
Review Comment:
[P1] Key this scoped `Table` by the full resolution identity.
`resolveScanTable()` selects a relation-specific table using `sysTableName`,
branch, and effective scan options, but this cache aliases every handle with
the same catalog/db/table. In a statement mixing a base table with a system
table or branch, or two different `@options` pins, the node can plan its
requested relation's ranges while serializing partition/schema properties from
the first node's cached table; neither the system-range bypass nor the outer
branch/options-aware reuse key protects this properties cache. Include all
three discriminators (or bypass system properties) and add mixed-identity tests.
--
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]
