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.

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

schema: Unexpected error when dropping an index + column, then inserting/upserting #46276

Open
andy-kimball opened this issue Mar 19, 2020 · 5 comments
Labels
A-schema-transactional C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@andy-kimball
Copy link
Contributor

andy-kimball commented Mar 19, 2020

REPRO:

root@:26257/defaultdb> CREATE TABLE t (
  a INT PRIMARY KEY,
  b DECIMAL(10,1) NOT NULL DEFAULT(0),
  UNIQUE INDEX t_secondary (b),
  FAMILY (a, b)
);
CREATE TABLE

root@:26257/defaultdb> INSERT INTO t VALUES (100, 500.5);
INSERT 1

root@:26257/defaultdb> BEGIN;
DROP INDEX t_secondary CASCADE;
ALTER TABLE t DROP COLUMN b;
ALTER TABLE

root@:26257/defaultdb  OPEN> INSERT INTO t SELECT a + 1 FROM t;
INSERT 1

root@:26257/defaultdb  OPEN> UPSERT INTO t SELECT a + 1 FROM t;
ERROR: column-id "2" does not exist

EXPECTED: No error.

Jira issue: CRDB-5097

@andy-kimball
Copy link
Contributor Author

andy-kimball commented Mar 19, 2020

This is low priority, since it's all in same transaction. But thought I'd file it anyway, in case it's something simple to fix and/or something that might cause more serious effects in some other scenario.

@jordanlewis jordanlewis assigned spaskob and unassigned jordanlewis Apr 27, 2020
@jordanlewis jordanlewis added A-schema-transactional C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. labels Apr 30, 2020
@spaskob
Copy link
Contributor

spaskob commented Jun 17, 2020

The actual error reported should be

ERROR: duplicate key value (b)=(0.0) violates unique constraint "t_secondary"
SQLSTATE: 23505

@spaskob
Copy link
Contributor

spaskob commented Jun 17, 2020

This is a stacktrace:

goroutine 1742 [running]:
runtime/debug.Stack(0xc001163b80, 0x10, 0xc0003c6400)
	/usr/local/go/src/runtime/debug/stack.go:24 +0x9d
github.com/cockroachdb/cockroach/pkg/sql/sqlbase.(*TableDescriptor).FindActiveColumnByID(0xc0016ab680, 0x2, 0x1, 0xc001518058, 0xb27bc20)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/sqlbase/structured.go:2723 +0x76
github.com/cockroachdb/cockroach/pkg/sql/sqlbase.GetColumnTypes(0xc0016ab680, 0xc001b25838, 0x1, 0x2, 0xc001b25800, 0x1, 0x2, 0x8, 0xc0029d5380)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/sqlbase/table.go:298 +0x81
github.com/cockroachdb/cockroach/pkg/sql/row.(*Fetcher).Init(0xc00145d138, 0xc00000e680, 0xc00000e680, 0x0, 0x0, 0x0, 0xc002180b40, 0xc00145d0d8, 0x1, 0x1, ...)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/row/fetcher.go:412 +0x83d
github.com/cockroachdb/cockroach/pkg/sql/row.NewUniquenessConstraintViolationError(0x8be3520, 0xc001cd6580, 0xc0016ab680, 0xc001163998, 0x4, 0x8, 0xc0029d5590, 0x4031771, 0x83b4490)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/row/errors.go:117 +0x4f8
github.com/cockroachdb/cockroach/pkg/sql/row.ConvertBatchError(0x8be3520, 0xc001cd6580, 0xc0016ab680, 0xc002125b80, 0x8b4f580, 0xc001518048)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/row/errors.go:64 +0x1e1
github.com/cockroachdb/cockroach/pkg/sql.(*tableWriterBase).finalize(0xc001e69c28, 0x8be3520, 0xc001cd6580, 0xc0016ab680, 0xc001976950, 0xc002122f00)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/tablewriter.go:162 +0xc6
github.com/cockroachdb/cockroach/pkg/sql.(*optTableUpserter).finalize(...)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/tablewriter_upsert_opt.go:217
github.com/cockroachdb/cockroach/pkg/sql.(*upsertNode).BatchedNext(0xc001e69c00, 0x8be3520, 0xc001cd6580, 0xc0029b4dc0, 0xc001976950, 0x7dcda00, 0xc00145d401, 0x0)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/upsert.go:133 +0x205
github.com/cockroachdb/cockroach/pkg/sql.(*rowCountNode).startExec(0xc0013421c0, 0x8be3520, 0xc001cd6580, 0xc0029b4dc0, 0xc001976950, 0xc0011a3500, 0x697e935)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/plan_batch.go:159 +0xcd
github.com/cockroachdb/cockroach/pkg/sql.startExec.func2(0x820fe81, 0x5, 0x8be5b20, 0xc0013421c0, 0x0, 0x0)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/plan.go:457 +0xb9
github.com/cockroachdb/cockroach/pkg/sql.(*planVisitor).visitInternal.func1(0xc00145dcd8, 0x820fe81, 0x5, 0x8be5b20, 0xc0013421c0)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/walk.go:144 +0x5d
github.com/cockroachdb/cockroach/pkg/sql.(*planVisitor).visitInternal(0xc0011a3cd8, 0x8be5b20, 0xc0013421c0, 0x820fe81, 0x5)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/walk.go:766 +0x2b0
github.com/cockroachdb/cockroach/pkg/sql.(*planVisitor).visit(0xc0011a3cd8, 0x8be5b20, 0xc0013421c0, 0xc001342560, 0x40053a4)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/walk.go:111 +0x8d
github.com/cockroachdb/cockroach/pkg/sql.walkPlan(...)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/walk.go:75
github.com/cockroachdb/cockroach/pkg/sql.startExec(0x8be3520, 0xc001cd6580, 0xc0029b4dc0, 0xc001976950, 0x8be5b20, 0xc0013421c0, 0x200000000, 0x2)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/plan.go:460 +0x1bb
github.com/cockroachdb/cockroach/pkg/sql.(*planNodeToRowSource).Start(0xc0029b2500, 0x8be3520, 0xc001cd6580, 0xb27ecc0, 0x7b33140)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/plan_node_to_row_source.go:117 +0xd6
github.com/cockroachdb/cockroach/pkg/sql/execinfra.(*ProcessorBase).Run(0xc0029b2500, 0x8be3520, 0xc001cd6580)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/execinfra/processorsbase.go:747 +0x52
github.com/cockroachdb/cockroach/pkg/sql/flowinfra.(*FlowBase).Run(0xc0029b70e0, 0x8be3520, 0xc001cd6580, 0x83b15d0, 0x0, 0x0)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/flowinfra/flow.go:369 +0x24a
github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).Run(0xc000d94160, 0xc0004ae770, 0xc0013041b0, 0xc001b03b30, 0xc001171880, 0xc001976a40, 0x0, 0x0)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:406 +0x5d4
github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).PlanAndRun(0xc000d94160, 0x8be35e0, 0xc0029f26f0, 0xc001976a40, 0xc0004ae770, 0xc0013041b0, 0x8be5b20, 0xc0013421c0, 0x0, 0x0, ...)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:1018 +0x1de
github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execWithDistSQLEngine(0xc001976580, 0x8be35e0, 0xc0029f26f0, 0xc001976950, 0x2, 0xefbb0a0, 0xc00014bc80, 0xc002464700, 0xc001a2fa98, 0x0, ...)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:904 +0x3f6
github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).dispatchToExecutionEngine(0xc001976580, 0x8be35e0, 0xc0029f26f0, 0xc001976950, 0xefbb0a0, 0xc00014bc80, 0x0, 0x0)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:801 +0x6fb
github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmtInOpenState(0xc001976580, 0x8be35e0, 0xc0029f26f0, 0x8c036a0, 0xc0010d9810, 0xc0014d0562, 0x21, 0x0, 0x2, 0x0, ...)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:488 +0xb23
github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmt(0xc001976580, 0x8be35e0, 0xc0029f26f0, 0x8c036a0, 0xc0010d9810, 0xc0014d0562, 0x21, 0x0, 0x2, 0x0, ...)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:99 +0x4f6
github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execCmd(0xc001976580, 0x8be3520, 0xc001948000, 0x0, 0x0)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1356 +0x1ba5
github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).run(0xc001976580, 0x8be3520, 0xc001360800, 0xc000319300, 0x5400, 0x15000, 0xc000319398, 0xc0013b0530, 0x0, 0x0)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1285 +0x1f2
github.com/cockroachdb/cockroach/pkg/sql.(*Server).ServeConn(0xc000cdd700, 0x8be3520, 0xc001360800, 0xc001976580, 0x5400, 0x15000, 0xc000319398, 0xc0013b0530, 0x0, 0x0)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:490 +0x104
github.com/cockroachdb/cockroach/pkg/sql/pgwire.(*conn).processCommandsAsync.func1(0xc0013a49e9, 0xc001393880, 0x8be3520, 0xc001360800, 0xc0013b0530, 0xc000cdd700, 0xc00014b600, 0x8c0ed60, 0xc0015cfec0, 0xc000ea9e60, ...)
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/pgwire/conn.go:595 +0x317
created by github.com/cockroachdb/cockroach/pkg/sql/pgwire.(*conn).processCommandsAsync
	/Users/spas/go/src/github.com/cockroachdb/cockroach/pkg/sql/pgwire/conn.go:523 +0x17e

@spaskob
Copy link
Contributor

spaskob commented Jun 17, 2020

The original unexpected error ERROR: column-id "2" does not exist is returned from

col, err := desc.FindActiveColumnByID(id)

because col, err := desc.FindActiveColumnByID(id) will not find the id for the dropped column b which is not active.

@thoszhang
Copy link
Contributor

I'm adding slightly more detail to this issue so that we don't forget what's happening when we revisit it later.

The problem is that the writes are happening in a state where column a is public, column b is write-only, and the unique index on b is write-only. In this case, writes will populate column b with its default value. Since the unique index still constrains the values in b, the second write fails because it would result in two rows in the table with the same value for b (its default value).

spaskob pushed a commit to spaskob/cockroach that referenced this issue Jul 3, 2020
Release note (bug fix):
If NewUniquenessConstraintViolationError cannot initialize a row fetcher
it will only report this error to the client without wrappibng it with
information about the actual constraint violation which can be confusing.

Old error:
`ERROR: column-id "2" does not exist`

New Error:
```
ERROR: duplicate key value (b)=(couldn't fetch value: column-id "2"
does not exist) violates unique constraint "t_secondary"
```

See cockroachdb#46276.
craig bot pushed a commit that referenced this issue Jul 6, 2020
45831: sql: add a comment that JSON behaviour is similar to JSONB r=jordanlewis a=giorgosp

Closes #44465
Release note : None

50369: sql: fix NewUniquenessConstraintViolationError reporting r=spaskob a=spaskob

Release note (bug fix):
If NewUniquenessConstraintViolationError cannot initialize a row fetcher
it will only report this error to the client without wrapping it with
information about the actual constraint violation. This is confusing.

See #46276.

Old error:
`ERROR: column-id "2" does not exist`

New Error:
```
ERROR: duplicate key value (b)=(couldn't fetch value: column-id "2"
does not exist) violates unique constraint "t_secondary"
```

50744: sql: fix drop database - sequence ownership bug r=solongordon a=arulajmani

Previously, `DROP DATABASE CASCADE` would not work if the database
contained a sequence owned by a table in the database. This would
happen because of two separate reasons:
- If the sequence was dropped before the table, the table would try to
"double drop" the sequence as it owned it; this would result in an
error.
- If the table was dropped before the sequence, the sequence would try
to remove the ownership dependency from the table descriptor, which had
already been dropped; this would also result in an error.

This PR addresses both these issues separately. Sequences are no longer
double dropped when dropping tables. Additionally, no attempt is made
to remove the ownership dependency from the table descriptor if the
table descriptor has already been dropped.

Fixes #50712

Release note (bug fix): `DROP DATABASE CASCADE` now works as expected
even when the database has a sequence with an owner in it.

50961: cmd/roachtest: fix --zones flag to work on AWS r=petermattis a=petermattis

Previously the `--zones` flag only worked on GCE and Azure and was
silently ignored on AWS. Noticed in passing while trying to run a
roachtest on a different AWS zone.

Release note: None

51000: acceptance: skip TestDockerCLI/test_demo_partitioning r=knz a=tbg

See: #50970

Release note: None

Co-authored-by: George Papadrosou <[email protected]>
Co-authored-by: Spas Bojanov <[email protected]>
Co-authored-by: arulajmani <[email protected]>
Co-authored-by: Peter Mattis <[email protected]>
Co-authored-by: Tobias Schottdorf <[email protected]>
@spaskob spaskob removed their assignment Aug 14, 2020
@jlinder jlinder added the T-sql-schema-deprecated Use T-sql-foundations instead label Jun 16, 2021
@exalate-issue-sync exalate-issue-sync bot added T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) and removed T-sql-schema-deprecated Use T-sql-foundations instead labels May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-schema-transactional C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

No branches or pull requests

5 participants