From d4b257bf06a4521d96f14bef068b3f8bad71a1b1 Mon Sep 17 00:00:00 2001
From: Raphael 'kena' Poss <knz@cockroachlabs.com>
Date: Mon, 8 Jul 2019 14:38:41 +0200
Subject: [PATCH] cli/dump: inform the user clearly when dumping no-col tables

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: https://github.com/cockroachdb/cockroach/issues/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.
---
 pkg/cli/cli_test.go | 21 +++++++++++++++++++++
 pkg/cli/dump.go     | 16 +++++++++++++++-
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/pkg/cli/cli_test.go b/pkg/cli/cli_test.go
index 38517e753f30..7d3a699a0fb0 100644
--- a/pkg/cli/cli_test.go
+++ b/pkg/cli/cli_test.go
@@ -382,6 +382,7 @@ func (c cliTest) runWithArgsUnredirected(origArgs []string) {
 		return Run(args)
 	}(); err != nil {
 		fmt.Println(err)
+		maybeShowErrorDetails(os.Stdout, err, false /*printNewLine*/)
 	}
 }
 
@@ -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
+}
diff --git a/pkg/cli/dump.go b/pkg/cli/dump.go
index 0023e92f1b15..5394e4795d6b 100644
--- a/pkg/cli/dump.go
+++ b/pkg/cli/dump.go
@@ -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"
 )
 
@@ -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,