github-actions[bot] commented on code in PR #66530:
URL: https://github.com/apache/doris/pull/66530#discussion_r4056348484


##########
fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveScanPlanProvider.java:
##########
@@ -137,6 +141,28 @@ public boolean usesHiveParquetInt96TimeZone() {
 
     @Override
     public List<ConnectorScanRange> planScan(ConnectorSession session, 
ConnectorScanRequest request) {
+        HiveTableHandle hiveHandle = (HiveTableHandle) 
request.getTableHandle();
+        if (session == null || !session.isExternalScanTaskReuseEnabled()) {
+            return doPlanScan(session, request);
+        }
+        if (hiveHandle.isTransactional()) {
+            // ACID / INSERT_ONLY reads open a per-scan read transaction with 
a write-id snapshot and
+            // a shared metastore lock; reusing the planned ranges would skip 
that transaction.
+            return doPlanScan(session, request);
+        }
+        // Statement-scoped reuse: within one statement the identical scan 
(same table, same
+        // partition set, same formats) plans once and every duplicated 
relation shares the result.
+        // The scope is NONE for offline planning and tests, in which case the 
loader runs on every
+        // call. Session variables are constant within a statement and 
deliberately absent.
+        String memoKey = SCAN_REUSE_NAMESPACE + ":" + session.getCatalogId() + 
":" + session.getQueryId();
+        Map<HiveScanReuseKey, List<ConnectorScanRange>> scanReuse = 
session.getStatementScope().computeIfAbsent(
+                memoKey, () -> new ConcurrentHashMap<>());
+        HiveScanReuseKey reuseKey = new HiveScanReuseKey(hiveHandle);
+        return scanReuse.computeIfAbsent(reuseKey,

Review Comment:
   [P1] Do not memoize a plan that skipped a failed directory. 
`listAndSplitFiles` catches `HiveDirectoryListingException` and lets 
`doPlanScan` return normally, so this `computeIfAbsent` installs an incomplete 
range list. The file-list cache intentionally does not cache that failure; 
before this wrapper, an identical later alias could retry and recover, but it 
now reuses the first alias's missing partitions. Please propagate/track the 
incomplete result so it is not installed (or fail the plan), and cover a 
fail-once/succeed-next lister.



##########
fe/fe-connector/fe-connector-hudi/src/main/java/org/apache/doris/connector/hudi/HudiScanPlanProvider.java:
##########
@@ -197,14 +201,69 @@ public List<ConnectorScanRange> planScan(ConnectorSession 
session, ConnectorScan
     }
 
     private List<ConnectorScanRange> planScanInScope(ConnectorSession session, 
ConnectorScanRequest request) {
+        // Statement-scoped reuse: within one statement the identical scan 
(same table, same
+        // instant/incremental pin, same partition set, same table generation) 
plans once and every
+        // duplicated relation shares the result. The generation token (latest 
completed instant)
+        // fences the key against same-path table recreation: without it, 
alias A's ranges (stamped
+        // with generation-A schema_id values) could be paired with alias B's 
generation-B
+        // history_schema_info dictionary, causing BE to map files through 
wrong field identities.
+        // The scope is NONE for offline planning and tests, in which case the 
loader runs on every
+        // call. Session variables are constant within a statement and 
deliberately absent.
+        if (session == null || !session.isExternalScanTaskReuseEnabled()) {
+            return doPlanScan(session, request);
+        }
+        HudiStatementTable statementTable = resolveHudiTable(
+                session, (HudiTableHandle) request.getTableHandle());
+        String memoKey = SCAN_REUSE_NAMESPACE + ":" + session.getCatalogId() + 
":" + session.getQueryId();
+        Map<HudiScanReuseKey, List<ConnectorScanRange>> scanReuse = 
session.getStatementScope().computeIfAbsent(
+                memoKey, () -> new ConcurrentHashMap<>());
+        HudiScanReuseKey reuseKey = hudiScanReuseKey(
+                (HudiTableHandle) request.getTableHandle(), 
statementTable.generation);
+        return scanReuse.computeIfAbsent(reuseKey,

Review Comment:
   [P1] Do not make a transient schema-ID fallback sticky. `doPlanScan` catches 
JNI/Avro and table/per-file InternalSchema lookup failures and can return 
native ranges without `hudi.schema_id`, so this memo installs the degraded 
BY_NAME plan. On an evolved COW table, alias A can read an unchanged column 
correctly during the hiccup, but a same-key alias B projecting a renamed column 
can no longer retry; even if B builds its history dictionary, BE sees no split 
schema ID, uses BY_NAME, and returns NULL for old files. Keep degraded plans 
uncached (or fail them), and cover a fail-once/succeed-next schema resolver.



-- 
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]

Reply via email to