Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
111403: sql: add new check relation to help build insert fast path uniqueness checks r=msirek a=msirek

sql: add new check relation to help build insert fast path uniqueness checks

This creates a `FastPathUniqueChecks` list as a child of `InsertExpr`,
similar to `UniqueChecks`, but consisting of relational expressions
which can be used to build a fast path uniqueness check using a
 particular index. Each item in the `FastPathUniqueChecks` list is a
`FastPathUniqueChecksItem` whose `Check` expression, if the unique
constraint is provisionally eligible to use a fast path uniqueness
check, is a filtered scan from the target table of an
`INSERT INTO ... VALUES` statement. It is only built if the
VALUES clause has a single row. The filter equates each of the unique
constraint key columns with its corresponding value from the VALUES
clause. The values may either be a `ValuesExpr` or a `WithScanExpr`
whose source is a `ValuesExpr`. During exploration,
GenerateConstrainedScans will find all valid constrained index scans
which are equivalent to the original filtered Scan. The built relations
are not executed or displayed in the EXPLAIN, but will be analyzed in a
later commit to specify the index and index key values to use for insert
fast path uniqueness checks.

FastPathUniqueChecksItemPrivate is added with elements which are
designed to communicate details of how to build a fast path uniqueness
check to the execbuilder.

Epic: CRDB-26290
Informs: #58047

Release note: None

----
memo: optbuild unit tests for fast path unique check exprs

This commit adds the `ExprFmtHideFastPathChecks` flag, which hides fast
path check expressions from the output of expression formatting. It is
used in all places except TestBuilder. Also, unit tests for
`buildFiltersForFastPathCheck` are added.

Epic: CRDB-26290
Informs: #58047

Release note: None

----
xform: opt test for insert fast path checks

This commit adds an `insert` transformation rules test which
displays optimized fast path checks which are built in the optbuilder
and later explored and optimized. This test is modified in a later
commit to test for rule firing.

Epic: CRDB-26290
Informs: #58047

Release note: None


111443: ccl: regionless restores with rbr table(s) fail fast, add version gate r=msbutler a=annrpom

### ccl: regionless restores with regional by row table(s) fail fast

Previously, backing up an MR cluster/db/table that had a regional by row table did not work out-of-the-box (users could not write to said table without performing some operations to ensure hidden column added by regional by row enablement, `crdb_region`, and zone config behaved according to the target RESTORE cluster. This was inadequate because we allowed users to perform a RESTORE where some tables were not "available" right away. This patch addresses this by making sure we "fail fast" if users do decide to attempt a RESTORE where the BACKUP object has a regional by row table.

Also: tests. A small amount of tests were fixed to separate the behavior of restoring a database vs a table (just dropping a table after we performed a db restore to test table restore is not a properly isolated test of behavior). Mixed-version tests were added as well.

Epic: none
Informs: #111348
Part of: https://cockroachlabs.atlassian.net/browse/CRDB-29129

Release note (sql change): Users are not able to perform a `remove_regions` RESTORE if the object being restored contains a regional by row table.

---
### ccl: add 23.2 version gate to regionless restore

This patch adds a >= 23.2 version gate to the `remove_regions`
RESTORE option, in addition to a mixed-version test to ensure
that RESTORE fails fast if it is on a cluster version < 23.2.

Epic: none
Fixes: #111348
Part of: https://cockroachlabs.atlassian.net/browse/CRDB-29129

Release note: none

111758: server: reduce a flake under race r=stevendanna a=knz

Informs #111742.

Will be superseded by #111753, but this one needs to be backported (the flake is on the release branch).

Epic: CRDB-28893

Release note: None

111761: sql: add select for update in udf tests r=rharding6373 a=rharding6373

This PR exercises simple SELECT FOR UPDATE patterns in UDFs, and also tests UDFs in transactions that are committed or rolled back.

Epic: CRDB-25388
Informs: #87289

Release note: None

Co-authored-by: Mark Sirek <[email protected]>
Co-authored-by: Annie Pompa <[email protected]>
Co-authored-by: Raphael 'kena' Poss <[email protected]>
Co-authored-by: rharding6373 <[email protected]>
  • Loading branch information
5 people committed Oct 4, 2023
5 parents fd181dd + b7468bb + 831d60b + b501e10 + 7299342 commit 69f0369
Show file tree
Hide file tree
Showing 37 changed files with 3,019 additions and 1,060 deletions.
2 changes: 2 additions & 0 deletions pkg/ccl/backupccl/datadriven_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ var localityCfgs = map[string]roachpb.Locality{
var clusterVersionKeys = map[string]clusterversion.Key{
"23_1_Start": clusterversion.V23_1Start,
"23_1_MVCCTombstones": clusterversion.V23_1_MVCCRangeTombstonesUnconditionallyEnabled,
"23_2_Start": clusterversion.V23_2Start,
"23_2": clusterversion.V23_2,
}

type sqlDBKey struct {
Expand Down
17 changes: 3 additions & 14 deletions pkg/ccl/backupccl/restore_multiregion_rbr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,9 @@ import (
"github.com/stretchr/testify/require"
)

// The goal of this test is to ensure that if a user ever performed a
// regionless restore where the backed-up target has a regional by row table,
// they would be able to get themselves out of a stuck state without needing
// an enterprise license (in addition to testing the ability to use remove_regions
// without said license).
// The goal of this test is to ensure that a user is able to perform a
// regionless restore and modify the restored object without an enterprise
// license.
func TestMultiRegionRegionlessRestoreNoLicense(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)
Expand All @@ -56,9 +54,6 @@ func TestMultiRegionRegionlessRestoreNoLicense(t *testing.T) {
INSERT INTO d.t VALUES (1), (2), (3);`,
)

// Make table regional by row.
mrSql.Exec(t, `ALTER TABLE d.t SET LOCALITY REGIONAL BY ROW;`)

if err := backuptestutils.VerifyBackupRestoreStatementResult(t, mrSql, `BACKUP DATABASE d INTO $1`, localFoo); err != nil {
t.Fatal(err)
}
Expand All @@ -85,12 +80,6 @@ func TestMultiRegionRegionlessRestoreNoLicense(t *testing.T) {
t.Fatal(err)
}

// Get us in the state that allows us to perform writes.
// This is the main purpose of this test - we want to ensure that this process is available
// to those without enterprise licenses.
sqlDB.Exec(t, `ALTER TABLE d.t ALTER COLUMN crdb_region SET DEFAULT 'us-east1';
ALTER TABLE d.t CONFIGURE ZONE DISCARD;`)

// Perform some writes to d's table.
sqlDB.Exec(t, `INSERT INTO d.t VALUES (4), (5), (6)`)

Expand Down
13 changes: 13 additions & 0 deletions pkg/ccl/backupccl/restore_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -1224,6 +1224,11 @@ func restorePlanHook(
return nil, nil, nil, false, err
}

if restoreStmt.Options.RemoveRegions && !p.ExecCfg().Settings.Version.IsActive(ctx, clusterversion.V23_2) {
return nil, nil, nil, false,
errors.Newf("to set the remove_regions option, cluster version must be >= %s", clusterversion.V23_2.String())
}

if !restoreStmt.Options.SchemaOnly && restoreStmt.Options.VerifyData {
return nil, nil, nil, false,
errors.New("to set the verify_backup_table_data option, the schema_only option must be set")
Expand Down Expand Up @@ -2030,6 +2035,14 @@ func doRestorePlan(
}
}

if restoreStmt.Options.RemoveRegions {
for _, t := range tablesByID {
if t.LocalityConfig.GetRegionalByRow() != nil {
return errors.Newf("cannot perform a remove_regions RESTORE with region by row enabled table %s in BACKUP target", t.Name)
}
}
}

if !restoreStmt.Options.SkipLocalitiesCheck {
if err := checkClusterRegions(ctx, p, typesByID); err != nil {
return err
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,11 @@ subtest end
subtest restore_regionless_on_single_region_table

exec-sql
DROP TABLE d.t;
DROP DATABASE d;
----

exec-sql
CREATE DATABASE d;
----

exec-sql
Expand All @@ -284,7 +288,7 @@ RESTORE TABLE d.t FROM LATEST IN 'nodelocal://1/table_backup/' WITH remove_regio
query-sql
SELECT table_name, locality FROM [SHOW TABLES FROM d] ORDER BY 1;
----
t REGIONAL BY TABLE IN PRIMARY REGION
t <nil>

# ensure that tables belonging to d can be modified & have the correct values
exec-sql
Expand All @@ -307,7 +311,8 @@ subtest restore_regionless_on_single_region_table/alter-region
exec-sql
ALTER DATABASE d ADD REGION "us-east-1";
----
pq: region "us-east-1" already added to database
pq: cannot add region "us-east-1" to database d
HINT: you must add a PRIMARY REGION first using ALTER DATABASE d PRIMARY REGION "us-east-1"

exec-sql
ALTER DATABASE d PRIMARY REGION "us-east-1";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
new-cluster name=s1 beforeVersion=23_2_Start disable-tenant localities=us-east-1,us-west-1,eu-central-1
----

exec-sql
CREATE DATABASE d PRIMARY REGION "us-east-1" REGIONS "us-west-1", "eu-central-1";
CREATE TABLE d.t (x INT);
INSERT INTO d.t VALUES (1), (2), (3);
----

query-sql
SELECT region FROM [SHOW REGIONS FROM DATABASE d] ORDER BY 1;
----
eu-central-1
us-east-1
us-west-1

query-sql
SHOW DATABASES;
----
d root us-east-1 {eu-central-1,us-east-1,us-west-1} zone
data root <nil> <nil> {} <nil>
defaultdb root <nil> <nil> {} <nil>
postgres root <nil> <nil> {} <nil>
system node <nil> <nil> {} <nil>

# backup a cluster
exec-sql
BACKUP INTO 'nodelocal://1/cluster_backup/';
----

new-cluster name=s2 beforeVersion=23_2_Start share-io-dir=s1 disable-tenant
----

# restore fails when cluster is in mixed version state while upgrading to 23.2
exec-sql
RESTORE FROM LATEST IN 'nodelocal://1/cluster_backup/' WITH remove_regions;
----
pq: to set the remove_regions option, cluster version must be >= 1000023.1-26

# upgrade cluster
upgrade-cluster version=23_2
----

exec-sql
RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/cluster_backup/' WITH remove_regions;
----

exec-sql
INSERT INTO d.t VALUES (4), (5);
----

query-sql
SELECT * FROM d.t
----
1
2
3
4
5
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,11 @@ subtest end
subtest restore_regionless_on_regionless_table

exec-sql
DROP TABLE d.t;
DROP DATABASE d;
----

exec-sql
CREATE DATABASE d;
----

exec-sql
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,186 +55,22 @@ BACKUP INTO 'nodelocal://1/rbr_cluster_backup/';

subtest end

# ensure restoring a cluster/db/table with a table that has regional by row on a single region cluster
# fails fast
new-cluster name=s2 share-io-dir=s1 allow-implicit-access localities=us-east-1
----

# restoring a cluster with a table that has regional by row on a single region cluster
subtest restore_regionless_cluster_rbr

exec-sql
RESTORE FROM LATEST IN 'nodelocal://1/rbr_cluster_backup/' WITH remove_regions;
----

# check cluster's regions
query-sql
SHOW REGIONS FROM CLUSTER;
----
us-east-1 {us-east1}

query-sql
SHOW DATABASES;
----
d root <nil> <nil> {} <nil>
data root <nil> <nil> {} <nil>
defaultdb root <nil> <nil> {} <nil>
postgres root <nil> <nil> {} <nil>
system node <nil> <nil> {} <nil>

# ensure that database d is regionless
query-sql
SELECT region FROM [SHOW REGIONS FROM DATABASE d] ORDER BY 1;
----

# show tables - make sure these are regionless as well
query-sql
SELECT table_name, locality FROM [SHOW TABLES FROM d] ORDER BY 1;
----
t <nil>

# ensure that tables belonging to d can be modified & have the correct values
exec-sql
INSERT INTO d.t VALUES (4), (5);
----
pq: default_to_database_primary_region(): current database defaultdb is not multi-region enabled

# here is how we unblock ourselves from the error above
exec-sql
ALTER TABLE d.t ALTER COLUMN crdb_region SET DEFAULT 'us-east-1';
----

exec-sql
ALTER TABLE d.t CONFIGURE ZONE DISCARD;
----

exec-sql
INSERT INTO d.t VALUES (4), (5);
----

query-sql
SELECT * FROM d.t;
----
1
2
3
4
5

subtest end

# restoring a database with a table that has regional by row on a single region cluster
subtest restore_regionless_rbr_db

exec-sql
DROP DATABASE d;
----
pq: cannot perform a remove_regions RESTORE with region by row enabled table t in BACKUP target

exec-sql
RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/rbr_database_backup/' WITH remove_regions;
----

# check to see if restored database, d, shows up
query-sql
SHOW DATABASES;
----
d root <nil> <nil> {} <nil>
data root <nil> <nil> {} <nil>
defaultdb root <nil> <nil> {} <nil>
postgres root <nil> <nil> {} <nil>
system node <nil> <nil> {} <nil>

# ensure that database d is regionless
query-sql
SELECT region FROM [SHOW REGIONS FROM DATABASE d] ORDER BY 1;
----

# show tables - make sure these are regionless as well
query-sql
SELECT table_name, locality FROM [SHOW TABLES FROM d] ORDER BY 1;
----
t <nil>

# ensure that tables belonging to d can be modified & have the correct values
exec-sql
INSERT INTO d.t VALUES (4), (5);
----
pq: default_to_database_primary_region(): current database defaultdb is not multi-region enabled

# here is how we unblock ourselves from the error above
exec-sql
ALTER TABLE d.t ALTER COLUMN crdb_region SET DEFAULT 'us-east-1';
----

exec-sql
ALTER TABLE d.t CONFIGURE ZONE DISCARD;
----

exec-sql
INSERT INTO d.t VALUES (4), (5);
----

query-sql
SELECT * FROM d.t;
----
1
2
3
4
5

subtest end

# restoring a table that has regional by row on a single region cluster
subtest restore_regionless_rbr_table

exec-sql
DROP TABLE d.t;
----
pq: cannot perform a remove_regions RESTORE with region by row enabled table t in BACKUP target

exec-sql
RESTORE TABLE d.t FROM LATEST IN 'nodelocal://1/rbr_table_backup/' WITH remove_regions;
----
pq: "crdb_internal_region" is not compatible with type "crdb_internal_region" existing in cluster: "crdb_internal_region" of type "ENUM" is not compatible with type "MULTIREGION_ENUM"

# let's drop the type
exec-sql
DROP TYPE d.public.crdb_internal_region;
----

exec-sql
RESTORE TABLE d.t FROM LATEST IN 'nodelocal://1/rbr_table_backup/' WITH remove_regions;
----

query-sql
SELECT table_name, locality FROM [SHOW TABLES FROM d] ORDER BY 1;
----
t <nil>

# ensure that tables belonging to d can be modified & have the correct values
exec-sql
INSERT INTO d.t VALUES (4), (5);
----
pq: default_to_database_primary_region(): current database defaultdb is not multi-region enabled

# here is how we unblock ourselves from the error above
exec-sql
ALTER TABLE d.t ALTER COLUMN crdb_region SET DEFAULT 'us-east-1';
----

exec-sql
ALTER TABLE d.t CONFIGURE ZONE DISCARD;
----

exec-sql
INSERT INTO d.t VALUES (4), (5);
----

query-sql
SELECT * FROM d.t;
----
1
2
3
4
5

subtest end
pq: cannot perform a remove_regions RESTORE with region by row enabled table t in BACKUP target
Loading

0 comments on commit 69f0369

Please sign in to comment.