Skip to content

fix: clean role privileges when dropping objects - #27745

Draft
ck89119 wants to merge 22 commits into
matrixorigin:mainfrom
ck89119:issue-27723-main
Draft

fix: clean role privileges when dropping objects#27745
ck89119 wants to merge 22 commits into
matrixorigin:mainfrom
ck89119:issue-27723-main

Conversation

@ck89119

@ck89119 ck89119 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

Fixes #27723

What this PR does / why we need it:

  • Remove table/view/sequence role privileges by relation logical ID and database/child privileges during object deletion.
  • Serialize object-scoped GRANT and persistent object replacement/deletion through database-before-relation catalog locks.
  • Preserve the logical identity and exact-view grants across ALTER VIEW; include ALTER VIEW in lifecycle admission.
  • Lock the mo_tables catalog row for source-relation DROP while skipping only the unavailable source storage lock.
  • Resolve prepared EXECUTE before transaction admission and use the prepared statement's binding database when classifying unqualified temporary-table aliases.
  • Include CREATE OR REPLACE VIEW, ALTER VIEW, and DROP SEQUENCE in the persistent lifecycle protocol.
  • Create lifecycle owner transactions as pessimistic RC on optimistic deployments.
  • Require reused transactions to be both pessimistic and RC; optimistic and pessimistic/SI generations are rejected before statement admission.
  • Preserve prior transaction work when admission is rejected: transaction finalization is enabled only for explicit COMMIT/ROLLBACK or after successful admission.
  • Admit outer DATA BRANCH DELETE TABLE/DATABASE before any shared derived execution; force only their private owners to pessimistic RC.
  • Force direct private owners for DROP ACCOUNT, snapshot restore, and PITR restore; shared user transactions are never implicitly replaced.
  • Classify ordinary DROP TABLE targets against session temporary aliases before admission, so temp-only drops remain valid in existing optimistic transactions while mixed/persistent drops use the catalog protocol.
  • Keep clone's snapshot source transaction independent from its private owner's required pessimistic-RC execution mode; DATA BRANCH and IF NOT EXISTS explicitly select the background snapshot transaction.
  • Consume pending one-shot isolation settings on the forced transaction generation.

Validation:

  • Exact-head regression TestIssue26087ConcurrentDataBranchQuota passed.
  • Focused effective-statement, prepared binding-database, owner-propagation, Pessimistic/SI rejection, admission-boundary, source-lock, temporary-alias, sequence cleanup, ALTER VIEW identity, and isolation tests passed.
  • Real Pessimistic/SI sequence verified rejected persistent DROP preserves both the previous INSERT and target object.
  • Real Optimistic/SI prepared DROP TABLE t bound in d1 was rejected despite an execute-time d2 temporary alias; d1.t remained.
  • Real sequence grant/drop verified zero remaining mo_role_privs rows.
  • Expanded lifecycle BVT: 52/52 passed.
  • clone_subscription.sql: 106/106 passed after separating clone owner mode from snapshot-operator selection.
  • pkg/frontend and pkg/sql/compile package runs passed; go vet ./pkg/frontend ./pkg/sql/compile passed.
  • Exact CI linter version/config: golangci-lint 2.6.2 -c .golangci.yml ./pkg/frontend ./pkg/sql/compile — 0 issues.
  • Final PR changed-statement coverage: 88.0%.

Additional review closures

  • Roll back and invalidate only a transaction generation newly published by TxnHandler.Create when storage initialization or validity checking fails; admission rejection still preserves a pre-existing transaction.
  • Carry the PREPARE-time binding database through execute-time AST authorization and implicit ownership cleanup, restoring the execute-time session database afterward.
  • Preserve sequence logical ID and exact grants across ALTER SEQUENCE, skip replacement-only DROP cleanup, and admit ALTER through the pessimistic-RC lifecycle protocol.
  • Reject grants on hidden __mo_index_ relations before catalog lookup/mutation, preventing internal index tables from becoming user authorization objects.

Additional validation:

  • Storage initialization and validity failure UTs: published transaction cleared, rollback exactly once.
  • Non-admin dual-database prepared DROP integration: d1 targets removed, both d2 tables and d2 ownership grant preserved.
  • ALTER SEQUENCE integration: logical ID preserved and exact grant count remains 1.
  • Dynamic hidden-index GRANT integration: rejected.
  • Expanded lifecycle BVT: 102/102 passed on Optimistic/SI.
  • Final changed-statement coverage: 80.9%.

Binary admission and legacy-index follow-up

  • Capture PrepareStmt.defaultDatabase in the binary COM_STMT_EXECUTE input before doComQuery, so transaction admission sees the correct PREPARE-time binding even though binary execution supplies the inner AST.
  • Reset prepared database and persistent DROP target state at every statement-generation boundary; a following direct statement in the same COM_QUERY cannot inherit prior PREPARE metadata.
  • Route hidden child deletion for both DROP INDEX and DROP TABLE through one database→relation lock/identity/cleanup helper, removing grants retained by old releases or rolling upgrades.

Validation:

  • Real go-sql-driver binary protocol on Optimistic/SI: binary DROP inside an existing unsafe transaction is rejected and d1.t remains.
  • Same-COM_QUERY prepared statement followed by a direct temporary DROP succeeds without stale binding.
  • Legacy hidden grants constructed for DROP INDEX and DROP TABLE: 2 before, 0 after.
  • Expanded lifecycle BVT: 124/124 passed.
  • Post-latest-main focused and full frontend/compile suites passed; vet/lint passed.
  • Changed-statement coverage before the latest unrelated main merges: 80.3%.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Something isn't working size/XXL Denotes a PR that changes 2000+ lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: DROP TABLE/DATABASE leaves orphan rows in mo_role_privs

2 participants