Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Disallow Insert with Duplicate key update and Replace Into queries on foreign key column, set locks on fk queries #13953

Merged
merged 10 commits into from
Sep 13, 2023
Merged

Disallow Insert with Duplicate key update and Replace Into queries on foreign key column, set locks on fk queries #13953

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
cbf44c0
moved code to operators
harshit-gangal Sep 11, 2023
44c41a8
moved around insert operator code
harshit-gangal Sep 11, 2023
a442c37
disallow insert on duplicate key update with foreign keys
harshit-gangal Sep 11, 2023
343fb89
reject update query only when fk columns have non-literal value
harshit-gangal Sep 12, 2023
b1e2910
fk required when unsharded table references sharded table
harshit-gangal Sep 12, 2023
7093339
reject replace into queries with fk
harshit-gangal Sep 12, 2023
f36cad7
addressed review comment
harshit-gangal Sep 12, 2023
ea92db1
fix cases for error messages
harshit-gangal Sep 12, 2023
5cdebad
set lock and comments on route, set lock in share mode for fk verify,…
harshit-gangal Sep 12, 2023
9fa5530
addressed review comments
harshit-gangal Sep 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 19 additions & 16 deletions go/vt/vtgate/planbuilder/insert.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,11 @@ func gen4InsertStmtPlanner(version querypb.ExecuteOptions_PlannerVersion, insStm
// Check single unsharded. Even if the table is for single unsharded but sequence table is used.
// We cannot shortcut here as sequence column needs additional planning.
ks, tables := ctx.SemTable.SingleUnshardedKeyspace()
if ks != nil && tables[0].AutoIncrement == nil {
if fkManagementNotRequiredForInsert(ctx, tables[0], sqlparser.UpdateExprs(insStmt.OnDup), insStmt.Action == sqlparser.ReplaceAct) {
FkPlanNeeded := false
harshit-gangal marked this conversation as resolved.
Show resolved Hide resolved
if ks != nil {
noAutoInc := tables[0].AutoIncrement == nil
FkPlanNeeded = fkManagementRequiredForInsert(ctx, tables[0], sqlparser.UpdateExprs(insStmt.OnDup), insStmt.Action == sqlparser.ReplaceAct)
if noAutoInc && !FkPlanNeeded {
plan := insertUnshardedShortcut(insStmt, ks, tables)
plan = pushCommentDirectivesOnPlan(plan, insStmt)
return newPlanResult(plan.Primitive(), operators.QualifiedTables(ks, tables)...), nil
Expand All @@ -58,7 +61,7 @@ func gen4InsertStmtPlanner(version querypb.ExecuteOptions_PlannerVersion, insStm
return nil, err
}

if err = errOutIfPlanCannotBeConstructed(ctx, tblInfo.GetVindexTable(), insStmt); err != nil {
if err = errOutIfPlanCannotBeConstructed(ctx, tblInfo.GetVindexTable(), insStmt, FkPlanNeeded); err != nil {
return nil, err
}

Expand Down Expand Up @@ -88,37 +91,37 @@ func gen4InsertStmtPlanner(version querypb.ExecuteOptions_PlannerVersion, insStm
return newPlanResult(plan.Primitive(), operators.TablesUsed(op)...), nil
}

func errOutIfPlanCannotBeConstructed(ctx *plancontext.PlanningContext, vTbl *vindexes.Table, insStmt *sqlparser.Insert) error {
if insStmt.Action == sqlparser.ReplaceAct {
if vTbl.AutoIncrement != nil {
return vterrors.VT12001("REPLACE INTO with auto-increment column")
}
return vterrors.VT12001("REPLACE INTO with foreign keys")
}
if vTbl.Keyspace.Sharded {
func errOutIfPlanCannotBeConstructed(ctx *plancontext.PlanningContext, vTbl *vindexes.Table, insStmt *sqlparser.Insert, fkPlanNeeded bool) error {
if vTbl.Keyspace.Sharded && ctx.SemTable.NotUnshardedErr != nil {
return ctx.SemTable.NotUnshardedErr
}
if insStmt.Action != sqlparser.ReplaceAct {
return nil
}
if fkPlanNeeded {
return vterrors.VT12001("REPLACE INTO with foreign keys")
}
return nil
}

// TODO: Handle all this in semantic analysis.
func fkManagementNotRequiredForInsert(ctx *plancontext.PlanningContext, vTbl *vindexes.Table, updateExprs sqlparser.UpdateExprs, replace bool) bool {
func fkManagementRequiredForInsert(ctx *plancontext.PlanningContext, vTbl *vindexes.Table, updateExprs sqlparser.UpdateExprs, replace bool) bool {
ksMode, err := ctx.VSchema.ForeignKeyMode(vTbl.Keyspace.Name)
if err != nil || ksMode != vschemapb.Keyspace_FK_MANAGED {
return true
return false
}

if len(vTbl.ParentFKsNeedsHandling(ctx.VerifyAllFKs, "")) > 0 {
return false
return true
}

childFks := vTbl.ChildFKsNeedsHandling(ctx.VerifyAllFKs, vindexes.UpdateAction)
if len(childFks) > 0 && replace {
return false
return true
}

// Check if any column in the parent table is being updated which has a child foreign key.
return !columnModified(updateExprs, func(expr *sqlparser.UpdateExpr) ([]vindexes.ParentFKInfo, []vindexes.ChildFKInfo) {
return columnModified(updateExprs, func(expr *sqlparser.UpdateExpr) ([]vindexes.ParentFKInfo, []vindexes.ChildFKInfo) {
return nil, childFks
})
}
Expand Down
Loading