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


##########
fe/fe-connector/fe-connector-hudi/src/main/java/org/apache/doris/connector/hudi/HudiScanPlanProvider.java:
##########
@@ -1009,4 +1028,89 @@ static Map<String, String> 
storageHadoopConfig(ConnectorContext context) {
     private ConnectorStorageContext storage() {
         return context.getStorageContext();
     }
+
+    /** Package-private for offline unit tests of key construction. */
+    static HudiScanReuseKey hudiScanReuseKey(HudiTableHandle handle) {
+        return new HudiScanReuseKey(handle);
+    }
+
+    /**
+     * Statement-scoped cache key for one Hudi scan.
+     *
+     * <p>Includes every input that changes the planned split list: table 
identity, the snapshot
+     * instant, the incremental window (begin/end instant + incremental 
options), the pruned
+     * partition set, the partition keys, and the JNI metadata carriers (input 
format / serde).
+     * Session variables are statement-constant and deliberately absent.
+     */
+    static final class HudiScanReuseKey {
+        private final String dbName;
+        private final String tableName;
+        private final String basePath;
+        private final String queryInstant;
+        private final String beginInstant;
+        private final String endInstant;
+        private final Map<String, String> incrementalParams;
+        private final List<String> prunedPartitionPaths;
+        private final List<String> partitionKeyNames;
+        private final String inputFormat;
+        private final String serdeLib;
+
+        // Hudi scan-planning identity is fully captured by the handle and its 
query/incremental
+        // parameters; request-level filter, columns, and countPushdown do not 
affect split planning
+        // and are deliberately excluded from the key.
+        private HudiScanReuseKey(HudiTableHandle handle) {
+            // Catalog and query isolation are provided by the statement-scope 
memo key.
+            this.dbName = handle.getDbName();
+            this.tableName = handle.getTableName();
+            this.basePath = handle.getBasePath();
+            this.queryInstant = handle.getQueryInstant();
+            this.beginInstant = handle.getBeginInstant();
+            this.endInstant = handle.getEndInstant();
+            this.incrementalParams = handle.getIncrementalParams() == null
+                    ? Collections.emptyMap()
+                    : Collections.unmodifiableMap(new 
HashMap<>(handle.getIncrementalParams()));
+            this.prunedPartitionPaths = handle.getPrunedPartitionPaths() == 
null

Review Comment:
   **[P1] Keep Hudi's unpruned and zero-match states distinct**
   
   For `FOR TIME AS OF`, FE intentionally carries an empty partition universe 
and lets the connector plan. A filtered alias can therefore reach this key with 
`prunedPartitionPaths=[]`, while an unfiltered alias at the same pin reaches it 
with `null`; `applySnapshot` preserves that distinction and gives both the same 
`queryInstant`. `resolvePartitions` treats empty as no files and `null` as 
enumerate all snapshot partitions, but these lines make the keys equal. If the 
filtered alias plans first, the unfiltered alias reuses `[]` and silently 
returns no rows. Preserve this discriminator and add a same-pin live-scope test.
   



##########
fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveScanPlanProvider.java:
##########
@@ -725,4 +759,72 @@ private static final class PartitionScanInfo {
     private ConnectorStorageContext storage() {
         return context.getStorageContext();
     }
+
+    /**
+     * Statement-scoped cache key for one Hive scan.
+     *
+     * <p>Includes every input that changes the planned split list: table 
identity, the file formats
+     * (input format / serialization lib / JSON single-column gate), the 
partition keys and the
+     * pruned partition set (each partition's location and values). ACID 
tables are excluded
+     * upstream, and session variables are statement-constant, so both stay 
out of the key.
+     */
+    private static final class HiveScanReuseKey {
+        private final String dbName;
+        private final String tableName;
+        private final String location;
+        private final String inputFormat;
+        private final String serializationLib;
+        private final boolean firstColumnIsString;
+        private final List<String> partitionKeyNames;
+        private final List<HmsPartitionInfo> prunedPartitions;
+
+        private HiveScanReuseKey(HiveTableHandle handle) {
+            // Catalog and query isolation are provided by the statement-scope 
memo key. The table
+            // location identifies the data source of unpartitioned tables, 
whose prunedPartitions
+            // is null.
+            this.dbName = handle.getDbName();
+            this.tableName = handle.getTableName();
+            this.location = handle.getLocation();
+            this.inputFormat = handle.getInputFormat();
+            this.serializationLib = handle.getSerializationLib();
+            this.firstColumnIsString = handle.isFirstColumnString();
+            this.partitionKeyNames = handle.getPartitionKeyNames() == null
+                    ? Collections.emptyList()
+                    : Collections.unmodifiableList(new 
ArrayList<>(handle.getPartitionKeyNames()));
+            this.prunedPartitions = handle.getPrunedPartitions() == null

Review Comment:
   **[P1] Preserve null versus zero-pruned Hive partitions**
   
   `null` and an empty `prunedPartitions` list have different planner meanings: 
`resolvePartitions` lists all HMS partitions for `null`, but returns no 
partitions for an empty list. The normal FE zero-partition short-circuit hides 
this only while `FILE_SCAN_PARTITION_PRUNE` runs. With the supported `SET 
disable_nereids_rules='FILE_SCAN_PARTITION_PRUNE'`, both aliases reach 
`planScan`; a zero-match alias can cache `[]`, then an unfiltered alias of the 
same table gets this identical key and incorrectly returns no rows. Preserve 
the null/empty discriminator and cover this disabled-rule ordering in a 
live-scope 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]

Reply via email to