924060929 commented on code in PR #64300:
URL: https://github.com/apache/doris/pull/64300#discussion_r3548436272
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/PluginDrivenScanNode.java:
##########
@@ -136,6 +151,57 @@ public static PluginDrivenScanNode create(PlanNodeId id,
TupleDescriptor desc,
scanContext, connector, session, handle);
}
+ /**
+ * Injects the Nereids partition-pruning result. Called by the translator
so the pruned
+ * partition set can be pushed down to the connector's scan plan (see
{@link #getSplits}).
+ */
+ public void setSelectedPartitions(SelectedPartitions selectedPartitions) {
+ this.selectedPartitions = selectedPartitions;
+ }
+
+ /**
+ * Resolves the pruned partition spec strings to push to the connector SPI.
+ *
+ * <p>Mirrors legacy {@code MaxComputeScanNode.getSplits()} three-state
handling:</p>
+ * <ul>
+ * <li>not pruned (NOT_PRUNED / non-partitioned) → {@code null}:
scan all partitions;</li>
+ * <li>pruned to a non-empty set → that set's partition names;</li>
+ * <li>pruned to zero partitions → empty list: caller
short-circuits with no splits.</li>
+ * </ul>
+ */
+ static List<String> resolveRequiredPartitions(SelectedPartitions
selectedPartitions) {
+ if (selectedPartitions == null || !selectedPartitions.isPruned) {
Review Comment:
**Batch-mode split generation lost for unpruned partitioned scans (high).**
This gate requires `selectedPartitions.isPruned == true`, but legacy
`MaxComputeScanNode.isBatchMode` gated on `selectedPartitions !=
SelectedPartitions.NOT_PRUNED && size >= numPartitions` (master :226-228). For
a partitioned MC table with no partition predicate,
`ExternalTable.initSelectedPartitions` returns a NON-sentinel
`SelectedPartitions(size, fullMap, false)` — isPruned=false but not NOT_PRUNED
— so legacy entered batch mode (async per-partition-batch split generation) for
full scans of large tables while this returns false and eagerly plans one
synchronous read session over ALL partitions. `supportsBatchScan()` returns
true with a 1024-partition default threshold, so a `SELECT *` / full
aggregation over a ≥1024-partition table now blows up FE plan-time latency and
memory (and EXPLAIN split output changes). The comment claiming `!isPruned`
subsumes both legacy gates is wrong exactly for the n
on-pruned partitioned case — restoring the `!= NOT_PRUNED` check fixes it.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/PluginDrivenExternalTable.java:
##########
@@ -141,6 +208,93 @@ protected synchronized void makeSureInitialized() {
}
}
+ @Override
+ public boolean isPartitionedTable() {
+ makeSureInitialized();
+ return !getPartitionColumns().isEmpty();
+ }
+
+ @Override
+ public List<Column> getPartitionColumns(Optional<MvccSnapshot> snapshot) {
+ return getPartitionColumns();
+ }
+
+ public List<Column> getPartitionColumns() {
+ makeSureInitialized();
+ return getSchemaCacheValue()
+ .map(value -> ((PluginDrivenSchemaCacheValue)
value).getPartitionColumns())
+ .orElse(Collections.emptyList());
+ }
+
+ @Override
+ public boolean supportInternalPartitionPruned() {
+ // Unconditional true, mirroring legacy MaxComputeExternalTable (and
IcebergExternalTable).
+ // This override is shared by every SPI-driven connector
(jdbc/es/trino/max_compute via
+ // CatalogFactory.SPI_READY_TYPES) and true is correct for all of
them, partitioned or not:
+ // - partitioned -> PruneFileScanPartition prunes to the
surviving partitions;
+ // - non-partitioned -> PruneFileScanPartition takes its IF branch
and pruneExternalPartitions
+ // returns NOT_PRUNED for empty partition
columns, so the scan reads all.
+ // It must NOT be gated on `!getPartitionColumns().isEmpty()`:
returning false for a
+ // non-partitioned table sends PruneFileScanPartition down its ELSE
branch, which overwrites the
+ // selection with SelectedPartitions(0, {}, isPruned=true).
PluginDrivenScanNode.getSplits() then
+ // reads that as "pruned to zero partitions" and short-circuits to no
splits, so a filtered query
+ // over a non-partitioned table silently returns zero rows (data
loss). See FIX-NONPART-PRUNE-DATALOSS.
+ return true;
+ }
+
+ @Override
+ public Map<String, PartitionItem>
getNameToPartitionItems(Optional<MvccSnapshot> snapshot) {
+ List<Column> partitionColumns = getPartitionColumns();
+ if (partitionColumns.isEmpty()) {
+ return Collections.emptyMap();
+ }
+ List<String> remoteNames = getSchemaCacheValue()
+ .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 = connector.getMetadata(session);
+ String dbName = db != null ? db.getRemoteName() : "";
+ Optional<ConnectorTableHandle> handleOpt =
metadata.getTableHandle(session, dbName, getRemoteName());
+ if (!handleOpt.isPresent()) {
+ return Collections.emptyMap();
+ }
+
+ // One round-trip, no FE-side partition-value cache (per CACHE-P1: the
cutover lists
Review Comment:
**Partition-values cache removed → a full ODPS `listPartitions` round-trip
per query plan (medium).** Legacy
`MaxComputeExternalTable.getPartitionValues()` served from
`MaxComputeExternalMetaCache` (an ExternalMetaCacheMgr loading cache,
invalidated on refresh); this PR deletes that cache and calls
`metadata.listPartitions()` synchronously during `initSelectedPartitions` for
EVERY query on a partitioned MC table. Tables with tens of thousands of
partitions now pay a multi-second paged ODPS enumeration per plan plus
API-throttling risk — a planning-latency regression vs master's warm-cache
behavior. If the 'no FE-side partition-value cache' choice is deliberate
(CACHE-P1), it's worth measuring against a large-partition-count catalog before
the pattern is inherited by other connectors.
--
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]
