github-actions[bot] commented on code in PR #67725:
URL: https://github.com/apache/doris/pull/67725#discussion_r3975372160
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java:
##########
@@ -166,6 +171,16 @@ private PluginDrivenMvccSnapshot materializeLatest(
ConnectorMvccSnapshot connectorSnapshot = existingFence.orElseGet(
() -> metadata.beginQuerySnapshot(session,
handle).orElseGet(this::emptySnapshot));
+ // A connector that can materialize a partition predicate remotely
must not populate the latest
+ // snapshot with every partition before Nereids has supplied that
predicate. Plain Hive reaches this
+ // MVCC table class because the catalog also serves snapshot-capable
sibling formats, but its latest
+ // pin is deliberately not a data snapshot and applySnapshot is a
no-op. Keep only that lightweight
+ // query-begin pin here; PruneFileScanPartition will request the
selected partition view later.
+ if (supportsConnectorPartitionPruning()) {
Review Comment:
[P1] Do not expose this empty query pin to MTMV partition alignment.
Scheduled MTMV refresh stores this exact result in
`MTMVTask.beforeMTMVRefresh`, then `alignMvPartition(mtmv, snapshots)` asks the
related Hive table for partition items using the pin. `getNameToPartitionItems`
returns the empty map unchanged, so the related partition set is empty and
alignment schedules every existing MTMV partition for deletion (creation
likewise generates no partitions). Keep the lightweight state query-specific,
or lazily materialize the full view for MTMV partition/freshness callers, and
add a partitioned-Hive MTMV refresh test.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/scan/PluginDrivenScanNode.java:
##########
@@ -1143,12 +1145,35 @@ protected TFileAttributes getFileAttributes() throws
UserException {
protected void doFinalize() throws UserException {
scanNodeProperties = null;
cachedPropertiesResult = null;
+ materializeDeferredSelectedPartitions();
// Nereids prunes scan slots between init and finalize; fencing the
init-time table-wide
// tuple would reject old backends even when the executable scan no
longer carries Variant.
checkVariantBackendCompatibilityForCurrentScan(backendPolicy.getBackends());
super.doFinalize();
}
+ private void materializeDeferredSelectedPartitions() throws UserException {
+ if (!selectedPartitions.isDeferredPartitionPruning()) {
+ return;
+ }
+ // A logical filter materializes this state earlier in
PruneFileScanPartition. Reaching finalize still
+ // deferred therefore means a no-filter full scan, which must recover
the complete map before the
+ // batch-mode gate so it keeps the legacy asynchronous
split-generation path.
+ PluginDrivenExternalTable table = (PluginDrivenExternalTable)
getTargetTable();
+ Optional<MvccSnapshot> snapshot =
MvccUtil.getSnapshotFromContext(table,
+ Optional.ofNullable(getQueryTableSnapshot()),
Optional.ofNullable(getScanParams()));
+ Map<String, PartitionItem> partitions =
table.getNameToPartitionItems(snapshot);
Review Comment:
[P1] Materialize from a real full partition view here, not the deferred MVCC
pin. For production Hive, the statement snapshot is the empty-map pin created
by `materializeLatest`, and dynamic dispatch to
`PluginDrivenMvccExternalTable.getNameToPartitionItems(snapshot)` simply
returns that empty map. This therefore produces a materialized 0/0 selection
while `resolveRequiredPartitions` still scans all partitions: batch mode is
disabled and partition-count EXPLAIN/SQL-block enforcement sees zero for a full
scan. The new unit test hides this by passing a fabricated nonempty map
directly. Add a rehydration path that bypasses the lightweight pin and cover
the real MVCC subclass.
##########
fe/fe-connector/fe-connector-hms/src/main/java/org/apache/doris/connector/hms/ThriftHmsClient.java:
##########
@@ -239,6 +239,13 @@ static short toThriftMaxParts(int maxParts) {
return maxParts <= 0 ? (short) -1 : (short) maxParts;
}
+ @Override
+ public List<HmsPartitionInfo> listPartitionsByFilter(String dbName, String
tableName, String filter) {
+ return execute(client -> client.listPartitionsByFilter(dbName,
tableName, filter, (short) -1).stream()
Review Comment:
[P1] Bound the cardinality of this full-partition response. A valid but
low-selectivity predicate can match hundreds of thousands of partitions, and
`-1` asks HMS to serialize every storage descriptor in one Thrift reply; this
bypasses the existing 5,000-item `HmsPartitionBatchExecutor` and its
frame/message-size backoff, so planning can hit a frame limit or exhaust FE
heap before the fallback helps. Request a safe threshold plus one and fall back
to the name + adaptive-batch path when it is saturated (or otherwise paginate),
and add a large-match test.
##########
fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveWriteUtils.java:
##########
@@ -203,6 +205,44 @@ static List<String> toPartitionValues(String
partitionName) {
return result;
}
+ /** Builds a metastore-rendered Hive partition name from declaration-order
keys and values. */
+ static String makePartName(List<String> partKeys, List<String> values) {
+ StringBuilder result = new StringBuilder();
+ for (int index = 0; index < partKeys.size(); index++) {
+ if (index != 0) {
+ result.append('/');
+ }
+
result.append(escapePathName(partKeys.get(index).toLowerCase(Locale.ROOT)))
Review Comment:
[P2] Preserve the metastore's canonical partition identity instead of
rebuilding it with a different locale. [Hive's
`FileUtils.makePartName`](https://github.com/apache/hive/blob/master/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FileUtils.java)
lowercases keys using the metastore JVM's default locale, while this uses
`Locale.ROOT`; with a Turkish-locale HMS and an API-created uppercase `I`
partition key, Hive renders dotless `ı=value` but Doris batches `i=value`.
`getExistingPartitionsWithStats` allows missing names, so batch mode silently
omits that matching partition. Carry the native filtered partition
metadata/name into batch planning (or obtain a server-rendered name) and add a
locale-sensitive round-trip test.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java:
##########
@@ -114,21 +117,49 @@ 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) {
+ Optional<Map<String, PartitionItem>> connectorPartitions =
+ ((PluginDrivenExternalTable)
externalTable).getNameToPartitionItemsByFilter(
+
ctx.getStatementContext().getSnapshot(externalTable,
+ scan.getTableSnapshot(),
scan.getScanParams()), connectorPredicate);
+ if (connectorPartitions.isPresent()) {
+ nameToPartitionItem = connectorPartitions.get();
+ connectorFilteredPartitions = true;
+ }
+ }
+ }
+ if (!connectorFilteredPartitions && nameToPartitionItem.isEmpty()
+ && (scan.getSelectedPartitions().isNotPruned()
+ || scan.getSelectedPartitions().isDeferredPartitionPruning()))
{
+ nameToPartitionItem = externalTable.getNameToPartitionItems(
Review Comment:
[P1] Make this a real full-view fallback for the MVCC Hive subclass. For
example, `WHERE year > 2024` converts to a connector expression, but Hive
extracts no equality/IN predicate and declines `applyFilter`; this line then
dynamically dispatches to
`PluginDrivenMvccExternalTable.getNameToPartitionItems`, which returns the
preloaded lightweight pin's empty map. The rewrite consequently materializes a
0/0 selection: the scan still reads all partitions, but local pruning/batching
and partition-count enforcement are lost, `require_partition_filter` rejects
the valid predicate, and `QueryPartitionCollector` records zero used
partitions. Use an explicit full-list path when connector pruning is declined
and cover a non-equality Hive partition predicate on the real MVCC table.
--
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]
