From e248fccc82316a556b7d5ee4cd6e312721c07b96 Mon Sep 17 00:00:00 2001 From: Rafi Shamim Date: Tue, 17 Sep 2024 14:46:22 -0400 Subject: [PATCH 1/2] tree: fix walkStmt for SHOW EXPERIMENTAL_FINGERPRINTS If SHOW EXPERIMENTAL_FINGERPRINTS FROM TABLE appeared in certain kinds of subqueries, the CRDB node could panic while walking the statement due to a missing nil check. No release note since this only affected an experimental feature. Release note: None --- pkg/sql/sem/tree/walk.go | 12 +++++++----- pkg/testutils/sqlutils/pretty.go | 9 +++++++++ 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/pkg/sql/sem/tree/walk.go b/pkg/sql/sem/tree/walk.go index 26038a58e519..c40153161ecd 100644 --- a/pkg/sql/sem/tree/walk.go +++ b/pkg/sql/sem/tree/walk.go @@ -1152,12 +1152,14 @@ func (n *ShowFingerprints) copyNode() *ShowFingerprints { // walkStmt is part of the walkableStmt interface. func (n *ShowFingerprints) walkStmt(v Visitor) Statement { ret := n - ts, changed := walkTenantSpec(v, n.TenantSpec) - if changed { - if ret == n { - ret = n.copyNode() + if n.TenantSpec != nil { + ts, changed := walkTenantSpec(v, n.TenantSpec) + if changed { + if ret == n { + ret = n.copyNode() + } + ret.TenantSpec = ts } - ret.TenantSpec = ts } if n.Options.StartTimestamp != nil { e, changed := WalkExpr(v, n.Options.StartTimestamp) diff --git a/pkg/testutils/sqlutils/pretty.go b/pkg/testutils/sqlutils/pretty.go index 3c4e7b716a64..17d5a471cac4 100644 --- a/pkg/testutils/sqlutils/pretty.go +++ b/pkg/testutils/sqlutils/pretty.go @@ -29,6 +29,15 @@ func VerifyStatementPrettyRoundtrip(t *testing.T, sql string) { for i := range stmts { origStmt := stmts[i].AST verifyStatementPrettyRoundTrip(t, sql, origStmt, false /* plpgsql */) + + // Verify that the AST can be walked. + if _, err := tree.SimpleStmtVisit( + origStmt, + func(expr tree.Expr) (recurse bool, newExpr tree.Expr, err error) { return }, + ); err != nil { + t.Fatalf("cannot walk stmt %s %v", stmts[i].SQL, err) + } + } } From 3516990fc018c5e63ec871fcdf196c336f441977 Mon Sep 17 00:00:00 2001 From: Rafi Shamim Date: Tue, 17 Sep 2024 14:54:36 -0400 Subject: [PATCH 2/2] tree: disallow creating partial index while table is used in LDR Release note: None --- pkg/sql/sem/tree/schema_helpers.go | 6 +++--- pkg/sql/sem/tree/schema_helpers_test.go | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/sql/sem/tree/schema_helpers.go b/pkg/sql/sem/tree/schema_helpers.go index 4ce418c17e96..135320e3e07d 100644 --- a/pkg/sql/sem/tree/schema_helpers.go +++ b/pkg/sql/sem/tree/schema_helpers.go @@ -40,9 +40,9 @@ func IsSetOrResetSchemaLocked(n Statement) bool { func IsAllowedLDRSchemaChange(n Statement) bool { switch s := n.(type) { case *CreateIndex: - // Only allow non-unique indexes to be created. A unique index on a - // destination table could cause inserts to fail. - return !s.Unique + // Only allow non-unique and non-partial indexes to be created. A unique or + // partial index on a destination table could cause inserts to fail. + return !s.Unique && s.Predicate == nil case *DropIndex: return true } diff --git a/pkg/sql/sem/tree/schema_helpers_test.go b/pkg/sql/sem/tree/schema_helpers_test.go index 25a8f8b733f6..d9d7fc4cd45e 100644 --- a/pkg/sql/sem/tree/schema_helpers_test.go +++ b/pkg/sql/sem/tree/schema_helpers_test.go @@ -35,6 +35,10 @@ func TestIsAllowedLDRSchemaChange(t *testing.T) { stmt: "CREATE UNIQUE INDEX idx ON t (a)", isAllowed: false, }, + { + stmt: "CREATE INDEX idx ON t (a) WHERE a > 10", + isAllowed: false, + }, { stmt: "DROP INDEX idx", isAllowed: true,