Skip to content

Commit

Permalink
sql: dump puts INTERLEAVE statements outside of CREATE TABLE statements
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Tyler314 committed Aug 9, 2019
1 parent 95968ee commit fa685c8
Show file tree
Hide file tree
Showing 7 changed files with 133 additions and 12 deletions.
4 changes: 3 additions & 1 deletion pkg/base/addr_validation_test.go
Expand Up @@ -64,7 +64,9 @@ func TestValidateAddrs(t *testing.T) {
// For the host name resolution error we can reliably expect "no such host"
// below, but before we test anything we need to ensure we indeed have
// a reliably non-resolvable host name.
_, err = net.DefaultResolver.LookupIPAddr(context.Background(), "nonexistent.example.com")
var derp []net.IPAddr
derp, err = net.DefaultResolver.LookupIPAddr(context.Background(), "nonexistent.example.com")
fmt.Println("DERP: ", derp)
if err == nil {
t.Fatal("expected host resolution failure, got no error")
}
Expand Down
74 changes: 74 additions & 0 deletions pkg/cli/dump_test.go
Expand Up @@ -441,3 +441,77 @@ INSERT INTO t (i, j) VALUES
t.Fatalf("unexpected output: %s", out)
}
}

func TestDumpInterleaved(t *testing.T) {
defer leaktest.AfterTest(t)()

defer leaktest.AfterTest(t)()

c := newCLITest(cliTestParams{t: t})
defer c.cleanup()

const create = `
CREATE DATABASE d;
CREATE TABLE d.t1 (a INT, b INT, PRIMARY KEY (a));
CREATE INDEX b_idx ON d.t1(a, b) INTERLEAVE IN PARENT d.t1 (a);
CREATE TABLE d.customers (id INT PRIMARY KEY, name STRING(50));
CREATE TABLE d.orders (
customer INT,
id INT,
total DECIMAL(20, 5),
PRIMARY KEY (customer, id),
CONSTRAINT fk_customer FOREIGN KEY (customer) REFERENCES d.customers
) INTERLEAVE IN PARENT d.customers (customer);
`

_, err := c.RunWithCaptureArgs([]string{"sql", "-e", create})
if err != nil {
t.Fatal(err)
}

dump1, err := c.RunWithCaptureArgs([]string{"dump", "d", "t1"})
if err != nil {
t.Fatal(err)
}

const want1 = `dump d t1
CREATE TABLE t1 (
a INT8 NOT NULL,
b INT8 NULL,
CONSTRAINT "primary" PRIMARY KEY (a ASC),
FAMILY "primary" (a, b)
);
CREATE INDEX b_idx ON t1 (a ASC, b ASC) INTERLEAVE IN PARENT t1 (a);
-- Validate foreign key constraints. These can fail if there was unvalidated data during the dump.
`
if dump1 != want1 {
t.Fatalf("expected: %s\ngot: %s", want1, dump1)
}

dump2, err := c.RunWithCaptureArgs([]string{"dump", "d", "orders"})
if err != nil {
t.Fatal(err)
}

const want2 = `dump d orders
CREATE TABLE orders (
customer INT8 NOT NULL,
id INT8 NOT NULL,
total DECIMAL(20,5) NULL,
CONSTRAINT "primary" PRIMARY KEY (customer ASC, id ASC),
FAMILY "primary" (customer, id, total)
) INTERLEAVE IN PARENT customers (customer);
ALTER TABLE orders ADD CONSTRAINT fk_customer FOREIGN KEY (customer) REFERENCES customers(id);
CREATE UNIQUE INDEX "primary" ON customers (customer ASC, id ASC) INTERLEAVE IN PARENT customers (customer);
-- Validate foreign key constraints. These can fail if there was unvalidated data during the dump.
ALTER TABLE orders VALIDATE CONSTRAINT fk_customer;
`

if dump2 != want2 {
t.Fatalf("expected: %s\ngot: %s", want2, dump2)
}
}
50 changes: 50 additions & 0 deletions pkg/sql/crdb_internal.go
Expand Up @@ -1155,6 +1155,7 @@ CREATE TABLE crdb_internal.create_statements (
idx := &allIdx[i]
if fk := &idx.ForeignKey; fk.IsSet() {
f := tree.NewFmtCtx(tree.FmtSimple)
// Create ALTER TABLE commands for Foreign Key Constraints.
f.WriteString("ALTER TABLE ")
f.FormatNode(tn)
f.WriteString(" ADD CONSTRAINT ")
Expand All @@ -1167,6 +1168,7 @@ CREATE TABLE crdb_internal.create_statements (
return err
}

// Validate the Foreign Key Constraints
f = tree.NewFmtCtx(tree.FmtSimple)
f.WriteString("ALTER TABLE ")
f.FormatNode(tn)
Expand All @@ -1177,6 +1179,54 @@ CREATE TABLE crdb_internal.create_statements (
return err
}
}
// Create CREATE INDEX commands for INTERLEAVE tables. These commands
// are included in the ALTER TABLE statements.
if len(idx.Interleave.Ancestors) > 0 {
f := tree.NewFmtCtx(tree.FmtSimple)
intl := idx.Interleave
// Get the parent table information.
parentTableID := intl.Ancestors[len(intl.Ancestors)-1].TableID
var parentName tree.TableName
if lCtx != nil {
parentTable, err := lCtx.getTableByID(parentTableID)
if err != nil {
return err
}
parentDbDesc, err := lCtx.getDatabaseByID(parentTable.ParentID)
if err != nil {
return err
}
parentName = tree.MakeTableName(tree.Name(parentDbDesc.Name), tree.Name(parentTable.Name))
parentName.ExplicitSchema = parentDbDesc.Name != contextName
} else {
parentName = tree.MakeTableName(tree.Name(""), tree.Name(fmt.Sprintf("[%d as parent]", parentTableID)))
parentName.ExplicitCatalog = false
parentName.ExplicitSchema = false
}
var sharedPrefixLen int
for _, ancestor := range intl.Ancestors {
sharedPrefixLen += int(ancestor.SharedPrefixLen)
}
// Write the CREATE INDEX statements.
f.WriteString("CREATE ")
f.WriteString(idx.SQLString(&parentName))
f.WriteString(" INTERLEAVE IN PARENT ")
fmtCtx := tree.NewFmtCtx(tree.FmtSimple)
fmtCtx.FormatNode(&parentName)
f.WriteString(fmtCtx.CloseAndGetString())
f.WriteString(" (")
// Get all of the columns and write them.
for i, name := range idx.ColumnNames[:sharedPrefixLen] {
if i > 0 {
f.WriteString(", ")
}
f.FormatNameP(&name)
}
f.WriteString(")")
if err := alterStmts.Append(tree.NewDString(f.CloseAndGetString())); err != nil {
return err
}
}
}
stmt, err = ShowCreateTable(ctx, tn, contextName, table, lCtx, IncludeFKs)
}
Expand Down
2 changes: 0 additions & 2 deletions pkg/sql/logictest/testdata/logic_test/interleaved
Expand Up @@ -227,8 +227,6 @@ all_interleaves CREATE TABLE all_interleaves (
CONSTRAINT "primary" PRIMARY KEY (b ASC),
INDEX all_interleaves_c_idx (c ASC),
UNIQUE INDEX all_interleaves_d_key (d ASC),
INDEX all_interleaves_c_d_idx (c ASC, d ASC) INTERLEAVE IN PARENT p1_1 (c),
UNIQUE INDEX all_interleaves_d_c_key (d ASC, c ASC) INTERLEAVE IN PARENT p1_1 (d),
FAMILY "primary" (b, c, d)
) INTERLEAVE IN PARENT p1_1 (b)

Expand Down
10 changes: 4 additions & 6 deletions pkg/sql/show_create.go
Expand Up @@ -201,15 +201,13 @@ func ShowCreateTable(
}
f.WriteString(foreignKey.String())
}
if idx.ID != desc.PrimaryIndex.ID {
// Only add indexes to CREATE TABLE statement if they are not associated
// with an INTERLEAVE statement.
if idx.ID != desc.PrimaryIndex.ID && len(idx.Interleave.Ancestors) == 0 {
// Showing the primary index is handled above.
f.WriteString(",\n\t")
f.WriteString(idx.SQLString(&sqlbase.AnonymousTable))
// Showing the INTERLEAVE and PARTITION BY for the primary index are
// handled last.
if err := showCreateInterleave(ctx, idx, &f.Buffer, dbPrefix, lCtx); err != nil {
return "", err
}
// Showing the PARTITION BY for the primary index are handled last.
if err := ShowCreatePartitioning(
a, desc, idx, &idx.Partitioning, &f.Buffer, 1 /* indent */, 0, /* colOffset */
); err != nil {
Expand Down
1 change: 0 additions & 1 deletion pkg/sql/show_create_test.go
Expand Up @@ -34,7 +34,6 @@ func TestStandAloneShowCreateTable(t *testing.T) {
payload BYTES NOT NULL,
CONSTRAINT "primary" PRIMARY KEY (id ASC),
CONSTRAINT fk FOREIGN KEY (status) REFERENCES "[52 as ref]"("???"),
INDEX jobs_status_created_idx (status ASC, created ASC) INTERLEAVE IN PARENT "[51 as parent]" (status),
FAMILY fam_0_id_status_created_payload (id, status, created, payload)
)`

Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/sqlbase/structured.go
Expand Up @@ -484,11 +484,11 @@ func (desc *IndexDescriptor) SQLString(tableName *tree.TableName) string {
f.WriteString("INVERTED ")
}
f.WriteString("INDEX ")
f.FormatNameP(&desc.Name)
if *tableName != AnonymousTable {
f.WriteString("ON ")
f.WriteString(" ON ")
f.FormatNode(tableName)
}
f.FormatNameP(&desc.Name)
f.WriteString(" (")
desc.ColNamesFormat(f)
f.WriteByte(')')
Expand Down

0 comments on commit fa685c8

Please sign in to comment.