diff --git a/pkg/sql/plan/build_dml_util.go b/pkg/sql/plan/build_dml_util.go index 71c7787c4af6e..45fd37c21db0e 100644 --- a/pkg/sql/plan/build_dml_util.go +++ b/pkg/sql/plan/build_dml_util.go @@ -120,8 +120,8 @@ type dmlPlanCtx struct { skipTargetDelete bool preserveUpdateSourceProjection bool // fkSetNullColumns records columns that are unconditionally NULL in every - // row of this recursive FK update source. A UNIQUE hidden index containing - // one of these columns has no replacement row to insert. + // row of this recursive FK update source. An index representation that + // compacts NULL keys has no replacement row to insert. fkSetNullColumns map[string]struct{} // isConditionalFkSetNullAction marks a combined FK SET NULL update whose // columns are NULL only on matching rows. Its hidden UNIQUE-index insert @@ -4143,6 +4143,7 @@ func appendDeleteIndexTablePlan( isUK bool, preserveProjection bool, preserveActionRows bool, + matchedDeleteOnly bool, ) (int32, error) { /******** NOTE: make sure to make the major change applied to secondary index, to IVFFLAT index as well. @@ -4365,8 +4366,9 @@ func appendDeleteIndexTablePlan( 2. SECONDARY INDEX: handling new inserts in ON DUPLICATE KEY UPDATE scenarios Note: The original assumption "secondary index won't have null situation" was incorrect. - While secondary indexes don't store NULL values, they DO need RIGHT JOIN to handle - new inserts that don't yet exist in the index table. + Single-part secondary indexes compact NULL keys, while composite indexes retain + NULL-containing keys via serial_full. RIGHT JOIN is also needed for new inserts + that do not yet exist in the hidden table. */ joinNode := &plan.Node{ NodeType: plan.Node_JOIN, @@ -4375,7 +4377,16 @@ func appendDeleteIndexTablePlan( IsRightJoin: true, OnList: joinConds, } - if preserveActionRows { + if matchedDeleteOnly { + // This source only feeds deletion of the old hidden row. The replacement + // composite SET NULL key is built from an independent action stream, so + // unmatched action rows must not flow into the delete pipeline. + joinNode.JoinType = plan.Node_INNER + joinNode.IsRightJoin = false + if !preserveProjection { + joinNode.ProjectList = projectList + } + } else if preserveActionRows { // Recursive FK maintenance must preserve the action source, not the // complete hidden-index scan. Express that ownership directly as a LEFT // join so the later physical right-join swap cannot invert the row domain. @@ -6199,6 +6210,24 @@ func buildDeleteRegularIndex(ctx CompilerContext, builder *QueryBuilder, bindCtx var isUk = indexdef.Unique var isSK = !isUk && catalog.IsRegularIndexAlgo(indexdef.IndexAlgo) + logicalIndexParts := 0 + indexBecomesNull := false + if isUpdate && len(delCtx.fkSetNullColumns) > 0 { + for _, part := range indexdef.Parts { + if catalog.IsAlias(part) { + continue + } + logicalIndexParts++ + if _, becomesNull := delCtx.fkSetNullColumns[catalog.ResolveAlias(part)]; becomesNull { + indexBecomesNull = true + } + } + } + skipIndexInsert := indexBecomesNull && + (isUk || (isSK && logicalIndexParts == 1)) + rebuildCompositeSetNullIndex := indexBecomesNull && isSK && logicalIndexParts > 1 + usePositionalIndexDelete := skipIndexInsert && + delCtx.sourceTag == 0 && !delCtx.preserveUpdateSourceProjection uniqueObjRef, uniqueTableDef, err := builder.compCtx.ResolveIndexTableByRef(delCtx.objRef, indexdef.IndexTableName, nil) if err != nil { @@ -6221,9 +6250,22 @@ func buildDeleteRegularIndex(ctx CompilerContext, builder *QueryBuilder, bindCtx preserveIndexProjection := delCtx.isFkRecursionCall || delCtx.preserveUpdateSourceProjection preserveActionRows := delCtx.isFkRecursionCall && (delCtx.sourceTag != 0 || delCtx.preserveUpdateSourceProjection) + if rebuildCompositeSetNullIndex { + // The replacement insert has its own action stream, so this branch only + // needs the positional old-row image plus the matched hidden row. + preserveIndexProjection = false + preserveActionRows = false + } + if usePositionalIndexDelete { + // This branch ends after deleting the old hidden row. Keep the join's + // established positional layout; preserving the recursive action's + // binding tags would leak those tags into the two-batch join executor. + preserveIndexProjection = false + preserveActionRows = false + } lastNodeId, err = appendDeleteIndexTablePlan( builder, bindCtx, uniqueObjRef, uniqueTableDef, indexdef, typMap, posMap, - lastNodeId, isUk, preserveIndexProjection, preserveActionRows, + lastNodeId, isUk, preserveIndexProjection, preserveActionRows, rebuildCompositeSetNullIndex, ) uniqueDeleteIdx = len(delCtx.tableDef.Cols) + delCtx.updateColLength uniqueTblPkPos = uniqueDeleteIdx + 1 @@ -6233,18 +6275,10 @@ func buildDeleteRegularIndex(ctx CompilerContext, builder *QueryBuilder, bindCtx return err } if isUpdate { - skipIndexInsert := false - if isUk && len(delCtx.fkSetNullColumns) > 0 { - for _, part := range indexdef.Parts { - if _, becomesNull := delCtx.fkSetNullColumns[catalog.ResolveAlias(part)]; becomesNull { - skipIndexInsert = true - break - } - } - } if skipIndexInsert { delNodeInfo := makeDeleteNodeInfo(builder.compCtx, uniqueObjRef, uniqueTableDef, uniqueDeleteIdx, false, uniqueTblPkPos, uniqueTblPkTyp, delCtx.lockTable) - delNodeInfo.preserveProjection = delCtx.isFkRecursionCall || delCtx.preserveUpdateSourceProjection + delNodeInfo.preserveProjection = !usePositionalIndexDelete && + (delCtx.isFkRecursionCall || delCtx.preserveUpdateSourceProjection) lastNodeId, err = makeOneDeletePlan(builder, bindCtx, lastNodeId, delNodeInfo, isUk, isSK, false) putDeleteNodeInfo(delNodeInfo) if err != nil { @@ -6275,7 +6309,21 @@ func buildDeleteRegularIndex(ctx CompilerContext, builder *QueryBuilder, bindCtx lastNodeId = appendSinkScanNode(builder, bindCtx, newSourceStep) } delNodeInfo := makeDeleteNodeInfo(builder.compCtx, uniqueObjRef, uniqueTableDef, uniqueDeleteIdx, false, uniqueTblPkPos, uniqueTblPkTyp, delCtx.lockTable) - delNodeInfo.preserveProjection = delCtx.isFkRecursionCall || delCtx.preserveUpdateSourceProjection + if rebuildCompositeSetNullIndex { + inputProjection := getProjectionByLastNode(builder, lastNodeId) + lastNodeId = builder.appendNode(&Node{ + NodeType: plan.Node_PROJECT, + Children: []int32{lastNodeId}, + ProjectList: []*Expr{ + inputProjection[uniqueDeleteIdx], + inputProjection[uniqueTblPkPos], + }, + }, bindCtx) + delNodeInfo.deleteIndex = 0 + delNodeInfo.pkPos = 1 + } else { + delNodeInfo.preserveProjection = delCtx.isFkRecursionCall || delCtx.preserveUpdateSourceProjection + } lastNodeId, err = makeOneDeletePlan(builder, bindCtx, lastNodeId, delNodeInfo, isUk, isSK, false) putDeleteNodeInfo(delNodeInfo) if err != nil { @@ -6285,7 +6333,13 @@ func buildDeleteRegularIndex(ctx CompilerContext, builder *QueryBuilder, bindCtx } // update uk plan { - if indexSourceTag != 0 { + if rebuildCompositeSetNullIndex { + // Composite secondary indexes retain rows whose keys contain NULL. + // Rebuild their replacement keys directly from the FK action image: + // the shared index-join source is also consumed by the delete branch + // and cannot provide a second independent stream here. + lastNodeId = appendSinkScanNode(builder, bindCtx, delCtx.sourceStep) + } else if indexSourceTag != 0 { lastNodeId = builder.appendTaggedSinkScan(bindCtx, newSourceStep, indexSourceTag) } else { lastNodeId = appendSinkScanNode(builder, bindCtx, newSourceStep) @@ -6293,12 +6347,25 @@ func buildDeleteRegularIndex(ctx CompilerContext, builder *QueryBuilder, bindCtx lastProject := builder.qry.Nodes[lastNodeId].ProjectList projectProjection := make([]*Expr, len(delCtx.tableDef.Cols)) for j, uCols := range delCtx.tableDef.Cols { + if _, becomesNull := delCtx.fkSetNullColumns[uCols.Name]; becomesNull { + nullType := uCols.Typ + nullType.NotNullable = false + projectProjection[j] = &plan.Expr{ + Typ: nullType, + Expr: &plan.Expr_Lit{Lit: &Const{Isnull: true}}, + } + continue + } if nIdx, ok := delCtx.updateColPosMap[uCols.Name]; ok { projectProjection[j] = lastProject[nIdx] } else { if uCols.Name == catalog.Row_ID { - // replace the origin table's row_id with unique table's row_id - projectProjection[j] = lastProject[len(lastProject)-2] + if rebuildCompositeSetNullIndex { + projectProjection[j] = lastProject[delCtx.rowIdPos] + } else { + // replace the origin table's row_id with unique table's row_id + projectProjection[j] = lastProject[len(lastProject)-2] + } } else { projectProjection[j] = lastProject[j] } diff --git a/pkg/sql/plan/build_dml_util_test.go b/pkg/sql/plan/build_dml_util_test.go index 355f2efbd8ab8..51f2eac864db3 100644 --- a/pkg/sql/plan/build_dml_util_test.go +++ b/pkg/sql/plan/build_dml_util_test.go @@ -660,7 +660,7 @@ func TestAppendDeleteIndexTablePlanUsesPrefixLookupKey(t *testing.T) { typMap, posMap, lastNodeID, - true, true, false, + true, true, false, false, ) require.NoError(t, err) @@ -689,7 +689,7 @@ func TestAppendDeleteIndexTablePlanUsesPrefixLookupKey(t *testing.T) { typMap, posMap, lastNodeID, - false, true, true, + false, true, true, false, ) require.NoError(t, err) @@ -700,6 +700,30 @@ func TestAppendDeleteIndexTablePlanUsesPrefixLookupKey(t *testing.T) { require.Equal(t, plan.Node_TABLE_SCAN, builder.qry.Nodes[joinNode.Children[1]].NodeType) }) + t.Run("composite set null delete keeps matched hidden rows only", func(t *testing.T) { + builder, bindCtx, lastNodeID := newBuilder(t) + + gotNodeID, err := appendDeleteIndexTablePlan( + builder, + bindCtx, + &plan.ObjectRef{ObjName: "idx_body_tenant"}, + indexTableDef, + &plan.IndexDef{Parts: []string{"body", "tenant"}}, + typMap, + posMap, + lastNodeID, + false, false, false, true, + ) + + require.NoError(t, err) + joinNode := builder.qry.Nodes[gotNodeID] + require.Equal(t, plan.Node_JOIN, joinNode.NodeType) + require.Equal(t, plan.Node_INNER, joinNode.JoinType) + require.False(t, joinNode.IsRightJoin) + require.NotEmpty(t, joinNode.ProjectList) + require.Equal(t, plan.Node_TABLE_SCAN, builder.qry.Nodes[joinNode.Children[0]].NodeType) + }) + t.Run("composite prefix part", func(t *testing.T) { builder, bindCtx, lastNodeID := newBuilder(t) @@ -715,7 +739,7 @@ func TestAppendDeleteIndexTablePlanUsesPrefixLookupKey(t *testing.T) { typMap, posMap, lastNodeID, - false, true, false, + false, true, false, false, ) require.NoError(t, err) @@ -745,7 +769,7 @@ func TestAppendDeleteIndexTablePlanUsesPrefixLookupKey(t *testing.T) { typMap, posMap, lastNodeID, - true, true, false, + true, true, false, false, ) require.NoError(t, err) diff --git a/pkg/sql/plan/build_test.go b/pkg/sql/plan/build_test.go index 62d85e4f4b96d..698b7767a9d56 100644 --- a/pkg/sql/plan/build_test.go +++ b/pkg/sql/plan/build_test.go @@ -4000,6 +4000,39 @@ func TestPreparedForeignKeyActionsMarkQueryUncacheable(t *testing.T) { }) } +func TestDeleteSetNullMaintainsCompositeSecondaryIndexEntry(t *testing.T) { + mock := NewMockOptimizer(true) + setMockEmpDeptForeignKeyAction(t, mock, plan.ForeignKeyDef_SET_NULL, plan.ForeignKeyDef_RESTRICT) + + emp := mock.ctxt.tables["emp"] + require.Len(t, emp.Indexes, 2) + emp.Indexes = emp.Indexes[1:] + require.False(t, emp.Indexes[0].Unique) + emp.Indexes[0].Parts = []string{"deptno", "ename", catalog.AliasPrefix + "empno"} + + logicPlan, err := runOneStmt(mock, t, "delete from dept where deptno = 10") + require.NoError(t, err) + query := logicPlan.GetQuery() + require.Equal(t, 1, countUpdateFkPlanNodes(query, plan.Node_PRE_INSERT_SK), + "a composite secondary index retains a row whose key has a NULL component") +} + +func TestDeleteSetNullDropsSingleColumnSecondaryIndexEntry(t *testing.T) { + mock := NewMockOptimizer(true) + setMockEmpDeptForeignKeyAction(t, mock, plan.ForeignKeyDef_SET_NULL, plan.ForeignKeyDef_RESTRICT) + + emp := mock.ctxt.tables["emp"] + require.Len(t, emp.Indexes, 2) + emp.Indexes = emp.Indexes[1:] + require.False(t, emp.Indexes[0].Unique) + emp.Indexes[0].Parts = []string{"deptno", catalog.AliasPrefix + "empno"} + + logicPlan, err := runOneStmt(mock, t, "delete from dept where deptno = 10") + require.NoError(t, err) + require.Zero(t, countUpdateFkPlanNodes(logicPlan.GetQuery(), plan.Node_PRE_INSERT_SK), + "a single-column secondary index compacts the NULL replacement key") +} + func TestPreparedInsertForeignKeyPlansRemainSensitiveAcrossChecks(t *testing.T) { statements := []struct { name string diff --git a/pkg/tests/issues/issue_27539_test.go b/pkg/tests/issues/issue_27539_test.go new file mode 100644 index 0000000000000..b231ef727a9a8 --- /dev/null +++ b/pkg/tests/issues/issue_27539_test.go @@ -0,0 +1,183 @@ +// Copyright 2021 - 2026 Matrix Origin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package issues + +import ( + "context" + "database/sql" + "fmt" + "testing" + "time" + + _ "github.com/go-sql-driver/mysql" + "github.com/stretchr/testify/require" + + "github.com/matrixorigin/matrixone/pkg/embed" + "github.com/matrixorigin/matrixone/pkg/tests/testutils" +) + +func TestIssue27539DeleteSetNullMaintainsSecondaryIndex(t *testing.T) { + embed.RunBaseClusterTests(t, func(c embed.Cluster) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + + cn, err := c.GetCNService(0) + require.NoError(t, err) + db, err := sql.Open("mysql", fmt.Sprintf( + "dump:111@tcp(127.0.0.1:%d)/", cn.GetServiceConfig().CN.Frontend.Port)) + require.NoError(t, err) + defer db.Close() + conn, err := db.Conn(ctx) + require.NoError(t, err) + defer conn.Close() + + dbName := testutils.GetDatabaseName(t) + mustExec(t, ctx, conn, fmt.Sprintf("create database `%s`", dbName)) + mustExec(t, ctx, conn, fmt.Sprintf("use `%s`", dbName)) + defer func() { + cleanupCtx, cleanupCancel := context.WithTimeout(context.Background(), time.Minute) + defer cleanupCancel() + mustExec(t, cleanupCtx, conn, "use mo_catalog") + mustExec(t, cleanupCtx, conn, fmt.Sprintf("drop database if exists `%s`", dbName)) + }() + + createTables := func(t *testing.T, prefix string, indexed bool) (string, string, string) { + t.Helper() + parent := prefix + "_parent" + child := prefix + "_child" + indexDDL := "" + if indexed { + indexDDL = ", key idx_parent(parent_id, note)" + } + mustExec(t, ctx, conn, fmt.Sprintf( + "create table %s(id int primary key, code varchar(20) unique)", parent)) + mustExec(t, ctx, conn, fmt.Sprintf(`create table %s( + child_id int primary key, parent_id int, note varchar(30)%s, + constraint fk_%s foreign key(parent_id) references %s(id) on delete set null)`, + child, indexDDL, prefix, parent)) + mustExec(t, ctx, conn, fmt.Sprintf( + "insert into %s values (100, 'p100'), (200, 'p200')", parent)) + mustExec(t, ctx, conn, fmt.Sprintf( + "insert into %s values (1,100,'s100-a'), (2,200,'s200-a'), (3,200,'s200-b')", child)) + hiddenIndexTable := "" + if indexed { + require.NoError(t, conn.QueryRowContext(ctx, fmt.Sprintf(`select index_table_name + from mo_catalog.mo_indexes where column_name = 'parent_id' and index_table_name != '' and table_id = + (select rel_id from mo_catalog.mo_tables where reldatabase = database() and relname = '%s') + order by ordinal_position limit 1`, child)).Scan(&hiddenIndexTable)) + require.NotEmpty(t, hiddenIndexTable) + var hiddenCount int + require.NoError(t, conn.QueryRowContext(ctx, + fmt.Sprintf("select count(*) from `%s`", hiddenIndexTable)).Scan(&hiddenCount)) + require.Equal(t, 3, hiddenCount) + } + return parent, child, hiddenIndexTable + } + + assertSetNullState := func(t *testing.T, child, hiddenIndexTable string) { + t.Helper() + var parentID sql.NullInt64 + require.NoError(t, conn.QueryRowContext(ctx, + fmt.Sprintf("select parent_id from %s where child_id = 1", child)).Scan(&parentID)) + require.False(t, parentID.Valid) + + var count int + if hiddenIndexTable != "" { + require.NoError(t, conn.QueryRowContext(ctx, + fmt.Sprintf("select count(*) from `%s`", hiddenIndexTable)).Scan(&count)) + require.Equal(t, 3, count) + require.NoError(t, conn.QueryRowContext(ctx, fmt.Sprintf( + "select count(*) from %s force index(idx_parent) where parent_id is null and note = 's100-a'", child)).Scan(&count)) + require.Equal(t, 1, count) + require.NoError(t, conn.QueryRowContext(ctx, fmt.Sprintf( + "select count(*) from %s force index(idx_parent) where parent_id = 100 and note = 's100-a'", child)).Scan(&count)) + require.Zero(t, count) + require.NoError(t, conn.QueryRowContext(ctx, fmt.Sprintf( + "select count(*) from %s force index(idx_parent) where parent_id = 200 and note in ('s200-a', 's200-b')", child)).Scan(&count)) + require.Equal(t, 2, count) + return + } + require.NoError(t, conn.QueryRowContext(ctx, fmt.Sprintf( + "select count(*) from %s where parent_id = 100", child)).Scan(&count)) + require.Zero(t, count) + require.NoError(t, conn.QueryRowContext(ctx, fmt.Sprintf( + "select count(*) from %s where parent_id = 200", child)).Scan(&count)) + require.Equal(t, 2, count) + } + + assertOriginalState := func(t *testing.T, parent, child, hiddenIndexTable string) { + t.Helper() + var count int + require.NoError(t, conn.QueryRowContext(ctx, + fmt.Sprintf("select count(*) from %s where id = 100", parent)).Scan(&count)) + require.Equal(t, 1, count) + require.NoError(t, conn.QueryRowContext(ctx, fmt.Sprintf( + "select count(*) from %s force index(idx_parent) where parent_id = 100 and note = 's100-a'", child)).Scan(&count)) + require.Equal(t, 1, count) + require.NoError(t, conn.QueryRowContext(ctx, fmt.Sprintf( + "select count(*) from %s force index(idx_parent) where parent_id is null and note = 's100-a'", child)).Scan(&count)) + require.Zero(t, count) + require.NoError(t, conn.QueryRowContext(ctx, + fmt.Sprintf("select count(*) from `%s`", hiddenIndexTable)).Scan(&count)) + require.Equal(t, 3, count) + } + + testDelete := func(t *testing.T, prefix string, indexed, prepared bool) { + t.Helper() + parent, child, hiddenIndexTable := createTables(t, prefix, indexed) + deleteSQL := fmt.Sprintf("delete from %s where id = ?", parent) + if prepared { + stmt, err := conn.PrepareContext(ctx, deleteSQL) + require.NoError(t, err) + defer stmt.Close() + _, err = stmt.ExecContext(ctx, 100) + require.NoError(t, err) + } else { + mustExec(t, ctx, conn, fmt.Sprintf("delete from %s where id = 100", parent)) + } + + assertSetNullState(t, child, hiddenIndexTable) + } + + t.Run("indexed literal delete", func(t *testing.T) { + testDelete(t, "indexed_literal", true, false) + }) + t.Run("indexed prepared delete", func(t *testing.T) { + testDelete(t, "indexed_prepared", true, true) + }) + t.Run("no index control", func(t *testing.T) { + testDelete(t, "no_index", false, false) + }) + t.Run("rollback restores composite secondary index", func(t *testing.T) { + parent, child, hiddenIndexTable := createTables(t, "rollback", true) + mustExec(t, ctx, conn, "begin") + mustExec(t, ctx, conn, fmt.Sprintf("delete from %s where id = 100", parent)) + assertSetNullState(t, child, hiddenIndexTable) + mustExec(t, ctx, conn, "rollback") + assertOriginalState(t, parent, child, hiddenIndexTable) + }) + t.Run("failed delete preserves composite secondary index", func(t *testing.T) { + parent, child, hiddenIndexTable := createTables(t, "failed_delete", true) + blocker := "failed_delete_blocker" + mustExec(t, ctx, conn, fmt.Sprintf(`create table %s( + id int primary key, parent_id int, + foreign key(parent_id) references %s(id) on delete restrict)`, blocker, parent)) + mustExec(t, ctx, conn, fmt.Sprintf("insert into %s values(1, 100)", blocker)) + _, err := conn.ExecContext(ctx, fmt.Sprintf("delete from %s where id = 100", parent)) + require.Error(t, err) + assertOriginalState(t, parent, child, hiddenIndexTable) + }) + }) +}