Skip to content

Commit

Permalink
cli/dump: inform the user clearly when dumping no-col tables
Browse files Browse the repository at this point in the history
Before:

```
kena@kenax ~/cockroach % ./cockroach dump --insecure defaultdb
CREATE TABLE t (,
        FAMILY "primary" (rowid)
);
Error: pq: at or near "from": syntax error
Failed running "dump"
```

After:

```
kena@kenax ~/cockroach % ./cockroach dump --insecure defaultdb
CREATE TABLE t (,
        FAMILY "primary" (rowid)
);
Error: table "defaultdb.public.t" has no visible columns
HINT: To proceed with the dump, either omit this table from the list of tables to dump, drop the table, or add some visible columns.
--
See: #37768
Failed running "dump"
```

Release note (cli change): `cockroach dump` will now more clearly
refer to issue #37768 when it encounters a table with no visible
columns, which (currently) cannot be dumped successfully.
  • Loading branch information
knz committed Jul 8, 2019
1 parent 861dc40 commit d4b257b
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 1 deletion.
21 changes: 21 additions & 0 deletions pkg/cli/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,7 @@ func (c cliTest) runWithArgsUnredirected(origArgs []string) {
return Run(args)
}(); err != nil {
fmt.Println(err)
maybeShowErrorDetails(os.Stdout, err, false /*printNewLine*/)
}
}

Expand Down Expand Up @@ -2555,3 +2556,23 @@ func Example_sqlfmt() {
// sqlfmt --no-simplify -e select (1+2)+3
// SELECT (1 + 2) + 3
}

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

c.RunWithArgs([]string{"sql", "-e", "create table t(x int); set sql_safe_updates=false; alter table t drop x"})
c.RunWithArgs([]string{"dump", "defaultdb"})

// Output:
// sql -e create table t(x int); set sql_safe_updates=false; alter table t drop x
// ALTER TABLE
// dump defaultdb
// CREATE TABLE t (,
// FAMILY "primary" (rowid)
// );
// table "defaultdb.public.t" has no visible columns
// HINT: To proceed with the dump, either omit this table from the list of tables to dump, drop the table, or add some visible columns.
// --
// See: https://github.com/cockroachdb/cockroach/issues/37768
}
16 changes: 15 additions & 1 deletion pkg/cli/dump.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/ctxgroup"
"github.com/cockroachdb/cockroach/pkg/util/timeofday"
"github.com/cockroachdb/cockroach/pkg/util/version"
"github.com/pkg/errors"
"github.com/cockroachdb/errors"
"github.com/spf13/cobra"
)

Expand Down Expand Up @@ -538,6 +538,20 @@ func dumpTableData(w io.Writer, conn *sqlConn, clusterTS string, bmd basicMetada
return err
}

if strings.TrimSpace(md.columnNames) == "" {
// A table with no columns may still have one or more rows. In
// fact, it can have arbitrary many rows, each with a different
// (hidden) PK value. Unfortunately, the dump command today simply
// omits the hidden PKs from the dump, so it is not possible to
// restore the invisible values.
// Instead of failing with an incomprehensible error below, inform
// the user more clearly.
err := errors.Newf("table %q has no visible columns", tree.ErrString(bmd.name))
err = errors.WithHint(err, "To proceed with the dump, either omit this table from the list of tables to dump, drop the table, or add some visible columns.")
err = errors.WithIssueLink(err, errors.IssueLink{IssueURL: "https://github.com/cockroachdb/cockroach/issues/37768"})
return err
}

bs := fmt.Sprintf("SELECT %s FROM %s AS OF SYSTEM TIME %s ORDER BY PRIMARY KEY %[2]s",
md.columnNames,
md.name,
Expand Down

0 comments on commit d4b257b

Please sign in to comment.