Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

sql: panic: ReturnType called on TypedExpr with empty typeAnnotation #36036

Closed
maddyblue opened this issue Mar 21, 2019 · 5 comments · Fixed by #36518
Closed

sql: panic: ReturnType called on TypedExpr with empty typeAnnotation #36036

maddyblue opened this issue Mar 21, 2019 · 5 comments · Fixed by #36518
Assignees
@jordanlewis @vivekmenezes
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-sqlsmith

Comments

@maddyblue
Copy link
Contributor 

CREATE TABLE t (
    _bool BOOL,
    _bytes BYTES,
    _date DATE,
    _decimal DECIMAL,
    _float4 FLOAT4,
    _float8 FLOAT8,
    _inet INET,
    _int4 INT4,
    _int8 INT8,
    _interval INTERVAL,
    _jsonb JSONB,
    _string STRING,
    _time TIME,
    _timestamp TIMESTAMP,
    _timestamptz TIMESTAMPTZ,
    _uuid UUID
);

ALTER TABLE defaultdb.public.t ADD COLUMN col_40 STRING NOT NULL AS (((SELECT tab_44.col_18 AS col_41 FROM defaultdb.public.t AS tab_44 CROSS JOIN defaultdb.public.t AS tab_45, [DELETE FROM defaultdb.public.t AS tab_46 WHERE true LIMIT 8:::INT8 RETURNING col_18 AS ret_1, col_30 AS ret_2, col_23 AS ret_3, 7555327040598613.505:::DECIMAL AS ret_4, '1970-01-04 09:29:00.000281+00:00':::TIMESTAMP AS ret_5, CASE WHEN (((NOT ((e'\\x98586bbb2a':::BYTES::BYTES || col_1::BYTES) IN ((col_13::BYTES || e'\\x6582393175697782':::BYTES::BYTES), NULL))) AND _bool) OR col_30) THEN col_24 ELSE col_25 END AS ret_6, 4274792555:::OID AS ret_7] LIMIT 1:::INT8))) STORED;

E190321 20:20:40.232181 317 sql/conn_executor.go:700  [n1,client=127.0.0.1:36686,user=root] a SQL panic has occurred while executing "ALTER TABLE defaultdb.public.t ADD COLUMN col_40 STRING NOT NULL AS (((SELECT tab_44.col_18 AS col_41 FROM defaultdb.public.t AS tab_44 CROSS JOIN defaultdb.public.t AS tab_45, [DELETE FROM defaultdb.public.t AS tab_46 WHERE true LIMIT 8:::INT8 RETURNING col_18 AS ret_1, col_30 AS ret_2, col_23 AS ret_3, 7555327040598613.505:::DECIMAL AS ret_4, '1970-01-04 09:29:00.000281+00:00':::TIMESTAMP AS ret_5, CASE WHEN (((NOT ((e'\\\\x98586bbb2a':::BYTES::BYTES || col_1::BYTES) IN ((col_13::BYTES || e'\\\\x6582393175697782':::BYTES::BYTES), NULL))) AND _bool) OR col_30) THEN col_24 ELSE col_25 END AS ret_6, 4274792555:::OID AS ret_7] LIMIT 1:::INT8))) STORED": internal error: ReturnType called on TypedExpr with empty typeAnnotation. Was the underlying Expr type-checked before asserting a type of TypedExpr?
E190321 20:20:40.232428 317 util/log/crash_reporting.go:216  [n1,client=127.0.0.1:36686,user=root] a panic has occurred!
panic: internal error: ReturnType called on TypedExpr with empty typeAnnotation. Was the underlying Expr type-checked before asserting a type of TypedExpr? [recovered]
	panic: panic while executing 1 statements: ALTER TABLE _._._ ADD COLUMN _ STRING NOT NULL AS (((SELECT _._ AS _ FROM _._._ AS _ CROSS JOIN _._._ AS _, [DELETE FROM _._._ AS _ WHERE _ LIMIT _:::INT8 RETURNING _ AS _, _ AS _, _ AS _, _:::DECIMAL AS _, _:::TIMESTAMP AS _, CASE WHEN (((NOT ((_:::BYTES::BYTES || _::BYTES) IN ((_::BYTES || _:::BYTES::BYTES), _))) AND _) OR _) THEN _ ELSE _ END AS _, _:::OID AS _] LIMIT _:::INT8))) STORED; caused by internal error: ReturnType called on TypedExpr with empty typeAnnotation. Was the underlying Expr type-checked before asserting a type of TypedExpr?

goroutine 317 [running]:
github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).closeWrapper(0xc001761600, 0x39606c0, 0xc00153f840, 0x312bcc0, 0xc0014a2200)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:714 +0x36d
github.com/cockroachdb/cockroach/pkg/sql.(*Server).ServeConn.func1(0xc001761600, 0x39606c0, 0xc00153f840)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:426 +0x61
panic(0x312bcc0, 0xc0014a2200)
	/usr/local/go/src/runtime/panic.go:513 +0x1b9
github.com/cockroachdb/cockroach/pkg/sql/sem/tree.typeAnnotation.assertTyped(0x0, 0x0)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/sem/tree/expr.go:139 +0x75
github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*Subquery).TypeCheck(0xc001688930, 0xc000f5a160, 0x397dec0, 0x5a716c0, 0x0, 0x0, 0xc001688930, 0x3060000)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/sem/tree/type_check.go:1144 +0x53
github.com/cockroachdb/cockroach/pkg/sql/sem/tree.TypeCheck(0x3966bc0, 0xc001688930, 0xc000f5a160, 0x397dec0, 0x5a716c0, 0xc000693c00, 0x11, 0x11, 0x3952dc0)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/sem/tree/type_check.go:259 +0x13b
github.com/cockroachdb/cockroach/pkg/sql/sqlbase.MakeComputedExprs(0xc0009be000, 0x1, 0x1, 0xc0017bac00, 0xc001cd6820, 0xc000602b40, 0xc00109eb60, 0x1, 0x0, 0x0, ...)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/sqlbase/computed_exprs.go:199 +0x971
github.com/cockroachdb/cockroach/pkg/sql/backfill.(*ColumnBackfiller).Init(0xc0006aa3e0, 0xc00109eb60, 0xc0017bac00, 0x3, 0x158e12d7f4f6bbf1)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/backfill/backfill.go:93 +0x403
github.com/cockroachdb/cockroach/pkg/sql/distsqlrun.newColumnBackfiller(0xc001772000, 0x0, 0x1, 0x5632bf4, 0x1, 0x3200000034, 0x3, 0x158e12d7f4f6bbf1, 0x0, 0xc001aa8f80, ...)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/distsqlrun/columnbackfiller.go:66 +0x39d
github.com/cockroachdb/cockroach/pkg/sql/distsqlrun.newProcessor(0x3960780, 0xc001688480, 0xc001772000, 0x0, 0xc00109e698, 0xc00109e760, 0x0, 0x0, 0x0, 0xc001cd8420, ...)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/distsqlrun/processors.go:1074 +0x19a4
github.com/cockroachdb/cockroach/pkg/sql/distsqlrun.(*Flow).makeProcessor(0xc001772000, 0x3960780, 0xc001688480, 0xc00109e680, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/distsqlrun/flow.go:350 +0x1d5
github.com/cockroachdb/cockroach/pkg/sql/distsqlrun.(*Flow).setupProcessors(0xc001772000, 0x3960780, 0xc001688480, 0xc0002b9180, 0x1, 0x1, 0x0, 0x0)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/distsqlrun/flow.go:425 +0x1cb
github.com/cockroachdb/cockroach/pkg/sql/distsqlrun.(*Flow).setup(0xc001772000, 0x3960780, 0xc001688480, 0xc0014f6100, 0x0, 0x0)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/distsqlrun/flow.go:525 +0xda
github.com/cockroachdb/cockroach/pkg/sql/distsqlrun.(*ServerImpl).setupFlow(0xc0001141e0, 0x3960780, 0xc0009b96e0, 0x0, 0x0, 0x0, 0xc0014f60f0, 0x394fb40, 0xc000872000, 0xc0011f2200, ...)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/distsqlrun/server.go:463 +0x82c
github.com/cockroachdb/cockroach/pkg/sql/distsqlrun.(*ServerImpl).SetupLocalSyncFlow(0xc0001141e0, 0x3960780, 0xc0009b96e0, 0x0, 0xc0014f60f0, 0x394fb40, 0xc000872000, 0xc0011f2200, 0x0, 0x0, ...)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/distsqlrun/server.go:518 +0xe7
github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).Run(0xc0000ecdc0, 0xc000602780, 0x0, 0xc001cd9c60, 0xc000872000, 0xc0011f2200, 0x0)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:241 +0x83d
github.com/cockroachdb/cockroach/pkg/sql.(*SchemaChanger).distBackfill.func2(0x3960780, 0xc0009b96e0, 0xc000f1d5f0, 0x0, 0x0)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/backfill.go:617 +0x5f6
github.com/cockroachdb/cockroach/pkg/internal/client.(*DB).Txn.func1(0x3960780, 0xc0009b96e0, 0xc000f1d5f0, 0x0, 0x0)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/internal/client/db.go:585 +0x43
github.com/cockroachdb/cockroach/pkg/internal/client.(*Txn).exec(0xc000f1d5f0, 0x3960780, 0xc0009b96e0, 0xc000315a60, 0x1, 0xc000f1d5f0)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/internal/client/txn.go:688 +0xd9
github.com/cockroachdb/cockroach/pkg/internal/client.(*DB).Txn(0xc0006c2780, 0x3960780, 0xc0009b96e0, 0xc0015eff80, 0x1b, 0xc001cda720)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/internal/client/db.go:584 +0xe3
github.com/cockroachdb/cockroach/pkg/sql.(*SchemaChanger).distBackfill(0xc000f5a0b0, 0x3960780, 0xc0009b96e0, 0xc0011f2200, 0xc001c9e0a0, 0xc000000003, 0x1, 0xc8, 0x335d000, 0x3, ...)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/backfill.go:540 +0x3c0
github.com/cockroachdb/cockroach/pkg/sql.(*SchemaChanger).truncateAndBackfillColumns(0xc000f5a0b0, 0x3960780, 0xc0009b96e0, 0xc0011f2200, 0xc001c9e0a0, 0x3, 0x3, 0xc000512820)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/backfill.go:940 +0x86
github.com/cockroachdb/cockroach/pkg/sql.(*SchemaChanger).runBackfill(0xc000f5a0b0, 0x3960780, 0xc0009b96e0, 0xc001c9e0a0, 0xc0011f2200, 0x0, 0x0)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/backfill.go:208 +0x7cb
github.com/cockroachdb/cockroach/pkg/sql.(*SchemaChanger).runStateMachineAndBackfill(0xc000f5a0b0, 0x3960780, 0xc0009b96e0, 0xc001c9e0a0, 0xc0011f2200, 0x0, 0x0)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/schema_changer.go:1101 +0x9d
github.com/cockroachdb/cockroach/pkg/sql.(*SchemaChanger).exec(0xc000f5a0b0, 0x3960780, 0xc0009b96e0, 0x1, 0xc0011f2200, 0x0, 0x0)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/schema_changer.go:852 +0x3ed
github.com/cockroachdb/cockroach/pkg/sql.(*schemaChangerCollection).execSchemaChanges(0xc0017618b8, 0x39606c0, 0xc00153f840, 0xc000664340, 0xc0017617d8, 0xc00094e6d0, 0xc00037c810, 0x0)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/exec_util.go:971 +0x1df
github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).txnStateTransitionsApplyWrapper(0xc001761600, 0x392c760, 0x5a71ed8, 0x2ed6d00, 0xc00037c810, 0x7f2904cd8a20, 0xc000bcc2c0, 0x58, 0x0, 0x0, ...)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1987 +0x707
github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).run(0xc001761600, 0x39606c0, 0xc00153f840, 0xc0000352f8, 0x5400, 0x15000, 0xc000035390, 0xc0004eb530, 0x0, 0x0)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1306 +0xd24
github.com/cockroachdb/cockroach/pkg/sql.(*Server).ServeConn(0xc000801520, 0x39606c0, 0xc00153f840, 0xc001761600, 0x5400, 0x15000, 0xc000035390, 0xc0004eb530, 0x0, 0x0)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:428 +0xce
github.com/cockroachdb/cockroach/pkg/sql/pgwire.(*conn).serveImpl.func4(0xc000801520, 0x39606c0, 0xc00153f840, 0xc00011d600, 0xc000324ba4, 0xc0004eb530, 0xc001761600, 0x5400, 0x15000, 0xc000035390, ...)
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/pgwire/conn.go:338 +0xe6
created by github.com/cockroachdb/cockroach/pkg/sql/pgwire.(*conn).serveImpl
	/home/mjibson/go/src/github.com/cockroachdb/cockroach/pkg/sql/pgwire/conn.go:321 +0x1080
@maddyblue maddyblue added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Mar 21, 2019
@maddyblue
Copy link
Contributor Author

maddyblue commented Apr 3, 2019 •
edited
Loading

Smaller repro:

CREATE TABLE t ();

ALTER TABLE t ADD COLUMN c STRING AS ((SELECT NULL)) STORED;

@jordanlewis
Copy link
Member

It looks to me like subqueries aren't properly handled by the column backfiller. Kind of messy. I think what should be happening here is that subqueries should be replaced ahead of time, before the backfiller runs - but no code is doing that. Hmm...

@maddyblue
Copy link
Contributor Author

We also incorrectly allow

CREATE TABLE t (i INT8 AS (1) STORED);

ALTER TABLE t ADD COLUMN c INT8 AS (i) STORED;

We just don't check the validity of computed columns during ALTER, only CREATE TABLE.

@jordanlewis
Copy link
Member

@vivekmenezes would you say that as a short-term fix for this issue we could ban subqueries within ALTER TABLE statements? That doesn't seem like an important use-case, but perhaps I'm wrong...

@maddyblue
Copy link
Contributor Author

I'm fixing it. It's not just a subqueries thing.

@maddyblue maddyblue mentioned this issue Apr 4, 2019
Merged
craig bot pushed a commit that referenced this issue Apr 5, 2019
@maddyblue
Merge #36518
8bb1bae 
36518: sql: validate computed columns during ALTER TABLE r=vivekmenezes a=mjibson

Previously these were only validated during CREATE, allowing the creation
of incorrect columns. validateComputedColumn has removed its dependency
on the tree.CreateTable struct so that the alter code can use it. Because
of this, all FKs must be computed prior to the computed validate checks.

Fixes #36036

Release note (bug fix): Correctly validate computed columns during ALTER
TABLE ADD COLUMN.

Co-authored-by: Matt Jibson <[email protected]>
@maddyblue maddyblue mentioned this issue Apr 5, 2019
Merged
@craig craig bot closed this as completed in #36518 Apr 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jordanlewis jordanlewis

@vivekmenezes vivekmenezes

Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-sqlsmith
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sql: validate computed columns during ALTER TABLE
3 participants
@maddyblue @jordanlewis @vivekmenezes