github-actions[bot] commented on code in PR #67725:
URL: https://github.com/apache/doris/pull/67725#discussion_r4068213285
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/scan/PluginDrivenScanNode.java:
##########
@@ -1150,18 +1166,130 @@ 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 {
+ // A null selection is the "nothing selected" state this node handles
everywhere else (see
+ // resolveRequiredPartitions, displayPartitionCounts,
shouldUseBatchMode and numApproximateSplits);
+ // there is no deferred view to materialize for it.
+ if (selectedPartitions == null ||
!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();
+ // The map is resolved per statement and table reference rather than
enumerated here: the MV partition
+ // compensator recorded the view it reasoned about, and its union
branch is restricted to exactly those
+ // partition names, so reading a later generation here would read
partitions that neither the MV branch
+ // nor the compensation union covers - rows silently missing from a
rewritten query.
+ Optional<TableSnapshot> tableSnapshot =
Optional.ofNullable(getQueryTableSnapshot());
+ Optional<TableScanParams> scanParams =
Optional.ofNullable(getScanParams());
+ Optional<MvccSnapshot> snapshot =
MvccUtil.getSnapshotFromContext(table, tableSnapshot, scanParams);
+ ConnectContext connectContext = ConnectContext.get();
+ StatementContext statementContext = connectContext == null ? null :
connectContext.getStatementContext();
+ Optional<Map<String, PartitionItem>> partitions = statementContext ==
null
+ ? table.getNameToPartitionItemsForScan(snapshot)
+ : statementContext.resolveScanPartitionView(table,
tableSnapshot, scanParams,
+ () -> table.getNameToPartitionItemsForScan(snapshot));
+ selectedPartitions =
materializeDeferredSelectedPartitions(selectedPartitions, partitions);
+ }
+
+ static SelectedPartitions
materializeDeferredSelectedPartitions(SelectedPartitions selectedPartitions,
+ Optional<Map<String, PartitionItem>> partitions) {
+ if (!selectedPartitions.isDeferredPartitionPruning()) {
+ return selectedPartitions;
+ }
+ // An UNAVAILABLE view (a connector entry that cannot be represented
as a Doris partition item) must not
+ // become an empty selection: keep NOT_PRUNED so the scan reads every
partition instead of none.
+ return partitions.map(items -> new SelectedPartitions(items.size(),
items, false))
+ .orElse(SelectedPartitions.NOT_PRUNED);
+ }
+
+ /**
+ * Completes the EXPLAIN {@code partition=N/M} total for a selection that
cannot know it.
+ *
+ * <p>A connector-filtered selection ({@code PruneFileScanPartition})
carries only the surviving partition
+ * names - not enumerating the table's full partition view is exactly what
pushing the predicate into the
+ * connector buys - so it leaves {@code totalPartitionNum} at
+ * {@link SelectedPartitions#UNKNOWN_TOTAL_PARTITION_NUM}. The reader of
an EXPLAIN still gets the real
+ * total: it is resolved from the connector's UNFILTERED view, the same
view a no-filter full scan
+ * materializes on this node before generating splits ({@link
#materializeDeferredSelectedPartitions}), and
+ * it is resolved here so a statement that renders no EXPLAIN string never
pays for it. An unavailable view
+ * leaves the count unknown, which the renderers write as {@code ?} -
never as a fabricated 0.</p>
+ */
+ private void resolveUnknownTotalPartitionNum() {
+ // Memoized, failure included: an UNAVAILABLE view leaves the count
unknown, so without the flag every
+ // later render of the same node (toString, getPlanTreeExplainStr,
Profile.updateSummary) would ask the
+ // connector again and rebuild the whole view for the same answer. A
FAILED attempt is remembered and
+ // rethrown instead of re-asked, because its caller may swallow the
throw: StmtExecutor.updateProfile
+ // catches Throwable with a WARN, and re-querying there would both
repeat the enumeration and keep
+ // aborting the profile update.
+ if (totalPartitionNumResolved) {
+ if (totalPartitionNumFailure != null) {
+ throw totalPartitionNumFailure;
+ }
+ return;
+ }
+ if (totalPartitionNum >= 0) {
+ totalPartitionNumResolved = true;
+ return;
+ }
+ // A metadata TVF scan (PluginDrivenSysTable) has no partition view to
ask about; its counts stay at
+ // their NOT_PRUNED default, so this resolver never fires for it, and
the cast below must not be reached
+ // through that shape.
+ TableIf table = desc.getTable();
+ if (!(table instanceof PluginDrivenExternalTable)) {
+ totalPartitionNumResolved = true;
+ return;
+ }
+ PluginDrivenExternalTable pluginDrivenTable =
(PluginDrivenExternalTable) table;
+ Optional<MvccSnapshot> snapshot =
MvccUtil.getSnapshotFromContext(pluginDrivenTable,
+ Optional.ofNullable(getQueryTableSnapshot()),
Optional.ofNullable(getScanParams()));
+ try {
+ totalPartitionNum =
totalPartitionNumFromUnfilteredView(totalPartitionNum,
+
pluginDrivenTable.getNameToPartitionItemsForScan(snapshot));
Review Comment:
[P1] Do not enumerate every partition just to render the denominator
A connector-filtered selection deliberately has an unknown total, but every
explain render reaches this unfiltered full-view build. Thus `EXPLAIN` of a
one-partition result over a large table allocates O(all partitions) and can
fail on a listing error even though the filtered executable plan was already
available; profile-enabled ordinary queries pay the same lookup through
`Profile.updateSummary`. Since the renderer already supports `?`, keep the
total unknown (or use a cheap connector count or an already-materialized view)
and add a test proving EXPLAIN/profile does not initiate an unfiltered listing.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/StatementContext.java:
##########
@@ -1358,6 +1366,114 @@ public Collection<ExternalTablePreloadInfo>
getExternalTablePreloadInfos() {
return
Collections.unmodifiableCollection(externalTablePreloadInfos.values());
}
+ /**
+ * The preload record of one external table, or empty when the table was
never registered for preload.
+ * Used by lock-sensitive consumers to reuse metadata the pre-lock preload
pass already materialized.
+ *
+ * @param tableId the table's id, as used by {@link
#registerExternalTableForPreload}
+ * @return ExternalTablePreloadInfo
+ */
+ public Optional<ExternalTablePreloadInfo> getExternalTablePreloadInfo(long
tableId) {
+ return Optional.ofNullable(externalTablePreloadInfos.get(tableId));
+ }
+
+ /**
+ * Materializes every deferred connector partition view the MV partition
collector can need, BEFORE the
+ * internal table read locks are taken. This step is unconditional: it
exists so the default configuration
+ * gets the lock scope, not only the opt-in {@code
enable_preload_external_metadata} pass.
+ *
+ * <p>WHY it is needed at all: {@code QueryPartitionCollector} runs from
+ * {@code InitMaterializationContextHook.afterRewrite}, i.e. while {@link
#lock()} is held, and an unfiltered
+ * connector-pruning file scan is still {@code DEFERRED} at that point, so
the collector would have to
+ * enumerate the table's whole partition view - an unbounded connector
round-trip plus an O(all partitions)
+ * build - with the statement's internal tables locked. Materializing the
same view here moves that work
+ * outside the lock window; the collector then reuses it.</p>
+ *
+ * <p>WHY it is skipped when no internal read lock is taken or no MV
rewrite is enabled at all: with no
+ * locked internal table the enumeration blocks nothing, and the
materialized view has exactly one consumer
+ * (the MV partition collector), which no planner hook runs when BOTH MV
rewrite switches are off. The gate
+ * is deliberately the OR of the two switches - the query hook is
registered from
+ * {@code enable_materialized_view_rewrite} and the DML hook from {@code
enable_dml_materialized_view_rewrite}
+ * - because gating on only one of them leaves the collector enumerating
under the lock in the other
+ * configuration.</p>
+ */
+ public void preloadDeferredScanPartitionViewsBeforeLock() {
+ ConnectContext connectContext = getConnectContext();
+ if (connectContext == null || connectContext.getSessionVariable() ==
null) {
+ return;
+ }
+ if
(!connectContext.getSessionVariable().isEnableMaterializedViewRewrite()
+ &&
!connectContext.getSessionVariable().isEnableDmlMaterializedViewRewrite()) {
+ return;
+ }
+ if (!hasAnyPlanReadLockTable()) {
+ return;
+ }
+ for (ExternalTablePreloadInfo preloadInfo :
externalTablePreloadInfos.values()) {
+ preloadDeferredScanPartitionView(preloadInfo);
+ }
+ }
+
+ /**
+ * Materializes one table's deferred scan partition view and records it on
its preload entry. No-op when the
+ * view is already materialized, or when the table has no LATEST
reference: the collector only reuses the
+ * view for a reference without a version selector, so warming any other
generation would be unused work.
+ */
+ public void preloadDeferredScanPartitionView(ExternalTablePreloadInfo
preloadInfo) {
+ if (preloadInfo.hasScanPartitionView() ||
!preloadInfo.shouldPreloadLatestSnapshot()) {
+ return;
+ }
+ ExternalTable table = preloadInfo.getTable();
+ if (!(table instanceof PluginDrivenExternalTable)
+ || !((PluginDrivenExternalTable)
table).supportsConnectorPartitionPruning()) {
+ return;
+ }
+ preloadInfo.setScanPartitionView(
+ ((PluginDrivenExternalTable)
table).getNameToPartitionItemsForScan(getSnapshot(table)));
Review Comment:
[P1] Avoid materializing the unfiltered view before selective pruning
This runs before `PruneFileScanPartition`, and it is enabled by default
whenever the statement also locks an internal table. A mixed query such as an
internal/Hive join with `hive.p = 1` therefore calls
`getNameToPartitionItemsForScan` here with no predicate, listing and building
every Hive partition, and only later performs the selective HMS filter; that
successful filtered branch never consumes this warmed map. On a large table
this restores the unbounded O(all partitions) planning work this PR is meant to
remove and retains the map for the statement. Please restrict full-view warmup
to scans proven to remain deferred/no-filter (or preload the predicate-filtered
handle), and add a mixed internal/Hive equality-filter test that verifies the
unfiltered listing is not called.
--
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]
