924060929 commented on code in PR #64143:
URL: https://github.com/apache/doris/pull/64143#discussion_r3544225096
##########
fe/fe-connector/fe-connector-hudi/src/main/java/org/apache/doris/connector/hudi/HudiConnectorMetadata.java:
##########
@@ -303,4 +340,113 @@ private Configuration buildHadoopConf() {
}
return conf;
}
+
+ // ========== Partition pruning helpers ==========
+ // Mirrors HiveConnectorMetadata's EQ/IN partition pruning. Duplicated
rather than
+ // shared because fe-connector-hudi depends on fe-connector-hms, not
fe-connector-hive;
+ // consolidate during the Hive (P7) migration. See P3-T05 design.
+
+ /**
+ * Extracts equality predicates on partition columns from the expression
tree.
+ * Supports: col = 'value', col IN ('v1', 'v2', ...), AND combinations.
+ */
+ private Map<String, List<String>> extractPartitionPredicates(
+ ConnectorExpression expr, List<String> partKeyNames) {
+ Set<String> partKeySet =
partKeyNames.stream().collect(Collectors.toSet());
+ Map<String, List<String>> result = new HashMap<>();
+ extractPredicatesRecursive(expr, partKeySet, result);
+ return result;
+ }
+
+ private void extractPredicatesRecursive(ConnectorExpression expr,
+ Set<String> partKeySet, Map<String, List<String>> result) {
+ if (expr instanceof ConnectorAnd) {
+ for (ConnectorExpression child : ((ConnectorAnd)
expr).getConjuncts()) {
+ extractPredicatesRecursive(child, partKeySet, result);
+ }
+ } else if (expr instanceof ConnectorComparison) {
+ ConnectorComparison cmp = (ConnectorComparison) expr;
+ if (cmp.getOperator() == ConnectorComparison.Operator.EQ) {
+ String colName = extractColumnName(cmp.getLeft());
+ String value = extractLiteralValue(cmp.getRight());
+ if (colName != null && value != null &&
partKeySet.contains(colName)) {
+ result.computeIfAbsent(colName, k -> new
ArrayList<>()).add(value);
+ }
+ }
+ } else if (expr instanceof ConnectorIn) {
+ ConnectorIn inExpr = (ConnectorIn) expr;
+ if (!inExpr.isNegated()) {
+ String colName = extractColumnName(inExpr.getValue());
+ if (colName != null && partKeySet.contains(colName)) {
+ List<String> values = new ArrayList<>();
+ for (ConnectorExpression item : inExpr.getInList()) {
+ String val = extractLiteralValue(item);
+ if (val != null) {
+ values.add(val);
+ }
+ }
+ if (!values.isEmpty()) {
+ result.computeIfAbsent(colName, k -> new
ArrayList<>()).addAll(values);
+ }
+ }
+ }
+ }
+ }
+
+ private String extractColumnName(ConnectorExpression expr) {
+ if (expr instanceof
org.apache.doris.connector.api.pushdown.ConnectorColumnRef) {
+ return
((org.apache.doris.connector.api.pushdown.ConnectorColumnRef)
expr).getColumnName();
+ }
+ return null;
+ }
+
+ private String extractLiteralValue(ConnectorExpression expr) {
+ if (expr instanceof ConnectorLiteral) {
+ Object val = ((ConnectorLiteral) expr).getValue();
+ return val != null ? String.valueOf(val) : null;
+ }
+ return null;
+ }
+
+ /**
+ * Prunes partition names based on extracted equality predicates.
+ * Partition names follow the Hive convention: key1=val1/key2=val2
+ */
+ private List<String> prunePartitionNames(List<String> allPartNames,
+ List<String> partKeyNames, Map<String, List<String>> predicates) {
+ List<String> matched = new ArrayList<>();
+ for (String partName : allPartNames) {
+ Map<String, String> partValues = parsePartitionName(partName,
partKeyNames);
+ if (matchesPredicates(partValues, predicates)) {
+ matched.add(partName);
+ }
+ }
+ return matched;
+ }
+
+ private Map<String, String> parsePartitionName(String partName,
+ List<String> partKeyNames) {
+ Map<String, String> values = new HashMap<>();
+ String[] parts = partName.split("/");
+ for (String part : parts) {
+ int eq = part.indexOf('=');
+ if (eq > 0) {
+ values.put(part.substring(0, eq), part.substring(eq + 1));
Review Comment:
**Partition values are matched without unescaping.** HMS stores partition
names Hive-escaped (`ts=2024-01-01 10%3A00%3A00`), but this splits the raw text
and `matchesPredicates` compares it against the raw predicate literal — any
partition value containing a char Hive escapes (space, `:`, `%`, `=`,
non-ASCII) can never match, so the partition is wrongly pruned OUT and its rows
silently vanish (residual filters can't recover rows from unscanned
partitions). Legacy unescapes: HiveUtil.toPartitionValues uses
`FileUtils.unescapePathName(...)` (master HiveUtil.java:190). The same gap
exists in the twin copy in fe-connector-hive. Verified still present at tip.
##########
fe/fe-connector/fe-connector-hudi/src/main/java/org/apache/doris/connector/hudi/HudiConnectorMetadata.java:
##########
@@ -150,17 +158,38 @@ public
Optional<FilterApplicationResult<ConnectorTableHandle>> applyFilter(
return Optional.empty();
}
- // List all partition names from HMS (e.g. "year=2024/month=01")
- // These are relative paths that double as partition identifiers
- List<String> partitionNames = hmsClient.listPartitionNames(
+ // Extract equality/IN predicates on partition columns from the
expression.
+ // No partition predicate -> leave the handle untouched so
resolvePartitions
+ // falls back to Hudi's own metadata listing
(HoodieTableMetadata.getAllPartitionPaths).
+ Map<String, List<String>> partitionPredicates =
extractPartitionPredicates(
+ constraint.getExpression(), partKeyNames);
+ if (partitionPredicates.isEmpty()) {
+ return Optional.empty();
+ }
+
+ // List candidate partition names from HMS (e.g.
"year=2024/month=01"). These
+ // relative paths double as partition identifiers consumed by
HudiScanPlanProvider.
+ // Keep maxParts=-1 (unlimited): no silent partition truncation.
+ List<String> allPartNames = hmsClient.listPartitionNames(
hudiHandle.getDbName(), hudiHandle.getTableName(), -1);
- if (partitionNames == null || partitionNames.isEmpty()) {
+ if (allPartNames == null || allPartNames.isEmpty()) {
+ return Optional.empty();
+ }
+
+ List<String> matchedPartNames = prunePartitionNames(
+ allPartNames, partKeyNames, partitionPredicates);
+ if (matchedPartNames.size() == allPartNames.size()) {
+ // No pruning effect
return Optional.empty();
}
- // Build updated handle with partition paths for scan planning
+ LOG.info("Partition pruning: {}.{} all={} pruned={}",
+ hudiHandle.getDbName(), hudiHandle.getTableName(),
+ allPartNames.size(), matchedPartNames.size());
+
+ // Build updated handle carrying only the matched partition paths for
scan planning.
HudiTableHandle updatedHandle = hudiHandle.toBuilder()
- .prunedPartitionPaths(partitionNames)
+ .prunedPartitionPaths(matchedPartNames)
Review Comment:
**HMS partition NAMES stored where Hudi storage paths are consumed.**
`matchedPartNames` are HMS names (`year=2024/month=01`), and
HudiScanPlanProvider.resolvePartitions returns them verbatim into
`fsView.getLatestBaseFilesBeforeOrOn(partitionPath, ...)`, which expects Hudi
*relative storage paths*. For non-hive-style partitioning
(`hoodie.datasource.write.hive_style_partitioning=false`, Hudi's default) the
storage layout is `2024/01`, so a filtered query finds 0 file slices → 0 rows
while the same query without the filter (via `getAllPartitionPaths()`) returns
rows. Legacy relativized the partition LOCATION instead: master
HudiScanNode.java:410-411
`FSUtils.getRelativePartitionPath(hudiClient.getBasePath(), new
StoragePath(partition.getPath()))`. Suggest resolving the HMS partition's
`sd.location` → relative path before storing. Verified still present at tip.
##########
fe/fe-connector/fe-connector-hudi/src/main/java/org/apache/doris/connector/hudi/HudiConnectorMetadata.java:
##########
@@ -303,4 +340,113 @@ private Configuration buildHadoopConf() {
}
return conf;
}
+
+ // ========== Partition pruning helpers ==========
+ // Mirrors HiveConnectorMetadata's EQ/IN partition pruning. Duplicated
rather than
+ // shared because fe-connector-hudi depends on fe-connector-hms, not
fe-connector-hive;
+ // consolidate during the Hive (P7) migration. See P3-T05 design.
+
+ /**
+ * Extracts equality predicates on partition columns from the expression
tree.
+ * Supports: col = 'value', col IN ('v1', 'v2', ...), AND combinations.
+ */
+ private Map<String, List<String>> extractPartitionPredicates(
+ ConnectorExpression expr, List<String> partKeyNames) {
+ Set<String> partKeySet =
partKeyNames.stream().collect(Collectors.toSet());
+ Map<String, List<String>> result = new HashMap<>();
+ extractPredicatesRecursive(expr, partKeySet, result);
+ return result;
+ }
+
+ private void extractPredicatesRecursive(ConnectorExpression expr,
+ Set<String> partKeySet, Map<String, List<String>> result) {
+ if (expr instanceof ConnectorAnd) {
+ for (ConnectorExpression child : ((ConnectorAnd)
expr).getConjuncts()) {
+ extractPredicatesRecursive(child, partKeySet, result);
+ }
+ } else if (expr instanceof ConnectorComparison) {
+ ConnectorComparison cmp = (ConnectorComparison) expr;
+ if (cmp.getOperator() == ConnectorComparison.Operator.EQ) {
+ String colName = extractColumnName(cmp.getLeft());
+ String value = extractLiteralValue(cmp.getRight());
+ if (colName != null && value != null &&
partKeySet.contains(colName)) {
+ result.computeIfAbsent(colName, k -> new
ArrayList<>()).add(value);
+ }
+ }
+ } else if (expr instanceof ConnectorIn) {
+ ConnectorIn inExpr = (ConnectorIn) expr;
+ if (!inExpr.isNegated()) {
+ String colName = extractColumnName(inExpr.getValue());
+ if (colName != null && partKeySet.contains(colName)) {
+ List<String> values = new ArrayList<>();
+ for (ConnectorExpression item : inExpr.getInList()) {
+ String val = extractLiteralValue(item);
+ if (val != null) {
+ values.add(val);
+ }
+ }
+ if (!values.isEmpty()) {
+ result.computeIfAbsent(colName, k -> new
ArrayList<>()).addAll(values);
+ }
+ }
+ }
+ }
+ }
+
+ private String extractColumnName(ConnectorExpression expr) {
+ if (expr instanceof
org.apache.doris.connector.api.pushdown.ConnectorColumnRef) {
+ return
((org.apache.doris.connector.api.pushdown.ConnectorColumnRef)
expr).getColumnName();
+ }
+ return null;
+ }
+
+ private String extractLiteralValue(ConnectorExpression expr) {
+ if (expr instanceof ConnectorLiteral) {
+ Object val = ((ConnectorLiteral) expr).getValue();
+ return val != null ? String.valueOf(val) : null;
Review Comment:
**Datetime literals can never match partition path text.** fe-core's
ExprToConnectorExpressionConverter wraps datetime literals as
`java.time.LocalDateTime` (ExprToConnectorExpressionConverter.java:316-320), so
`String.valueOf(val)` yields ISO `2024-01-01T10:00` (T separator, seconds
elided), which never equals Hive path text `2024-01-01 10%3A00%3A00`. An EQ/IN
predicate on a datetime partition column therefore prunes ALL partitions → 0
rows (DATE still works: `LocalDate.toString()` = `2024-01-01`). Boolean (`true`
vs `1`/`0`) and decimal trailing-zero forms share the untyped-string-compare
problem. Legacy compared typed values via ListPartitionPrunerV2. Verified still
present at tip.
##########
fe/fe-connector/fe-connector-hudi/src/main/java/org/apache/doris/connector/hudi/HudiScanPlanProvider.java:
##########
@@ -116,7 +116,7 @@ public List<ConnectorScanRange> planScan(
columnNames = avroSchema.getFields().stream()
.map(Schema.Field::name).collect(Collectors.toList());
Review Comment:
**Mixed-case Avro field names break the JNI reader.** `getTableSchema`
lowercases top-level names (HudiConnectorMetadata.java:278, matching legacy),
but this ships raw `Schema.Field::name` as `hudi_column_names`. BE builds
`required_fields` from the (lowercased) Doris slot names and
HadoopHudiJniScanner.initTableInfo does an exact-case lookup — `if
(!hudiColNameToType.containsKey(requiredField)) throw new
IllegalArgumentException(...)` — so any MOR/JNI split on a mixed-case Avro
schema fails at scan open. Legacy sent lowercased names (HudiScanNode.java:223
builds from Doris schema `Column::getName`). This PR's own HudiSchemaParityTest
uses `Id`/`Name`/`Addr` as the realistic case; the fix is one
`.toLowerCase(Locale.ROOT)` mirroring line 278. Verified against the BE scanner
in both checkouts.
--
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]
