github-actions[bot] commented on code in PR #67725:
URL: https://github.com/apache/doris/pull/67725#discussion_r4002323094
##########
fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveScanPlanProvider.java:
##########
@@ -279,6 +278,21 @@ public List<ConnectorScanRange> planScanForPartitionBatch(
return ranges;
}
+ private List<HmsPartitionInfo> resolveBatchPartitions(HiveTableHandle
handle, List<String> partitionBatch) {
+ if (handle.getPrunedPartitionsByName().isEmpty()) {
+ return loadPartitionsWithProfile(handle.getDbName(),
handle.getTableName(), partitionBatch);
+ }
+ List<HmsPartitionInfo> partitions = new
ArrayList<>(partitionBatch.size());
+ for (String partitionName : partitionBatch) {
+ HmsPartitionInfo partition =
handle.getPrunedPartitionsByName().get(partitionName);
Review Comment:
[P1] Do not assume the physical handle covers the logical batch
This new lookup can reject a name selected under different predicate
semantics. For a STRING-partitioned table with `p=1` and `p=01`, `CAST(p AS
INT)=1` is rejected by the Nereids connector converter, so typed logical
pruning selects both; the physical converter strips the CAST, and Hive builds a
handle/map only for bare `p='1'`. When batch mode is used, `p=01` now reaches
the missing-partition exception below. The base batch path fetched the
requested names directly and read both, so this is a new batch regression even
though the analogous synchronous defect predates this PR. This is also distinct
from the earlier two-generation thread: no HMS mutation is required. Please
fall back to loading batch names unless the native map is proven to represent
the same logical selection (or align the two converters), and add a
forced-batch CAST case.
##########
fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnectorMetadata.java:
##########
@@ -2492,6 +2587,99 @@ private List<String> prunePartitionNames(List<String>
allPartNames,
return matched;
}
+ private static String buildHmsPartitionFilter(List<String> partKeyNames,
Map<String, String> partKeyTypes,
+ Map<String, List<String>> partitionPredicates) {
+ List<String> filters = new ArrayList<>();
+ for (String partKeyName : partKeyNames) {
+ List<String> values = partitionPredicates.get(partKeyName);
+ if (values == null || values.isEmpty()) {
+ continue;
+ }
+ if (!isHmsFilterIdentifier(partKeyName)) {
+ return null;
+ }
+ List<String> valueFilters = new ArrayList<>();
+ for (String value : values) {
+ String literal = toHmsFilterLiteral(value,
partKeyTypes.get(partKeyName));
+ if (literal == null) {
+ return null;
+ }
+ valueFilters.add(partKeyName + " = " + literal);
+ }
+ filters.add(valueFilters.size() == 1 ? valueFilters.get(0)
+ : "(" + String.join(" OR ", valueFilters) + ")");
+ }
+ return filters.isEmpty() ? null : "(" + String.join(" AND ", filters)
+ ")";
+ }
+
+ private static boolean isHmsFilterIdentifier(String value) {
+ if (value.isEmpty() || !isHmsFilterLetterOrDigit(value.charAt(0))) {
Review Comment:
[P2] Reject Hive filter keywords as identifiers
A valid quoted/API-created partition key such as lowercase `date` passes
this check, but Hive 3.1.3's case-insensitive filter lexer tokenizes it as
`KW_DATE`, while a key operand must be `Identifier`. Every equality/IN query on
that schema therefore sends a predictably invalid RPC, taints/destroys the
borrowed client, logs a warning, and falls back to unbounded partition-name
enumeration; the uncached path repeats this on later queries. Please validate
against the filter grammar's reserved tokens (and other lexer collisions such
as all-digit names) and add a parser-backed `date` partition-key case.
##########
fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnectorMetadata.java:
##########
@@ -1288,6 +1279,106 @@ private List<ConnectorPartitionInfo>
listPartitionsUncached(HiveTableHandle hive
return result;
}
+ private PartitionPruningResult prunePartitions(ConnectorSession session,
HiveTableHandle hiveHandle,
+ ConnectorExpression expression) {
+ List<String> partKeyNames = hiveHandle.getPartitionKeyNames();
+ Map<String, List<String>> partitionPredicates =
extractPartitionPredicates(expression, partKeyNames);
+ if (partitionPredicates.isEmpty()) {
+ return null;
+ }
+
+ String hmsFilter = buildHmsPartitionFilter(partKeyNames,
hiveHandle.getPartitionKeyTypes(),
+ partitionPredicates);
+ if (hmsFilter != null) {
+ int predicateValueCount =
partitionPredicates.values().stream().mapToInt(List::size).sum();
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("HMS partition filter request for {}.{}
predicateValues={} filter={}",
+ hiveHandle.getDbName(), hiveHandle.getTableName(),
predicateValueCount,
+ summarizeHmsFilterForDebug(hmsFilter));
+ }
+ try {
+ List<HmsPartitionInfo> prunedPartitions =
hmsClient.listPartitionsByFilter(
+ hiveHandle.getDbName(), hiveHandle.getTableName(),
hmsFilter);
+ LOG.info("Partition pruning through HMS filter: {}.{}
predicateValues={} pruned={}",
+ hiveHandle.getDbName(), hiveHandle.getTableName(),
predicateValueCount,
+ prunedPartitions.size());
+ return new PartitionPruningResult(prunedPartitions, null);
+ } catch (HmsPartitionFilterSaturatedException e) {
+ LOG.info("HMS partition filter response saturated for {}.{}
predicateValues={}; "
+ + "falling back to local partition pruning",
+ hiveHandle.getDbName(), hiveHandle.getTableName(),
predicateValueCount);
+ } catch (HmsClientException | UnsupportedOperationException e) {
+ LOG.warn("Failed to prune Hive partitions through HMS filter
for {}.{} predicateValues={}; "
+ + "falling back to local partition pruning",
+ hiveHandle.getDbName(), hiveHandle.getTableName(),
predicateValueCount, e);
+ }
+ }
+
+ List<String> allPartNames = hmsClient.listPartitionNames(
+ hiveHandle.getDbName(), hiveHandle.getTableName(), -1);
+ List<String> matchedPartNames = prunePartitionNames(
Review Comment:
[P1] Keep the local fallback conservative for batched pruning
The result of this raw-name prefilter is now reused as the logical selected
view, so textually different but type-equal values can be lost before
`PartitionPruner` runs. For example, a valid quoted partition key such as `p-x`
forces the HMS builder to fall back; an INT partition value `01` and predicate
`` `p-x` = 1 `` both parse to integer 1, but `matchesPredicates` compares
`"01"` with `"1"` and drops that partition. Base batch planning still loaded
the typed logical name set directly; the new carried fallback makes batch scans
omit it too. Please decline connector filtering/materialize the full view on
fallback, or compare with the declared partition type, and add a
fallback-forced batch test with `01` and `1`.
--
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]
