diff --git a/docs/design/data_branch_privilege.md b/docs/design/data_branch_privilege.md index f24fadc2d6faf..8ec6483c0b40f 100644 --- a/docs/design/data_branch_privilege.md +++ b/docs/design/data_branch_privilege.md @@ -11,6 +11,10 @@ with only one side of the required permission create or modify branch objects. This document defines a complete privilege model for all user-facing `DATA BRANCH` statements and a concrete implementation plan. +The database-identity extension in section 12 is a proposed amendment for +issue #26068. Its implementation remains gated on independent approval of the +persisted-state and rollout contract recorded there. + The central rule is: > A data branch statement must require the same privileges as the ordinary SQL @@ -392,11 +396,13 @@ Execution safety: - If the database contains user data tables, every contained user data table must have an active `mo_branch_metadata` child row. - Fail before DDL if any contained user data table is not a branch child. -- If validation finds no active branch child tables, fail before DDL. Current - branch metadata is table-level only and cannot distinguish an empty - branch-created database from an ordinary empty database, so `DATA BRANCH - DELETE DATABASE` must fail closed for empty, view-only, or sequence-only - databases until a database-level branch identity exists. +- Before database-identity protocol v60 is active, if validation finds no active + branch child tables, fail before DDL. Current table-level metadata cannot + distinguish an empty branch-created database from an ordinary empty database. +- At protocol v60 or later, `mo_database.dat_type = 'data-branch'` is the + database-level identity. A marked database may contain no ordinary tables, + but every ordinary table that is present must still have an active + `mo_branch_metadata` child row. - Views are not recorded in `mo_branch_metadata` today. Do not require branch metadata for views during database-delete validation; they are covered by the required `DROP DATABASE` privilege. @@ -774,9 +780,10 @@ Required unit tests: - database owner fallback -> same; - database with non-branch user table -> fail before drop. - failed validation leaves the database and all contained objects untouched. - - empty database with normal `DROP DATABASE` privilege -> fail before drop. - - database containing only views/sequences and no active branch child tables - -> fail before drop. + - pre-v60 or unmarked empty database with normal `DROP DATABASE` privilege -> + fail before drop. + - v60 marked database containing only views/sequences and no active branch + child tables -> pass. - database containing only branch tables plus restored views -> pass. ### 9.2 BVT regression cases @@ -849,11 +856,10 @@ Implement these decisions in this change: destination database; do not allow a destination snapshot or overwrite an existing table. 2. Empty database created by `DATA BRANCH CREATE DATABASE` - - Reject `DATA BRANCH DELETE DATABASE` if validation finds no active branch - child tables. Current metadata is table-level only and cannot distinguish - an empty branch-created database from an ordinary empty database, so the - secure behavior is to fail closed until database-level branch identity is - implemented. + - Before the section 12 protocol is approved and active, reject `DATA BRANCH + DELETE DATABASE` if validation finds no active branch child tables. At v60 + or later, accept an empty database only when its catalog row carries the + approved database-level branch identity. 3. View handling in `CREATE DATABASE` - Require `SELECT` on each source view returned by `showFullTables`. - Preserve existing view security behavior through synthetic SELECT planning @@ -862,10 +868,10 @@ Implement these decisions in this change: - Do not require `mo_branch_metadata` rows for restored views during `DATA BRANCH DELETE DATABASE`; current metadata is table-level only. 4. Database-level branch identity - - Keep this fix table-metadata based. Adding database-level branch metadata - is the clean long-term model for empty, view-only, or sequence-only branch - databases, but it requires a metadata schema/upgrade change and should be - done as a separate design and implementation step. + - The privilege fix remains table-metadata based below v60. The proposed + issue #26068 extension is specified separately in section 12 and must not + emit or consume its persistent semantics until the cluster-wide capability + gate is active. 5. Metadata validation batching - Validate branch child metadata in bounded batches using the shared `dataBranchMetadataIDBatchSize` constant. This keeps validation and update @@ -876,3 +882,164 @@ Implement these decisions in this change: table names, and `TO ACCOUNT` account names, must use the shared identifier quoting helper. Do not combine user/catalog identifiers with raw backticks or bare `%s` formatting. + +## 12. Proposed Database-level Branch Identity Amendment + +- Status: proposed; implementation activation blocked pending independent + design approval +- Tracking issue: [matrixorigin/matrixone#26068](https://github.com/matrixorigin/matrixone/issues/26068) +- Implementation PR: [matrixorigin/matrixone#28272](https://github.com/matrixorigin/matrixone/pull/28272) +- Design revision: `data-branch-database-identity-2026-09-10-r12` +- Required rollout capability: `MORPCVersion60` + +Revision r12 moves the proposed capability to v60 because v48 through v59 are +now owned by the generation-aware CDC watermark catalog contract, vector-level +grouping-set projection expansion, the two ordered ODKU protocol contracts, +MySQL binary JSON subtype tags, ordered-stream distributed Top-N merge, and +catalog-authenticated proxy prepared-cache reuse, followed by session-owned +temporary DDL with transactional data and session-scoped AUTO_INCREMENT +increment/offset and provenance, and the Arrow LOAD external-scan pipeline +payload, followed by binary-string function semantics and runtime-domain +metadata, and typed numeric `FORMAT` arguments in remote expressions. + +### 12.1 Problem and invariant + +`DATA BRANCH CREATE DATABASE` commits the destination database even when no +ordinary table receipt is produced, but the pre-v60 delete validator recognizes +database branches only through active table receipts. The result is a database +that the matching delete statement cannot recognize after an empty, +view-only, or sequence-only clone, or after every cloned ordinary table is +dropped. + +The new invariant is: + +> Every database successfully created by a v60-capable `DATA BRANCH CREATE +> DATABASE` has durable database-level identity until that database is dropped, +> independent of the number and lifetime of its ordinary tables. + +Its negation remains fail-closed: an unmarked empty database is not inferred to +be a branch, and a marked database containing any ordinary table without an +active child receipt is not deletable through the data-branch path. + +The database catalog row is the first owner of database identity. Table lineage, +parent snapshots, and their cleanup remain owned by `mo_branch_metadata` and +`mo_snapshots`; the database marker does not replace or synthesize table +receipts. + +### 12.2 Persistent representation and transaction boundary + +Use the existing categorical `mo_database.dat_type` column with the value +`data-branch`. This adds a value, not a column or table, and therefore requires +no catalog schema migration. The internal clone path attaches the value to the +same `CREATE DATABASE` engine call that creates the catalog row. Database +creation, restored objects, table receipts, and branch-protection snapshots +remain in the existing background-executor transaction, so any error rolls all +of them back together. + +The delete transaction reads and locks the target database through the existing +path, validates the marker and every current ordinary table receipt, then drops +the database and performs lineage cleanup. Missing, subscription, unknown-type, +unmarked-empty, and mixed branch/non-branch targets fail before DDL. + +`data-branch` remains a user database category for publication purposes only +when v60 semantics are active. Subscription and unknown non-empty types remain +non-user databases. + +### 12.3 Capability, upgrade, and mixed-version behavior + +`MORPCVersion60` is the deployment capability for every producer and consumer +of the marker. `MOProtocolVersion` is maintained at the oldest live service +version, so all three frontend decisions use the same gate: + +| Common protocol | CREATE DATABASE branch | DELETE DATABASE branch | CREATE/ALTER PUBLICATION | +| --- | --- | --- | --- | +| `< v60` | new CN rejects before DDL; it does not emit a marker | use legacy table-receipt identity; empty/all-dropped targets fail closed | treat `data-branch` as non-user, matching old CNs | +| `>= v60` | persist `dat_type = 'data-branch'` atomically | accept a marked zero-table target; still validate every present ordinary table | treat `data-branch` as a user database | + +Rejecting create below v60 is deliberate. Merely omitting the marker would let a +new CN create another database that later becomes undeletable, while writing it +would expose unknown persistent semantics to an old CN. Existing pre-v60 +table-backed branches remain usable and deletable through their receipts during +rollout. Existing pre-v60 empty/all-dropped branches remain indistinguishable +from ordinary databases and cannot be migrated automatically. + +No session-local flag or per-call probe may override the common protocol. The +gate must be checked before privilege resolution, target locks, or DDL so a +rejected create has no catalog, table, receipt, or snapshot side effects. + +### 12.4 Downgrade and rollback + +Deployment must lower the common protocol below v60 before removing v60-capable +CNs. New CNs then immediately stop creating branch databases and use legacy +delete/publication behavior, matching old CNs. Existing marker rows remain +valid opaque catalog data: + +- ordinary SQL access and normal `DROP DATABASE` remain unchanged; +- table-backed branches with active receipts remain deletable through `DATA + BRANCH DELETE DATABASE`; +- empty or all-dropped marked branches fail closed until v60 is restored; +- publication create/alter rejects a marked database until v60 is restored. + +Rollback does not erase or rewrite markers because doing so would permanently +destroy trustworthy identity. Re-enabling v60 restores the new behavior without +migration. A permanent downgrade must first remove marked branch databases +while v60 is active; silently converting them into ordinary databases is not an +allowed fallback. + +### 12.5 Backup, snapshot, PITR, and restore + +Physical backup/checkpoint/replay already persists `mo_database.dat_type`; the +new categorical value requires no new serialization. Restore into a deployment +whose common protocol is below v60 is unsupported for snapshots containing a +marked database and must fail before an existing target database is dropped. + +Logical snapshot, account/cluster restore, and PITR recreate database catalog +rows rather than replaying them directly. The restore reader must therefore +read `dat_type` with `dat_createsql` and attach `data-branch` to the internal +database create at v60. It must not infer identity from table receipts, names, +views, sequences, or SQL text. Restoring an unmarked pre-v60 database leaves it +unmarked. Subscription restoration retains its existing create-SQL path. + +After database recreation, existing object and system-catalog restoration owns +table receipts and protection snapshots as before. A failed type read, capability +check, database create, object restore, or catalog restore returns through the +restore transaction and does not publish a partially restored identity. + +### 12.6 Alternatives + +- Keep table receipts only: preserves the bug for zero-table and all-dropped + databases and violates the new invariant. +- Reserve a synthetic `mo_branch_metadata.table_id`: overloads a table primary + key and would enter table-lineage DAG, reclamation, and snapshot cleanup as a + fake table. It also changes more persistent invariants than a database-owned + category. +- Add a database metadata table or new column: gives explicit schema shape but + requires catalog bootstrap/upgrade and another owner for a single categorical + fact already represented by `dat_type`. +- Infer from names, object contents, create SQL, or historical snapshots: none + is authoritative and each can misclassify an ordinary database, so all are + rejected. + +### 12.7 Deterministic validation + +Focused tests must cover both sides of the capability boundary: + +- v59 create rejects before any DDL; v60 create attaches the marker; +- old-created unmarked empty/all-dropped databases reject at both versions; +- old-created unmarked table-backed databases with valid receipts delete at + both versions; +- new-created marked empty/all-dropped databases reject at v59 and delete at + v60; +- new-created marked table-backed databases retain legacy receipt validation at + v59 and require the same receipts at v60; +- publication treats the marker as non-user at v59 and user at v60; +- snapshot/PITR restore rejects marked input before destructive work at v59 and + preserves the marker at v60; +- missing/subscription/unknown identity and locally added ordinary tables remain + negative controls; +- BVT at the active protocol proves empty, view-only, sequence-only, + all-dropped, cross-account, publication, and failure-atomicity behavior. + +No new goroutine, cache, retry, background worker, external I/O, or unbounded +state is introduced. The additional database-type catalog read replaces the +former existence read and does not change asymptotic work. diff --git a/pkg/catalog/types.go b/pkg/catalog/types.go index 86e255bd9f5d9..2d26b5e4da50b 100644 --- a/pkg/catalog/types.go +++ b/pkg/catalog/types.go @@ -501,6 +501,7 @@ const ( SystemColNoConstraint = "n" SystemDBTypeSubscription = "subscription" + SystemDBTypeDataBranch = "data-branch" MOPartitionMetadata = "mo_partition_metadata" MOPartitionTables = "mo_partition_tables" diff --git a/pkg/frontend/data_branch.go b/pkg/frontend/data_branch.go index 3b0979f535726..77eb4af728442 100644 --- a/pkg/frontend/data_branch.go +++ b/pkg/frontend/data_branch.go @@ -675,6 +675,9 @@ func dataBranchCreateDatabase( execCtx.reqCtx, tree.CloneLevelCtxKey{}, tree.NormalCloneLevelDatabase, ) execCtx.reqCtx = context.WithValue(execCtx.reqCtx, dataBranchCloneLockCtxKey{}, true) + execCtx.reqCtx = context.WithValue( + execCtx.reqCtx, defines.DatTypKey{}, catalog.SystemDBTypeDataBranch, + ) if !skipDataBranchPrivilegeCheck(ses) { if authStats, err = authenticateDataBranchCreateDatabase(execCtx.reqCtx, ses, stmt); err != nil { diff --git a/pkg/frontend/data_branch_privilege.go b/pkg/frontend/data_branch_privilege.go index 4603d0b309e59..a63e482399d96 100644 --- a/pkg/frontend/data_branch_privilege.go +++ b/pkg/frontend/data_branch_privilege.go @@ -659,9 +659,13 @@ func validateDataBranchDeleteDatabaseTarget( if err != nil { return nil, err } - if err = validateBranchDatabaseExists(ctx, ses, bh, accId, dbName); err != nil { + databaseType, err := loadBranchDatabaseType(ctx, ses, bh, accId, dbName) + if err != nil { return nil, err } + if databaseType != "" && databaseType != catalog.SystemDBTypeDataBranch { + return nil, inactiveBranchDatabaseError(ctx, dbName) + } sql := branchDeleteDatabaseTableIDsSQL(accId, dbName) sqlRet, err := runSql(ctx, ses, bh, sql, nil, nil) @@ -686,8 +690,8 @@ func validateDataBranchDeleteDatabaseTarget( } return true }) - if len(tableNames) == 0 { - return nil, moerr.NewInternalErrorf(ctx, "DATA BRANCH DELETE target %s is not an active branch database", dbName) + if len(tableNames) == 0 && databaseType != catalog.SystemDBTypeDataBranch { + return nil, inactiveBranchDatabaseError(ctx, dbName) } if err = validateActiveBranchChildTableIDs(ctx, ses, bh, tableNames); err != nil { return nil, err @@ -713,35 +717,49 @@ func branchDeleteDatabaseTableIDsSQL(accId uint32, dbName string) string { ) } -func validateBranchDatabaseExists( +func loadBranchDatabaseType( ctx context.Context, ses *Session, bh BackgroundExec, accId uint32, dbName string, -) error { - sql := fmt.Sprintf( - "select dat_id from %s.%s where account_id = %d and datname = %s", - catalog.MO_CATALOG, - catalog.MO_DATABASE, - accId, - quoteSQLStringLiteral(dbName), - ) - sqlRet, err := runSql(ctx, ses, bh, sql, nil, nil) +) (string, error) { + sqlRet, err := runSql(ctx, ses, bh, branchDatabaseTypeSQL(accId, dbName), nil, nil) if err != nil { - return err + return "", err } defer sqlRet.Close() + var databaseType string found := false sqlRet.ReadRows(func(rows int, cols []*vector.Vector) bool { - found = rows > 0 + if rows > 0 { + databaseType = executor.GetStringRows(cols[0])[0] + found = true + } return false }) if !found { - return moerr.NewBadDB(ctx, dbName) + return "", moerr.NewBadDB(ctx, dbName) } - return nil + return databaseType, nil +} + +func branchDatabaseTypeSQL(accId uint32, dbName string) string { + return fmt.Sprintf( + "select coalesce(%s, '') from %s.%s where %s = %d and %s = %s", + catalog.SystemDBAttr_Type, + catalog.MO_CATALOG, + catalog.MO_DATABASE, + catalog.SystemDBAttr_AccID, + accId, + catalog.SystemDBAttr_Name, + quoteSQLStringLiteral(dbName), + ) +} + +func inactiveBranchDatabaseError(ctx context.Context, dbName string) error { + return moerr.NewInternalErrorf(ctx, "DATA BRANCH DELETE target %s is not an active branch database", dbName) } func validateActiveBranchChildTableIDs( diff --git a/pkg/frontend/data_branch_privilege_test.go b/pkg/frontend/data_branch_privilege_test.go index 6db93c7f478dc..b5bc34c920127 100644 --- a/pkg/frontend/data_branch_privilege_test.go +++ b/pkg/frontend/data_branch_privilege_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/matrixorigin/matrixone/pkg/catalog" + "github.com/matrixorigin/matrixone/pkg/defines" "github.com/matrixorigin/matrixone/pkg/sql/parsers/tree" "github.com/stretchr/testify/require" ) @@ -107,6 +108,150 @@ func TestBranchDeleteDatabaseTableIDsSQLReusesCloneObjectFilter(t *testing.T) { require.NotContains(t, got, "relname not like") } +func TestValidateDataBranchDeleteDatabaseTargetUsesDatabaseIdentity(t *testing.T) { + const ( + accountID uint32 = 42 + dbName = "db1" + tableID uint64 = 101 + ) + ctx := defines.AttachAccountId(context.Background(), accountID) + + tests := []struct { + name string + databaseType string + tables [][]interface{} + activeIDs [][]interface{} + wantIDs []uint64 + wantErr string + wantSQLCount int + }{ + { + name: "marked empty database", + databaseType: catalog.SystemDBTypeDataBranch, + wantIDs: []uint64{}, + wantSQLCount: 2, + }, + { + name: "unmarked empty database", + wantErr: "not an active branch database", + wantSQLCount: 2, + }, + { + name: "subscription database", + databaseType: catalog.SystemDBTypeSubscription, + wantErr: "not an active branch database", + wantSQLCount: 1, + }, + { + name: "unknown database type", + databaseType: "unknown", + wantErr: "not an active branch database", + wantSQLCount: 1, + }, + { + name: "marked database with ordinary table", + databaseType: catalog.SystemDBTypeDataBranch, + tables: [][]interface{}{{int64(tableID), "local_t"}}, + wantErr: "not an active branch table", + wantSQLCount: 3, + }, + { + name: "marked database with branch table", + databaseType: catalog.SystemDBTypeDataBranch, + tables: [][]interface{}{{int64(tableID), "branch_t"}}, + activeIDs: [][]interface{}{{int64(tableID)}}, + wantIDs: []uint64{tableID}, + wantSQLCount: 3, + }, + { + name: "legacy database with branch table", + tables: [][]interface{}{{int64(tableID), "branch_t"}}, + activeIDs: [][]interface{}{{int64(tableID)}}, + wantIDs: []uint64{tableID}, + wantSQLCount: 3, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + ses := newValidateSession(t) + bh := &backgroundExecTest{} + bh.init() + bh.sql2result[branchDatabaseTypeSQL(accountID, dbName)] = branchStringResult( + "dat_type", [][]interface{}{{test.databaseType}}, + ) + bh.sql2result[branchDeleteDatabaseTableIDsSQL(accountID, dbName)] = branchTableResult(test.tables) + activeSQL := fmt.Sprintf( + "select table_id from %s.%s where table_deleted = false and level != 'alter' and table_id in (%d)", + catalog.MO_CATALOG, catalog.MO_BRANCH_METADATA, tableID, + ) + bh.sql2result[activeSQL] = branchUint64Result("table_id", test.activeIDs) + + ids, err := validateDataBranchDeleteDatabaseTarget(ctx, ses, bh, dbName) + if test.wantErr != "" { + require.ErrorContains(t, err, test.wantErr) + require.Nil(t, ids) + } else { + require.NoError(t, err) + require.Equal(t, test.wantIDs, ids) + } + require.Len(t, bh.executedSQLs, test.wantSQLCount) + }) + } +} + +func TestLoadBranchDatabaseTypeRejectsMissingDatabase(t *testing.T) { + const ( + accountID uint32 = 42 + dbName = "missing_db" + ) + ctx := defines.AttachAccountId(context.Background(), accountID) + ses := newValidateSession(t) + bh := &backgroundExecTest{} + bh.init() + bh.sql2result[branchDatabaseTypeSQL(accountID, dbName)] = branchStringResult("dat_type", nil) + + _, err := loadBranchDatabaseType(ctx, ses, bh, accountID, dbName) + require.ErrorContains(t, err, "Unknown database") +} + +func branchStringResult(columnName string, rows [][]interface{}) *MysqlResultSet { + result := &MysqlResultSet{} + column := &MysqlColumn{} + column.SetName(columnName) + column.SetColumnType(defines.MYSQL_TYPE_VARCHAR) + result.AddColumn(column) + for _, row := range rows { + result.AddRow(row) + } + return result +} + +func branchUint64Result(columnName string, rows [][]interface{}) *MysqlResultSet { + result := &MysqlResultSet{} + column := &MysqlColumn{} + column.SetName(columnName) + column.SetColumnType(defines.MYSQL_TYPE_LONGLONG) + column.SetSigned(false) + result.AddColumn(column) + for _, row := range rows { + result.AddRow(row) + } + return result +} + +func branchTableResult(rows [][]interface{}) *MysqlResultSet { + result := branchUint64Result("rel_id", nil) + nameColumn := &MysqlColumn{} + nameColumn.SetName("relname") + nameColumn.SetColumnType(defines.MYSQL_TYPE_VARCHAR) + result.AddColumn(nameColumn) + for _, row := range rows { + result.AddRow(row) + } + return result +} + func TestBuildTableInfoListWhereClauseUsesRelationKindForInternalObjects(t *testing.T) { got := buildTableInfoListWhereClause("db1", "", 42) diff --git a/pkg/frontend/publication_subscription.go b/pkg/frontend/publication_subscription.go index 2e2a7c7e05862..4659f5733bfb2 100644 --- a/pkg/frontend/publication_subscription.go +++ b/pkg/frontend/publication_subscription.go @@ -343,7 +343,7 @@ func createPublication(ctx context.Context, bh BackgroundExec, cp *tree.CreatePu return } } - if dbType != "" { //TODO: check the dat_type + if !isUserDatabaseType(dbType) { return moerr.NewInternalErrorf(ctx, "database '%s' is not a user database", cp.Database) } } else { @@ -554,7 +554,7 @@ func doAlterPublication(ctx context.Context, ses *Session, ap *tree.AlterPublica if dbId, dbType, err = getDbIdAndType(ctx, bh, dbName); err != nil { return err } - if dbType != "" { //TODO: check the dat_type + if !isUserDatabaseType(dbType) { return moerr.NewInternalErrorf(ctx, "database '%s' is not a user database", dbName) } } @@ -648,6 +648,10 @@ func doAlterPublication(ctx context.Context, ses *Session, ap *tree.AlterPublica return } +func isUserDatabaseType(databaseType string) bool { + return databaseType == "" || databaseType == catalog.SystemDBTypeDataBranch +} + func doDropPublication(ctx context.Context, ses *Session, dp *tree.DropPublication) (err error) { start := time.Now() defer func() { diff --git a/pkg/frontend/publication_subscription_test.go b/pkg/frontend/publication_subscription_test.go index 46b3b2c70ec1e..360a15210d332 100644 --- a/pkg/frontend/publication_subscription_test.go +++ b/pkg/frontend/publication_subscription_test.go @@ -20,6 +20,7 @@ import ( "time" "github.com/golang/mock/gomock" + "github.com/matrixorigin/matrixone/pkg/catalog" "github.com/matrixorigin/matrixone/pkg/cdc" "github.com/matrixorigin/matrixone/pkg/common/moerr" "github.com/prashantv/gostub" @@ -34,6 +35,13 @@ import ( "github.com/matrixorigin/matrixone/pkg/sql/parsers/tree" ) +func TestIsUserDatabaseType(t *testing.T) { + require.True(t, isUserDatabaseType("")) + require.True(t, isUserDatabaseType(catalog.SystemDBTypeDataBranch)) + require.False(t, isUserDatabaseType(catalog.SystemDBTypeSubscription)) + require.False(t, isUserDatabaseType("unknown")) +} + func Test_doCreatePublication(t *testing.T) { mockedAccountsResults := func(ctrl *gomock.Controller) []interface{} { er := mock_frontend.NewMockExecResult(ctrl) @@ -56,7 +64,7 @@ func Test_doCreatePublication(t *testing.T) { er := mock_frontend.NewMockExecResult(ctrl) er.EXPECT().GetRowCount().Return(uint64(1)).AnyTimes() er.EXPECT().GetUint64(gomock.Any(), uint64(0), uint64(0)).Return(uint64(0), nil).AnyTimes() - er.EXPECT().GetString(gomock.Any(), uint64(0), uint64(1)).Return("", nil).AnyTimes() + er.EXPECT().GetString(gomock.Any(), uint64(0), uint64(1)).Return(catalog.SystemDBTypeDataBranch, nil).AnyTimes() return []interface{}{er} } @@ -246,7 +254,7 @@ func Test_doAlterPublication(t *testing.T) { er := mock_frontend.NewMockExecResult(ctrl) er.EXPECT().GetRowCount().Return(uint64(1)).AnyTimes() er.EXPECT().GetUint64(gomock.Any(), uint64(0), uint64(0)).Return(uint64(0), nil).AnyTimes() - er.EXPECT().GetString(gomock.Any(), uint64(0), uint64(1)).Return("", nil).AnyTimes() + er.EXPECT().GetString(gomock.Any(), uint64(0), uint64(1)).Return(catalog.SystemDBTypeDataBranch, nil).AnyTimes() return []interface{}{er} } diff --git a/pkg/sql/compile/ddl.go b/pkg/sql/compile/ddl.go index 7248ebd0823e9..2e539e3d4d70b 100644 --- a/pkg/sql/compile/ddl.go +++ b/pkg/sql/compile/ddl.go @@ -115,7 +115,9 @@ func (s *Scope) CreateDatabase(c *Compile) error { } ctx = context.WithValue(ctx, defines.SqlKey{}, createDatabase.GetSql()) - datType := "" + // Internal database creators can attach a categorical type to the CREATE + // itself so the catalog row is atomic with the database definition. + datType, _ := ctx.Value(defines.DatTypKey{}).(string) // handle sub if subOption := createDatabase.SubscriptionOption; subOption != nil { datType = catalog.SystemDBTypeSubscription diff --git a/pkg/sql/compile/ddl_test.go b/pkg/sql/compile/ddl_test.go index de62b333e7a2a..bb4a65d68fc95 100644 --- a/pkg/sql/compile/ddl_test.go +++ b/pkg/sql/compile/ddl_test.go @@ -463,6 +463,7 @@ func TestCreateDatabaseChecksExistingBeforeSerializingAbsence(t *testing.T) { ifNotExists bool lookups []lookupResult lockErr error + databaseType string createErr error wantCreate bool wantErr error @@ -480,6 +481,17 @@ func TestCreateDatabaseChecksExistingBeforeSerializingAbsence(t *testing.T) { wantAffected: 1, wantEvents: []string{"lookup", "lock", "lookup", "create"}, }, + { + name: "internal database type", + databaseType: catalog.SystemDBTypeDataBranch, + lookups: []lookupResult{ + {err: moerr.GetOkExpectedEOB()}, + {err: moerr.GetOkExpectedEOB()}, + }, + wantCreate: true, + wantAffected: 1, + wantEvents: []string{"lookup", "lock", "lookup", "create"}, + }, { name: "if not exists fast no-op", ifNotExists: true, @@ -573,8 +585,9 @@ func TestCreateDatabaseChecksExistingBeforeSerializingAbsence(t *testing.T) { } if tc.wantCreate { eng.EXPECT().Create(gomock.Any(), "db1", gomock.Any()).DoAndReturn( - func(context.Context, string, client.TxnOperator) error { + func(ctx context.Context, _ string, _ client.TxnOperator) error { events = append(events, "create") + require.Equal(t, tc.databaseType, ctx.Value(defines.DatTypKey{})) return tc.createErr }, ) @@ -582,6 +595,9 @@ func TestCreateDatabaseChecksExistingBeforeSerializingAbsence(t *testing.T) { proc := testutil.NewProcess(t) ctx := defines.AttachAccountId(context.Background(), sysAccountId) + if tc.databaseType != "" { + ctx = context.WithValue(ctx, defines.DatTypKey{}, tc.databaseType) + } proc.Ctx = ctx proc.ReplaceTopCtx(ctx) c := &Compile{e: eng, proc: proc, affectRows: new(atomic.Uint64)} diff --git a/test/distributed/cases/git4data/branch/metadata/branch_database_identity.result b/test/distributed/cases/git4data/branch/metadata/branch_database_identity.result new file mode 100644 index 0000000000000..c99b52f576b95 --- /dev/null +++ b/test/distributed/cases/git4data/branch/metadata/branch_database_identity.result @@ -0,0 +1,97 @@ +drop database if exists issue26068_empty_src; +drop database if exists issue26068_empty_dst; +drop database if exists issue26068_view_src; +drop database if exists issue26068_view_dst; +drop database if exists issue26068_sequence_src; +drop database if exists issue26068_sequence_dst; +drop database if exists issue26068_dropall_src; +drop database if exists issue26068_dropall_dst; +drop database if exists issue26068_local_table_src; +drop database if exists issue26068_local_table_dst; +drop database if exists issue26068_ordinary_empty; +create database issue26068_empty_src; +data branch create database issue26068_empty_dst from issue26068_empty_src; +select dat_type from mo_catalog.mo_database where datname = 'issue26068_empty_dst'; +➤ dat_type[12,32,0] 𝄀 +data-branch +data branch delete database issue26068_empty_dst; +select count(*) as empty_branch_exists from mo_catalog.mo_database where datname = 'issue26068_empty_dst'; +➤ empty_branch_exists[-5,64,0] 𝄀 +0 +create database issue26068_view_src; +create view issue26068_view_src.v as select 1 as n; +select * from issue26068_view_src.v; +➤ n[-5,64,0] 𝄀 +1 +data branch create database issue26068_view_dst from issue26068_view_src; +select * from issue26068_view_dst.v; +➤ n[-5,64,0] 𝄀 +1 +data branch delete database issue26068_view_dst; +select count(*) as view_branch_exists from mo_catalog.mo_database where datname = 'issue26068_view_dst'; +➤ view_branch_exists[-5,64,0] 𝄀 +0 +create database issue26068_sequence_src; +create sequence issue26068_sequence_src.s increment 3 start with 7; +select count(*) as source_sequences from mo_catalog.mo_tables +where reldatabase = 'issue26068_sequence_src' and relkind = 'S'; +➤ source_sequences[-5,64,0] 𝄀 +1 +data branch create database issue26068_sequence_dst from issue26068_sequence_src; +select count(*) as cloned_sequences from mo_catalog.mo_tables +where reldatabase = 'issue26068_sequence_dst' and relkind = 'S'; +➤ cloned_sequences[-5,64,0] 𝄀 +1 +data branch delete database issue26068_sequence_dst; +select count(*) as sequence_branch_exists from mo_catalog.mo_database where datname = 'issue26068_sequence_dst'; +➤ sequence_branch_exists[-5,64,0] 𝄀 +0 +create database issue26068_dropall_src; +create table issue26068_dropall_src.t1(id int primary key); +create table issue26068_dropall_src.t2(id int primary key); +select count(*) as source_tables from mo_catalog.mo_tables +where reldatabase = 'issue26068_dropall_src' and relname in ('t1', 't2'); +➤ source_tables[-5,64,0] 𝄀 +2 +data branch create database issue26068_dropall_dst from issue26068_dropall_src; +select count(*) as cloned_tables from mo_catalog.mo_tables +where reldatabase = 'issue26068_dropall_dst' and relname in ('t1', 't2'); +➤ cloned_tables[-5,64,0] 𝄀 +2 +drop table issue26068_dropall_dst.t1; +drop table issue26068_dropall_dst.t2; +data branch delete database issue26068_dropall_dst; +select count(*) as dropped_table_branch_exists from mo_catalog.mo_database where datname = 'issue26068_dropall_dst'; +➤ dropped_table_branch_exists[-5,64,0] 𝄀 +0 +create database issue26068_local_table_src; +data branch create database issue26068_local_table_dst from issue26068_local_table_src; +create table issue26068_local_table_dst.local_t(id int primary key); +data branch delete database issue26068_local_table_dst; +internal error: DATA BRANCH DELETE target issue26068_local_table_dst.local_t is not an active branch table +select count(*) as protected_local_table from mo_catalog.mo_tables +where reldatabase = 'issue26068_local_table_dst' and relname = 'local_t'; +➤ protected_local_table[-5,64,0] 𝄀 +1 +drop table issue26068_local_table_dst.local_t; +data branch delete database issue26068_local_table_dst; +select count(*) as local_table_branch_exists from mo_catalog.mo_database where datname = 'issue26068_local_table_dst'; +➤ local_table_branch_exists[-5,64,0] 𝄀 +0 +create database issue26068_ordinary_empty; +data branch delete database issue26068_ordinary_empty; +internal error: DATA BRANCH DELETE target issue26068_ordinary_empty is not an active branch database +select count(*) as ordinary_empty_exists from mo_catalog.mo_database where datname = 'issue26068_ordinary_empty'; +➤ ordinary_empty_exists[-5,64,0] 𝄀 +1 +drop database if exists issue26068_empty_src; +drop database if exists issue26068_empty_dst; +drop database if exists issue26068_view_src; +drop database if exists issue26068_view_dst; +drop database if exists issue26068_sequence_src; +drop database if exists issue26068_sequence_dst; +drop database if exists issue26068_dropall_src; +drop database if exists issue26068_dropall_dst; +drop database if exists issue26068_local_table_src; +drop database if exists issue26068_local_table_dst; +drop database if exists issue26068_ordinary_empty; diff --git a/test/distributed/cases/git4data/branch/metadata/branch_database_identity.sql b/test/distributed/cases/git4data/branch/metadata/branch_database_identity.sql new file mode 100644 index 0000000000000..9dc5e7075f520 --- /dev/null +++ b/test/distributed/cases/git4data/branch/metadata/branch_database_identity.sql @@ -0,0 +1,84 @@ +-- DATA BRANCH CREATE DATABASE records database identity even when there are +-- no ordinary-table receipts, and DELETE uses that identity without weakening +-- validation for ordinary databases or locally-added tables. + +drop database if exists issue26068_empty_src; +drop database if exists issue26068_empty_dst; +drop database if exists issue26068_view_src; +drop database if exists issue26068_view_dst; +drop database if exists issue26068_sequence_src; +drop database if exists issue26068_sequence_dst; +drop database if exists issue26068_dropall_src; +drop database if exists issue26068_dropall_dst; +drop database if exists issue26068_local_table_src; +drop database if exists issue26068_local_table_dst; +drop database if exists issue26068_ordinary_empty; + +-- Empty source database: the database marker is the only branch receipt. +create database issue26068_empty_src; +data branch create database issue26068_empty_dst from issue26068_empty_src; +select dat_type from mo_catalog.mo_database where datname = 'issue26068_empty_dst'; +data branch delete database issue26068_empty_dst; +select count(*) as empty_branch_exists from mo_catalog.mo_database where datname = 'issue26068_empty_dst'; + +-- View-only source database: views remain usable and do not need table receipts. +create database issue26068_view_src; +create view issue26068_view_src.v as select 1 as n; +select * from issue26068_view_src.v; +data branch create database issue26068_view_dst from issue26068_view_src; +select * from issue26068_view_dst.v; +data branch delete database issue26068_view_dst; +select count(*) as view_branch_exists from mo_catalog.mo_database where datname = 'issue26068_view_dst'; + +-- Sequence-only source database: sequences are excluded from branch-table validation. +create database issue26068_sequence_src; +create sequence issue26068_sequence_src.s increment 3 start with 7; +select count(*) as source_sequences from mo_catalog.mo_tables + where reldatabase = 'issue26068_sequence_src' and relkind = 'S'; +data branch create database issue26068_sequence_dst from issue26068_sequence_src; +select count(*) as cloned_sequences from mo_catalog.mo_tables + where reldatabase = 'issue26068_sequence_dst' and relkind = 'S'; +data branch delete database issue26068_sequence_dst; +select count(*) as sequence_branch_exists from mo_catalog.mo_database where datname = 'issue26068_sequence_dst'; + +-- A database branch remains identifiable after every cloned table is dropped. +create database issue26068_dropall_src; +create table issue26068_dropall_src.t1(id int primary key); +create table issue26068_dropall_src.t2(id int primary key); +select count(*) as source_tables from mo_catalog.mo_tables + where reldatabase = 'issue26068_dropall_src' and relname in ('t1', 't2'); +data branch create database issue26068_dropall_dst from issue26068_dropall_src; +select count(*) as cloned_tables from mo_catalog.mo_tables + where reldatabase = 'issue26068_dropall_dst' and relname in ('t1', 't2'); +drop table issue26068_dropall_dst.t1; +drop table issue26068_dropall_dst.t2; +data branch delete database issue26068_dropall_dst; +select count(*) as dropped_table_branch_exists from mo_catalog.mo_database where datname = 'issue26068_dropall_dst'; + +-- The marker does not permit deleting a locally-added ordinary table. +create database issue26068_local_table_src; +data branch create database issue26068_local_table_dst from issue26068_local_table_src; +create table issue26068_local_table_dst.local_t(id int primary key); +data branch delete database issue26068_local_table_dst; +select count(*) as protected_local_table from mo_catalog.mo_tables + where reldatabase = 'issue26068_local_table_dst' and relname = 'local_t'; +drop table issue26068_local_table_dst.local_t; +data branch delete database issue26068_local_table_dst; +select count(*) as local_table_branch_exists from mo_catalog.mo_database where datname = 'issue26068_local_table_dst'; + +-- An ordinary empty database is still not a DATA BRANCH DELETE target. +create database issue26068_ordinary_empty; +data branch delete database issue26068_ordinary_empty; +select count(*) as ordinary_empty_exists from mo_catalog.mo_database where datname = 'issue26068_ordinary_empty'; + +drop database if exists issue26068_empty_src; +drop database if exists issue26068_empty_dst; +drop database if exists issue26068_view_src; +drop database if exists issue26068_view_dst; +drop database if exists issue26068_sequence_src; +drop database if exists issue26068_sequence_dst; +drop database if exists issue26068_dropall_src; +drop database if exists issue26068_dropall_dst; +drop database if exists issue26068_local_table_src; +drop database if exists issue26068_local_table_dst; +drop database if exists issue26068_ordinary_empty; diff --git a/test/distributed/cases/git4data/branch/privilege/branch_privilege.result b/test/distributed/cases/git4data/branch/privilege/branch_privilege.result index 6fe7ecf43ef55..72198e76b6400 100644 --- a/test/distributed/cases/git4data/branch/privilege/branch_privilege.result +++ b/test/distributed/cases/git4data/branch/privilege/branch_privilege.result @@ -16,6 +16,9 @@ set @quoted_acc_id = (select account_id from mo_catalog.mo_account where account select count(*) from mo_catalog.mo_database where account_id = @quoted_acc_id and datname = 'br_to_acc'; ➤ count(*)[-5,64,0] 𝄀 1 +select dat_type from mo_catalog.mo_database where account_id = @quoted_acc_id and datname = 'br_to_acc'; +➤ dat_type[12,32,0] 𝄀 +data-branch select count(*) from mo_catalog.mo_tables where account_id = @quoted_acc_id and reldatabase = 'br_to_acc' and relname = 't'; ➤ count(*)[-5,64,0] 𝄀 1 diff --git a/test/distributed/cases/git4data/branch/privilege/branch_privilege.sql b/test/distributed/cases/git4data/branch/privilege/branch_privilege.sql index 969af52ef95ab..25cd57fb37c76 100644 --- a/test/distributed/cases/git4data/branch/privilege/branch_privilege.sql +++ b/test/distributed/cases/git4data/branch/privilege/branch_privilege.sql @@ -17,6 +17,7 @@ create snapshot sp_to_account for database br_sys_src; data branch create database br_to_acc from br_sys_src{snapshot="sp_to_account"} to account `acc-branch`; set @quoted_acc_id = (select account_id from mo_catalog.mo_account where account_name = 'acc-branch'); select count(*) from mo_catalog.mo_database where account_id = @quoted_acc_id and datname = 'br_to_acc'; +select dat_type from mo_catalog.mo_database where account_id = @quoted_acc_id and datname = 'br_to_acc'; select count(*) from mo_catalog.mo_tables where account_id = @quoted_acc_id and reldatabase = 'br_to_acc' and relname = 't'; drop snapshot sp_to_account; drop database br_sys_src;