github-actions[bot] commented on code in PR #67725:
URL: https://github.com/apache/doris/pull/67725#discussion_r4079548197
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CollectRelation.java:
##########
@@ -319,4 +322,28 @@ private void collectFromTableStream(BaseTableStream
tableStream, CascadesContext
List<String> tableQualifier = tableStream.getBaseTableFullQualifiers();
statementContext.getAndCacheTable(tableQualifier, tableFrom,
unboundRelation);
}
+
+ private boolean isUnderInitialFilter(Plan plan, UnboundRelation relation) {
+ if (plan instanceof LogicalFilter && containsRelation(plan.child(0),
relation)) {
Review Comment:
[P1] Do not treat filter ancestry as proof this scan is filtered
This returns true for every relation anywhere below a `LogicalFilter`,
without proving that the predicate will reach that scan. For `Filter(i.k = 1)
-> Join(internal i, Hive h)`, `h` is therefore not marked as an unfiltered
latest relation, so the pre-lock warmup skips it; rewrite leaves `h` DEFERRED
and `QueryPartitionCollector` resolves its full HMS view after
`StatementContext.lock()`, holding `i`'s read lock across the external RPC and
O(all partitions) build. This is distinct from the existing eager-warmup
thread: an unrelated branch's predicate defeats the attempted fix for the
cold-unfiltered collector path. Because collection is still unbound and
barriers can also prevent pushdown, conservatively warm unless the parsed shape
proves a connector-prunable filter will reach this scan, and add a
filtered-join regression test.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java:
##########
@@ -714,9 +690,69 @@ static boolean schemaCacheDisabled(Connector connector) {
@Override
public Map<String, PartitionItem>
getNameToPartitionItems(Optional<MvccSnapshot> snapshot) {
+ if (supportsConnectorPartitionPruning()) {
+ PluginDrivenMvccSnapshot pin = getOrMaterialize(snapshot);
+ if (!pin.getNameToPartitionItem().isEmpty()) {
+ return pin.getNameToPartitionItem();
+ }
+ // The latest Hive query pin intentionally carries no partition
map so selective scans can send a
+ // predicate to HMS first. Consumers that explicitly ask for a
partition map (MTMV alignment,
+ // no-filter scan finalization, and a connector-declined pruning
fallback) require the real full
+ // view instead of treating that query-only pin as an empty table.
+ return super.getNameToPartitionItems(snapshot);
+ }
return getOrMaterialize(snapshot).getNameToPartitionItem();
}
+ @Override
+ public Optional<Map<String, PartitionItem>>
getNameToPartitionItemsForScan(Optional<MvccSnapshot> snapshot) {
+ if (supportsConnectorPartitionPruning()) {
+ PluginDrivenMvccSnapshot pin = getOrMaterialize(snapshot);
+ if (!pin.getNameToPartitionItem().isEmpty()) {
+ // This statement's pin already carries a materialized view:
reuse it instead of paying another
+ // connector round-trip that could observe a different remote
generation.
+ return Optional.of(pin.getNameToPartitionItem());
+ }
+ }
+ return super.getNameToPartitionItemsForScan(snapshot);
+ }
+
+ /**
+ * Threads this statement's MVCC pin onto the handle the partition view is
enumerated from, so a
+ * time-travel / {@code @options} query never prunes against the latest
generation: the data scan reads the
+ * pinned snapshot, and a latest view can be missing (or contain)
partitions it will never read.
+ */
+ @Override
+ protected ConnectorTableHandle pinPartitionViewHandle(ConnectorTableHandle
handle,
+ ConnectorMetadata metadata, ConnectorSession session,
Optional<MvccSnapshot> snapshot) {
+ if (snapshot.isPresent() && snapshot.get() instanceof
PluginDrivenMvccSnapshot) {
+ return metadata.applySnapshot(session, handle,
+ ((PluginDrivenMvccSnapshot)
snapshot.get()).getConnectorSnapshot());
+ }
+ return handle;
+ }
+
+ /**
+ * Materializes the complete partition view for an MTMV refresh before it
acquires base-table locks.
+ * The lightweight Hive query pin keeps the connector snapshot/freshness
kind but deliberately omits the
+ * partition map; MTMV alignment needs that map and must not load it while
holding internal table locks.
+ */
+ public MvccSnapshot materializePartitionViewForMtmv(MvccSnapshot snapshot)
{
+ if (!supportsConnectorPartitionPruning() || !(snapshot instanceof
PluginDrivenMvccSnapshot)) {
+ return snapshot;
+ }
+ PluginDrivenMvccSnapshot pin = (PluginDrivenMvccSnapshot) snapshot;
+ if (!pin.getNameToPartitionItem().isEmpty()) {
Review Comment:
[P1] Preserve the materialized-empty snapshot state
A partitioned Hive table with zero partitions produces an authoritative
empty map here, but the returned snapshot is still indistinguishable from the
lightweight deferred pin. During `syncPartitionsIfNeeded`, `alignMvPartition`
runs under `MetaLockUtils.readLockTables` and reaches `getAndCopyPartitionItems
-> getNameToPartitionItems`; line 695 sees the empty map and performs a second
live HMS enumeration, so it both restores external I/O under the locks and can
align against a newer generation than the pre-lock snapshot. This is a distinct
residual after the existing MTMV fix: the warmup did run successfully, but its
empty result lost the materialized state. Carry an explicit
deferred/materialized flag (with materialized-empty valid) and test that an
empty MTMV view is listed exactly once before locking.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/StatementContext.java:
##########
@@ -1358,6 +1378,116 @@ 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()
Review Comment:
[P1] Keep the pre-lock warmup for physical scan finalization
This early return assumes the MV partition collector is the only consumer,
but every no-filter connector-pruning scan is still initialized DEFERRED. In a
mixed internal/Hive query with both rewrite switches off, `NereidsPlanner`
acquires the internal read locks and its lock callback translates/finalizes the
plan; `PluginDrivenScanNode.doFinalize` then calls
`materializeDeferredSelectedPartitions -> resolveScanPartitionView ->
getNameToPartitionItemsForScan`, enumerating the full HMS view before
`releasePlannerResources` unlocks. This is distinct from the existing
collector-under-lock thread because that hook is disabled here and physical
finalization is the consumer. Gate warmup on the union of actual MV-collector
and physical-finalization consumers (without penalizing plan-only EXPLAIN), and
extend the disabled-MV test through scan finalization.
--
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]
