Skip to content

Commit 5a9967b

Browse files
michae2michae2
committed
sql/row: order primary index deletes before secondary index deletes
Change row.Deleter to order writes to the primary index first, before writes to the secondary index. This matches row.Inserter and row.Updater. This is needed to assist cockroachdb#147117 in achieving deterministic error ordering for LDR and DeleteSwap. Informs: cockroachdb#144503, cockroachdb#146117 Release note: None
1 parent fdb713a commit 5a9967b

File tree

4 files changed

+53
-48
lines changed

4 files changed

+53
-48
lines changed

‎pkg/sql/opt/exec/execbuilder/testdata/partial_index

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -275,27 +275,27 @@ query T kvtrace
275275
DELETE FROM t WHERE a = 5
276276
----
277277
Scan /Table/113/1/5/0 lock Exclusive (Block, Unreplicated)
278-
Del /Table/113/2/4/5/0
279278
Del /Table/113/1/5/0
279+
Del /Table/113/2/4/5/0
280280

281281
# Deleted row matches the first partial index.
282282
query T kvtrace
283283
DELETE FROM t WHERE a = 6
284284
----
285285
Scan /Table/113/1/6/0 lock Exclusive (Block, Unreplicated)
286+
Del /Table/113/1/6/0
286287
Del /Table/113/2/11/6/0
287288
Del /Table/113/3/11/6/0
288-
Del /Table/113/1/6/0
289289

290290
# Deleted row matches both partial indexes.
291291
query T kvtrace
292292
DELETE FROM t WHERE a = 12
293293
----
294294
Scan /Table/113/1/12/0 lock Exclusive (Block, Unreplicated)
295+
Del /Table/113/1/12/0
295296
Del /Table/113/2/11/12/0
296297
Del /Table/113/3/11/12/0
297298
Del /Table/113/4/"foo"/12/0
298-
Del /Table/113/1/12/0
299299

300300
# Deleted row does not match partial index with predicate column that is not
301301
# indexed.
@@ -311,8 +311,8 @@ query T kvtrace
311311
DELETE FROM u WHERE k = 2
312312
----
313313
Scan /Table/114/1/2/0 lock Exclusive (Block, Unreplicated)
314-
Del /Table/114/2/3/2/0
315314
Del /Table/114/1/2/0
315+
Del /Table/114/2/3/2/0
316316

317317
# ---------------------------------------------------------
318318
# UPDATE
@@ -900,9 +900,9 @@ query T kvtrace
900900
DELETE FROM inv WHERE a = 1
901901
----
902902
Scan /Table/107/1/1/0 lock Exclusive (Block, Unreplicated)
903+
Del /Table/107/1/1/0
903904
Del /Table/107/2/"num"/1/1/0
904905
Del /Table/107/2/"x"/"y"/1/0
905-
Del /Table/107/1/1/0
906906

907907
query T kvtrace
908908
DELETE FROM inv WHERE a = 2
@@ -1083,9 +1083,9 @@ DELETE FROM t57085_p WHERE p = 1;
10831083
----
10841084
Del (locking) /Table/115/1/1/0
10851085
Scan /Table/116/{1-2}
1086-
Del /Table/116/2/1/10/0
10871086
Del (locking) /Table/116/1/10/0
10881087
Del (locking) /Table/116/1/20/0
1088+
Del /Table/116/2/1/10/0
10891089

10901090
# Tests for partial vector indexes.
10911091

@@ -1157,8 +1157,8 @@ DELETE FROM vec WHERE a = 10
11571157
----
11581158
Scan /Table/108/1/10/0
11591159
Scan /Table/108/2/"foo"/{1/1-2}
1160-
Del /Table/108/2/"foo"/1/1/10/0
11611160
Del (locking) /Table/108/1/10/0
1161+
Del /Table/108/2/"foo"/1/1/10/0
11621162

11631163
query T kvtrace
11641164
DELETE FROM vec WHERE a = 20

‎pkg/sql/row/deleter.go

Lines changed: 40 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -146,47 +146,14 @@ func (rd *Deleter) DeleteRow(
146146
) error {
147147
b := &KVBatchAdapter{Batch: batch}
148148

149-
// Delete the row from any secondary indices.
150-
for i, index := range rd.Helper.Indexes {
151-
// If the index ID exists in the set of indexes to ignore, do not
152-
// attempt to delete from the index.
153-
if pm.IgnoreForDel.Contains(int(index.GetID())) {
154-
continue
155-
}
156-
157-
// We want to include empty k/v pairs because we want to delete all k/v's for this row.
158-
entries, err := rowenc.EncodeSecondaryIndex(
159-
ctx,
160-
rd.Helper.Codec,
161-
rd.Helper.TableDesc,
162-
index,
163-
rd.FetchColIDtoRowIndex,
164-
values,
165-
vh.GetDel(),
166-
true, /* includeEmpty */
167-
)
168-
if err != nil {
169-
return err
170-
}
171-
alreadyLocked := rd.secondaryLocked != nil && rd.secondaryLocked.GetID() == index.GetID()
172-
for _, e := range entries {
173-
if err = rd.Helper.deleteIndexEntry(
174-
ctx, b, index, &e.Key, alreadyLocked, rd.Helper.sd.BufferedWritesUseLockingOnNonUniqueIndexes,
175-
traceKV, rd.Helper.secIndexValDirs[i],
176-
); err != nil {
177-
return err
178-
}
179-
}
180-
}
181-
182149
primaryIndexKey, err := rd.Helper.encodePrimaryIndexKey(rd.FetchColIDtoRowIndex, values)
183150
if err != nil {
184151
return err
185152
}
186153

187-
// Delete the row.
154+
// Delete the row from the primary index.
188155
var called bool
189-
return rd.Helper.TableDesc.ForeachFamily(func(family *descpb.ColumnFamilyDescriptor) error {
156+
err = rd.Helper.TableDesc.ForeachFamily(func(family *descpb.ColumnFamilyDescriptor) error {
190157
if called {
191158
// HACK: MakeFamilyKey appends to its argument, so on every loop iteration
192159
// after the first, trim primaryIndexKey so nothing gets overwritten.
@@ -221,6 +188,44 @@ func (rd *Deleter) DeleteRow(
221188
rd.key = nil
222189
return nil
223190
})
191+
if err != nil {
192+
return err
193+
}
194+
195+
// Delete the row from any secondary indices.
196+
for i, index := range rd.Helper.Indexes {
197+
// If the index ID exists in the set of indexes to ignore, do not
198+
// attempt to delete from the index.
199+
if pm.IgnoreForDel.Contains(int(index.GetID())) {
200+
continue
201+
}
202+
203+
// We want to include empty k/v pairs because we want to delete all k/v's for this row.
204+
entries, err := rowenc.EncodeSecondaryIndex(
205+
ctx,
206+
rd.Helper.Codec,
207+
rd.Helper.TableDesc,
208+
index,
209+
rd.FetchColIDtoRowIndex,
210+
values,
211+
vh.GetDel(),
212+
true, /* includeEmpty */
213+
)
214+
if err != nil {
215+
return err
216+
}
217+
alreadyLocked := rd.secondaryLocked != nil && rd.secondaryLocked.GetID() == index.GetID()
218+
for _, e := range entries {
219+
if err = rd.Helper.deleteIndexEntry(
220+
ctx, b, index, &e.Key, alreadyLocked, rd.Helper.sd.BufferedWritesUseLockingOnNonUniqueIndexes,
221+
traceKV, rd.Helper.secIndexValDirs[i],
222+
); err != nil {
223+
return err
224+
}
225+
}
226+
}
227+
228+
return nil
224229
}
225230

226231
// encodeValueForPrimaryIndexFamily encodes the expected roachpb.Value

‎pkg/sql/testdata/index_mutations/delete_preserving_delete

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ kvtrace
2222
DELETE FROM ti WHERE a = 1
2323
----
2424
Scan /Table/106/1/1/0 lock Exclusive (Block, Unreplicated)
25-
Put (delete) /Table/106/2/2/100/1/0
2625
Del /Table/106/1/1/0
26+
Put (delete) /Table/106/2/2/100/1/0
2727

2828
# ---------------------------------------------------------
2929
# Partial Index With Delete Preserving Encoding
@@ -57,8 +57,8 @@ kvtrace
5757
DELETE FROM tpi WHERE a = 3
5858
----
5959
Scan /Table/107/1/3/0 lock Exclusive (Block, Unreplicated)
60-
Put (delete) /Table/107/2/"foo"/3/0
6160
Del /Table/107/1/3/0
61+
Put (delete) /Table/107/2/"foo"/3/0
6262

6363
# ---------------------------------------------------------
6464
# Expression Index With Delete Preserving Encoding
@@ -84,8 +84,8 @@ kvtrace
8484
DELETE FROM tei WHERE a + b = 102
8585
----
8686
Scan /Table/108/{1-2}
87-
Put (delete) /Table/108/2/102/1/0
8887
Del (locking) /Table/108/1/1/0
88+
Put (delete) /Table/108/2/102/1/0
8989

9090
# ---------------------------------------------------------
9191
# Inverted Index With Delete Preserving Encoding
@@ -111,11 +111,11 @@ kvtrace
111111
DELETE FROM tii WHERE a = 1
112112
----
113113
Scan /Table/109/1/1/0 lock Exclusive (Block, Unreplicated)
114+
Del /Table/109/1/1/0
114115
Put (delete) /Table/109/2/NULL/1/0
115116
Put (delete) /Table/109/2/1/1/0
116117
Put (delete) /Table/109/2/2/1/0
117118
Put (delete) /Table/109/2/3/1/0
118-
Del /Table/109/1/1/0
119119

120120
# ---------------------------------------------------------
121121
# Multicolumn Inverted Index With Delete Preserving Encoding
@@ -142,6 +142,6 @@ kvtrace
142142
DELETE FROM tmi WHERE a = 1
143143
----
144144
Scan /Table/110/1/1/0 lock Exclusive (Block, Unreplicated)
145+
Del /Table/110/1/1/0
145146
Put (delete) /Table/110/2/2/"a"/"foo"/1/0
146147
Put (delete) /Table/110/2/2/"b"/"bar"/1/0
147-
Del /Table/110/1/1/0

‎pkg/sql/testdata/index_mutations/delete_preserving_unique_index

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ kvtrace
2424
DELETE FROM ti WHERE a = 1
2525
----
2626
Scan /Table/106/1/1/0 lock Exclusive (Block, Unreplicated)
27-
Put (delete) (locking) /Table/106/2/1/100/0
2827
Del /Table/106/1/1/0
28+
Put (delete) (locking) /Table/106/2/1/100/0
2929

3030
kvtrace
3131
INSERT INTO ti VALUES (1, 1, 100)

0 commit comments

Comments
 (0)