github-actions[bot] commented on code in PR #67725:
URL: https://github.com/apache/doris/pull/67725#discussion_r3987459704
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/plugin/PluginDrivenExternalTable.java:
##########
@@ -949,17 +952,57 @@ public boolean supportInternalPartitionPruned() {
return true;
}
+ @Override
+ public SelectedPartitions initSelectedPartitions(Optional<MvccSnapshot>
snapshot) {
+ if (supportsConnectorPartitionPruning()) {
+ return SelectedPartitions.DEFERRED_PARTITION_PRUNING;
+ }
+ return super.initSelectedPartitions(snapshot);
+ }
+
+ /** Whether this table defers partition materialization until Nereids
supplies a connector predicate. */
+ public boolean supportsConnectorPartitionPruning() {
+ return
hasCapability(ConnectorCapability.SUPPORTS_CONNECTOR_PARTITION_PRUNING);
+ }
+
@Override
public Map<String, PartitionItem>
getNameToPartitionItems(Optional<MvccSnapshot> snapshot) {
+ return getNameToPartitionItems(snapshot, Optional.empty());
+ }
+
+ /**
+ * Builds the generic partition map from a connector-filtered partition
view. Callers use this only after
+ * converting a Nereids predicate into the neutral connector expression
grammar.
+ */
+ public Optional<Map<String, PartitionItem>>
getNameToPartitionItemsByFilter(Optional<MvccSnapshot> snapshot,
+ ConnectorExpression partitionFilter) {
List<Column> partitionColumns = getPartitionColumns(snapshot);
if (partitionColumns.isEmpty()) {
- return Collections.emptyMap();
+ return Optional.empty();
}
- List<String> remoteNames = getSchemaCacheValue(snapshot)
- .map(value -> ((PluginDrivenSchemaCacheValue)
value).getPartitionColumnRemoteNames())
- .orElse(Collections.emptyList());
- List<Type> types =
partitionColumns.stream().map(Column::getType).collect(Collectors.toList());
+ PluginDrivenExternalCatalog pluginCatalog =
(PluginDrivenExternalCatalog) catalog;
+ Connector connector = pluginCatalog.getConnector();
+ ConnectorSession session = pluginCatalog.buildConnectorSession();
+ ConnectorMetadata metadata = PluginDrivenMetadata.get(session,
connector);
+ Optional<ConnectorTableHandle> handleOpt =
resolveConnectorTableHandle(session, metadata);
+ if (!handleOpt.isPresent()) {
+ return Optional.empty();
+ }
+ Optional<FilterApplicationResult<ConnectorTableHandle>> filterResult =
metadata.applyFilter(
Review Comment:
[P1] Reuse the logical connector-filter result for the physical scan
This call applies the Hive filter while logical pruning builds
`SelectedPartitions`, but the returned handle is discarded;
`PluginDrivenScanNode.convertPredicate` later applies the same filter again to
the original handle. If HMS changes between those calls, batched scans combine
names from the first result with `prunedPartitionsByName` from the second, and
`resolveBatchPartitions` now throws for a partition that disappeared between
generations (while synchronous scans consume the second generation). Please
carry one filtered handle/result from pruning into physical planning, and cover
a mutation between the two current calls.
##########
fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnectorMetadata.java:
##########
@@ -1288,6 +1277,87 @@ 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) {
+ try {
+ List<HmsPartitionInfo> prunedPartitions =
hmsClient.listPartitionsByFilter(
+ hiveHandle.getDbName(), hiveHandle.getTableName(),
hmsFilter);
+ LOG.info("Partition pruning through HMS filter: {}.{}
filter={} pruned={}",
Review Comment:
[P2] Keep the generated predicate out of INFO/WARN logs
This logs every literal from an arbitrarily large `IN` predicate on each
successful prune. The deliberate >5,000-match fallback also logs the full
filter plus a stack trace at WARN, and the current logical/physical double
application can duplicate both records. That makes an ordinary low-selectivity
or large-IN query an unbounded hot-log source and exposes partition values
outside debug logging. Please log bounded counts/a digest here, reserve a
truncated expression for DEBUG, and avoid warning stacks for the expected
saturation fallback.
##########
fe/fe-connector/fe-connector-hms/src/main/java/org/apache/doris/connector/hms/ThriftHmsClient.java:
##########
@@ -239,6 +240,21 @@ static short toThriftMaxParts(int maxParts) {
return maxParts <= 0 ? (short) -1 : (short) maxParts;
}
+ @Override
+ public List<HmsPartitionInfo> listPartitionsByFilter(String dbName, String
tableName, String filter) {
+ List<Partition> partitions = execute(client ->
client.listPartitionsByFilter(
+ dbName, tableName, filter, (short) (MAX_FILTERED_PARTITIONS +
1)));
+ if (isFilteredPartitionResponseSaturated(partitions.size())) {
Review Comment:
[P1] Check saturation before applying the metastore filter hook
The concrete `HiveMetaStoreClient` caps the raw HMS call and then applies
`filterHook.filterPartitions`, so this size is post-hook. With more than 5,001
raw matches, one hook-hidden entry in the first page can reduce the returned
size to 5,000 even though a later visible partition was truncated; Doris then
accepts an incomplete result and can miss rows. Please preserve the raw
count/saturation bit across the wrapper (or otherwise fall back when the raw
page saturates), and test through the concrete client with a filtering hook
rather than the direct `IMetaStoreClient` proxy.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java:
##########
@@ -166,6 +172,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] Apply the MVCC snapshot before deferred partition materialization
This public capability can coexist with `SUPPORTS_MVCC_SNAPSHOT`, but the
deferred paths later resolve a fresh base handle: filtered materialization
calls `applyFilter` on it and full materialization calls `listPartitions` on it
without first calling `metadata.applySnapshot`. A snapshot query can then prune
against latest metadata (for example, produce an empty selection after a
partition was removed); although scan initialization later pins its handle, the
stale empty selection can short-circuit before that pinned handle reaches
`planScan`, returning wrong rows. Please materialize from the snapshot-pinned
handle, or explicitly reject the capability combination, and test base/pinned
handles with different partition sets.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/plugin/PluginDrivenExternalTable.java:
##########
@@ -969,12 +1012,24 @@ public Map<String, PartitionItem>
getNameToPartitionItems(Optional<MvccSnapshot>
return Collections.emptyMap();
}
- // One round-trip, no FE-side partition-value cache (per CACHE-P1: the
cutover lists
- // partitions per query instead of maintaining a second-level cache).
The connector returns
- // each partition's display name plus a raw-keyed value map; we
extract values in
- // partition-column order via the cached remote names.
- List<ConnectorPartitionInfo> partitions =
- metadata.listPartitions(session, handleOpt.get(),
Optional.empty());
+ return buildNameToPartitionItems(snapshot, metadata, session,
handleOpt.get(), partitionColumns,
+ partitionFilter);
+ }
+
+ private Map<String, PartitionItem>
buildNameToPartitionItems(Optional<MvccSnapshot> snapshot,
+ ConnectorMetadata metadata, ConnectorSession session,
ConnectorTableHandle handle,
+ List<Column> partitionColumns) {
+ return buildNameToPartitionItems(snapshot, metadata, session, handle,
partitionColumns, Optional.empty());
+ }
+
+ private Map<String, PartitionItem>
buildNameToPartitionItems(Optional<MvccSnapshot> snapshot,
+ ConnectorMetadata metadata, ConnectorSession session,
ConnectorTableHandle handle,
+ List<Column> partitionColumns, Optional<ConnectorExpression>
partitionFilter) {
+ List<String> remoteNames = getSchemaCacheValue(snapshot)
+ .map(value -> ((PluginDrivenSchemaCacheValue)
value).getPartitionColumnRemoteNames())
+ .orElse(Collections.emptyList());
+ List<Type> types =
partitionColumns.stream().map(Column::getType).collect(Collectors.toList());
+ List<ConnectorPartitionInfo> partitions =
metadata.listPartitions(session, handle, partitionFilter);
Review Comment:
[P1] Reuse the MVCC partition-item builder for deferred views
The eager path built each item from `orderedPartitionValues` plus
connector-supplied NULL flags and caught per-partition conversion failures so
an invalid item disabled pruning and preserved scan-all. Both newly deferred
paths now come through this helper, reconstruct values from the raw-key map,
drop the NULL flags, and feed the whole set to `TablePartitionValues`, where
one bad typed value throws. For example, an HMS/API-created non-numeric value
under an `INT` partition column now aborts no-filter planning instead of safely
degrading. Please share the existing source-agnostic builder/degradation
contract and test an unrepresentable typed partition plus an explicit non-NULL
sentinel string.
--
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]
