From 9181af08858fd046793553e0c1f535b2e7a5c9d4 Mon Sep 17 00:00:00 2001 From: Xiang Gu Date: Tue, 4 Oct 2022 18:24:37 -0400 Subject: [PATCH 1/3] jobs: check whether adoption is disabled before resuming a job There is one place where we resume a job without first checking whether adoption on that registry is disabled. This commit fixes that. --- pkg/jobs/wait.go | 4 +++- pkg/upgrade/upgrades/wait_for_schema_changes_test.go | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/jobs/wait.go b/pkg/jobs/wait.go index 27cc2474b20e..6f1ce37abac4 100644 --- a/pkg/jobs/wait.go +++ b/pkg/jobs/wait.go @@ -46,7 +46,9 @@ func (r *Registry) NotifyToResume(ctx context.Context, jobs ...jobspb.JobID) { _ = r.stopper.RunAsyncTask(ctx, "resume-jobs", func(ctx context.Context) { r.withSession(ctx, func(ctx context.Context, s sqlliveness.Session) { r.filterAlreadyRunningAndCancelFromPreviousSessions(ctx, s, m) - r.resumeClaimedJobs(ctx, s, m) + if !r.adoptionDisabled(ctx) { + r.resumeClaimedJobs(ctx, s, m) + } }) }) } diff --git a/pkg/upgrade/upgrades/wait_for_schema_changes_test.go b/pkg/upgrade/upgrades/wait_for_schema_changes_test.go index 812825feb025..87672e8b64e6 100644 --- a/pkg/upgrade/upgrades/wait_for_schema_changes_test.go +++ b/pkg/upgrade/upgrades/wait_for_schema_changes_test.go @@ -254,7 +254,6 @@ func TestWaitForSchemaChangeMigrationSynthetic(t *testing.T) { var waitCount int32 var secondWaitChan chan struct{} params.Knobs.JobsTestingKnobs = &jobs.TestingKnobs{ - DisableAdoptions: true, BeforeWaitForJobsQuery: func() { if secondWaitChan != nil { if atomic.AddInt32(&waitCount, 1) == 2 { From 714b4d3635a7546f93125be403677671fd069470 Mon Sep 17 00:00:00 2001 From: Xiang Gu Date: Tue, 4 Oct 2022 18:25:59 -0400 Subject: [PATCH 2/3] tests: skip a roachtest test We skipped a roachtest until master is on v23.1 --- ...version_job_compatibility_in_declarative_schema_changer.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/cmd/roachtest/tests/mixed_version_job_compatibility_in_declarative_schema_changer.go b/pkg/cmd/roachtest/tests/mixed_version_job_compatibility_in_declarative_schema_changer.go index a594823f85c9..68312ffaaa18 100644 --- a/pkg/cmd/roachtest/tests/mixed_version_job_compatibility_in_declarative_schema_changer.go +++ b/pkg/cmd/roachtest/tests/mixed_version_job_compatibility_in_declarative_schema_changer.go @@ -18,6 +18,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option" "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry" "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test" + "github.com/cockroachdb/cockroach/pkg/testutils/skip" "github.com/stretchr/testify/require" ) @@ -84,6 +85,9 @@ func registerDeclarativeSchemaChangerJobCompatibilityInMixedVersion(r registry.R Owner: registry.OwnerSQLSchema, Cluster: r.MakeClusterSpec(4), Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { + // Skip this roachtest until master is on 23.1 + skip.WithIssue(t, 89345) + if runtime.GOARCH == "arm64" { t.Skip("Skip under ARM64. See https://github.com/cockroachdb/cockroach/issues/89268") } From 9bcc2e705d8ebbbb80cd0dc3d59c85312f4cbaf5 Mon Sep 17 00:00:00 2001 From: Chengxiong Ruan Date: Wed, 5 Oct 2022 16:20:39 -0400 Subject: [PATCH 3/3] sql: benchmark for function expression type checking Release note: None --- pkg/bench/BUILD.bazel | 7 +++ pkg/bench/bench_test.go | 106 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+) diff --git a/pkg/bench/BUILD.bazel b/pkg/bench/BUILD.bazel index 4de5efed6bb5..04511c01295a 100644 --- a/pkg/bench/BUILD.bazel +++ b/pkg/bench/BUILD.bazel @@ -37,6 +37,12 @@ go_test( "//pkg/security/securitytest", "//pkg/security/username", "//pkg/server", + "//pkg/sql", + "//pkg/sql/parser", + "//pkg/sql/sem/tree", + "//pkg/sql/sessiondata", + "//pkg/sql/sessiondatapb", + "//pkg/sql/types", "//pkg/testutils/serverutils", "//pkg/testutils/skip", "//pkg/testutils/sqlutils", @@ -48,6 +54,7 @@ go_test( "//pkg/util/tracing", "@com_github_go_sql_driver_mysql//:mysql", "@com_github_lib_pq//:pq", + "@com_github_stretchr_testify//require", ], ) diff --git a/pkg/bench/bench_test.go b/pkg/bench/bench_test.go index 9ce8e63bfc8a..af1dede42960 100644 --- a/pkg/bench/bench_test.go +++ b/pkg/bench/bench_test.go @@ -23,6 +23,13 @@ import ( "testing" "github.com/cockroachdb/cockroach/pkg/base" + "github.com/cockroachdb/cockroach/pkg/security/username" + "github.com/cockroachdb/cockroach/pkg/sql" + "github.com/cockroachdb/cockroach/pkg/sql/parser" + "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" + "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" + "github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb" + "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" "github.com/cockroachdb/cockroach/pkg/testutils/skip" "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" @@ -33,6 +40,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/util/tracing" _ "github.com/go-sql-driver/mysql" _ "github.com/lib/pq" + "github.com/stretchr/testify/require" ) func runBenchmarkSelect1(b *testing.B, db *sqlutils.SQLRunner) { @@ -1420,3 +1428,101 @@ func BenchmarkNameResolution(b *testing.B) { b.StopTimer() }) } + +func BenchmarkFuncExprTypeCheck(b *testing.B) { + skip.UnderShort(b) + defer log.Scope(b).Close(b) + + s, db, kvDB := serverutils.StartServer(b, base.TestServerArgs{UseDatabase: "defaultdb"}) + defer s.Stopper().Stop(context.Background()) + + sqlDB := sqlutils.MakeSQLRunner(db) + sqlDB.ExecMultiple(b, + `CREATE SCHEMA sc1`, + `CREATE SCHEMA sc2`, + `CREATE FUNCTION abs(val INT) RETURNS INT CALLED ON NULL INPUT LANGUAGE SQL AS $$ SELECT val $$`, + `CREATE FUNCTION sc1.udf(val INT) RETURNS INT CALLED ON NULL INPUT LANGUAGE SQL AS $$ SELECT val $$`, + `CREATE FUNCTION sc1.udf(val STRING) RETURNS STRING LANGUAGE SQL AS $$ SELECT val $$`, + `CREATE FUNCTION sc1.udf(val FLOAT) RETURNS FLOAT LANGUAGE SQL AS $$ SELECT val $$`, + `CREATE FUNCTION sc2.udf(val INT) RETURNS INT LANGUAGE SQL AS $$ SELECT val $$`, + ) + + ctx := context.Background() + execCfg := s.ExecutorConfig().(sql.ExecutorConfig) + p, cleanup := sql.NewInternalPlanner("type-check-benchmark", + kvDB.NewTxn(ctx, "type-check-benchmark-planner"), + username.RootUserName(), + &sql.MemoryMetrics{}, + &execCfg, + sessiondatapb.SessionData{ + Database: "defaultdb", + }, + ) + + defer cleanup() + semaCtx := p.(sql.PlanHookState).SemaCtx() + sp := sessiondata.MakeSearchPath(append(sessiondata.DefaultSearchPath.GetPathArray(), "sc1", "sc2")) + semaCtx.SearchPath = &sp + + testCases := []struct { + name string + exprStr string + }{ + { + name: "builtin called on null input", + exprStr: "md5('some_string')", + }, + { + name: "builtin not called on null input", + exprStr: "parse_timetz('some_string')", + }, + { + name: "builtin aggregate", + exprStr: "corr(123, 321)", + }, + { + name: "builtin aggregate not called on null", + exprStr: "concat_agg(NULL)", + }, + { + name: "udf same name as builtin", + exprStr: "abs(123)", + }, + { + name: "udf across different schemas", + exprStr: "udf(123)", + }, + { + name: "unary operator", + exprStr: "-123", + }, + { + name: "binary operator", + exprStr: "123 + 321", + }, + { + name: "comparison operator", + exprStr: "123 > 321", + }, + { + name: "tuple comparison operator", + exprStr: "(1, 2, 3) > (1, 2, 4)", + }, + { + name: "tuple in operator", + exprStr: "1 in (1, 2, 3)", + }, + } + + for _, tc := range testCases { + b.Run(tc.name, func(b *testing.B) { + expr, err := parser.ParseExpr(tc.exprStr) + require.NoError(b, err) + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, err := tree.TypeCheck(ctx, expr, semaCtx, types.Any) + require.NoError(b, err) + } + }) + } +}