Skip to content

Commit 4ff7e87

Browse files
rafissrafiss
committed
ttljob: avoid using different versions of enum
This patch changes the SELECT/DELETE queries so they use the logical string representation of enums, rather than the encoded datum. This avoids an issue that was caused by the enum descriptor's version being passed in as part of the query placeholder argument. Release note (bug fix): Fixed a bug that could cause the row level TTL job to fail with a "comparison of two different versions of enum" error if an ENUM type referenced by the table experienced a schema change.
1 parent d02450c commit 4ff7e87

File tree

11 files changed

+100
-77
lines changed

11 files changed

+100
-77
lines changed

‎pkg/sql/ttl/ttlbase/BUILD.bazel‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ go_library(
1313
"//pkg/sql/catalog/catenumpb",
1414
"//pkg/sql/catalog/catpb",
1515
"//pkg/sql/catalog/tabledesc",
16+
"//pkg/sql/types",
1617
"@com_github_cockroachdb_errors//:errors",
1718
],
1819
)
@@ -23,6 +24,7 @@ go_test(
2324
embed = [":ttlbase"],
2425
deps = [
2526
"//pkg/sql/catalog/catenumpb",
27+
"//pkg/sql/types",
2628
"//pkg/util/leaktest",
2729
"//pkg/util/log",
2830
"@com_github_stretchr_testify//require",

‎pkg/sql/ttl/ttlbase/ttl_helpers.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catenumpb"
1717
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
1818
"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
19+
"github.com/cockroachdb/cockroach/pkg/sql/types"
1920
"github.com/cockroachdb/errors"
2021
)
2122

@@ -169,6 +170,7 @@ func BuildSelectQuery(
169170
relationName string,
170171
pkColNames []string,
171172
pkColDirs []catenumpb.IndexColumn_Direction,
173+
pkColTypes []*types.T,
172174
aostDuration time.Duration,
173175
ttlExpr catpb.Expression,
174176
numStartQueryBounds, numEndQueryBounds int,
@@ -217,6 +219,8 @@ func BuildSelectQuery(
217219
buf.WriteString(pkColNames[j])
218220
buf.WriteString(" = $")
219221
buf.WriteString(strconv.Itoa(j + placeholderOffset))
222+
buf.WriteString("::")
223+
buf.WriteString(pkColTypes[j].SQLStringFullyQualified())
220224
buf.WriteString(" AND ")
221225
}
222226
buf.WriteString(pkColNames[i])
@@ -227,6 +231,8 @@ func BuildSelectQuery(
227231
}
228232
buf.WriteString(" $")
229233
buf.WriteString(strconv.Itoa(i + placeholderOffset))
234+
buf.WriteString("::")
235+
buf.WriteString(pkColTypes[i].SQLStringFullyQualified())
230236
buf.WriteString(")")
231237
if !isLast {
232238
buf.WriteString(" OR")

‎pkg/sql/ttl/ttlbase/ttl_helpers_test.go‎

Lines changed: 46 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"testing"
1010

1111
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catenumpb"
12+
"github.com/cockroachdb/cockroach/pkg/sql/types"
1213
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
1314
"github.com/cockroachdb/cockroach/pkg/util/log"
1415
"github.com/stretchr/testify/require"
@@ -43,10 +44,10 @@ FROM relation_name
4344
AS OF SYSTEM TIME INTERVAL '-30 seconds'
4445
WHERE ((expire_at) <= $1)
4546
AND (
46-
(col0 > $3)
47+
(col0 > $3::INT8)
4748
)
4849
AND (
49-
(col0 <= $2)
50+
(col0 <= $2::INT8)
5051
)
5152
ORDER BY col0 ASC
5253
LIMIT 2`,
@@ -63,10 +64,10 @@ FROM relation_name
6364
AS OF SYSTEM TIME INTERVAL '-30 seconds'
6465
WHERE ((expire_at) <= $1)
6566
AND (
66-
(col0 < $3)
67+
(col0 < $3::INT8)
6768
)
6869
AND (
69-
(col0 >= $2)
70+
(col0 >= $2::INT8)
7071
)
7172
ORDER BY col0 DESC
7273
LIMIT 2`,
@@ -113,10 +114,10 @@ FROM relation_name
113114
AS OF SYSTEM TIME INTERVAL '-30 seconds'
114115
WHERE ((expire_at) <= $1)
115116
AND (
116-
(col0 >= $3)
117+
(col0 >= $3::INT8)
117118
)
118119
AND (
119-
(col0 <= $2)
120+
(col0 <= $2::INT8)
120121
)
121122
ORDER BY col0 ASC
122123
LIMIT 2`,
@@ -134,10 +135,10 @@ FROM relation_name
134135
AS OF SYSTEM TIME INTERVAL '-30 seconds'
135136
WHERE ((expire_at) <= $1)
136137
AND (
137-
(col0 <= $3)
138+
(col0 <= $3::INT8)
138139
)
139140
AND (
140-
(col0 >= $2)
141+
(col0 >= $2::INT8)
141142
)
142143
ORDER BY col0 DESC
143144
LIMIT 2`,
@@ -155,12 +156,12 @@ FROM relation_name
155156
AS OF SYSTEM TIME INTERVAL '-30 seconds'
156157
WHERE ((expire_at) <= $1)
157158
AND (
158-
(col0 > $4) OR
159-
(col0 = $4 AND col1 > $5)
159+
(col0 > $4::INT8) OR
160+
(col0 = $4::INT8 AND col1 > $5::INT8)
160161
)
161162
AND (
162-
(col0 < $2) OR
163-
(col0 = $2 AND col1 <= $3)
163+
(col0 < $2::INT8) OR
164+
(col0 = $2::INT8 AND col1 <= $3::INT8)
164165
)
165166
ORDER BY col0 ASC, col1 ASC
166167
LIMIT 2`,
@@ -178,11 +179,11 @@ FROM relation_name
178179
AS OF SYSTEM TIME INTERVAL '-30 seconds'
179180
WHERE ((expire_at) <= $1)
180181
AND (
181-
(col0 > $4)
182+
(col0 > $4::INT8)
182183
)
183184
AND (
184-
(col0 < $2) OR
185-
(col0 = $2 AND col1 <= $3)
185+
(col0 < $2::INT8) OR
186+
(col0 = $2::INT8 AND col1 <= $3::INT8)
186187
)
187188
ORDER BY col0 ASC, col1 ASC
188189
LIMIT 2`,
@@ -200,11 +201,11 @@ FROM relation_name
200201
AS OF SYSTEM TIME INTERVAL '-30 seconds'
201202
WHERE ((expire_at) <= $1)
202203
AND (
203-
(col0 > $3) OR
204-
(col0 = $3 AND col1 > $4)
204+
(col0 > $3::INT8) OR
205+
(col0 = $3::INT8 AND col1 > $4::INT8)
205206
)
206207
AND (
207-
(col0 <= $2)
208+
(col0 <= $2::INT8)
208209
)
209210
ORDER BY col0 ASC, col1 ASC
210211
LIMIT 2`,
@@ -222,12 +223,12 @@ FROM relation_name
222223
AS OF SYSTEM TIME INTERVAL '-30 seconds'
223224
WHERE ((expire_at) <= $1)
224225
AND (
225-
(col0 < $4) OR
226-
(col0 = $4 AND col1 < $5)
226+
(col0 < $4::INT8) OR
227+
(col0 = $4::INT8 AND col1 < $5::INT8)
227228
)
228229
AND (
229-
(col0 > $2) OR
230-
(col0 = $2 AND col1 >= $3)
230+
(col0 > $2::INT8) OR
231+
(col0 = $2::INT8 AND col1 >= $3::INT8)
231232
)
232233
ORDER BY col0 DESC, col1 DESC
233234
LIMIT 2`,
@@ -245,11 +246,11 @@ FROM relation_name
245246
AS OF SYSTEM TIME INTERVAL '-30 seconds'
246247
WHERE ((expire_at) <= $1)
247248
AND (
248-
(col0 < $4)
249+
(col0 < $4::INT8)
249250
)
250251
AND (
251-
(col0 > $2) OR
252-
(col0 = $2 AND col1 >= $3)
252+
(col0 > $2::INT8) OR
253+
(col0 = $2::INT8 AND col1 >= $3::INT8)
253254
)
254255
ORDER BY col0 DESC, col1 DESC
255256
LIMIT 2`,
@@ -267,11 +268,11 @@ FROM relation_name
267268
AS OF SYSTEM TIME INTERVAL '-30 seconds'
268269
WHERE ((expire_at) <= $1)
269270
AND (
270-
(col0 < $3) OR
271-
(col0 = $3 AND col1 < $4)
271+
(col0 < $3::INT8) OR
272+
(col0 = $3::INT8 AND col1 < $4::INT8)
272273
)
273274
AND (
274-
(col0 >= $2)
275+
(col0 >= $2::INT8)
275276
)
276277
ORDER BY col0 DESC, col1 DESC
277278
LIMIT 2`,
@@ -290,14 +291,14 @@ FROM relation_name
290291
AS OF SYSTEM TIME INTERVAL '-30 seconds'
291292
WHERE ((expire_at) <= $1)
292293
AND (
293-
(col0 > $5) OR
294-
(col0 = $5 AND col1 < $6) OR
295-
(col0 = $5 AND col1 = $6 AND col2 > $7)
294+
(col0 > $5::INT8) OR
295+
(col0 = $5::INT8 AND col1 < $6::INT8) OR
296+
(col0 = $5::INT8 AND col1 = $6::INT8 AND col2 > $7::INT8)
296297
)
297298
AND (
298-
(col0 < $2) OR
299-
(col0 = $2 AND col1 > $3) OR
300-
(col0 = $2 AND col1 = $3 AND col2 <= $4)
299+
(col0 < $2::INT8) OR
300+
(col0 = $2::INT8 AND col1 > $3::INT8) OR
301+
(col0 = $2::INT8 AND col1 = $3::INT8 AND col2 <= $4::INT8)
301302
)
302303
ORDER BY col0 ASC, col1 DESC, col2 ASC
303304
LIMIT 2`,
@@ -316,14 +317,14 @@ FROM relation_name
316317
AS OF SYSTEM TIME INTERVAL '-30 seconds'
317318
WHERE ((expire_at) <= $1)
318319
AND (
319-
(col0 < $5) OR
320-
(col0 = $5 AND col1 > $6) OR
321-
(col0 = $5 AND col1 = $6 AND col2 < $7)
320+
(col0 < $5::INT8) OR
321+
(col0 = $5::INT8 AND col1 > $6::INT8) OR
322+
(col0 = $5::INT8 AND col1 = $6::INT8 AND col2 < $7::INT8)
322323
)
323324
AND (
324-
(col0 > $2) OR
325-
(col0 = $2 AND col1 < $3) OR
326-
(col0 = $2 AND col1 = $3 AND col2 >= $4)
325+
(col0 > $2::INT8) OR
326+
(col0 = $2::INT8 AND col1 < $3::INT8) OR
327+
(col0 = $2::INT8 AND col1 = $3::INT8 AND col2 >= $4::INT8)
327328
)
328329
ORDER BY col0 DESC, col1 ASC, col2 DESC
329330
LIMIT 2`,
@@ -334,10 +335,15 @@ LIMIT 2`,
334335
t.Run(tc.desc, func(t *testing.T) {
335336
pkColDirs := tc.pkColDirs
336337
pkColNames := GenPKColNames(len(pkColDirs))
338+
pkColTypes := make([]*types.T, len(pkColDirs))
339+
for i := range pkColDirs {
340+
pkColTypes[i] = types.Int
341+
}
337342
actualQuery := BuildSelectQuery(
338343
relationName,
339344
pkColNames,
340345
pkColDirs,
346+
pkColTypes,
341347
DefaultAOSTDuration,
342348
ttlExpr,
343349
tc.numStartQueryBounds,

‎pkg/sql/ttl/ttljob/testdata/ttljob_plans‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@ FROM defaultdb.public.p@"p_pkey"
1818
AS OF SYSTEM TIME INTERVAL ''0 seconds''
1919
WHERE ((crdb_internal_expiration) <= ''2024-01-01 00:00:00'')
2020
AND (
21-
(id >= 0)
21+
(id >= 0::INT8)
2222
)
2323
AND (
24-
(id <= 100)
24+
(id <= 100::INT8)
2525
)
2626
ORDER BY id ASC
2727
LIMIT 500

‎pkg/sql/ttl/ttljob/ttljob_plans_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ func TestQueryPlansDataDriven(t *testing.T) {
125125
var tableID int64
126126
row := runner.QueryRow(t, fmt.Sprintf("SELECT '%s'::REGCLASS::OID;", tableName))
127127
row.Scan(&tableID)
128-
relationName, _, pkColNames, _, pkColDirs, _, _, err := getTableInfo(
128+
relationName, _, pkColNames, pkColTypes, pkColDirs, _, _, err := getTableInfo(
129129
ctx, db, descpb.ID(tableID),
130130
)
131131
require.NoError(t, err)
@@ -137,6 +137,7 @@ func TestQueryPlansDataDriven(t *testing.T) {
137137
RelationName: relationName,
138138
PKColNames: pkColNames,
139139
PKColDirs: pkColDirs,
140+
PKColTypes: pkColTypes,
140141
Bounds: selectBounds,
141142
SelectBatchSize: ttlbase.DefaultSelectBatchSizeValue,
142143
TTLExpr: catpb.DefaultTTLExpirationExpr,

‎pkg/sql/ttl/ttljob/ttljob_processor.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@ func (t *ttlProcessor) work(ctx context.Context) error {
242242
RelationName: relationName,
243243
PKColNames: pkColNames,
244244
PKColDirs: pkColDirs,
245+
PKColTypes: pkColTypes,
245246
Bounds: bounds,
246247
AOSTDuration: ttlSpec.AOSTDuration,
247248
SelectBatchSize: ttlSpec.SelectBatchSize,

‎pkg/sql/ttl/ttljob/ttljob_query_builder.go‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
1818
"github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb"
1919
"github.com/cockroachdb/cockroach/pkg/sql/ttl/ttlbase"
20+
"github.com/cockroachdb/cockroach/pkg/sql/types"
2021
"github.com/cockroachdb/cockroach/pkg/util/metric/aggmetric"
2122
"github.com/cockroachdb/cockroach/pkg/util/quotapool"
2223
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
@@ -49,6 +50,7 @@ type SelectQueryParams struct {
4950
RelationName string
5051
PKColNames []string
5152
PKColDirs []catenumpb.IndexColumn_Direction
53+
PKColTypes []*types.T
5254
Bounds QueryBounds
5355
AOSTDuration time.Duration
5456
SelectBatchSize int64
@@ -113,6 +115,7 @@ func (b *selectQueryBuilder) BuildQuery() string {
113115
b.RelationName,
114116
b.PKColNames,
115117
b.PKColDirs,
118+
b.PKColTypes,
116119
b.AOSTDuration,
117120
b.TTLExpr,
118121
len(b.Bounds.Start),
@@ -146,6 +149,13 @@ func (b *selectQueryBuilder) Run(
146149
}
147150
query = b.cachedQuery
148151
}
152+
// Convert any DEnum args to their logical representation to avoid the risk
153+
// of using the wrong version of the enum type descriptor.
154+
for i, arg := range b.cachedArgs {
155+
if enum, ok := arg.(*tree.DEnum); ok {
156+
b.cachedArgs[i] = enum.LogicalRep
157+
}
158+
}
149159

150160
tokens, err := b.SelectRateLimiter.Acquire(ctx, b.SelectBatchSize)
151161
if err != nil {
@@ -266,7 +276,13 @@ func (b *deleteQueryBuilder) Run(
266276
deleteArgs := b.cachedArgs[:1]
267277
for _, row := range rows {
268278
for _, col := range row {
269-
deleteArgs = append(deleteArgs, col)
279+
// Convert any DEnum args to their logical representation to avoid the risk
280+
// of using the wrong version of the enum type descriptor.
281+
if enum, ok := col.(*tree.DEnum); ok {
282+
deleteArgs = append(deleteArgs, enum.LogicalRep)
283+
} else {
284+
deleteArgs = append(deleteArgs, col)
285+
}
270286
}
271287
}
272288

‎pkg/sql/ttl/ttljob/ttljob_query_builder_test.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
2222
"github.com/cockroachdb/cockroach/pkg/sql/ttl/ttlbase"
2323
"github.com/cockroachdb/cockroach/pkg/sql/ttl/ttljob"
24+
"github.com/cockroachdb/cockroach/pkg/sql/types"
2425
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
2526
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
2627
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
@@ -301,6 +302,10 @@ func TestSelectQueryBuilder(t *testing.T) {
301302
pkColDirs := tc.pkColDirs
302303
numPKCols := len(pkColDirs)
303304
pkColNames := ttlbase.GenPKColNames(numPKCols)
305+
pkColTypes := make([]*types.T, numPKCols)
306+
for i := range pkColDirs {
307+
pkColTypes[i] = types.Int
308+
}
304309

305310
// Run CREATE TABLE statement.
306311
createTableStatement := genCreateTableStatement(pkColNames, pkColDirs)
@@ -336,6 +341,7 @@ func TestSelectQueryBuilder(t *testing.T) {
336341
RelationName: relationName,
337342
PKColNames: pkColNames,
338343
PKColDirs: pkColDirs,
344+
PKColTypes: pkColTypes,
339345
Bounds: tc.bounds,
340346
AOSTDuration: 0,
341347
SelectBatchSize: 2,

0 commit comments

Comments
 (0)