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]