Skip to content

Commit

Permalink
sql: assert that table writers use root txns
Browse files Browse the repository at this point in the history
We assume that all mutation statements use the Root txn, but this was
never enforced in practice. This commit adds an explicit check when
initializing the table writer that the root txn is provided.

Release note: None
  • Loading branch information
yuzefovich committed May 25, 2022
1 parent 709881d commit 1c45f17
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 8 deletions.
7 changes: 6 additions & 1 deletion pkg/sql/tablewriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/admission"
"github.com/cockroachdb/cockroach/pkg/util/admission/admissionpb"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/errors"
)

// expressionCarrier handles visiting sub-expressions.
Expand Down Expand Up @@ -155,7 +156,10 @@ var maxBatchBytes = settings.RegisterByteSizeSetting(

func (tb *tableWriterBase) init(
txn *kv.Txn, tableDesc catalog.TableDescriptor, evalCtx *eval.Context, settings *settings.Values,
) {
) error {
if txn.Type() != kv.RootTxn {
return errors.AssertionFailedf("unexpectedly non-root txn is used by the table writer")
}
tb.txn = txn
tb.desc = tableDesc
tb.lockTimeout = 0
Expand All @@ -171,6 +175,7 @@ func (tb *tableWriterBase) init(
tb.maxBatchByteSize = mutations.MaxBatchByteSize(batchMaxBytes, tb.forceProductionBatchSizes)
tb.sv = settings
tb.initNewBatch()
return nil
}

// setRowsWrittenLimit should be called before finalize whenever the
Expand Down
3 changes: 1 addition & 2 deletions pkg/sql/tablewriter_delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ func (td *tableDeleter) walkExprs(_ func(desc string, index int, expr tree.Typed
func (td *tableDeleter) init(
_ context.Context, txn *kv.Txn, evalCtx *eval.Context, sv *settings.Values,
) error {
td.tableWriterBase.init(txn, td.tableDesc(), evalCtx, sv)
return nil
return td.tableWriterBase.init(txn, td.tableDesc(), evalCtx, sv)
}

// row is part of the tableWriter interface.
Expand Down
3 changes: 1 addition & 2 deletions pkg/sql/tablewriter_insert.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ func (*tableInserter) desc() string { return "inserter" }
func (ti *tableInserter) init(
_ context.Context, txn *kv.Txn, evalCtx *eval.Context, sv *settings.Values,
) error {
ti.tableWriterBase.init(txn, ti.tableDesc(), evalCtx, sv)
return nil
return ti.tableWriterBase.init(txn, ti.tableDesc(), evalCtx, sv)
}

// row is part of the tableWriter interface.
Expand Down
3 changes: 1 addition & 2 deletions pkg/sql/tablewriter_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ func (*tableUpdater) desc() string { return "updater" }
func (tu *tableUpdater) init(
_ context.Context, txn *kv.Txn, evalCtx *eval.Context, sv *settings.Values,
) error {
tu.tableWriterBase.init(txn, tu.tableDesc(), evalCtx, sv)
return nil
return tu.tableWriterBase.init(txn, tu.tableDesc(), evalCtx, sv)
}

// row is part of the tableWriter interface.
Expand Down
4 changes: 3 additions & 1 deletion pkg/sql/tablewriter_upsert_opt.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@ var _ tableWriter = &optTableUpserter{}
func (tu *optTableUpserter) init(
ctx context.Context, txn *kv.Txn, evalCtx *eval.Context, sv *settings.Values,
) error {
tu.tableWriterBase.init(txn, tu.ri.Helper.TableDesc, evalCtx, sv)
if err := tu.tableWriterBase.init(txn, tu.ri.Helper.TableDesc, evalCtx, sv); err != nil {
return err
}

// rowsNeeded, set upon initialization, indicates pkg/sql/backfill.gowhether or not we want
// rows returned from the operation.
Expand Down

0 comments on commit 1c45f17

Please sign in to comment.