feat(sql): add experimental native materialized views with atomic full refresh

[dengn]

What type of PR is this?

• BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

Related to #30079 (staged native materialized views), #6593, #23355 and #24553. The tracker remains open for its broader scope; this does not add transparent rewrite, incremental maintenance or logical history/clone/restore integration.

What this PR does / why we need it:

Add experimental, disabled-by-default native read-only materialized views, with deferred population and manual/periodic full refresh. Native optimistic-SI publication commits stored rows, successful run/sequence and complete physical/logical source timestamp together. Failure preserves the last successful publication. Existing frontend privilege/account/definer admission, dependency guards, task lease/retry lifetime, native transaction ownership and writer budgets remain the owners.

SET experimental_materialized_view = 1;
SET time_zone = '+00:00';
CREATE MATERIALIZED VIEW sales_summary REFRESH MANUAL AS
  SELECT region, COUNT(*) AS n, SUM(amount) AS total
  FROM sales GROUP BY region;
REFRESH MATERIALIZED VIEW sales_summary;
SELECT * FROM sales_summary;

Current delivery — 50e7f92b

Fix the coverage-UT failure in TestViewDescriptionTwoCN: Push-mode peer freshness assertions now transfer the writer's commit frontier through existing SYNCCOMMIT. After restoring only the source, independently require CN1 to see its new physical source ID before executing the same prepared SHOW COLUMNS handle. All literal metadata/data, CTAS, stable View identity and restart assertions remain. No sleep/retry/reprepare, production change or CI-policy relaxation.

Validation: focused dependency-instrumented coverage pass, 10/10 repeated coverage runs, all 5 normal owning-package roots, gofmt/diff checks and vet/lint with zero issues. Instrumentation covers six key dependency packages, distinct from full CI's graph. Two single-path guard-removal probes still pass through redundant validation paths; they are explicitly inconclusive, not negative-control successes. Existing protocol120/native/race evidence remains scoped to unchanged product sources. Diagnosis · Test cases and scoped results.

Parent 3b4328cc ALL CI39152640600 failed coverage UT (job117749332951: expected restored varchar(60), got varchar(90)); its normal UT, SCA, shared build, both BVTs and both connectors succeeded. This correction is test-only. Current head 50e7f92bd08e6ba0491d8c63368f8720f874d9f5 completed ALL CI39157705591 SUCCESS, including shared build, normal UT, coverage UT, arm64 SCA, both complete BVTs, final coverage aggregation and CI Required. Iceberg39157708587 and Connector/J39157708624 also completed SUCCESS. Verified 2026-10-18 20:09 UTC: conflict-free, unmerged, with no newer changes-requested review; reviewer acceptance remains separate.

Prior protocol120 delivery — 3b4328cc

Merged main 75d16a2b, resolved its protocol allocation conflict and preserved main's coordinator-owned dependent programs at 119. MV authoring, frontend admission, scans/registration, catalog prerequisite and global cron admission now require 120. Typed controls reject119 before catalog/task publication and preserve ordinary dependent execution at119. Catalog4.0.17/offset5, definition format2 and executor17 remain unchanged.

Main's new dependent source/writer paths bypass ordinary compiler scope construction. Dependent MV sources now use the same population/protocol/source-identity validator. Ordinary and dependent DML share the transaction-bound MV target capability check at validateDMLTarget; three later ordinary-only checks are removed. Existing CCPR error precedence remains intact. There is no new worker, cache, lock or retry layer, and ordinary tables gain no catalog lookup.

Validation on the integrated candidate:

• 116/116 native entrypoint checks in each RC and SI configuration (70 functional +5 failure +14 consumer +9 cross-CN +18 bridge), plus independent14-case consumer,9-case concurrent topology and plain prepared probes. Every CN advertises120; independent cleanup is zero and services exit0.
• The three new consumer cases cover unpopulated/populated deep MV reads including LIMIT0/WHERE FALSE, and correlated INSERT/UPDATE/DELETE rejection with unchanged rows and complete publication metadata. An independent verbose EXPLAIN confirms dependent-program execution.
• 6/6 native commit-loss/SIGKILL/replay/cleanup cases, 7,882 root passes across complete migration/MV/planner/compiler/frontend packages, seven-package vet and incremental lint with zero issues. Removing the read or shared write admission separately makes its focused counterexample fail while CCPR controls pass.

Compatibility and execution-consumer design · Source-bound execution and counterexample results · Executable modes.

The native binary is accurately identified as95+staged main75+working fixes, with final production/client inputs unchanged during execution. The11,275-file non-doc fingerprint is 185ce660a1fd954ca04f91f1ae6f2c355cc9a084ab884525f8d03aeb02121823. Prior service-owner race evidence remains valid for its byte-identical synchronization code; new admission adds no Go shared state. No repeated full native upgrade or new benchmark is claimed; those older captures remain historical.

Parent 95dd4c0d completed ALL CI39145960351, both BVTs, regular UT, SCA, coverage and both connector workflows successfully. This head's ALL CI39152640600 subsequently failed coverage UT; normal UT/SCA/both BVTs and both connectors passed. See the current test-only correction above. Reviewer acceptance is separate; the PR is unmerged.

Prior metadata/retirement delivery — 95dd4c0d

Merged main 5f8d38f1 and regenerated the conflicted MySQL parser from the merged grammar, preserving both MV syntax and main's empty-CTE rejection. Protocol119, format2, migration4.0.17/offset5 and executor17 are unchanged.

• Prepared SHOW CREATE TABLE now rebinds its original AST on EXECUTE and bypasses both physical-compile cache admission sites. A handle prepared at60 seconds observes another CN's ALTER to120 and equals fresh SHOW; the independent manual-to120 probe also passes.
• All MV service metadata writers share the existing account lifecycle row lock in bounded RC transactions. Publication holds an outer read-only RC fence while its original optimistic SI transaction atomically replaces data/provenance; the SI snapshot starts after fence admission and stays fixed. No new persistent state, worker, queue or retry layer. The existing retry owner remains bounded to3 attempts.
• Retained negative evidence covers stale SHOW, a definition surviving DROP after restart/sync, and rejected intermediate concurrency designs. Fence failure/cancel/cursor errors, source snapshot drift and a lost outer-completion response are covered; removing only the row-lock statement makes both retirement and refresh counterexamples fail.
• Native two-CN 113/113 checks pass in each explicit RC and SI configuration (70 functional +5 failure +11 consumer +9 cross-CN +18 bridge), plus independent11-case consumers,9-case topology and plain prepared probes. Cleanup counts and service exits are zero. 6/6 native commit-loss/SIGKILL/replay/cleanup cases pass, using the existing recovery fixture's new --refresh-only mode.
• Full merged parser/planner/frontend package checks, final owning MV race,100-repeat focused race checks, vet and incremental lint pass. The final11,217-file non-doc input fingerprint is 92ee0309e31e8d150f33c3b88a6cf27e35523e9be0062551546f4c6e00d1b709.

Requirement/design/cost review · Source-bound results and negative proofs · Executable validation modes.

Native tests use the pre-main-merge14a+working-fixes candidate; all five changed production files and consumer client are byte-identical to delivery. Merged parser/planner/frontend tests and this head's CI cover the merge. The restricted frontend attempt failed on denied local socket creation; its authorized-network rerun passes. Earlier upgrade/query-performance captures remain historical. No new benchmark is claimed; the additional read-only transaction and account serialization can increase concurrent refresh latency.

Historical95 validation: ALL CI39145960351, Iceberg39145962602 and Connector/J39145962621 all completed successfully. The new integration above has its own CI.

Prior cross-CN DELETE delivery — 14a213e5

Fix the remaining cross-CN cached DELETE boundary on top of main4226537e and the DUMP/LOAD, source DML, SHOW export and two-phase COPY corrections below. GitHub confirms mergeable=true with no conflict or behind state. Protocol119, format2, migration4.0.17/offset5 and executor17 are unchanged.

A native probe found that PREPARE on CN1 before CN2 creates an MV retained CanTruncate: EXECUTE failed with the dependency guard while fresh direct DELETE succeeded. The earlier same-session case advanced session DDL generation and masked that cache boundary. One bounded planner predicate now makes the existing frontend owners rebind the logical plan and bypass ordinary/physical prepared caches for truncating DELETE. Filtered/row DELETE and ordinary SELECT retain their policy; no new catalog query, worker, cache, retry layer or schema mutation is added. Eligible DELETE pays logical rebinding plus its existing indexed dependency lookup per execution.

The consumer mode creates the MV from another connection/CN with existing causal synccommit handoffs. Complete planner/frontend suites pass (6,332 root passes); focused controls, owning vet and incremental lint pass with zero issues. The cache-fix product binary passes the 112-check authenticated two-CN entrypoint. After adding only two causal handoff SELECTs to the test client, final 10/10 consumer checks plus the plain prepared reproducer pass on each explicit PESSIMISTIC/RC and OPTIMISTIC/SI two-CN configuration. All service exits and independent cleanup counts are zero. Final client vet/lint pass. The final11,214-file delivery fingerprint has zero mismatches; product sources are unchanged between those native captures. Binary identity is accurately5c25+working changes, not a pristine publication-head build.

Cache requirement/design/cases · Counterexample and final execution receipts.

Historical14a validation: ALL CI39136197577, Iceberg39136198979 and Connector/J39136198998 all completed successfully. These are prior-head results.

Prior main integration — 5c25c816

Includes consumer fix 4280e1f7, two-phase COPY integration 9ecf544f (actual main e97e54ff) and the subsequent actual merge of main 4226537e. GitHub confirms mergeable=true and no behind/conflict state. Protocol119, format2, migration4.0.17/offset5 and executor17 remain unchanged.

• Recheck MV dependencies after COPY publication locks and RC snapshot advancement. An MV created during physical preparation must prevent source replacement even when source identity/schema/data timestamp remain unchanged. Retain fail-fast admission before preparation.
• Four deterministic cases reuse the upstream phase fixture: existing dependency, dependency published during preparation, independent source and canceled publication probe. All pass; removing only the late guard makes the late-creation counterexample fail at the publication oracle.
• The native consumer mode now has ten checks, including COPY rejection with unchanged publication and no prepared-relation residue.

The e97 integration passes the complete compiler suite (1,499 roots), 36 focused MV/planner/frontend/compiler roots, compiler/client vet and configured incremental golangci-lint2.14.0 (zero issues). A fresh production binary and authenticated two-CN entrypoint pass 112 checks (70 functional +5 failure +10 consumer +9 cross-CN +18 SQL bridge), plus separate 10/10 consumer smoke. Both processes exit0; owned metadata/tasks are absent. Its11,214-file input fingerprint is unchanged after execution. These native results are bound to the e97 integration candidate, not represented as a pristine later-head binary. After merging4226537e, the complete compiler suite passes again (9.824s); MV fix sources/cases are unchanged by those nine upstream paths.

Integration design/cases · Integration receipts/source limits.

That preceding run is superseded by14a213e5. Build, SCA, coverage UT, both connector workflows and both BVT jobs passed before the new push; its pending ordinary UT is not claimed as a completed pass.

Prior consumer-fix capture — 4280e1f7

Addresses review 5562448876 on top of 79484a43, preserving format2, protocol119, migration4.0.17/offset5 and the existing owners.

• Reject native DUMP/LOAD TABLE on MV relation kind before file admission or native writes, preserving rows and publication provenance.
• Keep unrestricted source DELETE as row DML when dependencies pin its identity. Perform one indexed dependency probe at physical eligibility and share that decision with index maintenance; explicit TRUNCATE stays protected.
• Share authoritative MV DDL formatting between standard and explicit SHOW CREATE, including current altered refresh policy and quoted identities.
• Add nine native consumer checks to the existing BVT entrypoint: prepared-before-creation and feature-disabled DELETE, affected rows/empty-source boundary, unique/secondary maintenance, unchanged last-good rows/full TS, native import admission, and manual/altered-policy DDL replay.

Linux/amd64 Go1.27.1 validation: complete MV/planner/frontend/compiler suites pass, final physical-path planner suite passes again, and owning/final planner/client vet passes. The final native two-CN entrypoint passes 111 checks (70 functional +5 failure +9 consumer +9 cross-CN +18 SQL bridge), plus a separate 9/9 smoke; both service processes exit0 and owned databases/metadata/tasks are absent. The 11,210-file non-documentation fingerprint has zero delivery mismatches. Native binaries identify pre-delivery79484a43 + working changes, not a pristine later publication build.

Correction design and case inventory · Inspectable execution evidence. Fresh-main79952341 merge-tree preflight passes; new-head CI is running and will be reported separately. No new crash/upgrade or performance claim is made for these unchanged owners.

Integration at 79484a4

Cumulative integration of source 4a5fb3fd89df9037db5bc76d0b06b27f93a63ada onto latest main 3fc00709eccc277c175c7a09eed2605ca1aaee29. All 24 original commit objects, including 12 merges, and the exact source tree are archived locally with original authorship preserved. This is a cumulative rebase, not a claim that the original commit IDs remain ancestors.

• Preserve all 40 main overlap owners and resolve seven conflicts. Main's entire 4.0.15/4.0.16 migration directories, charset protocol118, function identities and plan protobuf remain unchanged. MV uses migration 4.0.17, offset 5, and capability 119 across authoring, scan, task registration and upgrade. These are allocations against this frozen main; recheck them when other capability PRs merge.
• Persist format 2 query semantics in the existing immutable definition: explicitly present division precision (including valid zero), full SQL mode (including valid empty mode), and named timezone rules or an explicit fixed offset. Refresh uses the captured context through the existing executor resolver; internal compiler forwarding now includes scoped SQL mode alongside precision/DOP. Control-plane SQL keeps its own escaping/parser context.
• Named timezone load failure does not silently become a fixed offset. A rule-bearing Local without portable identity requires an explicit named or fixed time_zone at creation. Stored format1 remains readable/SHOWable/removable and its policy can be disabled, but refresh/re-enabling requires recreation because its original precision is unknown. Migration pauses legacy periodic policies and advances their epoch; no default4 backfill or scale guessing.
• Retain main's complete table-definition resolution before LIMIT0 pruning. MV-containing literal-zero subtrees reach the existing population/source validator instead of bypassing it through the physical compiler shortcut. Populated MV LIMIT0 retains metadata and live source privileges; unpopulated LIMIT0/WHERE FALSE still reject. No duplicate catalog validator or general read-path query is added.
• Keep bounded retention (16 views, two128-row pages/view, terminal-only conditional deletes and overlap deduplication). Count raw rows per page across result batches; either full page keeps cleanup due now, both short pages postpone one hour. The existing 30-second reconciler owns catch-up, retry and rotation; no new queue, worker or cursor state machine.
• Remove the duplicate EXPLAIN base64 options field; reuse main's existing opaque-options representation and renderer owner. Catalog inventories retain main's extra entries: sys65 and tenant42.

Design correction: docs/design/materialized_view_semantic_context_v2.md. Original behavior/recovery/performance ledgers retain their historical revisions and topology limits; this section supersedes earlier current-allocation claims.

Historical validation at 79484a4 (macOS)

Go1.27.1/darwin-arm64, CPU. Task-owned native artifacts pass repository provenance checks; original libraries are reused. Official builds pass for exact main, the current instrumented recovery service, and the CGO-free SQL probe.

• Complete normal suites: 518 roots / 2,546 test nodes across 11 owner packages, including bootstrap and main's existing migration owners. Selected normal consumers: 89 root executions / 405 nodes across16 packages, from88 distinct selected names. One focused -race -count=1 pass succeeds for that closure.
• One shared authenticated SQL fixture asserts actual stored strings and wire scales at precision0/4/10/30 despite changed refresh settings; SQL-mode-sensitive arithmetic; named New York winter/summer and fixed+8 results; populated/unpopulated LIMIT0/WHERE FALSE; direct/prepared source-grant revoke/regrant; failed refresh preservation of rows/run/sequence/full TS; dependency rejection; drop/recreate identity; and actual sys catalog count65.
• Retention tests cover127/128 boundaries, either/both full overlapping pages, multiple result batches, cancellation, errors and a subsequent drained invocation, while checking actual task CreateBatch publication. Catalog tests check both definition representations, missing/null/invalid semantic values, legacy refresh rejection and named rules. Named/fixed authoring controls are portable; the rule-bearing Local case explicitly skips on hosts whose Local is fixed and executes on this machine.
• New fixture cleanup/import corrections pass narrow normal/race checks. The final configured 21-package incremental SCA exits0 with zero golangci-lint2.14.0 issues. The earlier seven fixture findings are fixed and their failed receipt retained. Molint's21 unchanged-main and six retained standalone-harness reporting/recovery advisory sites are separately classified; they are not presented as newly introduced production violations.
• Current native recovery: 13/13 PASS, exit0, against actual main4.0.16/protocol118 and current4.0.17/protocol119. Covers old-CN upgrade/task exclusion, interrupted migration rollback/recovery/repetition, downgrade refusal, durable commit response loss, SIGKILL before/after publication, restart/same-run replay and cleanup of owned processes/data. The first attempt stopped after six passes because its harness DSN omitted UTC; that failure remains recorded. The corrected driver reuses the same binaries and completes all13 groups.
• Parser regeneration is byte-identical with zero conflicts and bound to the final reviewed fingerprint. Task proto/generated output is unchanged from the original feature pair; descriptor review confirms only executor17 is added. Main-only665 paths are preserved apart from the explicit compiler-context owner extension. Formatting/diff checks pass.
• Required actual gpt-6.1-sol/xhigh design and complete112-path final review pass with zero source blockers. The review's remaining generated-output receipt condition is now closed by the matching final source fingerprint and regeneration receipt. Commands, hashes, exact selections, terminal receipts and native binary/results are retained in the maintenance record.

No new TPS/speedup, zero-overhead, GPU, fleet or production release-acceptance claim is made. The older macOS record and the new Linux consumer/BVT record above have distinct source identities; new-head CI remains separately observed. Ordinary SELECT gains no catalog SQL; otherwise eligible unrestricted DELETE adds one bounded indexed dependency probe; ordinary literal LIMIT0 planning gains a bounded local metadata walk. Existing batch/context writer checks and full-refresh scan/delete/write/MVCC costs remain. Downstream review, new-head CI and existing release-readiness requirements remain separate; no review dismissal, self-approval or merge is performed.

[dengn]

Updated PR #30085 to 50e7f92bd08e6ba0491d8c63368f8720f874d9f5 with a test-only correction for coverage UT's ordinary-View cross-CN restore assertion. Transfer the writer's commit frontier, then independently assert the peer sees the restored source ID before executing the same prepared handle. All original result/identity/restart assertions remain; no sleep, retry, reprepare or CI-policy changes.

Focused dependency coverage and 10 repeated runs pass; the normal multicn package passes all5 roots; vet/lint zero issues. Two individual guard-removal probes are recorded as inconclusive because redundant cache validation remains. Diagnosis · Test cases and scoped results.

Parent3b: coverage UT failed; normal UT/SCA/shared build/both BVTs/both connectors passed. Current head 50e7f92bd08e6ba0491d8c63368f8720f874d9f5 completed ALL CI39157705591 SUCCESS, including shared build, normal UT, coverage UT, arm64 SCA, both complete BVTs, final coverage aggregation and CI Required. Iceberg39157708587 and Connector/J39157708624 also completed SUCCESS. Verified 2026-10-18 20:09 UTC: conflict-free, unmerged, with no newer changes-requested review; reviewer acceptance remains separate.

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[XuPeng-SH]

mo-bot review · mo-bot-local · done

Commit: 15ef9e056fc44a076fd548a93403d40ecfff58b8

Review completed. Read the assessment.

[XuPeng-SH]

mo-bot review · mo-bot-local · review

Commit: 15ef9e056fc44a076fd548a93403d40ecfff58b8

Assessment: request_changes

Reviewed the complete 93-file delta, callers, tests, documentation and supplied evidence. Reusing SQL transactions for atomic refresh is a reasonable design, but integration with account suspension and deletion is incomplete.

Model: gpt-6-astra / xhigh

Session: 01a12d8f-7dea-77c2-a565-38d45b5564e4 · slot 0 · generation 0

• P1 pkg/frontend/materialized_view.go:282: Create a periodic MV in a tenant, then suspend or restrict that account before its next scheduled refresh.
  Impact: Scheduled refreshes can continue reading sources and publishing writes after administrative suspension/restriction. Check the account's live status before admitting background execution, and test suspension followed by reopening.
  Evidence: validateMaterializedViewDefiner checks only mo_user, role membership and user unlock status. pkg/mv/task.go:85-90 creates an internal SQL session directly; internal_executor.go:145-239 bypasses AuthenticateUser, which owns the account-suspension check. The restricted-account check in mysql_cmd_executor.go:4933 also requires a Routine, absent from this internal session. Account suspension kills connections and pauses CDC but does not disable these MV tasks. This violates the RFC's active-account requirement.

• P2 pkg/sql/compile/materialized_view.go:195: Create an MV in a non-system account, then execute DROP ACCOUNT for that tenant without first dropping the MV.
  Impact: DROP ACCOUNT fails with 'database contains materialized views; drop them first' instead of removing the tenant. Integrate MV retirement and metadata cleanup into the existing account-drop transaction while preserving ordinary DROP DATABASE restrictions.
  Evidence: doDropAccount in pkg/frontend/authenticate.go executes tenant DROP DATABASE statements at lines 4314-4320, before deleting account-qualified cluster-table rows at lines 4440-4455. The new DropDatabase hook therefore encounters the still-present MV and returns this error, aborting account deletion. tenantIsolationCases explicitly drops its MVs before dropping accounts, so its passing result does not cover this trigger.

Validation:

• The frozen-head evidence snapshot reports successful build, Ubuntu UT, arm64 SCA, coverage, PROXY BVT, pessimistic BVT and CI Required checks. local_validation contains no retained worker gates.

• Fresh parser generation matched the committed parser after command-header/path normalization, with zero conflicts. Receipt: checks/1791962842406441721-2.json; retained output: y.output.

• Final source audit passed: task protobuf descriptor adds only enum 17; BVT shell syntax, plugin-dispatch checks, report parsing, port-source fingerprints and clean source status verified. Receipt: checks/1791963069278410481-4.json; logs: source-audit.stdout and source-audit.stderr under the assigned attempt directory.

• Both findings are supported by production call-chain analysis. Existing tests cover refresh atomicity, source privileges and tenant isolation but omit suspended-account refresh and account deletion with surviving MVs.

• Recomputed historical 50-pair performance results: 7.43× grouped aggregation, 8.89× rollup, 5.36× join and 1.04× high-cardinality control. These are historical explicit-MV measurements, not current-head regression measurements.

Limitations:

• No new native SQL reproduction was executed; the findings are source-proven rather than runtime-reproduced.

• Author runtime directories were unavailable. Historical source fingerprints matched only partially, so those reports were not relabeled as complete current-head execution evidence.

• Ordinary DDL adds dependency queries, optimistic DDL adds account fencing, and DML writers add budget checks. Current-head overhead was not measured. Mixed-version, crash-at-commit and large-scale scheduler validation remain documented release gates.

[LeftHandCold]

Reviewed head 15ef9e056fc44a076fd548a93403d40ecfff58b8, including the versioned P1 design, parser/binder, catalog and upgrade ownership, frontend authorization, compiler/writer guards, SI refresh publication, task replay/recovery, and restore/account lifecycle consumers.

The selected design is reasonable for experimental P1: stable relation identity, replacement plus success metadata in one native SI transaction, and reuse of taskservice rather than a second scheduler. I am not blocking on unimplemented rewrite/incremental/history phases or on speculative performance claims.

Three reachable integration defects need changes:

1. P1 — Ordinary DDL depends on MV catalogs before the 4.0.15 migration. Every production CN registers the MV service unconditionally; the new DDL guards use that registration as readiness and query the new tables even with the feature disabled and no MVs. A new binary serving an existing catalog with auto-upgrade=false can therefore break ordinary ALTER/DROP/TRUNCATE/DROP DATABASE with NoSuchTable.
2. P1 — Scheduled refresh bypasses tenant suspension/restriction. The definer check validates the user and role, but not the account's live status. These internally constructed SQL sessions do not pass through login account checks and have no Routine for restricted-account admission.
3. P2 — DROP ACCOUNT cannot retire a tenant containing an MV. Its internal DROP DATABASE hits the new restriction before account-qualified MV metadata is removed.

I independently confirm the suspension/deletion findings in the existing review; the inline comments below identify the concrete owners and counterexamples, and add the pre-migration ordinary-DDL regression.

Evidence: latest-head CI run 38510716395 is completed/success. Read-only call-chain analysis establishes the triggers above. A lightweight, dependency-free Go harness extracts the two actual DDL guard bodies (only qualified type names adapted) and confirms six boundary cases, including empty installed-catalog controls, surviving-MV account deletion, and missing pre-migration catalogs in pessimistic/optimistic DDL. That is source-extraction/fixture evidence, not a fresh native SQL reproduction or kernel UT. Shell syntax, checked-in JSON parsing, diff whitespace, head freshness and clean review-worktree checks also pass. No heavyweight local build, full UT run, or live environment mutation was performed. Existing passing tenant tests explicitly drop MVs before DROP ACCOUNT and do not close these cases.

[LeftHandCold]

[P1] Keep ordinary DDL working before the MV catalog migration

initInternalSQlExecutor unconditionally installs mv.RuntimeKey on every CN, so the check at lines 116–118 does not establish that the four 4.0.15 catalog tables exist. On an existing 4.0.14 catalog, starting this binary with the supported auto-upgrade=false path leaves those tables absent: Bootstrap recognizes the existing cluster, and BootstrapUpgrade is only started when AutomaticUpgrade is enabled. SQL admission can still accept the existing catalog/protocol; the startup catalog fence checks ordinary-view metadata, not these MV tables. Even with experimental_materialized_view=0 and no MVs, ALTER/DROP/TRUNCATE/CREATE INDEX/DROP INDEX now execute this query and fail with NoSuchTable; DROP DATABASE has the same problem in its parallel guard, and optimistic DDL fails earlier in DependencyGuard.

Please use authoritative MV catalog/upgrade readiness before adding this dependency to ordinary DDL, while continuing to enforce guards once MVs can exist. A session feature flag alone is not a safe substitute. Add a pre-migration existing-cluster control (ordinary DDL still succeeds) and a post-migration dependent-source rejection. The extracted production guard bodies reproduce the error propagation for both ordinary-table and database DDL; the production startup/upgrade call chain establishes the missing-catalog trigger.

[dengn]

Addressed in e55a8a6 and retained at current head c0bab97. Both ordinary DDL guards call mv.CatalogReady in the owning transaction before DependencyGuard or any MV-only catalog query. Zero installed catalogs is a no-op; partial migration and query errors fail closed; installed catalogs still enforce dependencies. TestMaterializedViewDDLReadiness covers table/database guards in both transaction modes. The real 4.0.14 kernel before/after fixture records the earlier normal-login version fence and its separate test-only authentication bypass explicitly, so it is not presented as normal-login mixed-version proof. Current-head coverage UT and both BVT groups pass. Source/evidence: https://github.com/matrixorigin/matrixone/blob/c0bab97f8cae9ecd0bb4611b9490e1eb3d94b1b2/docs/design/materialized_view_ci_triage.md . SCA/ordinary UT are being rerun after checkout TLS failures; review resolution remains with the reviewer.

[LeftHandCold]

[P1] Check the account's live status before admitting scheduled refresh

Create a periodic MV in a tenant, then suspend or restrict that account before the next run. This query checks only mo_user, its unlock status, and current role membership; none changes when the account itself is suspended/restricted. TaskExecutor constructs a new InternalExecutor session directly with the stored IDs, so it does not run Session.AuthenticateUser (which checks mo_account.status), and the restricted-account check in authenticateUserCanExecuteStatement requires a Routine that this session does not have. The task can consequently keep scanning sources and publishing writes after administrative suspension or storage restriction.

Please resolve the live account status through the existing account lifecycle owner and reject scheduled refresh for inactive/restricted accounts before source execution, preserving last-good rows and allowing recovery after reopening. Add suspension and restriction controls with source changes: no publication while blocked, then successful refresh after reopening. This independently confirms the same issue in the existing review; current source-ACL/user-role tests do not exercise account status.

[dengn]

Addressed in e55a8a6 and retained at c0bab97: scheduled definer validation first reads system-owned mo_account.status and admits only OPEN before tenant user/role/source execution. InternalExecutor also synchronizes the session account ID with the final tenant override. The real scheduledAccountLifecycleCases modifies source data, suspends/restricts the tenant, verifies no new publication and unchanged last-good rows/provenance, then reopens and verifies successful refresh. These use normal authenticated CN sessions. Current-head coverage UT and both BVT groups pass; final ordinary UT/SCA rerun is still pending. Evidence and scope: https://github.com/matrixorigin/matrixone/blob/c0bab97f8cae9ecd0bb4611b9490e1eb3d94b1b2/docs/design/materialized_view_review_lifecycle_results.json .

[LeftHandCold]

[P2] Retire MVs inside the existing DROP ACCOUNT transaction

A tenant with any surviving MV can no longer be deleted by DROP ACCOUNT. doDropAccount first enumerates the tenant databases and executes drop database if exists ... (authenticate.go around 4314), and only later deletes account-qualified cluster-table rows (around 4444). The new DropDatabase hook therefore sees the still-present mo_materialized_views row here and returns database contains materialized views; drop them first, rolling back account deletion. There is no account-lifecycle exception or prior MV retirement on this path.

Integrate MV/task fencing and dependency/state/history cleanup into the existing account-drop transaction before dropping the owned databases; preserve the ordinary user-facing DROP DATABASE restriction. Add a test that drops an account with populated and periodic MVs still present and verifies object/metadata cleanup and no later publication. tenantIsolationCases currently drops each MV explicitly before its deferred DROP ACCOUNT, so its passing result masks this trigger. This independently confirms the existing review's account-deletion finding.

[dengn]

Addressed in e55a8a6, with subsequent empty-account and cross-mode native DROP corrections in 1ec43a9/c0bab97f. doDropAccount calls retireAccountMaterializedViews before tenant database removal, within its existing transaction: fence/disable definitions, bounded native drops with exact account/table/txn capabilities, then task/dependency/history/state cleanup. Ordinary DROP DATABASE remains RESTRICT. Native tenant cases leave populated/periodic MVs in place; injected failure after an earlier native drop verifies rollback of objects and metadata before a successful account deletion. RFC revision 3 and F13/C09 now select this same policy. Current-head coverage UT and both BVT groups pass; SCA/ordinary UT rerun after checkout TLS errors is pending. Evidence: https://github.com/matrixorigin/matrixone/blob/c0bab97f8cae9ecd0bb4611b9490e1eb3d94b1b2/docs/design/materialized_view_ci_triage.md . No reviewer Request changes has been dismissed.

[aptend]

Distinct DESIGN-FIRST review phase for exact head 15ef9e056fc44a076fd548a93403d40ecfff58b8, base/merge-base fcad62a8ff06000e682824802f1155cbeea02a2f: REQUEST_CHANGES. This is a design-contract finding, not a demand for an unavailable approval label or a claim that another reviewer’s approval is required.

Trigger: the complete 93-file feature adds persistent MV catalogs/relkind, protocol/task consumers, atomic refresh, authorization boundaries and background scheduling across more than five production owners. It therefore requires a distinct design-first decision before implementation approval.

Reviewed the complete 859-line RFC revision 2, 269-line staged validation plan, 288-line self-review and reference manifest, the full issue #30079, both historical reviews, all three unresolved inline threads and conversation comments. The frozen RFC blob is c0ef212f0895ecae00a2a823faf0d29f7ab35b15 (SHA256 c9e8309fb027cf3b95d50b2955965d56f8085d21269b3a4bcf0d9f67e77f1ec8); validation-plan SHA256 cec14528eac33b4abf44fc0c861caea3ab1f7cba6ccf9bb50ac64925a35eabb7.

The stable relation, explicit full-refresh SI transaction, atomic rows/provenance/success publication, sequence/epoch fencing, current-CN write capability, bounded task-service integration and conservative invoker-read model are reasonable P1 choices. Rewrite/incremental/history extensions and the historical performance measurements are not being promoted into P1 or current-head evidence.

One blocking lifecycle decision remains contradictory: the main P1 contract requires transactional MV/dependency retirement for DROP DATABASE/ACCOUNT, but the P1 notes select DROP DATABASE RESTRICT, while the acceptance matrix still promises complete database/account cleanup. A restricted ordinary database drop can be a reasonable conservative choice, but it needs an explicit, coherent account-lifecycle escape/retirement contract rather than leaving administrative tenant deletion subject to that restriction. The inline finding gives the two competing oracles and requested closure.

Limited implementation inspection was used only to establish scope and the concrete risk: the current database guard rejects a surviving MV, and doDropAccount’s tenant database loop invokes DROP DATABASE before account-qualified metadata retirement. This is consistent with the account-drop risk already reported by others; it is not a fresh native SQL reproduction. I did not run the complete implementation/native/compatibility/performance matrix or claim a full 93-file code approval while this upstream design phase remains blocked.

Please record one accepted P1 database/account retirement policy, align the normative RFC and acceptance tests, and define transaction/lock/refresh-task fencing and rollback owners. Then the separate implementation review can validate the full change map against that decision, including the existing pre-migration and suspended-account findings. No document or production source was modified in this external review. Head/state freshness and diff checks were verified before submission.

[aptend]

[P2] Resolve the contradictory P1 database/account retirement contract

Here P1 DROP DATABASE/ACCOUNT transactionally removes the dependency closure and owned MVs. The P1 notes at line 779 instead specify “DROP DATABASE restricts while MVs exist,” while F13/C09 still require database/account cleanup and no later task publication. These are different acceptance oracles, not editorial wording.

Concrete fixture: create a tenant with a native source plus a populated/periodic MV, then issue DROP DATABASE and administrative DROP ACCOUNT without explicitly dropping the MV first. This paragraph expects successful transactional retirement; the implemented database guard at compile/materialized_view.go:195 rejects a surviving MV. doDropAccount invokes tenant DROP DATABASE before its account-qualified metadata cleanup, so simply choosing database RESTRICT also leaves the account owner without a defined retirement path.

Please state the selected P1 policy explicitly. RESTRICT for ordinary DROP DATABASE is acceptable if deliberately chosen and its oracles are aligned, but account deletion must define prior MV retirement, definition/task fencing, catalog/grant cleanup, lock ordering and rollback in the existing account transaction. Alternatively implement/document the promised cascading database closure. Record the decision and align the current normative text and tests before design PASS; an author-written “approved” status alone cannot resolve this lifecycle mismatch.

[dengn]

Revision 3 in e55a8a6baa9eac0e45e60f38b10ea5aad1660803 explicitly selects ordinary DROP DATABASE RESTRICT and transactional administrative DROP ACCOUNT retirement. The normative lifecycle text, F13/C09 oracles and implementation notes now agree; ownership, mode-specific admission/ordering, bounded native drops, task/definition fencing and whole-transaction rollback are documented and tested. This is the author's concrete policy correction, not self-declared external approval; design re-review remains pending. Versioned RFC. Validation/evidence boundaries: #30085 (comment)

[iamlinjunhong]

Review of exact head 15ef9e056fc44a076fd548a93403d40ecfff58b8: REQUEST_CHANGES, with two P1 findings and one P2 finding.

Inspected the 93-file diff, parser/binder and generated artifacts, catalog migration, authorization and internal-executor paths, DDL/writer guards, atomic refresh and task recovery, restore consumers, and the added unit/native/BVT tests. The atomic replacement approach is reasonable, but the following reachable integration defects remain.

1. [P1] Preserve ordinary DDL before the MV catalog migration. pkg/sql/compile/materialized_view.go:141

   Start this binary against an existing 4.0.14 cluster with auto-upgrade=false, with the experimental feature disabled and no MVs. initInternalSQlExecutor unconditionally installs mv.RuntimeKey (server.go:1319), so the guard's runtime-service check does not establish catalog readiness. Bootstrap returns for an initialized cluster (bootstrap/service.go:193–195); BootstrapUpgrade is launched only when AutomaticUpgrade is enabled (cnservice/server.go:1445–1461). The existing startup catalog fence operates on ordinary-view metadata, not the four MV tables. Ordinary ALTER/DROP/TRUNCATE/CREATE INDEX/DROP INDEX therefore reach this query and propagate NoSuchTable; optimistic DDL fails even earlier in DependencyGuard. DROP DATABASE has the same missing-catalog dependency at lines 182–190.

   Gate these guards on authoritative MV catalog/migration readiness while retaining dependency enforcement once MVs can exist. Add an existing-cluster, migration-disabled ordinary-DDL control and a migrated dependent-source rejection. The added guard test mocks an installed catalog and does not cover missing tables.

2. [P1] Reject scheduled refresh for suspended or restricted accounts. pkg/frontend/materialized_view.go:282

   Create a periodic MV in a tenant, then suspend or restrict the account before its next run. This validation checks user unlock status and role membership, but never reads the live account status. mv/task.go:85–90 constructs the stored identity and executes through InternalExecutor; internal_executor.go:198–240 creates the session without Session.AuthenticateUser, where account suspension is checked (session.go:3288–3315). Restricted-account statement admission also requires a Routine (mysql_cmd_executor.go:4933), absent from this session. Account status changes kill/restrict connections and pause CDC (authenticate.go:3599–3651), without disabling these MV tasks. A fresh scheduled task can consequently read sources and publish writes after administrative suspension or storage restriction.

   Check the account's live status through the account lifecycle owner before admitting background refresh. Test suspend/restrict with changed source rows, preservation of last-good rows while blocked, and successful refresh after reopening. User/source-ACL checks do not cover this boundary.

3. [P2] Retire MVs within DROP ACCOUNT before dropping tenant databases. pkg/sql/compile/materialized_view.go:195

   Create any MV in a non-system account and execute administrative DROP ACCOUNT without first dropping that MV. doDropAccount executes tenant DROP DATABASE statements at authenticate.go:4312–4317, before removing account-qualified cluster-table rows at 4443–4451. The new database guard sees the surviving MV and returns database contains materialized views; drop them first, aborting account deletion. Ordinary DROP DATABASE RESTRICT can remain, but account deletion needs transactional MV/task fencing and catalog retirement before database removal. Add a DROP ACCOUNT test with surviving populated and periodic MVs, including cleanup and no later publication. tenantIsolationCases explicitly drops the MV at additional.go:377 before its deferred account deletion, masking this trigger.

Validation and evidence boundaries: the supplied snapshot contains 30 completed checks (13 success, 15 skipped, 2 neutral), with CI Required successful. All 93 inspected head blobs match the complete diff's blob identifiers; changed JSON files parse and both changed BVT shell scripts pass bash -n. Findings are established by current-head production call chains, not a new native SQL reproduction. This workspace contains only the Loom snapshot and no Git repository/object store, so no clone, checkout, worktree, build or kernel/native test was performed. Exact-head blobs and public main/head metadata were read without credentials; main currently resolves to a33dcfb2e54196e17abe21de4346200a18009d83, while the reviewed PR diff is against base fcad62a8ff06000e682824802f1155cbeea02a2f.

Historical reviews, inline discussion and conversation were inspected. Other reviewers reported overlapping issues; they were independently confirmed above. The captured author comment addresses parser/port/CI integration and does not reply to these findings. The two current P1 defects require REQUEST_CHANGES regardless of the nonblocking P2 or CI success.

[iamlinjunhong]

[P1] Keep ordinary DDL working before the MV migration

On an existing 4.0.14 cluster started with auto-upgrade=false, the new MV catalogs are absent, but every CN still installs mv.RuntimeKey. With experimental_materialized_view=0 and no MVs, ordinary ALTER/DROP/TRUNCATE/CREATE INDEX/DROP INDEX reach this query and propagate NoSuchTable; optimistic DDL fails first in DependencyGuard, and DROP DATABASE has the same issue at lines 182–190. Bootstrap returns for an initialized cluster, and BootstrapUpgrade is conditional on AutomaticUpgrade; the startup ordinary-view fence does not create the MV catalogs. Use authoritative catalog/migration readiness before introducing this dependency to ordinary DDL. Add pre-migration ordinary-DDL and post-migration dependent-source controls.

[dengn]

Fixed in e55a8a6baa9eac0e45e60f38b10ea5aad1660803: both guards use transaction-snapshot four-table readiness before DependencyGuard/MV queries, retaining enforcement on installed catalogs. Zero/partial/full/error controls and both transaction modes are covered. Native isolated 4.0.14 kernel before/after evidence records the separate earlier login fence and test-only session boundary explicitly. Details and evidence: #30085 (comment)

[iamlinjunhong]

[P1] Check live account status before scheduled refresh

Create a periodic MV in a tenant, then suspend or restrict that account before its next run. This query only verifies the user and role, which remain valid after an account-status change. TaskExecutor creates an InternalExecutor session directly (task.go:85–90), bypassing Session.AuthenticateUser and its mo_account.status check. Restricted-account admission requires a Routine (mysql_cmd_executor.go:4933), absent from this session, and the account-status lifecycle only handles connections/CDC. Scheduled refresh can therefore continue scanning and publishing after administrative suspension or storage restriction. Reject inactive/restricted accounts through the lifecycle owner before execution; test blocked publication, last-good preservation and recovery after reopening.

[dengn]

Fixed in e55a8a6baa9eac0e45e60f38b10ea5aad1660803: the internal-session owner now synchronizes account identity, and scheduled definer admission checks the live account is OPEN before tenant user/role/source execution. Real suspended/restricted cases preserve rows/provenance and pass again after reopening; both normally authenticated CNs run the new code. Details: #30085 (comment)

[iamlinjunhong]

[P2] Retire MVs before DROP ACCOUNT drops tenant databases

DROP ACCOUNT for a tenant with any surviving MV now fails here with database contains materialized views; drop them first. doDropAccount executes tenant DROP DATABASE at authenticate.go:4312–4317, before account-qualified cluster-table cleanup at 4443–4451, so this guard sees the MV metadata and aborts deletion. Integrate MV/task fencing and metadata retirement into the existing account-drop transaction before database removal, retaining ordinary DROP DATABASE RESTRICT. Add a populated/periodic-MV account-deletion control and verify cleanup plus no later publication. The current tenant-isolation runner explicitly drops its MV before deleting the account, so its passing result does not cover this case.

[dengn]

Fixed in e55a8a6baa9eac0e45e60f38b10ea5aad1660803: account retirement fences/disables definitions and drops each native MV with its exact account/table/txn capability inside the existing account-drop transaction, before tenant database cleanup. Surviving populated/periodic MVs, metadata/object cleanup, rollback after a first native drop and obsolete task admission are covered. Ordinary DROP DATABASE stays RESTRICT. Details: #30085 (comment)

[XuPeng-SH]

mo-bot review · mo-bot-local · done

Commit: d50c4c77513c08837cf6ba351fae047ff9728bd2

Review completed. Read the assessment.

[XuPeng-SH]

mo-bot review · mo-bot-local · review

Commit: d50c4c77513c08837cf6ba351fae047ff9728bd2

Assessment: request_changes

Revision 3 resolves the earlier database/account lifecycle contradiction. Reusing the transaction and task-service owners is a reasonable design, but required compatibility and crash-recovery validation remains open. These are validation requirements, not claims of observed production failures.

Model: gpt-6-astra / xhigh

Session: 01a13dcf-1586-7a01-9000-d45e20995cf8 · slot 1 · generation 1

• P2 pkg/bootstrap/versions/v4_0_15/upgrade.go:35: Upgrade an existing 4.0.14 deployment, interrupt migration, run mixed-version CNs, or attempt downgrade with populated/periodic MVs.
  Impact: Validation requirement: the new persistent relation kind, four catalogs and task executor lack the required integrated proof of migration recovery, old-node exclusion and unsafe-downgrade rejection. The default-off creation flag does not isolate the catalog upgrade.
  Evidence: RFC lines 656–669 and 730 and validation case C12 require these checks. TestMaterializedViewUpgrade uses NewMemTxnExecutor; protocol tests substitute runtime version values. Both current-head compatibility CI jobs were SKIPPED. The author explicitly excludes mixed-version/downgrade validation, and the pre-migration fixture bypasses normal login. Before approval, retain a native compatibility run covering interrupted/repeated migration, old-CN SQL/task exclusion and downgrade refusal with existing MVs.

• P2 pkg/mv/service.go:354: A refresh loses its commit response or its CN exits between durable publication and task acknowledgement, followed by restart and task replay.
  Impact: Validation requirement: last-good retention, durable success recovery and replay behavior across the actual transaction/task boundary remain unverified. These are central acceptance contracts for atomic refresh.
  Evidence: RFC line 724 and validation cases C07/C10 require restart/fault integration. The lost-response unit test directly assigns state.LastRun in an in-memory executor; runRestartCheck only checks previously committed objects after a graceful restart. The CI harness runs functional, failure and multi-CN modes without a crash-at-commit scenario, which the author explicitly lists as unexecuted. Add deterministic native fault runs around commit/acknowledgement and restart, asserting rows, source timestamp, run status and replay outcome together.

Validation:

• Reviewed RFC revision 3, acceptance requirements, all supplied review concerns and replies, and the relevant refresh, scheduling, authorization, lifecycle and upgrade owners.

• Frozen evidence reports current-head build, UT, coverage, SCA and multi-CN BVT success; compatibility jobs were skipped. General CI success does not cover the two required fault/topology scenarios.

• Static evidence audit passed with the assigned check-run: evidence-audit.log and checks/1792124733224928443-2.json under /home/xupeng/.local/state/mo-bot/jobs/9c2d2ffacb4e81cdd675792f/attempt-1. Verified command, exit 0, exact head and clean-source fingerprint.

• Performance evidence remains historical prototype evidence: warm explicit-query gains were reported, while full refresh retains scan/delete/write and MVCC costs. No current-head performance measurement was claimed.

Limitations:

• The upstream validation gate blocks acceptance; exhaustive implementation, generated-artifact and test review across the complete 100-file delta remains unfinished.

• local_validation is empty. Referenced /workspace/mv-ci logs/input ledgers and /workspace/mv-build artifacts are unavailable here; author execution summaries were not treated as independently reconciled local runs.

• No product tests were executed locally. Missing artifacts or environment availability are not themselves the findings.

[XuPeng-SH]

mo-bot review · mo-bot-local · done

Commit: 4a5fb3fd89df9037db5bc76d0b06b27f93a63ada

Review completed. Read the assessment.

[XuPeng-SH]

mo-bot review · mo-bot-local · review

Commit: 4a5fb3fd89df9037db5bc76d0b06b27f93a63ada

Assessment: request_changes

The 109-file delta has a coherent core design: stable relation identity, atomic publication through native SI transactions, and reuse of the existing task service. Earlier account-lifecycle and upgrade/recovery concerns are addressed. Three defects remain: incorrect decimal results, failed LIMIT 0 queries, and non-convergent history retention.

Model: gpt-6-astra / xhigh

Session: 01a13dcf-1586-7a01-9000-d45e20995cf8 · slot 1 · generation 1

• P1 pkg/mv/service.go:307: Set div_precision_increment=10; create src(a DECIMAL(10,0)); insert 1; create an MV AS SELECT a/3 AS q FROM src; then refresh it.
  Impact: Refresh silently uses default division precision instead of the defining session's precision, persisting incorrect results. Persist and restore this setting, or consistently normalize definition binding and refresh execution.
  Evidence: Authenticated native probe TestReviewMVSemanticContext returned 0.3333333333 for the defining query but stored 0.3333000000 after refresh. MVDefinition omits this setting and this resolver falls back to DefaultResolveVariable. Default-precision controls matched at 0.3333. See semantic-context-test.log and checks/1792154097899289443-2.json (exit 1).

• P2 pkg/frontend/authenticate.go:7223: Run SELECT * FROM stored LIMIT 0 against a successfully refreshed materialized view.
  Impact: Valid metadata queries fail, breaking clients that discover result schemas using LIMIT 0. Route MVs through full-definition resolution while preserving source-privilege checks.
  Evidence: TestReviewMVPopulatedLimitZero reproduced Error 20101: materialized view definition is missing. The LIMIT 0 shortcut accepts relation kind 'm', but BuildTableDefByMoColumns returns no Props/Defs; this new authorization decoder therefore rejects it. Ordinary populated reads succeed. See populated-limit-test.log and checks/1792154417236731965-2.json (exit 1).

• P2 pkg/mv/task.go:244: Keep one manual MV refreshing every 10 seconds on a single-CN deployment.
  Impact: Refresh history grows without bound despite the latest-100/seven-day retention policy. Keep each cleanup invocation bounded, but revisit full pages promptly until the backlog drains instead of always postponing maintenance for an hour.
  Evidence: Each refresh inserts an attempt. Cleanup selects at most two 128-ID pages, then unconditionally advances next_cleanup by one hour. Thus 360 attempts/hour exceed the maximum 256 deletions/hour, leaving at least 104 additional rows/hour indefinitely. Manual refresh has no rate limit. Existing retention tests return only one overlapping ID and miss full-page backlog.

Validation:

• Artifacts are retained under /home/xupeng/.local/state/mo-bot/jobs/09788166d6abfca28870f340/attempt-1. validation-receipt-index.json reconciles commands, source fingerprints, exits, log hashes and probe inputs. Preflight passed; repository-owned native preparation passed after verifying 192 inputs and 62 artifacts.

• Three authenticated SQL probes executed through the supplied test-exec and failed assertions for the reproduced defects; setup succeeded. WHERE FALSE and default-precision controls passed within those probes. Source remained unchanged.

• Regenerated the MySQL parser: exact match apart from its invocation comment, zero conflicts. Decoded the protobuf descriptor: only MaterializedView=17 was added. Delta whitespace and changed shell-entrypoint syntax checks passed.

• Reconstructed all 11,061 author non-documentation inputs; their canonical SHA256 matches the committed recovery ledger. Reviewed the 13-group native upgrade/crash/replay evidence and repeated production BVT evidence, including process exits and cleanup observations.

• The supplied exact-head CI snapshot reports successful build, Ubuntu UT, arm64 SCA, coverage, proxy/pessimistic BVT and CI Required. Skipped compatibility jobs were not counted as passes.

• Historical paired measurements show 7.43x aggregate, 8.89x rollup, 5.36x join and 1.04x high-cardinality speedups; the million-row aggregate refresh took 151.27 ms. These are not current-head performance measurements.

Limitations:

• Author /workspace binaries and raw logs were unavailable locally. Recovery evidence was assessed from committed matching fingerprints, SQL/process ledgers and diagnostic excerpts; destructive recovery scenarios were not repeated.

• No new throughput, cold-cache, tail-latency or ordinary-SQL benchmark was run. Review traced full-refresh rewrite costs, metadata reads, optimistic DDL fencing and added batch sizing; current-head performance at scale remains unmeasured.

• History growth is established by the implemented admission and cleanup capacity, not a week-long execution. Fixes need regressions for captured division precision, populated/unpopulated LIMIT 0, and retention with full pages.

[XuPeng-SH]

Updated at 79484a43c2a4c984b9cb9e0290805822d6d6a275, cumulatively rebased onto latest main 3fc00709eccc277c175c7a09eed2605ca1aaee29; original author and archived source history retained.

The three latest substantive review findings are fixed at their existing owners:

1. Captured refresh semantics: format2 persists explicitly present precision0..30, full SQL mode including empty, and named/fixed timezone identity. Refresh restores those settings through the existing executor resolver; the internal compiler forwards scoped SQL mode too. Real stored strings/wire scales at0/4/10/30, unsigned subtraction and winter/summer timezone controls pass with different refresh-session settings. Legacy format1 remains readable but requires recreation before refresh; the migration disables its periodic policy and advances its epoch instead of guessing default4.
2. LIMIT0: preserve main's full table-definition resolution, and keep MV-containing literal-zero subtrees on the existing scan validation path. Actual populated/unpopulated/WHERE FALSE, result metadata and direct/prepared source-grant revoke/regrant checks pass. No second catalog validator is added.
3. History backlog: count raw rows per page; either full page keeps cleanup due now, both short pages schedule an hour later. Existing bounds, terminal-only deletion, overlap deduplication and task owner remain. Boundary, overlap, multi-batch, error/cancellation and subsequent-drain controls pass while asserting actual task publication.

Main's v15/v16 migration directories and charset118 remain intact; MV uses v17/offset5/protocol119. Duplicate EXPLAIN opaque-options output is removed. The final112-path gpt-6.1-sol/xhigh review reports zero source blockers, and its final parser-reproduction receipt condition is satisfied at the exact reviewed fingerprint.

Current native upgrade/crash/replay completes13/13 groups, exit0, against the exact main4.0.16/protocol118 binary. Normal owner/consumer checks, one scoped race pass, corrected fixture checks and final21-package configured SCA all pass (zero lint issues). Initial fixture/harness failures remain recorded as failures; no historical evidence is relabeled. The updated template body and design/native recovery documents state compatibility, reproduction and remaining CI/release/fleet/performance limits. No self-approval or review dismissal.

[XuPeng-SH]

mo-bot review · mo-bot-xupeng · done

Commit: 79484a43c2a4c984b9cb9e0290805822d6d6a275

Review completed. Read the assessment.

[XuPeng-SH]

mo-bot review · mo-bot-xupeng · review

Commit: 79484a43c2a4c984b9cb9e0290805822d6d6a275

Assessment: request_changes

Reviewed the 112-file delta, P1 design, callers, tests and latest review concerns. Reusing native transactions and taskservice is appropriate, but public-SQL reproductions confirm three integration defects affecting read-only enforcement, source DELETE and DDL export.

Model: gpt-6-astra / xhigh

Session: 01a14d34-4e51-79a3-a62d-6c9c16279dc8 · slot 0 · generation 0

• P1 pkg/frontend/table_dump.go:529: Dump a populated MV, delete its source rows, then drop/recreate the same MV and refresh it to an empty result. LOAD TABLE from the earlier dump succeeds.
  Impact: Native loading changes the read-only MV without updating refresh provenance. Readers receive rows inconsistent with the recorded successful source snapshot. Reject MV targets at this restore boundary.
  Evidence: The kind switch omits SystemMaterializedViewRel; handleLoadTable reaches rel.Write at line 1480 without compiler write admission. consumer-probe3.log:1714 records LOAD error=, count changing from 0 to 1, and unchanged last_run, successful_sequence and full source timestamp. Reproducer: review_consumers_v3_test.go; receipt: checks/1792316951014041843-3.json.

• P2 pkg/sql/plan/build_dml_util.go:1093: With default delete_opt_to_truncate=1, create an MV over facts and execute DELETE FROM facts without a WHERE clause.
  Impact: Creating an MV breaks valid source DML: DELETE becomes physical TRUNCATE and the dependency guard rejects it. Disable this optimization for dependent sources while retaining explicit TRUNCATE protection; keep the index-delete decision at line 7098 consistent.
  Evidence: compile.go:2204 routes CanTruncate to TruncateTable, whose new guard at ddl.go:4144 rejects dependent sources. consumer-probe2.log:1606 records 'source has dependent materialized views' and remaining=1; a filtered DELETE subsequently succeeds. Receipt: checks/1792316858630810574-3.json.

• P2 pkg/sql/plan/build_show.go:193: Execute SHOW CREATE TABLE for an MV and replay the returned DDL under another target name.
  Impact: The standard SHOW/export path emits CREATE TABLE with an internal property that CREATE TABLE rejects, rather than reconstructing the MV and refresh policy. Route this consumer through the authoritative MV formatter.
  Evidence: Only ordinary views are redirected at line 162. MVs reach ConstructCreateTableSQL and emit PROPERTIES('mo_materialized_view'=...). consumer-probe2.log:1219 confirms replay fails with 'materialized view properties are reserved', violating the RFC's export contract. Receipt: checks/1792316858630810574-3.json.

Validation:

• Artifacts are retained under /home/xupeng/.local/state/mo-bot/jobs/a142f8116fc01d6a669f2f0e/attempt-1. evidence-audit.json correlates commands, source fingerprints, exits, logs and probe inputs.

• Isolation preflight passed. The initial native-owner check rejected missing/incompatible shared artifacts before test execution. A diagnostic-directory make cgo build then passed; the final audit verified 192 native source inputs and 46 artifact hashes.

• Go 1.27.1 probes used the existing authenticated embedded-cluster fixture, assigned caches, read-only overlays and supplied test-exec. Probe 2 confirmed DELETE and DDL-replay failures; probe 3 confirmed LOAD publication bypass. Both exited 1 from contract assertions. Earlier incomplete probe outcomes remain separately recorded.

• Fresh parser generation matched after command-header/path normalization, with zero conflicts. Final audit verified the protobuf descriptor adds only TaskCode 17, checked shell/Python syntax and 11 JSON reports, and confirmed clean source status. Passing receipt: checks/1792317030724811928-3.json; log: evidence-audit-v2.log.

• The frozen exact-head CI snapshot reports successful build, Ubuntu UT, arm64 SCA, coverage, proxy BVT, pessimistic multi-CN BVT and CI Required. Source/test review checked the latest precision, LIMIT 0, retention, pre-migration DDL and account-lifecycle corrections.

Limitations:

• Local reproductions used one Linux/amd64 embedded CN. No new race, multi-CN, crash/upgrade or performance runs were performed; the confirmed defects justify this verdict before exhaustive runtime acceptance.

• local_validation was empty. Referenced current author crash/upgrade artifacts were unavailable, while checked-in recovery reports describe older inputs; these were not treated as independently verified current-head results. Ordinary-SQL overhead was not measured locally.

[XuPeng-SH]

mo-bot review · mo-bot-xupeng · done

Commit: 50e7f92bd08e6ba0491d8c63368f8720f874d9f5

Review completed. Read the assessment.

[XuPeng-SH]

mo-bot review · mo-bot-xupeng · review

Commit: 50e7f92bd08e6ba0491d8c63368f8720f874d9f5

Assessment: request_changes

Reviewed the complete delta, ownership, lifecycle, compatibility, tests and prior review concerns. One native read path demonstrably bypasses the unpopulated materialized-view contract.

Model: gpt-6-astra / xhigh

Session: 01a14d34-4e51-79a3-a62d-6c9c16279dc8 · slot 0 · generation 0

• P2 pkg/sql/compile/materialized_view.go:93: Create an unrefreshed manual MV named unpopulated and an empty ordinary table comparison with matching columns, then execute DATA BRANCH DIFF unpopulated AGAINST comparison.
  Impact: DIFF succeeds with an empty result, treating an MV that has never been populated as valid empty data. The population check only protects compiled scans; native branch readers need equivalent admission or explicit MV rejection.
  Evidence: The native TestReviewMVNativeReaders/unpopulated probe reproduced SELECT failing with MV_NOT_POPULATED while DIFF returned result=[] error=. requireBranchReadOnTableName builds a privilege-check plan without executing the compile guard; getTableStuff opens engine relations directly. This violates docs/rfcs/00000000_materialized_views.md:189. Retained evidence: review_mv_branch_v2_test.go, review-mv-branch-v2.log:1736 and checks/1792355181611809628-3.json under the assigned attempt directory; receipt records the exact head, clean source and exit 1.

Validation:

• Artifacts are retained under /home/xupeng/.local/state/mo-bot/jobs/fd5a5468cd42a3b5855a9db7/attempt-1. final-evidence-audit.json reconciles eight command/source/exit receipts with log hashes; its own check 1792355270799938937-3 exited 0. Source remains unchanged.

• Parser regeneration matched the checked-in parser with zero conflicts. Source audit verified the 130-file inventory, whitespace, changed JSON/Python/shell syntax and protobuf descriptor delta. Receipts: 1792354621859879956-3 and 1792354711811867113-4, both exit 0.

• Isolation preflight passed. Before native execution, verified 190 native inputs and 46 retained artifact hashes against their successful repository build receipt; native-reuse.json records provenance.

• The native probe used the supplied test-exec and assigned caches. Source-privilege and historical-read controls passed; only unpopulated DIFF admission failed.

• Reconciled frozen exact-head CI outcomes and author evidence inputs. Build, Ubuntu unit tests, coverage, static analysis and multi-CN BVT checks report success; these do not cover the reproduced failure.

Limitations:

• Full suites, crash recovery, upgrade scenarios and performance benchmarks were not rerun locally. Raw author logs under /workspace/mv-ci were unavailable; checked-in reports were assessed with their stated provenance limits.

• An initial native setup failure was recovered using verified compatible libraries. The first probe stopped at the expected timezone restriction; neither is classified as a product failure.

• The confirmed finding concerns DIFF read admission. No privilege bypass, historical-read bypass or branch-merge data loss is claimed.