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

opt: panic on insert when a column is being dropped and a check constraint is present #35611

Closed
thoszhang opened this issue Mar 11, 2019 · 1 comment · Fixed by #35624
Closed

opt: panic on insert when a column is being dropped and a check constraint is present #35611

thoszhang opened this issue Mar 11, 2019 · 1 comment · Fixed by #35624
Labels
A-schema-changes A-sql-execution Relating to SQL execution. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. S-2-temp-unavailability Temp crashes or other availability problems. Can be worked around or resolved by restarting.

Comments

@thoszhang
Copy link
Contributor

thoszhang commented Mar 11, 2019 •
edited
Loading

Starting with this table:

CREATE DATABASE t;
CREATE TABLE t.test (k INT PRIMARY KEY, v INT CHECK (v > 0), a INT);

If I drop column a with ALTER TABLE t.test DROP COLUMN a, and try to insert values for (k, v) while a is in the delete-only state, I get this panic:

panic: could not find return column [recovered]
	panic: could not find return column [recovered]
	panic: panic while executing 1 statements: INSERT INTO _._(_, _) VALUES (_, _); caused by could not find return column

goroutine 397 [running]:
github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).closeWrapper(0xc000f30880, 0x6e8a1c0, 0xc001277180, 0x62fb7c0, 0x6e4aa70)
	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:715 +0x36d
github.com/cockroachdb/cockroach/pkg/sql.(*Server).ServeConn.func1(0xc000f30880, 0x6e8a1c0, 0xc001277180)
	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:427 +0x61
panic(0x62fb7c0, 0x6e4aa70)
	/usr/local/go/src/runtime/panic.go:513 +0x1b9
github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*Builder).Build.func1(0xc000f9e8a0)
	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/builder.go:142 +0x9a
panic(0x62fb7c0, 0x6e4aa70)
	/usr/local/go/src/runtime/panic.go:513 +0x1b9
github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*mutationBuilder).mapToReturnColID(...)
	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/mutation_builder.go:477
github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*mutationBuilder).disambiguateColumns(0xc001681340)
	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/mutation_builder.go:409 +0x369
github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*mutationBuilder).addCheckConstraintCols(0xc001681340)
	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/mutation_builder.go:379 +0x72
github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*mutationBuilder).buildInsert(0xc001681340, 0x0, 0x0, 0x0)
	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/insert.go:592 +0x2f
github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*Builder).buildInsert(0xc001421770, 0xc0012823c0, 0xc001241000, 0x0)
	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/insert.go:242 +0x606
github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*Builder).buildStmt(0xc001421770, 0x6e8f340, 0xc0012823c0, 0xc001241000, 0x4)
	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/builder.go:205 +0x3b2
github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*Builder).Build(0xc001421770, 0x0, 0x0)
	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/builder.go:156 +0x111
github.com/cockroachdb/cockroach/pkg/sql.(*optPlanningCtx).buildExecMemo(0xc000f31330, 0x6e8a280, 0xc00123ecf0, 0x0, 0x6e8a280, 0xc00123ecf0, 0x4060800)
	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/plan_opt.go:418 +0x183
github.com/cockroachdb/cockroach/pkg/sql.(*planner).makeOptimizerPlan(0xc000f30bd0, 0x6e8a280, 0xc00123ecf0, 0x671ed47, 0x19, 0x841ad092ad45, 0x2d98e5b8)
	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/plan_opt.go:152 +0x9f
github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).makeExecPlan(0xc000f30880, 0x6e8a280, 0xc00123ecf0, 0xc000f30bd0, 0x6, 0x400e9df)
	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:975 +0x131
github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).dispatchToExecutionEngine(0xc000f30880, 0x6e8a280, 0xc00123ecf0, 0xc000f30bd0, 0xc3b4ac0, 0xc000721340, 0x0, 0x0)
	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:864 +0x148
github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmtInOpenState(0xc000f30880, 0x6e8a280, 0xc00123ecf0, 0x6e8f340, 0xc0012823c0, 0xc00129d000, 0x27, 0x0, 0x0, 0x0, ...)
	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:457 +0xdd4
github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmt(0xc000f30880, 0x6e8a280, 0xc00123ecf0, 0x6e8f340, 0xc0012823c0, 0xc00129d000, 0x27, 0x0, 0x0, 0x0, ...)
	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:103 +0x610
github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).run(0xc000f30880, 0x6e8a1c0, 0xc001277180, 0xc000110c38, 0x5400, 0x15000, 0xc000110cd0, 0xc0003ff720, 0x0, 0x0)
	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1178 +0x21bb
github.com/cockroachdb/cockroach/pkg/sql.(*Server).ServeConn(0xc0001ac1a0, 0x6e8a1c0, 0xc001277180, 0xc000f30880, 0x5400, 0x15000, 0xc000110cd0, 0xc0003ff720, 0x0, 0x0)
	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:429 +0xce
github.com/cockroachdb/cockroach/pkg/sql/pgwire.(*conn).serveImpl.func4(0xc0001ac1a0, 0xc000117600, 0xc000288c64, 0xc0003ff720, 0x6e8a1c0, 0xc001277180, 0xc000f30880, 0x5400, 0x15000, 0xc000110cd0, ...)
	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/pgwire/conn.go:332 +0xcf
created by github.com/cockroachdb/cockroach/pkg/sql/pgwire.(*conn).serveImpl
	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/pgwire/conn.go:315 +0x1040
FAIL	github.com/cockroachdb/cockroach/pkg/sql	0.201s
make: *** [test] Error 1

I don't think there's any way to reproduce reliably in a sql console, but here's a schema changer test that reproduces the problem, by issuing the DROP COLUMN in a separate goroutine, and waiting to insert the row until the backfill to drop the column begins: master...lucy-zhang:drop-column-panic

Bisecting turned up be12e8f as the first commit where this happens.
@thoszhang thoszhang added A-sql-execution Relating to SQL execution. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. labels Mar 11, 2019
@awoods187 awoods187 added the S-2-temp-unavailability Temp crashes or other availability problems. Can be worked around or resolved by restarting. label Mar 11, 2019
@thoszhang
Copy link
Contributor Author

I found this while testing #35300, but I think this is basically unrelated: there are no active schema changes for the check constraint when the panic occurs, and the check constraint doesn't reference the dropped column.

@andy-kimball andy-kimball mentioned this issue Mar 11, 2019
Merged
andy-kimball added a commit to andy-kimball/cockroach that referenced this issue Mar 12, 2019
@andy-kimball
opt: Fix panic on insert with check constraint and dropping column
ddcab18 
The panic occurs because the delete-only column is never referenced nor used by
the Insert statement, and yet the disambiguateColumns method was trying to get
its ColumnID. The fix is to skip over the column, since there's no need to
disambiguate a column that's never referenced.

Fixes cockroachdb#35611

Release note: None
craig bot pushed a commit that referenced this issue Mar 12, 2019
@andy-kimball
Merge #35624
036640f 
35624: opt: Fix panic on insert with check constraint and dropping column r=andy-kimball a=andy-kimball

The panic occurs because the delete-only column is never referenced nor used by
the Insert statement, and yet the disambiguateColumns method was trying to get
its ColumnID. The fix is to skip over the column, since there's no need to
disambiguate a column that's never referenced.

Fixes #35611

Release note: None

Co-authored-by: Andrew Kimball <[email protected]>
@craig craig bot closed this as completed in #35624 Mar 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-schema-changes A-sql-execution Relating to SQL execution. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. S-2-temp-unavailability Temp crashes or other availability problems. Can be worked around or resolved by restarting.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

opt: Fix panic on insert with check constraint and dropping column andy-kimball/cockroach
2 participants
@thoszhang @awoods187