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

cli/dump: cockroach dump produces unrestorable output for CREATE TABLE statements with interleaved indices #35462

Closed
rolandcrosby opened this issue Mar 6, 2019 · 6 comments · Fixed by #39486
Assignees
Labels
A-disaster-recovery C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) O-support Would prevent or help troubleshoot a customer escalation - bugs, missing observability/tooling, docs S-1 High impact: many users impacted, serious risk of high unavailability or data loss

Comments

@rolandcrosby
Copy link

Interleaved indices can't be added as part of a CREATE TABLE statement; this is a known limitation tracked by #9148. However, cockroach dump is unaware of this limitation: if you dump a table with an interleaved index, the index is emitted inline in the CREATE TABLE. This produces dump files that can't be restored without manual editing.

In the past we've made changes to ShowCreateTable and the crdb_internal table to produce usable CREATE TABLE statements in dumps (see 65c0bfb). To solve this issue we either need to fix the root cause (#9148) or make another change to the dump logic to add interleaved indices after table creation instead of in the CREATE TABLE.

Relates to #28948 (master issue of cockroach dump inadequacies).

The manual edit you have to make: change

CREATE TABLE t (
  somecolumndef,
  somecolumndef,
  somecolumndef,
  INDEX i (somecolumndef, somecolumndef) INTERLEAVE IN PARENT t (somecolumndef)
);

into

CREATE TABLE t (
  somecolumndef,
  somecolumndef,
  somecolumndef
);
CREATE INDEX i (somecolumndef, somecolumndef) INTERLEAVE IN PARENT t (somecolumndef);

Full repro on a 19.1 build:

Repro steps
root@:26257/test> create table t1 (a int, b int, primary key (a));
CREATE TABLE

Time: 17.7833ms

root@:26257/test> create index b_idx on t1(a, b) interleave in parent t1 (a);
CREATE INDEX

Time: 191.1104ms

root@:26257/test> show create table t1;
  table_name |                      create_statement
+------------+-------------------------------------------------------------+
  t1         | CREATE TABLE t1 (
             |     a INT8 NOT NULL,
             |     b INT8 NULL,
             |     CONSTRAINT "primary" PRIMARY KEY (a ASC),
             |     INDEX b_idx (a ASC, b ASC) INTERLEAVE IN PARENT t1 (a),
             |     FAMILY "primary" (a, b)
             | )
(1 row)

Time: 4.2069ms

root@:26257/test> \q
PS C:\Users\roland\Desktop> cockroach-20190211.exe dump --insecure test
CREATE TABLE t1 (
        a INT8 NOT NULL,
        b INT8 NULL,
        CONSTRAINT "primary" PRIMARY KEY (a ASC),
        INDEX b_idx (a ASC, b ASC) INTERLEAVE IN PARENT t1 (a),
        FAMILY "primary" (a, b)
);
PS C:\Users\roland\Desktop> cockroach-20190211.exe sql --insecure
# Welcome to the cockroach SQL interface.
# All statements must be terminated by a semicolon.
# To exit: CTRL + D.
#
# Server version: CockroachDB CCL v2.2.0-alpha.20190211 (x86_64-w64-mingw32, built 2019/02/07 23:54:03, go1.11.4) (same version as client)
# Cluster ID: cf81eacb-45cd-4cfe-8113-6b1ea1f2f744
#
# Enter \? for a brief introduction.
#
root@:26257/defaultdb> create database test2; use test2;
SET

Time: 16.5387ms

root@:26257/test2> CREATE TABLE t1 (
                ->         a INT8 NOT NULL,
                ->         b INT8 NULL,
                ->         CONSTRAINT "primary" PRIMARY KEY (a ASC),
                ->         INDEX b_idx (a ASC, b ASC) INTERLEAVE IN PARENT t1 (a),
                ->         FAMILY "primary" (a, b)
                -> );
pq: unimplemented: use CREATE INDEX to make interleaved indexes
HINT: See: https://github.com/cockroachdb/cockroach/issues/9148
root@:26257/test2>
@awoods187 awoods187 added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Mar 6, 2019
@tim-o tim-o added O-support Would prevent or help troubleshoot a customer escalation - bugs, missing observability/tooling, docs S-1 High impact: many users impacted, serious risk of high unavailability or data loss labels Mar 11, 2019
@tim-o
Copy link
Contributor

tim-o commented Mar 14, 2019

Zendesk ticket #3068 has been linked to this issue.

@jdoupe
Copy link

jdoupe commented Apr 2, 2019

This issue is referenced from the documentation as a workaround for fixing interleaved tables. However, this only applies to interleaved indices.

Is there a workaround or alternate solution for interleaved tables?

(I should clarify that the problem exists with import, not necessarily with cockroach sql < dump.sql)

@kenliu kenliu added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Jul 3, 2019
@kenliu
Copy link

kenliu commented Jul 23, 2019

@dt would this be a good one for @tyler314 to pick up next?

@dt
Copy link
Member

dt commented Jul 23, 2019

I think @jordanlewis is already working on this in #38317 ?

@jordanlewis
Copy link
Member

I was, but I don't have bandwidth to finish that work right now. It needs more testing still. If @tyler314 wants to pick this up, I'm completely okay with that.

@dt
Copy link
Member

dt commented Jul 29, 2019

@tyleroberts I think we can move all the index creation statements to the end like @mjibson suggested -- for IMPORT I think it won't matter and for regular insert, I think it might be faster than making every INSERT do index writes.

@tyler314 tyler314 self-assigned this Jul 30, 2019
@tyler314 tyler314 self-assigned this Aug 9, 2019
tyler314 pushed a commit to tyler314/cockroach that referenced this issue Aug 9, 2019
Fix bug where dump would include INTERLEAVE IN PARENT statements inside
of CREATE TABLE statements. CRDB currently does not support this, thus
forcing the user to manually edit the dump files to remove the
INTERLEAVE statements themselves.

Previousily, crdb_internal.go would use show_create.go to create the
SHOW CREATE TABLE statements which would include INTERLEAVE.
crdb_internal.go would also add ALTER STATEMENTs separately to the
alter_statements of the create statements of the table. This change
removes the addition of the INTERLEAVE within show_create.go, and
included the additon of adding the INTERLEAVE statements to the
alter_statements within crdb_internal.go.

A bug was also fixed in structured.go in the SQLString method that
created the SQL string out of order when a non-empty table name is
passed in. The bug caused the "ON tablename" to be printed before
the index and without space in between the two.

Resolves: cockroachdb#35462

Release Note (bug fix): dump now works properly when handling
INTERLEAVED tables, printing them outside of CREATE TABLE statements.
tyler314 pushed a commit to tyler314/cockroach that referenced this issue Aug 12, 2019
Fix bug where dump would include INTERLEAVE IN PARENT statements inside
of CREATE TABLE statements. CRDB currently does not support this, thus
forcing the user to manually edit the dump files to remove the
INTERLEAVE statements themselves.

Previousily, crdb_internal.go would use show_create.go to create the
SHOW CREATE TABLE statements which would include INTERLEAVE, for the
create_nofk columns. crdb_internal.go would also add ALTER STATEMENTs
separately to the alter_statements of the create statements of the
table. This change removes the addition of the INTERLEAVE within
show_create.go for the create_nofk columns, and includes the additon
of adding the INTERLEAVE statements to the alter_statements within
crdb_internal.go.

A bug was also fixed in structured.go in the SQLString method that
created the SQL string out of order when a non-empty table name is
passed in. The bug caused the "ON tablename" to be printed before
the index and without space in between the two.

Resolves: cockroachdb#35462

Release Note (bug fix): dump now works properly when handling
INTERLEAVED tables, printing them outside of CREATE TABLE statements.
tyler314 pushed a commit to tyler314/cockroach that referenced this issue Aug 13, 2019
Fix bug where dump would include INTERLEAVE IN PARENT statements for
indexes inside of CREATE TABLE statements. CRDB currently does not
support this, thus forcing the user to manually edit the dump files
to remove the INTERLEAVE statements themselves.

Previousily, crdb_internal.go would use show_create.go to create the
SHOW CREATE TABLE statements which would include INTERLEAVE, for the
create_nofk columns. crdb_internal.go would also add ALTER STATEMENTs
separately to the alter_statements of the create statements of the
table. This change removes the addition of the INTERLEAVE within
show_create.go for the create_nofk columns, and includes the additon
of adding the INTERLEAVE statements to the alter_statements within
crdb_internal.go.

A bug was also fixed in structured.go in the SQLString method that
created the SQL string out of order when a non-empty table name is
passed in. The bug caused the "ON tablename" to be printed before
the index and without space in between the two.

Resolves: cockroachdb#35462

Release Note (bug fix): dump now works properly when handling
INTERLEAVED tables, printing them outside of CREATE TABLE statements.
tyler314 pushed a commit to tyler314/cockroach that referenced this issue Aug 14, 2019
Fix bug where dump would include INTERLEAVE IN PARENT statements for
indexes inside of CREATE TABLE statements. CRDB currently does not
support this, thus forcing the user to manually edit the dump files
to remove the INTERLEAVE statements themselves.

Previousily, crdb_internal.go would use show_create.go to create the
SHOW CREATE TABLE statements which would include INTERLEAVE, for the
create_nofk columns. crdb_internal.go would also add ALTER STATEMENTs
separately to the alter_statements of the create statements of the
table. This change removes the addition of the INTERLEAVE within
show_create.go for the create_nofk columns, and includes the additon
of adding the INTERLEAVE statements to the alter_statements within
crdb_internal.go.

A bug was also fixed in structured.go in the SQLString method that
created the SQL string out of order when a non-empty table name is
passed in. The bug caused the "ON tablename" to be printed before
the index and without space in between the two.

Resolves: cockroachdb#35462

Release Note (bug fix): dump now works properly when handling
INTERLEAVED tables, printing them outside of CREATE TABLE statements.
tyler314 pushed a commit to tyler314/cockroach that referenced this issue Aug 15, 2019
Fix bug where dump would include INTERLEAVE IN PARENT statements for
indexes inside of CREATE TABLE statements. CRDB currently does not
support this, thus forcing the user to manually edit the dump files
to remove the INTERLEAVE statements themselves.

Previousily, crdb_internal.go would use show_create.go to create the
SHOW CREATE TABLE statements which would include INTERLEAVE, for the
create_nofk columns. crdb_internal.go would also add ALTER STATEMENTs
separately to the alter_statements of the create statements of the
table. This change removes the addition of the INTERLEAVE within
show_create.go for the create_nofk columns, and includes the additon
of adding the INTERLEAVE statements to the alter_statements within
crdb_internal.go.

A bug was also fixed in structured.go in the SQLString method that
created the SQL string out of order when a non-empty table name is
passed in. The bug caused the "ON tablename" to be printed before
the index and without space in between the two.

Resolves: cockroachdb#35462

Release Note (bug fix): dump now works properly when handling
INTERLEAVED tables, printing them outside of CREATE TABLE statements.
tyler314 pushed a commit to tyler314/cockroach that referenced this issue Aug 15, 2019
Fix bug where dump would include INTERLEAVE IN PARENT statements for
indexes inside of CREATE TABLE statements. CRDB currently does not
support this, thus forcing the user to manually edit the dump files
to remove the INTERLEAVE statements themselves.

Previousily, crdb_internal.go would use show_create.go to create the
SHOW CREATE TABLE statements which would include INTERLEAVE, for the
create_nofk columns. crdb_internal.go would also add ALTER STATEMENTs
separately to the alter_statements of the create statements of the
table. This change removes the addition of the INTERLEAVE within
show_create.go for the create_nofk columns, and includes the additon
of adding the INTERLEAVE statements to the alter_statements within
crdb_internal.go.

A bug was also fixed in structured.go in the SQLString method that
created the SQL string out of order when a non-empty table name is
passed in. The bug caused the "ON tablename" to be printed before
the index and without space in between the two.

Resolves: cockroachdb#35462

Release Note (bug fix): dump now works properly when handling
INTERLEAVED tables, printing them outside of CREATE TABLE statements.
tyler314 pushed a commit to tyler314/cockroach that referenced this issue Aug 17, 2019
Fix bug where dump would include INTERLEAVE IN PARENT statements for
indexes inside of CREATE TABLE statements. CRDB currently does not
support this, thus forcing the user to manually edit the dump files
to remove the INTERLEAVE statements themselves.

Previousily, crdb_internal.go would use show_create.go to create the
SHOW CREATE TABLE statements which would include INTERLEAVE, for the
create_nofk columns. crdb_internal.go would also add ALTER STATEMENTs
separately to the alter_statements of the create statements of the
table. This change removes the addition of the INTERLEAVE within
show_create.go for the create_nofk columns, and includes the additon
of adding the INTERLEAVE statements to the alter_statements within
crdb_internal.go.

A bug was also fixed in structured.go in the SQLString method that
created the SQL string out of order when a non-empty table name is
passed in. The bug caused the "ON tablename" to be printed before
the index and without space in between the two.

Resolves: cockroachdb#35462

Release Note (bug fix): dump now works properly when handling
INTERLEAVED tables, printing them outside of CREATE TABLE statements.
tyler314 pushed a commit to tyler314/cockroach that referenced this issue Aug 19, 2019
Fix bug where dump would include INTERLEAVE IN PARENT clauses for
indexes inside of CREATE TABLE statements. CRDB currently does not
support this, thus forcing the user to manually edit the dump files
to remove the INTERLEAVE clauses themselves.

Previousily, crdb_internal.go would use show_create.go to create the
SHOW CREATE TABLE statements which would include INTERLEAVE, for the
create_nofk columns. crdb_internal.go would also add ALTER STATEMENTs
separately to the alter_statements of the create statements of the
table. This change removes the addition of the INTERLEAVE within
show_create.go for the create_nofk columns, and includes the additon
of adding the INTERLEAVE clauses to the alter_statements within
crdb_internal.go.

A bug was also fixed in structured.go in the SQLString method that
created the SQL string out of order when a non-empty table name is
passed in. The bug caused the "ON tablename" to be printed before
the index and without space in between the two.

Resolves: cockroachdb#35462

Release Note (bug fix): dump now works properly when handling
INTERLEAVED tables, printing them outside of CREATE TABLE statements.
tyler314 pushed a commit to tyler314/cockroach that referenced this issue Aug 20, 2019
Fix bug where dump would include INTERLEAVE IN PARENT clauses for
indexes inside of CREATE TABLE statements. CRDB currently does not
support this, thus forcing the user to manually edit the dump files
to remove the INTERLEAVE clauses themselves.

Previousily, crdb_internal.go would use show_create.go to create the
SHOW CREATE TABLE statements which would include INTERLEAVE, for the
create_nofk columns. crdb_internal.go would also add ALTER STATEMENTs
separately to the alter_statements of the create statements of the
table. This change removes the addition of the INTERLEAVE within
show_create.go for the create_nofk columns, and includes the additon
of adding the INTERLEAVE clauses to the alter_statements within
crdb_internal.go.

A bug was also fixed in structured.go in the SQLString method that
created the SQL string out of order when a non-empty table name is
passed in. The bug caused the "ON tablename" to be printed before
the index and without space in between the two.

Resolves: cockroachdb#35462

Release Note (bug fix): dump now works properly when handling
INTERLEAVED tables, printing them outside of CREATE TABLE statements.
craig bot pushed a commit that referenced this issue Aug 20, 2019
39486: sql: dump puts INTERLEAVE statements outside of CREATE TABLE statements r=tyleroberts a=tyleroberts

Fix bug where dump would include INTERLEAVE IN PARENT statements inside
of CREATE TABLE statements. CRDB currently does not support this, thus
forcing the user to manually edit the dump files to remove the
INTERLEAVE statements themselves.

Previousily, crdb_internal.go would use show_create.go to create the
SHOW CREATE TABLE statements which would include INTERLEAVE.
crdb_internal.go would also add ALTER STATEMENTs separately to the
alter_statements of the create statements of the table. This change
removes the addition of the INTERLEAVE within show_create.go, and
included the additon of adding the INTERLEAVE statements to the
alter_statements within crdb_internal.go.

A bug was also fixed in structured.go in the SQLString method that
created the SQL string out of order when a non-empty table name is
passed in. The bug caused the "ON tablename" to be printed before
the index and without space in between the two.

Resolves: #35462

Release Note (bug fix): dump now works properly when handling
INTERLEAVED tables, printing them outside of CREATE TABLE statements.

39763: sql: quieter TestFuncNulls r=dt a=dt

Before this was running every combination of builtin func and type as a separate, named subtest.
It is unlikely we need such granular ability to run just one combination (the test runs <1s),
while generating so many named substests resulted in 5655 lines / 400kb of test output.

Removing the subtest cuts this to about 140 lines / 14kb (there are some stacktraces logged).

Release note: none.

39764: pgwire: make TestEncodings quieter r=dt a=dt

The creation of named subtests for every combination of test-case and
parameters meant this test was creating over 1k named tests. Along with
test logging, that meant this one test was emitting over 400kb of output.
Additionally this meant this one test was contributing a non-trivial
fraction of our overall total named tests that pass/fail individually,
are tracked over time, etc. The hyper-granularity here isn't buying us
much vs the extra overhead.

This change inverts the loop vs subtests, making hand-defined subtests
for encode and decode of each format, each of which then check all the
test cases.

Release note: none.

Co-authored-by: Tyler314 <tyler@cockroachlabs.com>
Co-authored-by: David Taylor <tinystatemachine@gmail.com>
@craig craig bot closed this as completed in a3d8f54 Aug 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-disaster-recovery C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) O-support Would prevent or help troubleshoot a customer escalation - bugs, missing observability/tooling, docs S-1 High impact: many users impacted, serious risk of high unavailability or data loss
Projects
None yet
8 participants