Merge #151563

151563: sql: fix "rows affected" for deleteRange with multiple column families r=yuzefovich a=yuzefovich

In 807c1f2 we began paginating DelRange requests that are used by the deleteRangeNode. Namely, we now delete at most 600 keys via a single BatchRequest. That change introduced a minor bug that when we have multiple column families AND the pagination stops in the middle of a SQL row, then we would count that row twice for the purposes of "rows affected" counter. This bug is now fixed by maintaining the row prefix across BatchRequests.

Fixes: #151505.

Release note (bug fix): CockroachDB could previously elevate the number of rows deleted on table with multiple column families in some cases. The bug has been present since 19.2 version and is now fixed. Note that the data was deleted correctly, just "rows affected" number was wrong.

Co-authored-by: Yahor Yuzefovich <[email protected]>

Author: craig[bot]

pkg/sql/delete_range.go

@@ -47,6 +47,11 @@ type deleteRangeNode struct {
 
 	// rowCount will be set to the count of rows deleted.
 	rowCount int
+
+	// curRowPrefix is the prefix for all KVs (i.e. for all column families) of
+	// the SQL row that increased rowCount last. It is maintained across
+	// different BatchRequests in order to not double count the same SQL row.
+	curRowPrefix []byte
 }
 
 var _ planNode = &deleteRangeNode{}
@@ -194,10 +199,12 @@ func (d *deleteRangeNode) processResults(
 	results []kv.Result, resumeSpans []roachpb.Span,
 ) (roachpb.Spans, error) {
 	for _, r := range results {
-		var prev []byte
+		// TODO(yuzefovich): when the table has 1 column family, we don't need
+		// to compare the key prefixes since each deleted key corresponds to a
+		// different deleted row.
 		for _, keyBytes := range r.Keys {
 			// If prefix is same, don't bother decoding key.
-			if len(prev) > 0 && bytes.HasPrefix(keyBytes, prev) {
+			if len(d.curRowPrefix) > 0 && bytes.HasPrefix(keyBytes, d.curRowPrefix) {
 				continue
 			}
 
@@ -206,8 +213,8 @@ func (d *deleteRangeNode) processResults(
 				return nil, err
 			}
 			k := keyBytes[:len(keyBytes)-len(after)]
-			if !bytes.Equal(k, prev) {
-				prev = k
+			if !bytes.Equal(k, d.curRowPrefix) {
+				d.curRowPrefix = k
 				d.rowCount++
 			}
 		}

pkg/sql/logictest/testdata/logic_test/delete

@@ -603,3 +603,24 @@ statement error pgcode 42803 sum\(\): aggregate functions are not allowed in ORD
 DELETE FROM t108166 ORDER BY COALESCE(sum(a), 1) LIMIT 1;
 
 subtest end
+
+# Regression test for double-counting some rows for "rows affected" statistic in
+# deleteRange when a single SQL row is deleted via different KV batches.
+subtest regression_151505
+
+statement ok
+CREATE TABLE t (
+  k INT PRIMARY KEY,
+  c INT,
+  FAMILY (k),
+  FAMILY (c)
+);
+
+statement ok
+INSERT INTO t SELECT i, NULL FROM generate_series(1, 599) AS g(i);
+INSERT INTO t VALUES (600, 2);
+
+statement count 600
+DELETE FROM t WHERE k > 0 AND k < 1000;
+
+subtest end