Don't use the table writer for zones config output.

This will make it easier to copy/paste. Example: ``` $ ./cockroach zone --dev set 1000 /tmp/zone $ ./cockroach zone --dev set 1010 /tmp/zone $ ./cockroach zone --dev get 1000 replicas: - attrs: [us-east-1a, ssd] - attrs: [us-east-1b, ssd] - attrs: [us-west-1b, ssd] range_min_bytes: 8388608 range_max_bytes: 67108864 $./cockroach zone --dev ls Object 1000: replicas: - attrs: [us-east-1a, ssd] - attrs: [us-east-1b, ssd] - attrs: [us-west-1b, ssd] range_min_bytes: 8388608 range_max_bytes: 67108864 Object 1010: replicas: - attrs: [us-east-1a, ssd] - attrs: [us-east-1b, ssd] - attrs: [us-west-1b, ssd] range_min_bytes: 8388608 range_max_bytes: 67108864 ```
cockroachdb · Sep 16, 2015 · 1e7bf25 · 1e7bf25
1 parent 9910822
commit 1e7bf25
Show file tree

Hide file tree

Showing 5 changed files with 109 additions and 46 deletions.
diff --git a/cli/sql.go b/cli/sql.go
@@ -111,7 +111,7 @@ func runTerm(cmd *cobra.Command, args []string) {
 		fullStmt := strings.Join(stmt, "\n")
 		liner.AppendHistory(fullStmt)
 
-		if err := runQuery(db, fullStmt); err != nil {
+		if err := runPrettyQuery(db, fullStmt); err != nil {
 			fmt.Fprintln(osStdout, err)
 		}
 

diff --git a/cli/sql_util.go b/cli/sql_util.go
@@ -49,61 +49,73 @@ func makeSQLClient() *sql.DB {
 type fmtMap map[string]func(interface{}) string
 
 // runQuery takes a 'query' with optional 'parameters'.
-// It runs the sql query and writes pretty output to osStdout.
-func runQuery(db *sql.DB, query string, parameters ...interface{}) error {
+// It runs the sql query and returns a list of columns names and a list of rows.
+func runQuery(db *sql.DB, query string, parameters ...interface{}) (
+	[]string, [][]string, error) {
+	return runQueryWithFormat(db, nil, query, parameters...)
+}
+
+// runQuery takes a 'query' with optional 'parameters'.
+// It runs the sql query and returns a list of columns names and a list of rows.
+// If 'format' is not nil, the values with column name
+// found in the map are run through the corresponding callback.
+func runQueryWithFormat(db *sql.DB, format fmtMap, query string, parameters ...interface{}) (
+	[]string, [][]string, error) {
 	rows, err := db.Query(query, parameters...)
 	if err != nil {
-		return fmt.Errorf("query error: %s", err)
+		return nil, nil, fmt.Errorf("query error: %s", err)
 	}
 
 	defer rows.Close()
-	return printQueryOutput(rows, nil)
+	return sqlRowsToStrings(rows, format)
 }
 
-// runQueryWithFormat takes a 'query' with optional 'parameters'.
+// runPrettyQueryWithFormat takes a 'query' with optional 'parameters'.
 // It runs the sql query and writes pretty output to osStdout.
-func runQueryWithFormat(db *sql.DB, format fmtMap, query string, parameters ...interface{}) error {
-	rows, err := db.Query(query, parameters...)
+func runPrettyQuery(db *sql.DB, query string, parameters ...interface{}) error {
+	return runPrettyQueryWithFormat(db, nil, query, parameters...)
+}
+
+// runPrettyQueryWithFormat takes a 'query' with optional 'parameters'.
+// It runs the sql query and writes pretty output to osStdout.
+// If 'format' is not nil, the values with column name
+// found in the map are run through the corresponding callback.
+func runPrettyQueryWithFormat(db *sql.DB, format fmtMap, query string, parameters ...interface{}) error {
+	cols, allRows, err := runQueryWithFormat(db, format, query, parameters...)
 	if err != nil {
-		return fmt.Errorf("query error: %s", err)
+		return err
 	}
-
-	defer rows.Close()
-	return printQueryOutput(rows, format)
+	return printQueryOutput(cols, allRows)
 }
 
-// printQueryOutput takes a set of sql rows and writes a pretty table
-// to osStdout, or "OK" if no rows are returned.
+// sqlRowsToStrings turns 'rows' into a list of rows, each of which
+// is a  list of column values.
 // 'rows' should be closed by the caller.
-// If 'formatter' is not nil, the values with column name
+// If 'format' is not nil, the values with column name
 // found in the map are run through the corresponding callback.
-func printQueryOutput(rows *sql.Rows, format fmtMap) error {
+// It returns the header row followed by all data rows.
+// If both the header row and list of rows are empty, it means no row
+// information was returned (eg: statement was not a query).
+func sqlRowsToStrings(rows *sql.Rows, format fmtMap) ([]string, [][]string, error) {
 	cols, err := rows.Columns()
 	if err != nil {
-		return fmt.Errorf("rows.Columns() error: %s", err)
+		return nil, nil, fmt.Errorf("rows.Columns() error: %s", err)
 	}
 
 	if len(cols) == 0 {
-		// This operation did not return rows, just show success.
-		fmt.Fprintln(osStdout, "OK")
-		return nil
+		return nil, nil, nil
 	}
 
-	// Initialize tablewriter and set column names as the header row.
-	table := tablewriter.NewWriter(osStdout)
-	table.SetAutoFormatHeaders(false)
-	table.SetAutoWrapText(false)
-	table.SetHeader(cols)
-
-	// Stringify all data and append rows to tablewriter.
 	vals := make([]interface{}, len(cols))
 	for i := range vals {
 		vals[i] = new(interface{})
 	}
-	rowStrings := make([]string, len(cols))
+
+	allRows := [][]string{}
 	for rows.Next() {
+		rowStrings := make([]string, len(cols))
 		if err := rows.Scan(vals...); err != nil {
-			return fmt.Errorf("scan error: %s", err)
+			return nil, nil, fmt.Errorf("scan error: %s", err)
 		}
 		for i, v := range vals {
 			if f, ok := format[cols[i]]; ok {
@@ -112,7 +124,29 @@ func printQueryOutput(rows *sql.Rows, format fmtMap) error {
 				rowStrings[i] = formatVal(*v.(*interface{}))
 			}
 		}
-		if err := table.Append(rowStrings); err != nil {
+		allRows = append(allRows, rowStrings)
+	}
+
+	return cols, allRows, nil
+}
+
+// printQueryOutput takes a list of column names and a list of row contents
+// writes a pretty table to osStdout, or "OK" if empty.
+func printQueryOutput(cols []string, allRows [][]string) error {
+	if len(cols) == 0 {
+		// This operation did not return rows, just show success.
+		fmt.Fprintln(osStdout, "OK")
+		return nil
+	}
+
+	// Initialize tablewriter and set column names as the header row.
+	table := tablewriter.NewWriter(osStdout)
+	table.SetAutoFormatHeaders(false)
+	table.SetAutoWrapText(false)
+	table.SetHeader(cols)
+
+	for _, row := range allRows {
+		if err := table.Append(row); err != nil {
 			return err
 		}
 	}

diff --git a/cli/sql_util_test.go b/cli/sql_util_test.go
@@ -22,6 +22,7 @@ import (
 	"database/sql"
 	"fmt"
 	"os"
+	"reflect"
 	"testing"
 
 	"github.com/cockroachdb/cockroach/security"
@@ -55,7 +56,7 @@ func TestRunQuery(t *testing.T) {
 	}()
 
 	// Non-query statement.
-	if err := runQuery(db, `SET DATABASE=system`); err != nil {
+	if err := runPrettyQuery(db, `SET DATABASE=system`); err != nil {
 		t.Fatal(err)
 	}
 
@@ -68,7 +69,26 @@ OK
 	b.Reset()
 
 	// Use system database for sample query/output as they are fairly fixed.
-	if err := runQuery(db, `SHOW COLUMNS FROM system.namespace`); err != nil {
+	cols, rows, err := runQuery(db, `SHOW COLUMNS FROM system.namespace`)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	expectedCols := []string{"Field", "Type", "Null"}
+	if !reflect.DeepEqual(expectedCols, cols) {
+		t.Fatalf("expected:\n%v\ngot:\n%v", expectedCols, cols)
+	}
+
+	expectedRows := [][]string{
+		{`"parentID"`, `"INT"`, `true`},
+		{`"name"`, `"STRING"`, `true`},
+		{`"id"`, `"INT"`, `true`},
+	}
+	if !reflect.DeepEqual(expectedRows, rows) {
+		t.Fatalf("expected:\n%v\ngot:\n%v", expectedRows, rows)
+	}
+
+	if err := runPrettyQuery(db, `SHOW COLUMNS FROM system.namespace`); err != nil {
 		t.Fatal(err)
 	}
 
@@ -88,7 +108,7 @@ OK
 	b.Reset()
 
 	// Test placeholders.
-	if err := runQuery(db, `SELECT * FROM system.namespace WHERE name=$1`, "descriptor"); err != nil {
+	if err := runPrettyQuery(db, `SELECT * FROM system.namespace WHERE name=$1`, "descriptor"); err != nil {
 		t.Fatal(err)
 	}
 
@@ -109,7 +129,7 @@ OK
 		return fmt.Sprintf("--> %#v <--", val)
 	}
 
-	if err := runQueryWithFormat(db, fmtMap{"name": newFormat},
+	if err := runPrettyQueryWithFormat(db, fmtMap{"name": newFormat},
 		`SELECT * FROM system.namespace WHERE name=$1`, "descriptor"); err != nil {
 		t.Fatal(err)
 	}

diff --git a/cli/user.go b/cli/user.go
@@ -40,7 +40,7 @@ func runGetUser(cmd *cobra.Command, args []string) {
 		return
 	}
 	db := makeSQLClient()
-	err := runQuery(db, `SELECT * FROM system.users WHERE username=$1`, args[0])
+	err := runPrettyQuery(db, `SELECT * FROM system.users WHERE username=$1`, args[0])
 	if err != nil {
 		log.Error(err)
 		return
@@ -63,7 +63,7 @@ func runLsUsers(cmd *cobra.Command, args []string) {
 		return
 	}
 	db := makeSQLClient()
-	err := runQuery(db, `SELECT username FROM system.users`)
+	err := runPrettyQuery(db, `SELECT username FROM system.users`)
 	if err != nil {
 		log.Error(err)
 		return
@@ -86,7 +86,7 @@ func runRmUser(cmd *cobra.Command, args []string) {
 		return
 	}
 	db := makeSQLClient()
-	err := runQuery(db, `DELETE FROM system.users WHERE username=$1`, args[0])
+	err := runPrettyQuery(db, `DELETE FROM system.users WHERE username=$1`, args[0])
 	if err != nil {
 		log.Error(err)
 		return
@@ -120,7 +120,7 @@ func runSetUser(cmd *cobra.Command, args []string) {
 	}
 	db := makeSQLClient()
 	// TODO(marc): switch to UPSERT.
-	err = runQuery(db, `INSERT INTO system.users VALUES ($1, $2)`, args[0], hashed)
+	err = runPrettyQuery(db, `INSERT INTO system.users VALUES ($1, $2)`, args[0], hashed)
 	if err != nil {
 		log.Error(err)
 		return

diff --git a/cli/zone.go b/cli/zone.go
@@ -82,19 +82,21 @@ func runGetZone(cmd *cobra.Command, args []string) {
 	}
 
 	db := makeSQLClient()
-	err = runQueryWithFormat(db, fmtMap{"config": formatZone}, `SELECT * FROM system.zones WHERE id=$1`, id)
+	_, rows, err := runQueryWithFormat(db, fmtMap{"config": formatZone},
+		`SELECT * FROM system.zones WHERE id=$1`, id)
 	if err != nil {
 		log.Error(err)
 		return
 	}
 
-	if err != nil {
-		log.Error(err)
+	if len(rows) == 0 {
+		log.Errorf("Object %d: no zone config found", id)
 		return
 	}
+	fmt.Fprintln(osStdout, rows[0][1])
 }
 
-// A lsZonesCmd command displays a list of zone configs by objet ID.
+// A lsZonesCmd command displays a list of zone configs by object ID.
 var lsZonesCmd = &cobra.Command{
 	Use:   "ls [options]",
 	Short: "list all zone configs by object ID",
@@ -111,11 +113,19 @@ func runLsZones(cmd *cobra.Command, args []string) {
 		return
 	}
 	db := makeSQLClient()
-	err := runQueryWithFormat(db, fmtMap{"config": formatZone}, `SELECT * FROM system.zones`)
+	_, rows, err := runQueryWithFormat(db, fmtMap{"config": formatZone}, `SELECT * FROM system.zones`)
 	if err != nil {
 		log.Error(err)
 		return
 	}
+
+	if len(rows) == 0 {
+		log.Error("No zone configs founds")
+		return
+	}
+	for _, r := range rows {
+		fmt.Fprintf(osStdout, "Object %s:\n%s\n", r[0], r[1])
+	}
 }
 
 // A rmZoneCmd command removes a zone config by ID.
@@ -142,12 +152,11 @@ func runRmZone(cmd *cobra.Command, args []string) {
 	}
 
 	db := makeSQLClient()
-	err = runQuery(db, `DELETE FROM system.zones WHERE id=$1`, id)
+	err = runPrettyQuery(db, `DELETE FROM system.zones WHERE id=$1`, id)
 	if err != nil {
 		log.Error(err)
 		return
 	}
-	fmt.Printf("Deleted zone key %q\n", args[0])
 }
 
 // A setZoneCmd command creates a new or updates an existing zone config.
@@ -222,7 +231,7 @@ func runSetZone(cmd *cobra.Command, args []string) {
 
 	db := makeSQLClient()
 	// TODO(marc): switch to UPSERT.
-	err = runQuery(db, `INSERT INTO system.zones VALUES ($1, $2)`, id, buf)
+	err = runPrettyQuery(db, `INSERT INTO system.zones VALUES ($1, $2)`, id, buf)
 	if err != nil {
 		log.Error(err)
 		return