github-actions[bot] commented on code in PR #66287:
URL: https://github.com/apache/doris/pull/66287#discussion_r3759602454
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/stream/BaseTableStream.java:
##########
@@ -113,10 +116,21 @@ public BaseTableStream(String streamName, List<Column>
fullSchema, TableIf baseT
}
public TableIf getBaseTableNullable() {
- if (baseTable == null) {
- baseTable = baseTableInfo.getTableNullable();
+ TableIf cachedBaseTable = baseTable;
+ if (cachedBaseTable != null) {
+ if (cachedBaseTable instanceof Table && ((Table)
cachedBaseTable).isDropped) {
Review Comment:
**[P2] Keep cached bases unavailable until their database recovers**
A stream can live in `ds` while its base `A` lives in `db`. During `RECOVER
DATABASE db`, `recoverAllTables()` calls `Database.registerTable(A)` and clears
`A.isDropped` before `InternalCatalog.recoverDatabase()` republishes and
unmarks `db`. If the stream retained its pre-drop cached pointer (because it
was not read while `db` was dropped), this branch returns `A` in that gap, so
metadata and planning expose it while its owning database is unavailable; if
the later catalog lock/name check fails, that exposure can outlive the recovery
attempt because the recycle entries were already removed. This is distinct from
the existing `RECOVER TABLE` cache-miss thread because here the table flag
already passes. Please fence on the owning database lifecycle as well (or defer
clearing member-table dropped flags), and cover cross-database recovery with a
latch test.
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVPlanUtil.java:
##########
@@ -224,7 +224,10 @@ public static Pair<Set<TableIf>, Set<TableIf>>
getBaseTableFromQuery(String quer
try {
NereidsPlanner planner = new
NereidsPlanner(ctx.getStatementContext());
planner.planWithLock(logicalPlan, PhysicalProperties.ANY,
ExplainLevel.ANALYZED_PLAN);
- return
Pair.of(Sets.newHashSet(ctx.getStatementContext().getTables().values()),
+ Set<TableIf> baseTables =
Sets.newHashSet(ctx.getStatementContext().getTables().values());
+ // Implicit dependencies are all-level tables, not relations
written at the first query level.
+
baseTables.addAll(ctx.getStatementContext().getImplicitTableDependencies());
Review Comment:
**[P1] Migrate persisted stream dependencies on upgrade**
These additions repair newly analyzed creation and refresh, but an MTMV
image can already contain stream `S` without stable base `T`: old dependency
collection could name-resolve replacement `B` while the stream wrapper still
scanned its cached `T`, then persist a successful refresh as `{S, B}` with no
`T` snapshot. On load, `MTMVRelation.compatible()` only normalizes stored
entries and `MTMV.compatible()` re-registers them unchanged. `T` therefore has
no commit-event edge or recorded snapshot, while freshness treats `S` as
synchronous; the MTMV can remain rewrite-eligible with stale rows until a later
refresh succeeds. Please expand persisted stream dependencies before
registration (or conservatively invalidate them) and add an old-image upgrade
test.
--
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]
