From 15e31d9dbfed1d8f9414ef23f5d77855e539f3e7 Mon Sep 17 00:00:00 2001 From: Rafi Shamim Date: Wed, 1 Dec 2021 13:01:18 -0500 Subject: [PATCH 1/2] randgen: fix Postgres mutator to handle expression-based indexes Release note: None --- pkg/sql/randgen/mutator.go | 26 ++++-- pkg/sql/randgen/mutator_test.go | 145 ++++++++++++++++++++++++++++++++ 2 files changed, 165 insertions(+), 6 deletions(-) diff --git a/pkg/sql/randgen/mutator.go b/pkg/sql/randgen/mutator.go index 467d04192d7a..08b41f7078bc 100644 --- a/pkg/sql/randgen/mutator.go +++ b/pkg/sql/randgen/mutator.go @@ -723,9 +723,16 @@ func postgresCreateTableMutator( // to their own statement. var newCols tree.IndexElemList for _, col := range def.Columns { - // Postgres doesn't support box2d as a btree index key. - colTypeFamily := colTypes[string(col.Column)].Family() - if colTypeFamily == types.Box2DFamily { + isBox2d := false + // NB: col.Column is empty for expression-based indexes. + if col.Expr == nil { + // Postgres doesn't support box2d as a btree index key. + colTypeFamily := colTypes[string(col.Column)].Family() + if colTypeFamily == types.Box2DFamily { + isBox2d = true + } + } + if isBox2d { changed = true } else { newCols = append(newCols, col) @@ -752,9 +759,16 @@ func postgresCreateTableMutator( case *tree.UniqueConstraintTableDef: var newCols tree.IndexElemList for _, col := range def.Columns { - // Postgres doesn't support box2d as a btree index key. - colTypeFamily := colTypes[string(col.Column)].Family() - if colTypeFamily == types.Box2DFamily { + isBox2d := false + // NB: col.Column is empty for expression-based indexes. + if col.Expr == nil { + // Postgres doesn't support box2d as a btree index key. + colTypeFamily := colTypes[string(col.Column)].Family() + if colTypeFamily == types.Box2DFamily { + isBox2d = true + } + } + if isBox2d { changed = true } else { newCols = append(newCols, col) diff --git a/pkg/sql/randgen/mutator_test.go b/pkg/sql/randgen/mutator_test.go index 152f7e2edc78..816710f7405d 100644 --- a/pkg/sql/randgen/mutator_test.go +++ b/pkg/sql/randgen/mutator_test.go @@ -17,6 +17,151 @@ import ( "github.com/cockroachdb/cockroach/pkg/util/randutil" ) +func TestPostgresCreateTableMutator(t *testing.T) { + q := ` +CREATE TABLE table1 ( + col1_0 + TIMESTAMP, + col1_1 + REGPROC, + col1_2 + BOX2D NOT NULL, + col1_3 + "char" NOT NULL, + col1_4 + GEOGRAPHY NULL, + col1_5 + GEOGRAPHY, + col1_6 + REGROLE NOT NULL, + col1_7 + REGROLE NOT NULL, + col1_8 + "char", + col1_9 + INTERVAL, + col1_10 + STRING AS (lower(col1_3)) STORED NOT NULL, + col1_11 + STRING AS (lower(CAST(col1_1 AS STRING))) STORED, + col1_12 + STRING AS (lower(CAST(col1_5 AS STRING))) STORED, + col1_13 + STRING AS (lower(CAST(col1_0 AS STRING))) VIRTUAL, + col1_14 + STRING AS (lower(CAST(col1_4 AS STRING))) STORED NULL, + col1_15 + STRING AS (lower(CAST(col1_7 AS STRING))) STORED NOT NULL, + col1_16 + STRING AS (lower(CAST(col1_0 AS STRING))) STORED, + INDEX (col1_9, col1_1, col1_12 ASC) + WHERE + ( + (table1.col1_8 != 'X':::STRING OR table1.col1_16 < e'\x00':::STRING) + AND table1.col1_15 <= '':::STRING + ) + AND table1.col1_14 >= e'\U00002603':::STRING, + UNIQUE ( + col1_1 ASC, + lower(CAST(col1_5 AS STRING)) ASC, + col1_0, + col1_10, + col1_11 ASC, + col1_12 ASC, + col1_2 ASC, + col1_7, + col1_3, + col1_6, + col1_16 + ) + WHERE + ( + ( + ( + ( + ( + ( + ( + ( + table1.col1_0 < '-4713-11-24 00:00:00':::TIMESTAMP + AND table1.col1_14 = e'\'':::STRING + ) + AND table1.col1_3 = '"':::STRING + ) + OR table1.col1_8 >= e'\U00002603':::STRING + ) + OR table1.col1_13 > '':::STRING + ) + AND table1.col1_12 < '':::STRING + ) + OR table1.col1_11 >= 'X':::STRING + ) + AND table1.col1_10 <= e'\'':::STRING + ) + OR table1.col1_16 != '':::STRING + ) + OR table1.col1_15 >= '':::STRING, + UNIQUE (col1_1, col1_16 DESC, col1_6, col1_11 DESC, col1_9 ASC, col1_3 DESC, col1_2 ASC), + UNIQUE (col1_14 DESC, col1_15 DESC) + WHERE + ( + ( + ( + ( + ( + ( + ( + (table1.col1_14 >= e'\'':::STRING OR table1.col1_16 >= 'X':::STRING) + OR table1.col1_3 <= 'X':::STRING + ) + AND table1.col1_15 = 'X':::STRING + ) + OR table1.col1_13 > '"':::STRING + ) + OR table1.col1_12 >= '':::STRING + ) + AND table1.col1_11 != '':::STRING + ) + OR table1.col1_10 != e'\U00002603':::STRING + ) + AND table1.col1_8 != e'\U00002603':::STRING + ) + OR table1.col1_0 <= '-4713-11-24 00:00:00':::TIMESTAMP, + INVERTED INDEX ( + col1_13, + col1_16, + col1_12, + col1_8, + col1_3 ASC, + col1_6, + col1_2, + col1_7, + col1_0 DESC, + lower(CAST(col1_7 AS STRING)) DESC, + col1_1, + col1_15 DESC, + col1_4 + ) +)` + rng, _ := randutil.NewTestRand() + // Set a deterministic seed so that PostgresCreateTableMutator performs a + // deterministic transformation. + rng.Seed(123) + mutated, changed := ApplyString(rng, q, PostgresCreateTableMutator) + if !changed { + t.Fatal("expected changed") + } + expect := ` +CREATE TABLE table1 (col1_0 TIMESTAMP, col1_1 REGPROC, col1_2 BOX2D NOT NULL, col1_3 "char" NOT NULL, col1_4 GEOGRAPHY NULL, col1_5 GEOGRAPHY, col1_6 REGROLE NOT NULL, col1_7 REGROLE NOT NULL, col1_8 "char", col1_9 INTERVAL, col1_10 STRING NOT NULL AS (lower(col1_3)) STORED, col1_11 STRING AS (CASE WHEN col1_1 IS NULL THEN 'L*h':::STRING ELSE '#':::STRING END) STORED, col1_12 STRING AS (lower(CAST(col1_5 AS STRING))) STORED, col1_13 STRING AS (CASE WHEN col1_0 IS NULL THEN e'\x1c\t':::STRING ELSE e'(,Zh\x05\x1dW':::STRING END) VIRTUAL, col1_14 STRING NULL AS (lower(CAST(col1_4 AS STRING))) STORED, col1_15 STRING NOT NULL AS (CASE WHEN col1_7 IS NULL THEN e'\x0e,\x162/BJ\x12':::STRING ELSE e'#\x17\x07;\x0emi':::STRING END) STORED, col1_16 STRING AS (CASE WHEN col1_0 IS NULL THEN e'\x1eM\x02\x12_\x05\r':::STRING ELSE e'[jUDO\nt':::STRING END) STORED); +CREATE INDEX ON table1 (col1_9, col1_1, col1_12 ASC); +CREATE UNIQUE INDEX ON table1 (col1_1 ASC, lower(CAST(col1_5 AS STRING)) ASC, col1_0, col1_10, col1_11 ASC, col1_12 ASC, col1_7, col1_3, col1_6, col1_16); +CREATE UNIQUE INDEX ON table1 (col1_1, col1_16 DESC, col1_6, col1_11 DESC, col1_9 ASC, col1_3 DESC); +CREATE UNIQUE INDEX ON table1 (col1_14 DESC, col1_15 DESC);` + if strings.TrimSpace(mutated) != strings.TrimSpace(expect) { + t.Fatalf("expected:\n%s\ngot:\n%s", expect, mutated) + } +} + func TestPostgresMutator(t *testing.T) { q := ` CREATE TABLE t (s STRING FAMILY fam1, b BYTES, FAMILY fam2 (b), PRIMARY KEY (s ASC, b DESC), INDEX (s) STORING (b)) From 547bfce231ac0857af1dbbaa726c62029d6d8bbb Mon Sep 17 00:00:00 2001 From: Chengxiong Ruan Date: Wed, 1 Dec 2021 13:22:47 -0500 Subject: [PATCH 2/2] kv/kvserver/uncertainty: remove observedts_test target from bazel build file The test files have been deleted. Leaving observedts_test target in build file fails CI for some PRs :( --- pkg/BUILD.bazel | 1 - pkg/kv/kvserver/uncertainty/BUILD.bazel | 33 ------------------------- 2 files changed, 34 deletions(-) diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel index ce8ad0ec328f..8f78622bb3bf 100644 --- a/pkg/BUILD.bazel +++ b/pkg/BUILD.bazel @@ -149,7 +149,6 @@ ALL_TESTS = [ "//pkg/kv/kvserver/tscache:tscache_test", "//pkg/kv/kvserver/txnrecovery:txnrecovery_test", "//pkg/kv/kvserver/txnwait:txnwait_test", - "//pkg/kv/kvserver/uncertainty:observedts_test", "//pkg/kv/kvserver/uncertainty:uncertainty_test", "//pkg/kv/kvserver:kvserver_test", "//pkg/kv:kv_test", diff --git a/pkg/kv/kvserver/uncertainty/BUILD.bazel b/pkg/kv/kvserver/uncertainty/BUILD.bazel index acad9cbfa295..613e8dc2363b 100644 --- a/pkg/kv/kvserver/uncertainty/BUILD.bazel +++ b/pkg/kv/kvserver/uncertainty/BUILD.bazel @@ -1,38 +1,5 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") -go_library( - name = "observedts", - srcs = [ - "doc.go", - "limit.go", - "uncertainty.go", - ], - importpath = "github.com/cockroachdb/cockroach/pkg/kv/kvserver/observedts", - visibility = ["//visibility:public"], - deps = [ - "//pkg/kv/kvserver/kvserverpb", - "//pkg/roachpb:with-mocks", - "//pkg/util/hlc", - ], -) - -go_test( - name = "observedts_test", - size = "small", - srcs = [ - "limit_test.go", - "uncertainty_test.go", - ], - embed = [":observedts"], - deps = [ - "//pkg/kv/kvserver/kvserverpb", - "//pkg/roachpb:with-mocks", - "//pkg/util/hlc", - "//pkg/util/leaktest", - "@com_github_stretchr_testify//require", - ], -) - go_library( name = "uncertainty", srcs = [