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: error on internal validation query for foreign keys referencing hidden columns #59582

Closed
thoszhang opened this issue Jan 29, 2021 · 3 comments · Fixed by #59659
Closed

sql: error on internal validation query for foreign keys referencing hidden columns #59582

thoszhang opened this issue Jan 29, 2021 · 3 comments · Fixed by #59659
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@thoszhang
Copy link
Contributor

This came up in the context of adding user-defined hidden columns in #58923, but also applies to rowid now. On master, adding foreign key constraints referencing hidden columns doesn't work:

root@:26257/defaultdb> CREATE TABLE t4(c INT);
CREATE TABLE
root@:26257/defaultdb> CREATE TABLE t5(a INT);
CREATE TABLE
root@:26257/defaultdb> ALTER TABLE t4 ADD FOREIGN KEY (c) REFERENCES t5 (rowid);
ERROR: validate fk constraint: column "t.rowid" does not exist
SQLSTATE: 42703

This comes from the internal executor validating the constraint for existing rows. Here's the query we're actually executing:

I210129 15:59:27.481167 3052 sql/check.go:290 ⋮ [n1,job=628705089054212097,scExec,id=52,mutation=1] 118  validating FK ‹"fk_c_ref_t5"› (‹"t4"› [‹[c rowid]›] -> ‹"t5"› [‹[rowid]›]) with query ‹"SELECT s.c, s.rowid FROM \n\t\t  (SELECT c, rowid FROM [52 AS src]@{IGNORE_FOREIGN_KEYS} WHERE c IS NOT NULL) AS s\n\t\t\tLEFT OUTER JOIN\n\t\t\t(SELECT * FROM [53 AS target]) AS t\n\t\t\tON s.c = t.rowid\n\t\t WHERE t.rowid IS NULL  LIMIT 1"›

This query also doesn't work on its own, so the problem isn't isolated to the schema change:

root@:26257/defaultdb> SELECT s.c, s.rowid
  FROM (SELECT c, rowid FROM [52 AS src]@{IGNORE_FOREIGN_KEYS} WHERE c IS NOT NULL) AS s
       LEFT JOIN (SELECT * FROM [53 AS target]) AS t ON s.c = t.rowid
 WHERE t.rowid IS NULL
 LIMIT 1;
ERROR: column "t.rowid" does not exist
SQLSTATE: 42703

Here's the stack trace for the error when running it in the internal executor:

I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo.NewUndefinedColumnError
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo/column_resolver.go:210
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*scope).Resolve
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/scope.go:887
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*ColumnItem).Resolve
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/sem/tree/name_resolution.go:237
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*scope).VisitPre
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/scope.go:931
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*scope).VisitPre
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/scope.go:928
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql/sem/tree.WalkExpr
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/sem/tree/walk.go:714
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*ComparisonExpr).Walk
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/sem/tree/walk.go:191
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql/sem/tree.WalkExpr
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/sem/tree/walk.go:717
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*scope).walkExprTree
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/scope.go:412
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*scope).resolveAndRequireType
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/scope.go:470
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*Builder).buildJoin
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/join.go:125
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*Builder).buildDataSource
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/select.go:71
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*Builder).buildFromTablesRightDeep
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/select.go:1183
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*Builder).buildFromTables
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/select.go:1160
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*Builder).buildFrom
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/select.go:1087
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*Builder).buildSelectClause
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/select.go:1008
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*Builder).buildSelectStmtWithoutParens
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/select.go:956
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*Builder).buildSelect.func1
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/select.go:929
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*Builder).processWiths
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/with.go:29
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*Builder).buildSelect
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/select.go:928
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*Builder).buildStmt
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/builder.go:265
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*Builder).buildStmtAtRoot
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/builder.go:229
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder.(*Builder).Build
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder/builder.go:200
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql.(*optPlanningCtx).buildExecMemo
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/plan_opt.go:507
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql.(*planner).makeOptimizerPlan
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/plan_opt.go:193
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).makeExecPlan
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:910
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).dispatchToExecutionEngine
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:796
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmtInOpenState
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:641
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmt
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:119
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execCmd.func1
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1446
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execCmd
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1448
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).run
I210129 16:03:01.820361 6475 jobs/registry.go:1135 ⋮ [n1] 212 +  | 	/Users/lucy/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1373

Should this query work? Are we generating the incorrect query in the first place?
@thoszhang thoszhang added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Jan 29, 2021
@RaduBerinde
Copy link
Member

t is the result of (SELECT * FROM ..) which does not include rowid (it is hidden from *). It would need to be added explicitly as a select target. Is the SELECT * necessary at all? (using [53 .. ] directly doesn't work?)

@thoszhang
Copy link
Contributor Author

Oh, good point. I don't think there's any reason for the SELECT * (probably we restricted the columns originally, then changed to * after realizing we needed more columns for logging). We can change that.

@RaduBerinde
Copy link
Member

I don't know the details of the logging you mention, but the optimizer will only scan the columns that are needed for the query (so replacing a column list with * shouldn't get you anything in this case).

@thoszhang thoszhang mentioned this issue Feb 1, 2021
Merged
@craig craig bot closed this as completed in f37712a Feb 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sql: fix bug preventing adding FKs referencing hidden columns thoszhang/cockroach
2 participants
@thoszhang @RaduBerinde