cli: extract the value formatting code to a sub-package (1/)

pkg/BUILD.bazel

@@ -45,6 +45,7 @@ ALL_TESTS = [
  "//pkg/ccl/utilccl:utilccl_test",
  "//pkg/ccl/workloadccl/allccl:allccl_test",
  "//pkg/ccl/workloadccl:workloadccl_test",
+ "//pkg/cli/clisqlclient:clisqlclient_test",
  "//pkg/cli/exit:exit_test",
  "//pkg/cli:cli_test",
  "//pkg/clusterversion:clusterversion_test",

pkg/cli/BUILD.bazel

@@ -47,7 +47,6 @@ go_library(
  "quit.go",
  "sql.go",
  "sql_format_table.go",
- "sql_format_value.go",
  "sql_util.go",
  "sqlfmt.go",
  "start.go",
@@ -99,6 +98,7 @@ go_library(
  "//pkg/ccl/sqlproxyccl",
  "//pkg/ccl/sqlproxyccl/tenantdirsvr",
  "//pkg/cli/cliflags",
+ "//pkg/cli/clisqlclient",
  "//pkg/cli/exit",
  "//pkg/cli/syncbench",
  "//pkg/clusterversion",
@@ -147,7 +147,6 @@ go_library(
  "//pkg/sql/row",
  "//pkg/sql/sem/builtins",
  "//pkg/sql/sem/tree",
- "//pkg/sql/sessiondatapb",
  "//pkg/sqlmigrations",
  "//pkg/storage",
  "//pkg/storage/cloud",
@@ -302,7 +301,6 @@ go_test(
  "nodelocal_test.go",
  "quit_test.go",
  "sql_format_table_test.go",
- "sql_format_value_test.go",
  "sql_test.go",
  "sql_util_test.go",
  "sqlfmt_test.go",

pkg/cli/clisqlclient/BUILD.bazel

@@ -0,0 +1,32 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+
+go_library(
+ name = "clisqlclient",
+ srcs = [
+ "doc.go",
+ "sql_format_value.go",
+ ],
+ importpath = "github.com/cockroachdb/cockroach/pkg/cli/clisqlclient",
+ visibility = ["//visibility:public"],
+ deps = [
+ "//pkg/sql/lex",
+ "//pkg/sql/sessiondatapb",
+ "//pkg/util/log",
+ "//pkg/util/timeutil",
+ "@com_github_lib_pq//:pq",
+ ],
+)
+
+go_test(
+ name = "clisqlclient_test",
+ srcs = [
+ "main_test.go",
+ "sql_format_value_test.go",
+ ],
+ deps = [
+ "//pkg/build",
+ "//pkg/cli",
+ "//pkg/server",
+ "//pkg/testutils/serverutils",
+ ],
+)

pkg/cli/clisqlclient/doc.go

@@ -0,0 +1,15 @@
+// Copyright 2021 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+// Package clisqlclient contains code common to all CLI commands
+// that establish SQL connections, including but not exclusively
+// the SQL interactive shell. It also supports commands like
+// 'cockroach node ls'.
+package clisqlclient

pkg/cli/clisqlclient/main_test.go

@@ -0,0 +1,31 @@
+// Copyright 2021 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+package clisqlclient_test
+
+import (
+ "os"
+ "testing"
+
+ "github.com/cockroachdb/cockroach/pkg/build"
+ "github.com/cockroachdb/cockroach/pkg/server"
+ "github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
+)
+
+func TestMain(m *testing.M) {
+ // CLI tests are sensitive to the server version, but test binaries don't have
+ // a version injected. Pretend to be a very up-to-date version.
+ defer build.TestingOverrideTag("v999.0.0")()
+
+ serverutils.InitTestServerFactory(server.TestServerFactory)
+ os.Exit(m.Run())
+}
+
+//go:generate ../../util/leaktest/add-leaktest.sh *_test.go

pkg/cli/sql_format_value.go → pkg/cli/clisqlclient/sql_format_value.go

@@ -8,7 +8,7 @@
 // by the Apache License, Version 2.0, included in the file
 // licenses/APL.txt.
 
-package cli
+package clisqlclient
 
 import (
  "context"
@@ -37,7 +37,9 @@ func isNotGraphicUnicodeOrTabOrNewline(r rune) bool {
  return r != '\t' && r != '\n' && !unicode.IsGraphic(r)
 }
 
-func formatVal(
+// FormatVal formats a value retrieved by a SQL driver into a string
+// suitable for displaying to the user.
+func FormatVal(
  val driver.Value, colType string, showPrintableUnicode bool, showNewLinesAndTabs bool,
 ) string {
  log.VInfof(context.Background(), 2, "value: go %T, sql %q", val, colType)
@@ -168,7 +170,7 @@ func formatArray(
  // A parsing failure is not a catastrophe; we can still print out
  // the array as a byte slice. This will do in many cases.
  log.VInfof(context.Background(), 1, "unable to parse %q (sql %q) as array: %v", b, colType, err)
- return formatVal(b, "BYTEA", showPrintableUnicode, showNewLinesAndTabs)
+ return FormatVal(b, "BYTEA", showPrintableUnicode, showNewLinesAndTabs)
  }
 
  // We have a go array in "backingArray". Now print it out.
@@ -182,7 +184,7 @@ func formatArray(
  // Access the i-th element in the backingArray.
  arrayVal := driver.Value(v.Index(i).Interface())
  // Format the value recursively into a string.
- vs := formatVal(arrayVal, colType, showPrintableUnicode, showNewLinesAndTabs)
+ vs := FormatVal(arrayVal, colType, showPrintableUnicode, showNewLinesAndTabs)
 
  // If the value contains special characters or a comma, enclose in double quotes.
  // Also escape the special characters.

pkg/cli/sql_format_value_test.go → pkg/cli/clisqlclient/sql_format_value_test.go

@@ -8,10 +8,12 @@
 // by the Apache License, Version 2.0, included in the file
 // licenses/APL.txt.
 
-package cli
+package clisqlclient_test
+
+import "github.com/cockroachdb/cockroach/pkg/cli"
 
 func Example_sql_format() {
- c := NewCLITest(TestCLIParams{})
+ c := cli.NewCLITest(cli.TestCLIParams{})
  defer c.Cleanup()
 
  c.RunWithArgs([]string{"sql", "-e", "create database t; create table t.times (bare timestamp, withtz timestamptz)"})

pkg/cli/clisqlshell/BUILD.bazel

@@ -0,0 +1,8 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
+ name = "clisqlshell",
+ srcs = ["doc.go"],
+ importpath = "github.com/cockroachdb/cockroach/pkg/cli/clisqlshell",
+ visibility = ["//visibility:public"],
+)

pkg/cli/clisqlshell/doc.go

@@ -0,0 +1,17 @@
+// Copyright 2021 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+// Package clisqlshell contains the code that powers CockroachDB's
+// interactive SQL shell.
+//
+// Note that the common code that is shared with other CLI commands
+// that are not interactive SQL shells but establish a SQL connection
+// to a server should be placed in package clisqlclient instead.
+package clisqlshell

pkg/cli/sql.go

@@ -27,6 +27,7 @@ import (
  "time"
 
  "github.com/cockroachdb/cockroach/pkg/cli/cliflags"
+ "github.com/cockroachdb/cockroach/pkg/cli/clisqlclient"
  "github.com/cockroachdb/cockroach/pkg/docs"
  "github.com/cockroachdb/cockroach/pkg/roachpb"
  "github.com/cockroachdb/cockroach/pkg/sql"
@@ -848,7 +849,7 @@ func (c *cliState) refreshTransactionStatus() {
  return
  }
 
- txnString := formatVal(dbVal, dbColType,
+ txnString := clisqlclient.FormatVal(dbVal, dbColType,
  false /* showPrintableUnicode */, false /* shownewLinesAndTabs */)
 
  // Change the prompt based on the response from the server.
@@ -887,7 +888,7 @@ func (c *cliState) refreshDatabaseName() string {
  " Use SET database = <dbname> to change, CREATE DATABASE to make a new database.")
  }
 
- dbName := formatVal(dbVal, dbColType,
+ dbName := clisqlclient.FormatVal(dbVal, dbColType,
  false /* showPrintableUnicode */, false /* shownewLinesAndTabs */)
 
  // Preserve the current database name in case of reconnects.

pkg/cli/sql_util.go

@@ -23,6 +23,7 @@ import (
 
  "github.com/cockroachdb/cockroach-go/crdb"
  "github.com/cockroachdb/cockroach/pkg/build"
+ "github.com/cockroachdb/cockroach/pkg/cli/clisqlclient"
  "github.com/cockroachdb/cockroach/pkg/roachpb"
  "github.com/cockroachdb/cockroach/pkg/security"
  "github.com/cockroachdb/cockroach/pkg/sql/catalog/catconstants"
@@ -425,12 +426,12 @@ func (c *sqlConn) getLastQueryStatistics() (
  return 0, 0, 0, 0, 0, containsJobLat, err
  }
 
- parseLatencyRaw = formatVal(row[0], iter.colTypes[0], false, false)
- planLatencyRaw = formatVal(row[1], iter.colTypes[1], false, false)
- execLatencyRaw = formatVal(row[2], iter.colTypes[2], false, false)
- serviceLatencyRaw = formatVal(row[3], iter.colTypes[3], false, false)
+ parseLatencyRaw = clisqlclient.FormatVal(row[0], iter.colTypes[0], false, false)
+ planLatencyRaw = clisqlclient.FormatVal(row[1], iter.colTypes[1], false, false)
+ execLatencyRaw = clisqlclient.FormatVal(row[2], iter.colTypes[2], false, false)
+ serviceLatencyRaw = clisqlclient.FormatVal(row[3], iter.colTypes[3], false, false)
  if containsJobLat {
- jobsLatencyRaw = formatVal(row[4], iter.colTypes[4], false, false)
+ jobsLatencyRaw = clisqlclient.FormatVal(row[4], iter.colTypes[4], false, false)
  }
 
  nRows++
@@ -1075,7 +1076,7 @@ func getColumnStrings(rows *sqlRows, showMoreChars bool) []string {
  srcCols := rows.Columns()
  cols := make([]string, len(srcCols))
  for i, c := range srcCols {
- cols[i] = formatVal(c, "NAME", showMoreChars, showMoreChars)
+ cols[i] = clisqlclient.FormatVal(c, "NAME", showMoreChars, showMoreChars)
  }
  return cols
 }
@@ -1114,7 +1115,7 @@ func getNextRowStrings(rows *sqlRows, colTypes []string, showMoreChars bool) ([]
 
  rowStrings := make([]string, len(cols))
  for i, v := range vals {
- rowStrings[i] = formatVal(v, colTypes[i], showMoreChars, showMoreChars)
+ rowStrings[i] = clisqlclient.FormatVal(v, colTypes[i], showMoreChars, showMoreChars)
  }
  return rowStrings, nil
 }