diff --git a/pkg/cmd/roachtest/tests/BUILD.bazel b/pkg/cmd/roachtest/tests/BUILD.bazel index ca1e8e7d30f5..7af0f6f26efc 100644 --- a/pkg/cmd/roachtest/tests/BUILD.bazel +++ b/pkg/cmd/roachtest/tests/BUILD.bazel @@ -97,7 +97,6 @@ go_library( "rapid_restart.go", "rebalance_load.go", "registry.go", - "remove_invalid_database_privileges.go", "replicagc.go", "reset_quorum.go", "restart.go", @@ -138,7 +137,6 @@ go_library( "util_disk_usage.go", "util_if_local.go", "util_load_group.go", - "validate_grant_option.go", "validate_system_schema_after_version_upgrade.go", "version.go", "version_upgrade_public_schema.go", diff --git a/pkg/cmd/roachtest/tests/registry.go b/pkg/cmd/roachtest/tests/registry.go index 788ea63b9ee2..cda41c2a3b7f 100644 --- a/pkg/cmd/roachtest/tests/registry.go +++ b/pkg/cmd/roachtest/tests/registry.go @@ -120,7 +120,6 @@ func RegisterTests(r registry.Registry) { registerKVBench(r) registerTypeORM(r) registerLoadSplits(r) - registerValidateGrantOption(r) registerVersion(r) registerYCSB(r) registerTPCHBench(r) @@ -128,7 +127,6 @@ func RegisterTests(r registry.Registry) { registerMultiTenantUpgrade(r) registerMultiTenantFairness(r) registerVersionUpgradePublicSchema(r) - registerRemoveInvalidDatabasePrivileges(r) registerValidateSystemSchemaAfterVersionUpgrade(r) registerIndexBackfill(r) } diff --git a/pkg/cmd/roachtest/tests/remove_invalid_database_privileges.go b/pkg/cmd/roachtest/tests/remove_invalid_database_privileges.go deleted file mode 100644 index 6b279a7dc1c6..000000000000 --- a/pkg/cmd/roachtest/tests/remove_invalid_database_privileges.go +++ /dev/null @@ -1,173 +0,0 @@ -// Copyright 2021 The Cockroach Authors. -// -// Use of this software is governed by the Business Source License -// included in the file licenses/BSL.txt. -// -// As of the Change Date specified in that file, in accordance with -// the Business Source License, use of this software will be governed -// by the Apache License, Version 2.0, included in the file -// licenses/APL.txt. - -package tests - -import ( - "context" - "fmt" - - "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster" - "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry" - "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test" - "github.com/cockroachdb/cockroach/pkg/util/version" - "github.com/stretchr/testify/require" -) - -func registerRemoveInvalidDatabasePrivileges(r registry.Registry) { - r.Add(registry.TestSpec{ - Name: "remove-invalid-database-privileges", - Owner: registry.OwnerSQLExperience, - Cluster: r.MakeClusterSpec(3), - Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { - runRemoveInvalidDatabasePrivileges(ctx, t, c, *t.BuildVersion()) - }, - }) -} - -func runRemoveInvalidDatabasePrivileges( - ctx context.Context, t test.Test, c cluster.Cluster, buildVersion version.Version, -) { - // This is meant to test a migration from 21.2 to 22.1. - if buildVersion.Major() != 22 || buildVersion.Minor() != 1 { - t.L().PrintfCtx(ctx, "skipping test because build version is %s", buildVersion) - return - } - const mainVersion = "" - // We kick off our test on version 21.1.12 since 21.1 was the last version - // we supported granting invalid database privileges. - const version21_1_12 = "21.1.12" - const version21_2_4 = "21.2.4" - u := newVersionUpgradeTest(c, - uploadAndStart(c.All(), version21_1_12), - waitForUpgradeStep(c.All()), - preventAutoUpgradeStep(1), - preventAutoUpgradeStep(2), - preventAutoUpgradeStep(3), - - // No nodes have been upgraded; Grant incompatible database privileges - // to testuser before upgrading. - execSQL("CREATE USER testuser;", "", 1), - execSQL("CREATE DATABASE test;", "", 1), - execSQL("GRANT CREATE, SELECT, INSERT, UPDATE, DELETE ON DATABASE test TO testuser;", "", 1), - - showGrantsOnDatabase("system", [][]string{ - {"system", "admin", "GRANT"}, - {"system", "admin", "SELECT"}, - {"system", "root", "GRANT"}, - {"system", "root", "SELECT"}, - }), - showGrantsOnDatabase("test", [][]string{ - {"test", "admin", "ALL"}, - {"test", "root", "ALL"}, - {"test", "testuser", "CREATE"}, - {"test", "testuser", "DELETE"}, - {"test", "testuser", "INSERT"}, - {"test", "testuser", "SELECT"}, - {"test", "testuser", "UPDATE"}, - }), - - binaryUpgradeStep(c.Node(1), version21_2_4), - allowAutoUpgradeStep(1), - binaryUpgradeStep(c.Node(2), version21_2_4), - allowAutoUpgradeStep(2), - binaryUpgradeStep(c.Node(3), version21_2_4), - allowAutoUpgradeStep(3), - - waitForUpgradeStep(c.All()), - - binaryUpgradeStep(c.Node(1), mainVersion), - allowAutoUpgradeStep(1), - binaryUpgradeStep(c.Node(2), mainVersion), - allowAutoUpgradeStep(2), - binaryUpgradeStep(c.Node(3), mainVersion), - allowAutoUpgradeStep(3), - - waitForUpgradeStep(c.All()), - - showGrantsOnDatabase("system", [][]string{ - {"system", "admin", "CONNECT"}, - {"system", "root", "CONNECT"}, - }), - showGrantsOnDatabase("test", [][]string{ - {"test", "admin", "ALL"}, - {"test", "root", "ALL"}, - {"test", "testuser", "CREATE"}, - }), - - showDefaultPrivileges("test", [][]string{ - {"tables", "testuser", "SELECT"}, - {"tables", "testuser", "INSERT"}, - {"tables", "testuser", "DELETE"}, - {"tables", "testuser", "UPDATE"}, - {"types", "public", "USAGE"}, - }), - - showDefaultPrivileges("system", [][]string{ - {"types", "public", "USAGE"}, - }), - ) - u.run(ctx, t) -} - -func showGrantsOnDatabase(dbName string, expectedPrivileges [][]string) versionStep { - return func(ctx context.Context, t test.Test, u *versionUpgradeTest) { - conn, err := u.c.ConnE(ctx, t.L(), loadNode) - require.NoError(t, err) - rows, err := conn.Query( - fmt.Sprintf("SELECT database_name, grantee, privilege_type FROM [SHOW GRANTS ON DATABASE %s]", - dbName)) - require.NoError(t, err) - var name, grantee, privilegeType string - i := 0 - for rows.Next() { - privilegeRow := expectedPrivileges[i] - err = rows.Scan(&name, &grantee, &privilegeType) - require.NoError(t, err) - - require.Equal(t, privilegeRow[0], name) - require.Equal(t, privilegeRow[1], grantee) - require.Equal(t, privilegeRow[2], privilegeType) - i++ - } - - if i != len(expectedPrivileges) { - t.Errorf("expected %d rows, found %d rows", len(expectedPrivileges), i) - } - } -} - -func showDefaultPrivileges(dbName string, expectedDefaultPrivileges [][]string) versionStep { - return func(ctx context.Context, t test.Test, u *versionUpgradeTest) { - conn, err := u.c.ConnE(ctx, t.L(), loadNode) - require.NoError(t, err) - _, err = conn.Exec(fmt.Sprintf("USE %s", dbName)) - require.NoError(t, err) - rows, err := conn.Query("SELECT object_type, grantee, privilege_type FROM [SHOW DEFAULT PRIVILEGES FOR ALL ROLES]") - require.NoError(t, err) - - var objectType, grantee, privilegeType string - i := 0 - for rows.Next() { - defaultPrivilegeRow := expectedDefaultPrivileges[i] - err = rows.Scan(&objectType, &grantee, &privilegeType) - require.NoError(t, err) - require.Equal(t, defaultPrivilegeRow[0], objectType) - require.Equal(t, defaultPrivilegeRow[1], grantee) - require.Equal(t, defaultPrivilegeRow[2], privilegeType) - - i++ - } - - if i != len(expectedDefaultPrivileges) { - t.Errorf("expected %d rows, found %d rows", len(expectedDefaultPrivileges), i) - } - } -} diff --git a/pkg/cmd/roachtest/tests/validate_grant_option.go b/pkg/cmd/roachtest/tests/validate_grant_option.go deleted file mode 100644 index 8d1ba174ec26..000000000000 --- a/pkg/cmd/roachtest/tests/validate_grant_option.go +++ /dev/null @@ -1,184 +0,0 @@ -// Copyright 2021 The Cockroach Authors. -// -// Use of this software is governed by the Business Source License -// included in the file licenses/BSL.txt. -// -// As of the Change Date specified in that file, in accordance with -// the Business Source License, use of this software will be governed -// by the Apache License, Version 2.0, included in the file -// licenses/APL.txt. - -package tests - -import ( - "context" - - "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster" - "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry" - "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test" - "github.com/cockroachdb/cockroach/pkg/util/version" - "github.com/stretchr/testify/require" -) - -func registerValidateGrantOption(r registry.Registry) { - r.Add(registry.TestSpec{ - Name: "validate-grant-option", - Owner: registry.OwnerSQLExperience, - Cluster: r.MakeClusterSpec(3), - Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { - runRegisterValidateGrantOption(ctx, t, c, *t.BuildVersion()) - }, - }) -} - -// execSQL executes the SQL statement as "root". -func execSQL(sqlStatement string, expectedErrText string, node int) versionStep { - return func(ctx context.Context, t test.Test, u *versionUpgradeTest) { - conn, err := u.c.ConnE(ctx, t.L(), node) - require.NoError(t, err) - t.L().PrintfCtx(ctx, "user root on node %d executing: %s", node, sqlStatement) - _, err = conn.Exec(sqlStatement) - if len(expectedErrText) == 0 { - require.NoError(t, err) - } else { - require.EqualError(t, err, expectedErrText) - } - } -} - -// execSQLAsUser executes the SQL statement as the specified user. -func execSQLAsUser(sqlStatement string, user string, expectedErrText string, node int) versionStep { - return func(ctx context.Context, t test.Test, u *versionUpgradeTest) { - conn, err := u.c.ConnEAsUser(ctx, t.L(), node, user) - require.NoError(t, err) - t.L().PrintfCtx(ctx, "user %s on node %d executing: %s", user, node, sqlStatement) - _, err = conn.Exec(sqlStatement) - if len(expectedErrText) == 0 { - require.NoError(t, err) - } else { - require.EqualError(t, err, expectedErrText) - } - } -} - -func runRegisterValidateGrantOption( - ctx context.Context, t test.Test, c cluster.Cluster, buildVersion version.Version, -) { - // This is meant to test a migration from 21.2 to 22.1. - if buildVersion.Major() != 22 || buildVersion.Minor() != 1 { - t.L().PrintfCtx(ctx, "skipping test because build version is %s", buildVersion) - return - } - const mainVersion = "" - predecessorVersion, err := PredecessorVersion(buildVersion) - if err != nil { - t.Fatal(err) - } - - u := newVersionUpgradeTest(c, - uploadAndStart(c.All(), predecessorVersion), - waitForUpgradeStep(c.All()), - preventAutoUpgradeStep(1), - preventAutoUpgradeStep(2), - preventAutoUpgradeStep(3), - - // No nodes have been upgraded; any user can grant or revoke a privilege that they hold (grant options - // are ignored). - execSQL("CREATE USER foo;", "", 1), - execSQL("CREATE USER foo2;", "", 2), - execSQL("CREATE USER foo3;", "", 2), - execSQL("CREATE USER foo4;", "", 2), - execSQL("CREATE USER foo5;", "", 2), - execSQL("CREATE USER target;", "", 1), - - execSQL("CREATE DATABASE d;", "", 2), - execSQL("CREATE SCHEMA s;", "", 3), - execSQL("CREATE TABLE t1();", "", 2), - execSQL("CREATE TYPE ty AS ENUM();", "", 3), - - execSQL("GRANT ALL PRIVILEGES ON DATABASE d TO foo;", "", 1), - execSQL("GRANT ALL PRIVILEGES ON SCHEMA s TO foo2;", "", 3), - execSQL("GRANT ALL PRIVILEGES ON TABLE t1 TO foo3;", "", 1), - execSQL("GRANT ALL PRIVILEGES ON TYPE ty TO foo4;", "", 1), - - execSQLAsUser("SELECT * FROM t1;", "foo3", "", 2), - execSQLAsUser("GRANT CREATE ON DATABASE d TO target;", "foo", "", 1), - execSQLAsUser("GRANT USAGE ON SCHEMA s TO target;", "foo2", "", 3), - execSQLAsUser("GRANT SELECT ON TABLE t1 TO target;", "foo3", "", 1), - execSQLAsUser("GRANT USAGE ON TYPE ty TO target;", "foo4", "", 1), - - // Node 1 is upgraded, and nodes 2 and 3 are on the previous version. - binaryUpgradeStep(c.Node(1), mainVersion), - allowAutoUpgradeStep(1), - execSQLAsUser("GRANT CREATE ON DATABASE d TO target;", "foo", "", 1), - execSQLAsUser("GRANT CREATE ON DATABASE defaultdb TO foo;", "root", "", 1), - execSQLAsUser("CREATE TABLE t2();", "foo", "", 1), // t2 created on a new node. - execSQLAsUser("CREATE TABLE t3();", "foo", "", 2), // t3 created on an old node. - execSQLAsUser("GRANT CREATE ON TABLE t2 TO foo2;", "foo", "", 1), // foo2 does not get GRANT or grant option on t2. - execSQLAsUser("GRANT GRANT, CREATE ON TABLE t3 TO foo2;", "foo", "", 2), // foo2 gets GRANT and grant option on t3 - execSQLAsUser("GRANT CREATE ON TABLE t2 TO foo3;", "foo", "", 1), // foo3 does not get GRANT or grant option on t2. - execSQLAsUser("GRANT GRANT, CREATE ON TABLE t3 TO foo3;", "foo", "", 2), // foo3 gets GRANT, but not grant option on t3 - - execSQLAsUser("GRANT CREATE ON TABLE t2 TO foo4 WITH GRANT OPTION;", "foo", "pq: version 21.2-22 must be finalized to use grant options", 1), - execSQLAsUser("GRANT CREATE ON TABLE t3 TO foo4 WITH GRANT OPTION;", "foo", "pq: at or near \"with\": syntax error", 2), - - execSQLAsUser("GRANT CREATE ON TABLE t2 TO foo3;", "foo2", - "pq: user foo2 does not have GRANT privilege on relation t2", 1), - execSQLAsUser("GRANT CREATE ON TABLE t2 TO foo3;", "foo2", - "pq: user foo2 does not have GRANT privilege on relation t2", 2), // Same error from node 2. - execSQLAsUser("GRANT CREATE ON TABLE t3 TO foo3;", "foo2", "", 1), - execSQLAsUser("GRANT CREATE ON TABLE t3 TO foo3;", "foo2", "", 2), - execSQLAsUser("GRANT CREATE ON TABLE t2 TO foo4;", "foo3", - "pq: user foo3 does not have GRANT privilege on relation t2", 1), - execSQLAsUser("GRANT CREATE ON TABLE t2 TO foo4;", "foo3", - "pq: user foo3 does not have GRANT privilege on relation t2", 2), // Same error from node 2. - execSQLAsUser("GRANT CREATE ON TABLE t3 TO foo4;", "foo3", "", 1), - execSQLAsUser("GRANT CREATE ON TABLE t3 TO foo4;", "foo3", "", 2), - - execSQLAsUser("GRANT USAGE ON SCHEMA s TO target;", "foo2", "", 3), - execSQLAsUser("GRANT INSERT ON TABLE t1 TO target;", "foo3", "", 2), - execSQLAsUser("GRANT GRANT ON TYPE ty TO target;", "foo4", "", 2), - - // Node 2 is upgraded. - binaryUpgradeStep(c.Node(2), mainVersion), - allowAutoUpgradeStep(2), - execSQLAsUser("GRANT ALL PRIVILEGES ON DATABASE d TO target;", "foo", "", 3), - execSQLAsUser("GRANT ALL PRIVILEGES ON SCHEMA s TO target;", "foo2", "", 2), - execSQLAsUser("GRANT ALL PRIVILEGES ON TABLE t1 TO target;", "foo3", "", 1), - execSQLAsUser("GRANT ALL PRIVILEGES ON TYPE ty TO target;", "foo4", "", 1), - execSQLAsUser("GRANT GRANT ON DATABASE d TO foo2;", "foo", "", 1), - execSQLAsUser("GRANT GRANT ON SCHEMA s TO foo3;", "foo2", "", 1), - execSQLAsUser("GRANT GRANT, SELECT ON TABLE t1 TO foo4;", "foo3", "", 2), - execSQLAsUser("GRANT DELETE ON TABLE t1 TO foo4;", "foo3", "", 2), - - // Node 3 is upgraded; because all the nodes have been upgraded, the migration will begin running after - // allowAutoUpgradeStep(3). The roachtest will ensure that the migration will be complete before moving - // beyond waitForUpgradeStep(c.All()). - binaryUpgradeStep(c.Node(3), mainVersion), - allowAutoUpgradeStep(3), - waitForUpgradeStep(c.All()), - - // All the nodes have been upgraded and the migration has finished; existing users will have had their grant - // option bits set equal to their privilege bits if they hold "GRANT" or "ALL" privileges. Grant options will - // now be enforced (an error will be returned if a user tries to grant a privilege they do not hold grant - // options for, which corresponds to a nonempty string in the errorExpected field). - execSQLAsUser("GRANT DELETE ON TABLE t1 TO foo2;", "foo3", "", 3), - execSQLAsUser("GRANT ALL PRIVILEGES ON SCHEMA s TO foo3;", "target", "", 2), - execSQLAsUser("GRANT CREATE ON DATABASE d TO foo3;", "foo2", "pq: user foo2 does not have CREATE privilege on database d", 2), - execSQLAsUser("GRANT USAGE ON SCHEMA s TO foo;", "foo3", "", 2), - execSQLAsUser("GRANT INSERT ON TABLE t1 TO foo;", "foo4", "pq: user foo4 does not have INSERT privilege on relation t1", 1), - execSQLAsUser("GRANT SELECT ON TABLE t1 TO foo;", "foo4", "", 1), - execSQLAsUser("GRANT DELETE ON TABLE t1 TO target;", "foo2", "pq: user foo2 missing WITH GRANT OPTION privilege on DELETE", 1), - execSQLAsUser("GRANT DELETE ON TABLE t1 TO target;", "foo4", "", 1), - execSQLAsUser("GRANT SELECT ON TABLE t1 TO target;", "foo4", "", 1), - - execSQLAsUser("GRANT SELECT ON TABLE t2 TO foo4 WITH GRANT OPTION;", "foo", "", 1), - execSQLAsUser("GRANT SELECT ON TABLE t3 TO foo4 WITH GRANT OPTION;", "foo", "", 2), - - execSQLAsUser("GRANT CREATE ON TABLE t2 TO foo3;", "foo2", "pq: user foo2 missing WITH GRANT OPTION privilege on CREATE", 1), - execSQLAsUser("GRANT CREATE ON TABLE t3 TO target;", "foo2", "", 2), - execSQLAsUser("GRANT CREATE ON TABLE t2 TO foo4;", "foo3", "pq: user foo3 missing WITH GRANT OPTION privilege on CREATE", 3), - execSQLAsUser("GRANT CREATE ON TABLE t3 TO target;", "foo3", "", 3), - ) - u.run(ctx, t) -} diff --git a/pkg/sql/colexec/colbuilder/execplan.go b/pkg/sql/colexec/colbuilder/execplan.go index 056277091dff..ef15183726fc 100644 --- a/pkg/sql/colexec/colbuilder/execplan.go +++ b/pkg/sql/colexec/colbuilder/execplan.go @@ -1237,6 +1237,7 @@ func NewColOperator( } typs = append(typs, typ) argIdxs[i] = castIdx + argTypes[i] = typ } partitionColIdx := tree.NoColumnIdx diff --git a/pkg/sql/opt/xform/BUILD.bazel b/pkg/sql/opt/xform/BUILD.bazel index 70cf0cbe8810..f51af7ab55bc 100644 --- a/pkg/sql/opt/xform/BUILD.bazel +++ b/pkg/sql/opt/xform/BUILD.bazel @@ -50,7 +50,6 @@ go_library( "//pkg/util/buildutil", "//pkg/util/errorutil", "//pkg/util/log", - "//pkg/util/randutil", "//pkg/util/treeprinter", "@com_github_cockroachdb_errors//:errors", "@com_github_cockroachdb_redact//:redact", diff --git a/pkg/sql/opt/xform/coster.go b/pkg/sql/opt/xform/coster.go index 646f890aa263..c6c7a2759f5e 100644 --- a/pkg/sql/opt/xform/coster.go +++ b/pkg/sql/opt/xform/coster.go @@ -559,7 +559,12 @@ func (c *coster) ComputeCost(candidate memo.RelExpr, required *physical.Required // Don't perturb the cost if we are forcing an index. if cost < hugeCost { // Get a random value in the range [-1.0, 1.0) - multiplier := 2*c.rng.Float64() - 1 + var multiplier float64 + if c.rng == nil { + multiplier = 2*rand.Float64() - 1 + } else { + multiplier = 2*c.rng.Float64() - 1 + } // If perturbation is p, and the estimated cost of an expression is c, // the new cost is in the range [max(0, c - pc), c + pc). For example, diff --git a/pkg/sql/opt/xform/optimizer.go b/pkg/sql/opt/xform/optimizer.go index 916fa2283d82..c45c4457a752 100644 --- a/pkg/sql/opt/xform/optimizer.go +++ b/pkg/sql/opt/xform/optimizer.go @@ -24,7 +24,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/util" "github.com/cockroachdb/cockroach/pkg/util/errorutil" "github.com/cockroachdb/cockroach/pkg/util/log" - "github.com/cockroachdb/cockroach/pkg/util/randutil" "github.com/cockroachdb/errors" ) @@ -127,9 +126,8 @@ func (o *Optimizer) Init(evalCtx *eval.Context, catalog cat.Catalog) { o.mem = o.f.Memo() o.explorer.init(o) seed := evalCtx.SessionData().TestingOptimizerRandomCostSeed - o.rng, _ = randutil.NewPseudoRand() if seed != 0 { - o.rng.Seed(seed) + o.rng = rand.New(rand.NewSource(seed)) // If we've been initialized with a random seed but not an explicit // perturbation value, use the max perturbation. This is used for tests // that set the seed with testing_optimizer_random_cost_seed, and will allow