Skip to content

Commit

Permalink
cli: move the value formatting code to clisqlclient
Browse files Browse the repository at this point in the history
Release note: None
  • Loading branch information
knz committed Jul 10, 2021
1 parent a80a0d5 commit e1e9d81
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 20 deletions.
1 change: 1 addition & 0 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 1 addition & 3 deletions pkg/cli/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
28 changes: 26 additions & 2 deletions pkg/cli/clisqlclient/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,8 +1,32 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

go_library(
name = "clisqlclient",
srcs = ["doc.go"],
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",
],
)
31 changes: 31 additions & 0 deletions pkg/cli/clisqlclient/main_test.go
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package cli
package clisqlclient

import (
"context"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)"})
Expand Down
5 changes: 3 additions & 2 deletions pkg/cli/sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
15 changes: 8 additions & 7 deletions pkg/cli/sql_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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++
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit e1e9d81

Please sign in to comment.