Skip to content

Commit

Permalink
sql: fix queuing behavior for checks and triggers
Browse files Browse the repository at this point in the history
This commit fixes a bug in `PlanAndRunPostQueries` that could cause
check queries to be run more than once. This would cause problems with
unclosed resources after query completion. This commit also fixes a
minor bug that could cause triggers queued by triggers to be run
before newly-queued checks and cascades, instead of after. The following
commit includes a regression test.

Fixes #133792

Release note: None
  • Loading branch information
DrewKimball committed Nov 5, 2024
1 parent e26cd44 commit c00f562
Showing 1 changed file with 5 additions and 3 deletions.
8 changes: 5 additions & 3 deletions pkg/sql/distsql_running.go
Original file line number Diff line number Diff line change
Expand Up @@ -2204,15 +2204,16 @@ func (dsp *DistSQLPlanner) PlanAndRunPostQueries(
// TODO(yuzefovich): the planObserver logic in
// planAndRunChecksInParallel will need to be adjusted when we switch to
// using the DistSQL spec factory.
for i := cascadesIdx; i < len(plan.checkPlans); i++ {
for i := checksIdx; i < len(plan.checkPlans); i++ {
if plan.checkPlans[i].plan.isPhysicalPlan() {
runParallelChecks = false
break
}
}
}
if runParallelChecks {
if err := dsp.planAndRunChecksInParallel(ctx, plan.checkPlans, planner, evalCtxFactory, recv); err != nil {
checksToRun := plan.checkPlans[checksIdx:]
if err := dsp.planAndRunChecksInParallel(ctx, checksToRun, planner, evalCtxFactory, recv); err != nil {
recv.SetError(err)
return false
}
Expand Down Expand Up @@ -2245,7 +2246,8 @@ func (dsp *DistSQLPlanner) PlanAndRunPostQueries(
}

// Finally, run triggers.
for ; triggersIdx < len(plan.triggers); triggersIdx++ {
numTriggers := len(plan.triggers)
for ; triggersIdx < numTriggers; triggersIdx++ {
trigger := &plan.triggers[triggersIdx]
hasBuffer, numBufferedRows := checkPostQueryBuffer(plan.triggers[triggersIdx])
if hasBuffer && numBufferedRows == 0 {
Expand Down

0 comments on commit c00f562

Please sign in to comment.