sql: update MaxBatchSize comments

The comments for MaxBatchSize incorrectly state that it limits the number
of KV batch entries. It actually limits the number of SQL-level batch rows.
It includes only primary index rows, not secondary index rows. This commit
updates all the comments to reflect what the code is actually doing.

Epic: none

Release note: None

Author: andy-kimball

pkg/sql/create_table.go

@@ -596,8 +596,8 @@ func (n *createTableNode) startExec(params runParams) error {
 					break
 				}
 
-				// Periodically flush out the batches, so that we don't issue gigantic
-				// raft commands.
+				// Periodically flush out the SQL-level batches, so that we don't
+				// issue gigantic raft commands.
 				if ti.currentBatchSize >= ti.maxBatchSize ||
 					ti.b.ApproximateMutationBytes() >= ti.maxBatchByteSize {
 					if err := ti.flushAndStartNewBatch(params.ctx); err != nil {

pkg/sql/delete.go

@@ -138,7 +138,7 @@ func (d *deleteNode) processBatch(params runParams) (lastBatch bool, err error)
 			return false, err
 		}
 
-		// Are we done yet with the current batch?
+		// Are we done yet with the current SQL-level batch?
 		if d.run.td.currentBatchSize >= d.run.td.maxBatchSize ||
 			d.run.td.b.ApproximateMutationBytes() >= d.run.td.maxBatchByteSize {
 			break

pkg/sql/insert.go

@@ -335,7 +335,7 @@ func (n *insertNode) processBatch(params runParams) (lastBatch bool, err error)
 			return false, err
 		}
 
-		// Are we done yet with the current batch?
+		// Are we done yet with the current SQL-level batch?
 		if n.run.ti.currentBatchSize >= n.run.ti.maxBatchSize ||
 			n.run.ti.b.ApproximateMutationBytes() >= n.run.ti.maxBatchByteSize {
 			break

pkg/sql/mutations/mutations_util.go

@@ -29,10 +29,9 @@ var testingMaxBatchByteSize = metamorphic.ConstantWithTestRange(
 	32<<20, /* max */
 )
 
-// MaxBatchSize returns the max number of entries in the KV batch for a
-// mutation operation (delete, insert, update, upsert) - including secondary
-// index updates, FK cascading updates, etc - before the current KV batch is
-// executed and a new batch is started.
+// MaxBatchSize returns the max number of rows in the SQL-level batch for a
+// mutation operation (delete, insert, update, upsert). This only includes
+// modified rows from the primary index, not any secondary index rows.
 //
 // If forceProductionMaxBatchSize is true, then the "production" value will be
 // returned regardless of whether the build is metamorphic or not. This should

pkg/sql/opt/exec/execbuilder/mutation.go

@@ -200,7 +200,7 @@ func (b *Builder) tryBuildFastPathInsert(
 	insInput := ins.Input
 	values, ok := insInput.(*memo.ValuesExpr)
 	// Values expressions containing subqueries or UDFs, or having a size larger
-	// than the max mutation batch size are disallowed.
+	// than the max mutation SQL-level batch size are disallowed.
 	if !ok || !memo.ValuesLegalForInsertFastPath(values) {
 		return execPlan{}, colOrdMap{}, false, nil
 	}

pkg/sql/opt/memo/insert_fastpath.go

@@ -15,14 +15,15 @@ import (
 )
 
 // ValuesLegalForInsertFastPath tests if `values` is a Values expression that
-// has no subqueries or UDFs and has less rows than the max number of entries in
-// a KV batch for a mutation operation.
+// has no subqueries or UDFs and has less rows than the max number of rows in
+// the SQL-level batch for a mutation operation.
 func ValuesLegalForInsertFastPath(values *ValuesExpr) bool {
 	//  - The input is Values with at most mutations.MaxBatchSize, and there are no
 	//    subqueries;
-	//    (note that mutations.MaxBatchSize() is a quantity of keys in the batch
-	//     that we send, not a number of rows. We use this as a guideline only,
-	//     and there is no guarantee that we won't produce a bigger batch.)
+	//    (note that mutations.MaxBatchSize() is a quantity of modified rows in
+	//     the SQL-level batch that we send, not the number of KV batch entries.
+	//     We use this as a guideline only, and there is no guarantee that we
+	//     won't produce a bigger batch.)
 	if values.ChildCount() > mutations.MaxBatchSize(false /* forceProductionMaxBatchSize */) ||
 		values.Relational().HasSubquery ||
 		values.Relational().HasUDF {

pkg/sql/tablewriter.go

@@ -49,12 +49,14 @@ type tableWriterBase struct {
 	// deadlockTimeout specifies the amount of time that the writer will wait
 	// on a lock before checking if there is a race condition.
 	deadlockTimeout time.Duration
-	// maxBatchSize determines the maximum number of entries in the KV batch
-	// for a mutation operation. By default, it will be set to 10k but can be
-	// a different value in tests.
+	// maxBatchSize determines the maximum number of rows in the SQL-level batch
+	// for a mutation operation. By default, it will be set to 10k but can be a
+	// different value in tests.
 	maxBatchSize int
 	// maxBatchByteSize determines the maximum number of key and value bytes in
 	// the KV batch for a mutation operation.
+	// NOTE: This is based on the bytes in the KV batch, while maxBatchSize is
+	// based on the rows in the SQL-level batch.
 	maxBatchByteSize int
 	// currentBatchSize is the size of the current SQL-level batch (i.e. not the
 	// KV-level batch). It is updated on every row() call and is reset once a new

pkg/sql/update.go

@@ -146,7 +146,7 @@ func (u *updateNode) processBatch(params runParams) (lastBatch bool, err error)
 			return false, err
 		}
 
-		// Are we done yet with the current batch?
+		// Are we done yet with the current SQL-level batch?
 		if u.run.tu.currentBatchSize >= u.run.tu.maxBatchSize ||
 			u.run.tu.b.ApproximateMutationBytes() >= u.run.tu.maxBatchByteSize {
 			break

pkg/sql/upsert.go

@@ -106,7 +106,7 @@ func (n *upsertNode) processBatch(params runParams) (lastBatch bool, err error)
 			return false, err
 		}
 
-		// Are we done yet with the current batch?
+		// Are we done yet with the current SQL-level batch?
 		if n.run.tw.currentBatchSize >= n.run.tw.maxBatchSize ||
 			n.run.tw.b.ApproximateMutationBytes() >= n.run.tw.maxBatchByteSize {
 			break