sql/tests: TestRandomSyntaxGeneration failed [ALTER TENANT (SELECT 'foo') START REPLICATION ... immediately crashes the database process]

[cockroach-teamcity]

sql/tests.TestRandomSyntaxGeneration failed with artifacts on release-24.2.5-rc @ beddec16a5a438d66bd3f5f4202085cf21c0973c:

*   | 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/type_check.go:355
*   | github.com/cockroachdb/cockroach/pkg/sql.(*planner).analyzeExpr
*   | 	github.com/cockroachdb/cockroach/pkg/sql/analyze_expr.go:34
*   | github.com/cockroachdb/cockroach/pkg/sql.(*planner).planTenantSpec
*   | 	github.com/cockroachdb/cockroach/pkg/sql/tenant_spec.go:63
*   | github.com/cockroachdb/cockroach/pkg/sql.(*planner).LookupTenantInfo
*   | 	github.com/cockroachdb/cockroach/pkg/sql/tenant_spec.go:148
*   | github.com/cockroachdb/cockroach/pkg/ccl/crosscluster/physical.alterReplicationJobHook.func1
*   | 	github.com/cockroachdb/cockroach/pkg/ccl/crosscluster/physical/alter_replication_job.go:205
*   | github.com/cockroachdb/cockroach/pkg/sql.(*hookFnNode).startExec.func1
*   | 	github.com/cockroachdb/cockroach/pkg/sql/planhook.go:194
*   | github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTaskEx.func2
*   | 	github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:480
*   | runtime.goexit
*   | 	src/runtime/asm_amd64.s:1695
* Wraps: (4) ReturnType called on TypedExpr with empty typeAnnotation. Was the underlying Expr type-checked before asserting a type of TypedExpr?
* Error types: (1) *withstack.withStack (2) *assert.withAssertionFailure (3) *withstack.withStack (4) *errutil.leafError
*
panic: ReturnType called on TypedExpr with empty typeAnnotation. Was the underlying Expr type-checked before asserting a type of TypedExpr? [recovered]
	panic: ReturnType called on TypedExpr with empty typeAnnotation. Was the underlying Expr type-checked before asserting a type of TypedExpr?

goroutine 420896 [running]:
github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).recover(0x454825?, {0x85580b0, 0xc00dbaa570})
	github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:226 +0x65
panic({0x64ffe00?, 0xc018b907b0?})
	GOROOT/src/runtime/panic.go:770 +0x132
github.com/cockroachdb/cockroach/pkg/sql/sem/tree.typeAnnotation.assertTyped(...)
	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/expr.go:148
github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*Subquery).TypeCheck(0x8558078?, {0xc008b7fea0?, 0x7f610785bae0?}, 0xc006822908?, 0x0?)
	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/type_check.go:1742 +0xd3
github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*ArrayFlatten).TypeCheck(0xc01f909e60, {0x85580b0?, 0xc00dbaa570?}, 0x6c8b272?, 0x850000c006822908?)
	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/type_check.go:1922 +0x46
github.com/cockroachdb/cockroach/pkg/sql/sem/tree.TypeCheck({0x85580b0?, 0xc00dbaa570?}, {0x8558f58?, 0xc01f909e60?}, 0x6bb7284?, 0xc0015353e0?)
	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/type_check.go:345 +0x8a
github.com/cockroachdb/cockroach/pkg/sql/sem/tree.TypeCheckAndRequire({0x85580b0?, 0xc00dbaa570?}, {0x8558f58?, 0xc01f909e60?}, 0x0?, 0xc365320, {0x6c79266, 0x21})
	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/type_check.go:355 +0x54
github.com/cockroachdb/cockroach/pkg/sql.(*planner).analyzeExpr(0xc0148d86b8, {0x85580b0, 0xc00dbaa570}, {0x8558f58?, 0xc01f909e60?}, {{0x0, 0x0, 0x0}, {0x0, 0x0}}, ...)
	github.com/cockroachdb/cockroach/pkg/sql/analyze_expr.go:34 +0xaf
github.com/cockroachdb/cockroach/pkg/sql.(*planner).planTenantSpec(0xc00dbaa570?, {0x85580b0?, 0xc00dbaa570?}, 0xc00000001e?, {0x6c79266?, 0xc01539d680?})
	github.com/cockroachdb/cockroach/pkg/sql/tenant_spec.go:63 +0x190
github.com/cockroachdb/cockroach/pkg/sql.(*planner).LookupTenantInfo(0xc0148d86b8, {0x85580b0, 0xc00dbaa570}, 0x8536580?, {0x6c79266?, 0xc006822908?})
	github.com/cockroachdb/cockroach/pkg/sql/tenant_spec.go:148 +0x27
github.com/cockroachdb/cockroach/pkg/ccl/crosscluster/physical.alterReplicationJobHook.func1({0x85580b0, 0xc00dbaa570}, {0x0?, 0xc006199f00?, 0xd54e14?}, 0xc00e4f3f80)
	github.com/cockroachdb/cockroach/pkg/ccl/crosscluster/physical/alter_replication_job.go:205 +0x150
github.com/cockroachdb/cockroach/pkg/sql.(*hookFnNode).startExec.func1({0x85580b0, 0xc00dbaa570})
	github.com/cockroachdb/cockroach/pkg/sql/planhook.go:194 +0x4a
github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTaskEx.func2()
	github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:480 +0x13a
created by github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTaskEx in goroutine 420884
	github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:471 +0x3fe

Help

See also: How To Investigate a Go Test Failure (internal)

Same failure on other branches

• sql/tests: TestRandomSyntaxGeneration failed [CREATE TENANT statement is not valid after parsing then formatting] #132558 sql/tests: TestRandomSyntaxGeneration failed [CREATE TENANT statement is not valid after parsing then formatting] [C-test-failure O-robot P-2 T-db-server branch-release-24.1.6-rc]

/cc @cockroachdb/sql-foundations

This test on roachdash | Improve this report!

Jira issue: CRDB-44704

[spilchen]

We are hitting this assert:

cockroach/pkg/sql/sem/tree/expr.go

Lines 146 to 152 in a6bcc1a

	func (ta typeAnnotation) assertTyped() {
	if ta.typ == nil {
	panic(errors.AssertionFailedf(
	"ReturnType called on TypedExpr with empty typeAnnotation. " +
	"Was the underlying Expr type-checked before asserting a type of TypedExpr?"))
	}
	}

I can't seem to locate the SQL statement that was being parsed, which would be helpful for debugging.

[blathers-crl]

cc @cockroachdb/disaster-recovery

[rafiss]

Repro on a fresh cluster with no other setup:

root@localhost:26257/defaultdb> ALTER TENANT (SELECT 'foo') START REPLICATION OF 'bar' ON 'baz' WITH RETENTION = '1s';
ERROR: unexpected EOF
warning: error retrieving the transaction status: connection closed unexpectedly: conn closed
warning: connection lost!
opening new connection: all session settings will be lost
warning: error retrieving the database name: failed to connect to `host=localhost user=root database=`: dial error (dial tcp [::1]:26257: connect: connection refused)

Failures on other branches: #136274 and #136339

[msbutler]

@rafiss thanks for digging up a precise repro! I'm noticing something strange with how we parse invalid statements. In this experimental branch, I play with the test TestAlterTenantStartReplicationAfterRestore and notice that if i only add a subquery to the ALTER TENANT stmt, i.e.:

db.Exec(t, "ALTER TENANT (select 't1') START REPLICATION OF t1 ON $1", u.String())

the client receives the expected pq: subqueries are not allowed in ALTER VIRTUAL CLUSTER REPLICATION.

However, if the user passes a bogus pgurl ('foo'), we do not conduct the subquery check, and panic:

db.Exec(t, "ALTER TENANT (select 't1') START REPLICATION OF t1 ON 'foo'")

Has this TestRandomSyntaxGeneration test hit similar errors before? I don't understand why this subqueries check is only hit on the happy pgurl path.

[msbutler]

Oh wait, actually, if foo is set via placeholder, we do see the correct subquery check

db.Exec(t, "ALTER TENANT (select 't1') START REPLICATION OF t1 ON $1", "foo")

returns pq: subqueries are not allowed in ALTER VIRTUAL CLUSTER REPLICATION

while this panics:

db.Exec(t, "ALTER TENANT (select 't1') START REPLICATION OF t1 ON 'foo'")

So, now i'm wondering why place holder evaluation triggers the right subquery check.

[msbutler]

So we catch the subquery error for placeholders in alterReplicationJobTypeCheck, which for some reason, only runs when the stmt has placeholders. I think we should be running the type checker regardless of there being place holders.
https://github.com/msbutler/cockroach/blob/butler-debug-alter-parse/pkg/crosscluster/physical/alter_replication_job.go#L128