github-actions[bot] commented on code in PR #67725:
URL: https://github.com/apache/doris/pull/67725#discussion_r3968743356
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/plugin/PluginDrivenExternalTable.java:
##########
@@ -949,8 +950,35 @@ public boolean supportInternalPartitionPruned() {
return true;
}
+ @Override
+ public SelectedPartitions initSelectedPartitions(Optional<MvccSnapshot>
snapshot) {
Review Comment:
[P1] Defer the production Hive snapshot, not only this later logical
initializer. Plain Hive tables are instantiated as
PluginDrivenMvccExternalTable because Hive declares SUPPORTS_MVCC_SNAPSHOT.
BindRelation calls StatementContext.loadSnapshots before constructing
LogicalFileScan, and the ordinary latest path reaches materializeLatest ->
listLatestPartitions -> metadata.listPartitions(..., Optional.empty()),
building the full partition map before this method returns DEFERRED. Cold
queries therefore still enumerate every HMS partition (and warm ones still
rebuild/carry the full per-statement map). Please make the plain-Hive snapshot
path lightweight/deferred too and test the real binding subclass.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java:
##########
@@ -114,21 +117,43 @@ private SelectedPartitions
pruneExternalPartitions(ExternalTable externalTable,
}
Map<String, PartitionItem> nameToPartitionItem =
scan.getSelectedPartitions().selectedPartitions;
+ boolean connectorFilteredPartitions = false;
+ if (nameToPartitionItem.isEmpty()
+ && scan.getSelectedPartitions().isDeferredPartitionPruning()
+ && externalTable instanceof PluginDrivenExternalTable
+ && ((PluginDrivenExternalTable)
externalTable).supportsConnectorPartitionPruning()) {
+ ConnectorExpression connectorPredicate =
+
NereidsToConnectorExpressionConverter.convert(filter.getPredicate());
+ if (connectorPredicate != null) {
+ nameToPartitionItem = ((PluginDrivenExternalTable)
externalTable).getNameToPartitionItemsByFilter(
+ ctx.getStatementContext().getSnapshot(externalTable,
+ scan.getTableSnapshot(),
scan.getScanParams()), connectorPredicate);
+ connectorFilteredPartitions = true;
+ }
+ }
+ if (!connectorFilteredPartitions && nameToPartitionItem.isEmpty()
+ && (scan.getSelectedPartitions().isNotPruned()
+ || scan.getSelectedPartitions().isDeferredPartitionPruning()))
{
+ nameToPartitionItem = externalTable.getNameToPartitionItems(
+ ctx.getStatementContext().getSnapshot(externalTable,
+ scan.getTableSnapshot(), scan.getScanParams()));
+ }
+ final Map<String, PartitionItem> partitionItems = nameToPartitionItem;
Optional<SortedPartitionRanges<String>> sortedPartitionRanges =
Optional.empty();
boolean enableBinarySearch = ctx.getConnectContext() == null
||
ctx.getConnectContext().getSessionVariable().enableBinarySearchFilteringPartitions;
- if (enableBinarySearch && !nameToPartitionItem.isEmpty()) {
+ if (enableBinarySearch && !partitionItems.isEmpty()) {
sortedPartitionRanges =
scan.getSelectedPartitions().sortedPartitionRanges
.or(() -> (Optional)
externalTable.getSortedPartitionRanges(scan))
Review Comment:
[P1] Build sorted ranges from the connector-filtered map. This branch gets
partitionItems from a live HMS-filter result, but
getSortedPartitionRanges(scan) uses PluginDrivenMvccExternalTable's separately
cached full snapshot. After an external add, binary search can omit a matching
key from the logical selected-name set used by partition-batch scans; after a
delete, a stale matching range can reach the missing-item check below and abort
planning. It also reintroduces the full-map work this path is meant to avoid.
When connectorFilteredPartitions is true, bypass the table-wide cache and build
ranges from exactly partitionItems (or carry a matching generation token).
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalFileScan.java:
##########
@@ -299,11 +299,24 @@ private boolean
hasSameScanParams(Optional<TableScanParams> left, Optional<Table
* Mainly for hive table partition pruning.
*/
public static class SelectedPartitions {
+ public enum State {
+ NOT_PRUNED,
+ DEFERRED,
+ MATERIALIZED
+ }
+
+ public static final long UNKNOWN_TOTAL_PARTITION_NUM = -1L;
+
// NOT_PRUNED means the Nereids planner does not handle the partition
pruning.
// This can be treated as the initial value of SelectedPartitions.
// Or used to indicate that the partition pruning is not processed.
public static SelectedPartitions NOT_PRUNED = new
SelectedPartitions(0, ImmutableMap.of(), false, false,
- Optional.empty());
+ Optional.empty(), State.NOT_PRUNED);
+ // DEFERRED_PARTITION_PRUNING means a connector will materialize the
partition view after Nereids has
+ // supplied a predicate. It must stay distinct from NOT_PRUNED because
PluginDrivenScanNode preserves
+ // batch split generation for a no-predicate full scan by
materializing this state before dispatch.
+ public static SelectedPartitions DEFERRED_PARTITION_PRUNING = new
SelectedPartitions(
Review Comment:
[P1] Do not encode an unmaterialized full scan as an empty used-partition
set. A no-filter Hive LogicalFileScan never reaches PruneFileScanPartition, so
QueryPartitionCollector reads this sentinel's empty map before scan
finalization and records that the query uses zero partitions.
PartitionCompensator treats that differently from ALL_PARTITIONS, and the
async-MV path then excludes/rejects otherwise eligible partitioned MVs. Please
teach the logical MV collector to handle DEFERRED explicitly (or materialize it
before collection) and cover an unfiltered Hive-base-table MV rewrite.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java:
##########
@@ -114,21 +117,43 @@ private SelectedPartitions
pruneExternalPartitions(ExternalTable externalTable,
}
Map<String, PartitionItem> nameToPartitionItem =
scan.getSelectedPartitions().selectedPartitions;
+ boolean connectorFilteredPartitions = false;
+ if (nameToPartitionItem.isEmpty()
+ && scan.getSelectedPartitions().isDeferredPartitionPruning()
+ && externalTable instanceof PluginDrivenExternalTable
+ && ((PluginDrivenExternalTable)
externalTable).supportsConnectorPartitionPruning()) {
+ ConnectorExpression connectorPredicate =
+
NereidsToConnectorExpressionConverter.convert(filter.getPredicate());
+ if (connectorPredicate != null) {
+ nameToPartitionItem = ((PluginDrivenExternalTable)
externalTable).getNameToPartitionItemsByFilter(
+ ctx.getStatementContext().getSnapshot(externalTable,
+ scan.getTableSnapshot(),
scan.getScanParams()), connectorPredicate);
+ connectorFilteredPartitions = true;
Review Comment:
[P2] Only mark/use this path when the connector actually applied a partition
filter. A predicate such as `data_col = 1` converts successfully, but Hive
extracts no partition predicate; because the Optional filter is still present,
listPartitions bypasses partitionViewCache and rebuilds the complete
ConnectorPartitionInfo/PartitionItem view in O(all partitions). The lower name
cache may avoid an HMS names RPC, but this line still labels the full result
connector-filtered and reports an unknown total. Propagate an
applied/not-applied result (or pre-check partition slots) so non-partition
filters keep the cached full-view path.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java:
##########
@@ -114,21 +117,43 @@ private SelectedPartitions
pruneExternalPartitions(ExternalTable externalTable,
}
Map<String, PartitionItem> nameToPartitionItem =
scan.getSelectedPartitions().selectedPartitions;
+ boolean connectorFilteredPartitions = false;
+ if (nameToPartitionItem.isEmpty()
+ && scan.getSelectedPartitions().isDeferredPartitionPruning()
+ && externalTable instanceof PluginDrivenExternalTable
+ && ((PluginDrivenExternalTable)
externalTable).supportsConnectorPartitionPruning()) {
+ ConnectorExpression connectorPredicate =
+
NereidsToConnectorExpressionConverter.convert(filter.getPredicate());
+ if (connectorPredicate != null) {
+ nameToPartitionItem = ((PluginDrivenExternalTable)
externalTable).getNameToPartitionItemsByFilter(
Review Comment:
[P1] Keep Hive partition materialization outside internal table locks.
Production Hive leaves supportsLatestSnapshotPreload false, so the pre-lock
preload phase relies on initSelectedPartitions; it now receives only DEFERRED
and does no partition warmup. Nereids then locks internal tables before
BindRelation performs the full snapshot/list materialization and this rewrite
performs the live get_partitions_by_filter RPC (or fallback). A mixed
internal/Hive query therefore holds internal metadata locks across both
operations. Please resolve/preload this view before locking, or use the
preloaded full view for this lock-sensitive path.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java:
##########
@@ -139,7 +164,9 @@ private SelectedPartitions
pruneExternalPartitions(ExternalTable externalTable,
"pruned partition %s is missing in the selected partitions
snapshot", name);
selectedPartitionItems.put(name, item);
}
- return new SelectedPartitions(nameToPartitionItem.size(),
selectedPartitionItems, true,
+ long totalPartitionNum = connectorFilteredPartitions
+ ? SelectedPartitions.UNKNOWN_TOTAL_PARTITION_NUM :
partitionItems.size();
+ return new SelectedPartitions(totalPartitionNum,
selectedPartitionItems, true,
Review Comment:
[P1] Preserve that the connector applied a partition predicate. HMS has
already narrowed partitionItems before PartitionPruner runs; reapplying the
same equality therefore retains the entire narrowed map, making
result.hasPartitionPredicate false. That false value reaches SqlBlockRuleMgr,
so require_partition_filter rejects a valid `WHERE partition_col = ...` query.
Please combine the connector-filter fact with the local result (or otherwise
retain the original-universe semantics) and add a rule-to-SQL-block test.
##########
fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ConnectorCapability.java:
##########
@@ -167,6 +167,16 @@ public enum ConnectorCapability {
* whose scan path supports storage-level predicate pruning.</p>
*/
SUPPORTS_STORAGE_PREDICATE_PRUNING,
+ /**
+ * Indicates that the connector can materialize a partitioned table's
selected partition view directly from
+ * a connector predicate during Nereids partition pruning. The engine
defers eager full partition
+ * materialization for such tables and asks the connector for the filtered
view instead; when the connector
+ * cannot apply a predicate it must retain its existing full-list fallback.
+ *
+ * <p><b>Scope: per-table only.</b> A heterogeneous connector such as Hive
can support this for plain HMS
+ * tables while delegating sibling table formats to connectors with
different partition semantics.</p>
+ */
+ SUPPORTS_CONNECTOR_PARTITION_PRUNING,
Review Comment:
[P1] Update the connector-plugin surface version with this enum.
ConnectorPluginSurfaceTest freezes every ConnectorCapability constant, but
connector-plugin-surface.txt has no entry for this value and both the POM and
the pinned assertion still say 7.0. The existing test therefore fails, and more
importantly an older major-7 FE can admit a newly built major-7 Hive plugin
before linkage reaches this missing field. Please regenerate the baseline, bump
connector.plugin.api.version to 8.0, and update the pinned assertion in this
change.
##########
fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnectorMetadata.java:
##########
@@ -2456,6 +2497,57 @@ private List<String> prunePartitionNames(List<String>
allPartNames,
return matched;
}
+ private static String buildHmsPartitionFilter(List<String> partKeyNames,
+ 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);
+ 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))) {
+ return false;
+ }
+ for (int index = 1; index < value.length(); index++) {
+ char character = value.charAt(index);
+ if (!isHmsFilterLetterOrDigit(character) && character != '_') {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ private static boolean isHmsFilterLetterOrDigit(char value) {
+ return value >= 'a' && value <= 'z'
+ || value >= 'A' && value <= 'Z'
+ || value >= '0' && value <= '9';
+ }
+
+ private static String toHmsFilterLiteral(String value) {
+ if (value.indexOf('\\') >= 0 || value.indexOf('\'') >= 0) {
+ return null;
+ }
+ return "'" + value + "'";
Review Comment:
[P1] Preserve literal type when rendering the HMS filter. In [Hive 2.3's
filter
grammar](https://github.com/apache/hive/blob/rel/release-2.3.9/metastore/src/java/org/apache/hadoop/hive/metastore/parser/Filter.g),
a quoted token is parsed as String, and MetaStoreDirectSql rejects it against
an integral partition column; the JDO integral fallback is disabled by default.
Thus common INT year/month predicates first issue a failing filtered RPC and
then enumerate all partition names, defeating this feature. Render
integral/date grammar forms when supported (and skip the HMS attempt for
unsupported types), with a real typed-client test rather than the current
STRING-only fake.
--
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]
