Merge #89348 #89438

89348: tests: Skip a roachtest test until master is on 23.1 r=Xiang-Gu a=Xiang-Gu

Commit 1: Fixed a bug where we forgot to check whether
job adoption is disabled before resuming a job. It also uncovered
a flaw in another test which we fix.

Commit 2: Skip a roachtest that we recently see failure until
master is on 23.1. The reason is detailed in issue #89345.

Fixes: #88921
Backport commit 1 will also fix #89091

Release note: None

89438: sql: benchmark for function expression type checking r=chengxiong-ruan a=chengxiong-ruan

Release note: None

Co-authored-by: Xiang Gu <[email protected]>
Co-authored-by: Chengxiong Ruan <[email protected]>

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",
     ],
 )
 

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)
+			}
+		})
+	}
+}

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")
 			}

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)
+			}
 		})
 	})
 }

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 {