Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
26550: sql: auto-generate column names like in PostgreSQL r=knz a=knz

Fixes  cockroachdb#26236.

Prior to this patch, CockroachDB would generate names for results not
explicitly named with AS using a pretty-printing algorithm for the
entire rendered expression.

Example before:

```
> SELECT cos(x + y)::INT FROM xy;
+-----------------+
| cos(x + y)::INT |
+-----------------+
|               0 |
+-----------------+
> SELECT 'abc'::BYTES;
+--------------+
| 'abc'::BYTES |
+--------------+
| abc          |
+--------------+
> SELECT 1 + 2
+-------+
| 1 + 2 |
+-------+
|     3 |
+-------+
```

There were multiple drawbacks from doing so:

- the generated column names were incompatible with PostgreSQL. This
  showed up in edge cases like `select nullif(a,b,c)` where the column
  name is expected to be "`nullif`" not the pretty-printed full
  expression `NULLIF(a, b, c)`.
- the algorithm incurred many mandatory memory allocations (to
  assemble the string) for every single column rendered, in every
  query.
- the tree recursion and string interpolations needed to construct the
  string were using CPU cycles that do not contribute to the query
  results.

Instead, this patch implements PostgreSQL's algorithm, which has the
following properties:

- it only produces pre-existing strings in memory, either from static
  string constants or from the text of the SQL query (parsed
  tokens). No additional memory allocations or string computations are
  needed.
- it traverses fewer nodes from the AST in memory to compute its
  result.

- it produces the same results as PostgreSQL for the same input
  queries.

Example after:

```
> SELECT cos(x + y)::INT FROM xy;
+-----+
| cos |
+-----+
|   0 |
+-----+
> SELECT 'abc'::BYTES;
+-------+
| bytes |
+-------+
| abc   |
+-------+
> SELECT 1 + 2
+----------+
| ?column? |
+----------+
|        3 |
+----------+
```

The algorithm produces a string and a confidence level (0-2) and is
recursively defined as follows:

- things that get constant strings with confidence 2:
  - column reference get named after the unqualified column name.
  - function applications get named after the name of the function,
    e.g. `cos(x)` gets named `cos`.
  - function-like operators get named after the operator. For example
    `coalesce(a,b)` gets named `coalesce`, `row(a,b)` gets named
    `row`, `nullif(a,b)` is `nullif`, `exists(subquery...)` is
    `exists`, etc.
  - `ARRAY(E)` and `ARRAY[...]` gets named `array`.
  - boolean literals (`true`/`false`) get named `bool` because pg
    internally implements them as `'t'::BOOL` and `'f'::BOOL`.
  - `(SELECT E AS x)` gets named just `x`.
  - `(E).x` gets named just `x`.
  - `((E) AS x)` gets named just `x`.
  - `(VALUES (E))` gets named just `column1` (for consistency with
    the standalone `VALUES` clause).
- things that are named recursively:
  - `(E)` gets named after E.
  - `E[N]` gets named after E.
  - `E COLLATE X` gets named after E.
  - `(SELECT E)` (no `AS` in subquery) gets named after E.
  - `E::T` and `E:::T` gets named after E, and if the confidence is <=
    1 then after T with confidence 1.  e.g. `t::INT` is named `t`,
    `123::INT` is named `int`.
  - `CASE A THEN B ELSE C END` gets named after C, and if the
    confidence is <= 1 then just `case` with confidence 1.
  - `CASE A THEN B END` (no ELSE) gets named just `case` with
    confidence 1.
- everything else receives no name with confidence 0.

When the algorithm complete, if there was no name produced the final
name becomes `"?column?"`.
The original code is to be found in pg's source in
`src/backend/parser/parse_target.c`.

Release note (sql change): CockroachDB now computes automatic column
names for SELECT expressions that do not use AS using a simpler and
more efficient algorithm, which also produces names more compatible
with PostgreSQL.

Release note (backward-incompatible change): CockroachDB now uses a
different algorithm to generate column names for complex expressions
in SELECT clauses when AS is not used. The results are more compatible
with PostgreSQL but may appear different to client applications. This
does not impact most uses of SQL, where the rendered expressions are
sufficiently simple (simple function applications, reuses of existing
columns) or when AS is used explicitly.

Co-authored-by: Raphael 'kena' Poss <[email protected]>
  • Loading branch information
craig[bot] and knz committed Jun 10, 2018
2 parents 7ece796 + 4413d78 commit dde686a
Show file tree
Hide file tree
Showing 87 changed files with 2,159 additions and 1,892 deletions.
96 changes: 48 additions & 48 deletions pkg/cli/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,30 +399,30 @@ func Example_logging() {
c := newCLITest(cliTestParams{})
defer c.cleanup()

c.RunWithArgs([]string{"sql", "--logtostderr=false", "-e", "select 1"})
c.RunWithArgs([]string{"sql", "--log-backtrace-at=foo.go:1", "-e", "select 1"})
c.RunWithArgs([]string{"sql", "--log-dir=", "-e", "select 1"})
c.RunWithArgs([]string{"sql", "--logtostderr=true", "-e", "select 1"})
c.RunWithArgs([]string{"sql", "--verbosity=0", "-e", "select 1"})
c.RunWithArgs([]string{"sql", "--vmodule=foo=1", "-e", "select 1"})
c.RunWithArgs([]string{`sql`, `--logtostderr=false`, `-e`, `select 1 as "1"`})
c.RunWithArgs([]string{`sql`, `--log-backtrace-at=foo.go:1`, `-e`, `select 1 as "1"`})
c.RunWithArgs([]string{`sql`, `--log-dir=`, `-e`, `select 1 as "1"`})
c.RunWithArgs([]string{`sql`, `--logtostderr=true`, `-e`, `select 1 as "1"`})
c.RunWithArgs([]string{`sql`, `--verbosity=0`, `-e`, `select 1 as "1"`})
c.RunWithArgs([]string{`sql`, `--vmodule=foo=1`, `-e`, `select 1 as "1"`})

// Output:
// sql --logtostderr=false -e select 1
// sql --logtostderr=false -e select 1 as "1"
// 1
// 1
// sql --log-backtrace-at=foo.go:1 -e select 1
// sql --log-backtrace-at=foo.go:1 -e select 1 as "1"
// 1
// 1
// sql --log-dir= -e select 1
// sql --log-dir= -e select 1 as "1"
// 1
// 1
// sql --logtostderr=true -e select 1
// sql --logtostderr=true -e select 1 as "1"
// 1
// 1
// sql --verbosity=0 -e select 1
// sql --verbosity=0 -e select 1 as "1"
// 1
// 1
// sql --vmodule=foo=1 -e select 1
// sql --vmodule=foo=1 -e select 1 as "1"
// 1
// 1
}
Expand Down Expand Up @@ -704,12 +704,12 @@ func Example_zone() {

func Example_demo() {
testData := [][]string{
{"demo", "-e", "show database"},
{"demo", "-e", "show application_name"},
{"demo", "--format=pretty", "-e", "show database"},
{"demo", "-e", "select 1", "-e", "select 3"},
{"demo", "--echo-sql", "-e", "select 1"},
{"demo", "--set=errexit=0", "-e", "select nonexistent", "-e", "select 123"},
{`demo`, `-e`, `show database`},
{`demo`, `-e`, `show application_name`},
{`demo`, `--format=pretty`, `-e`, `show database`},
{`demo`, `-e`, `select 1 as "1"`, `-e`, `select 3 as "3"`},
{`demo`, `--echo-sql`, `-e`, `select 1 as "1"`},
{`demo`, `--set=errexit=0`, `-e`, `select nonexistent`, `-e`, `select 123 as "123"`},
}

// Ensure that CLI error messages and anything meant for the
Expand Down Expand Up @@ -739,16 +739,16 @@ func Example_demo() {
// | defaultdb |
// +-----------+
// (1 row)
// demo -e select 1 -e select 3
// demo -e select 1 as "1" -e select 3 as "3"
// 1
// 1
// 3
// 3
// demo --echo-sql -e select 1
// > select 1
// demo --echo-sql -e select 1 as "1"
// > select 1 as "1"
// 1
// 1
// demo --set=errexit=0 -e select nonexistent -e select 123
// demo --set=errexit=0 -e select nonexistent -e select 123 as "123"
// pq: column "nonexistent" does not exist
// 123
// 123
Expand All @@ -758,44 +758,44 @@ func Example_sql() {
c := newCLITest(cliTestParams{})
defer c.cleanup()

c.RunWithArgs([]string{"sql", "-e", "show application_name"})
c.RunWithArgs([]string{"sql", "-e", "create database t; create table t.f (x int, y int); insert into t.f values (42, 69)"})
c.RunWithArgs([]string{"sql", "-e", "select 3", "-e", "select * from t.f"})
c.RunWithArgs([]string{"sql", "-e", "begin", "-e", "select 3", "-e", "commit"})
c.RunWithArgs([]string{"sql", "-e", "select * from t.f"})
c.RunWithArgs([]string{"sql", "--execute=show databases"})
c.RunWithArgs([]string{"sql", "-e", "select 1; select 2"})
c.RunWithArgs([]string{"sql", "-e", "select 1; select 2 where false"})
c.RunWithArgs([]string{`sql`, `-e`, `show application_name`})
c.RunWithArgs([]string{`sql`, `-e`, `create database t; create table t.f (x int, y int); insert into t.f values (42, 69)`})
c.RunWithArgs([]string{`sql`, `-e`, `select 3 as "3"`, `-e`, `select * from t.f`})
c.RunWithArgs([]string{`sql`, `-e`, `begin`, `-e`, `select 3 as "3"`, `-e`, `commit`})
c.RunWithArgs([]string{`sql`, `-e`, `select * from t.f`})
c.RunWithArgs([]string{`sql`, `--execute=show databases`})
c.RunWithArgs([]string{`sql`, `-e`, `select 1 as "1"; select 2 as "2"`})
c.RunWithArgs([]string{`sql`, `-e`, `select 1 as "1"; select 2 as "@" where false`})
// CREATE TABLE AS returns a SELECT tag with a row count, check this.
c.RunWithArgs([]string{"sql", "-e", "create table t.g1 (x int)"})
c.RunWithArgs([]string{"sql", "-e", "create table t.g2 as select * from generate_series(1,10)"})
c.RunWithArgs([]string{`sql`, `-e`, `create table t.g1 (x int)`})
c.RunWithArgs([]string{`sql`, `-e`, `create table t.g2 as select * from generate_series(1,10)`})
// It must be possible to access pre-defined/virtual tables even if the current database
// does not exist yet.
c.RunWithArgs([]string{"sql", "-d", "nonexistent", "-e", "select count(*) from \"\".information_schema.tables limit 0"})
c.RunWithArgs([]string{`sql`, `-d`, `nonexistent`, `-e`, `select count(*) from "".information_schema.tables limit 0`})
// It must be possible to create the current database after the
// connection was established.
c.RunWithArgs([]string{"sql", "-d", "nonexistent", "-e", "create database nonexistent; create table foo(x int); select * from foo"})
c.RunWithArgs([]string{`sql`, `-d`, `nonexistent`, `-e`, `create database nonexistent; create table foo(x int); select * from foo`})
// COPY should return an intelligible error message.
c.RunWithArgs([]string{"sql", "-e", "copy t.f from stdin"})
c.RunWithArgs([]string{`sql`, `-e`, `copy t.f from stdin`})
// --echo-sql should print out the SQL statements.
c.RunWithArgs([]string{"user", "ls", "--echo-sql"})
c.RunWithArgs([]string{`user`, `ls`, `--echo-sql`})
// --set changes client-side variables before executing commands.
c.RunWithArgs([]string{"sql", "--set=errexit=0", "-e", "select nonexistent", "-e", "select 123"})
c.RunWithArgs([]string{"sql", "--set", "echo=true", "-e", "select 123"})
c.RunWithArgs([]string{"sql", "--set", "unknownoption", "-e", "select 123"})
c.RunWithArgs([]string{`sql`, `--set=errexit=0`, `-e`, `select nonexistent`, `-e`, `select 123 as "123"`})
c.RunWithArgs([]string{`sql`, `--set`, `echo=true`, `-e`, `select 123 as "123"`})
c.RunWithArgs([]string{`sql`, `--set`, `unknownoption`, `-e`, `select 123 as "123"`})

// Output:
// sql -e show application_name
// application_name
// cockroach sql
// sql -e create database t; create table t.f (x int, y int); insert into t.f values (42, 69)
// INSERT 1
// sql -e select 3 -e select * from t.f
// sql -e select 3 as "3" -e select * from t.f
// 3
// 3
// x y
// 42 69
// sql -e begin -e select 3 -e commit
// sql -e begin -e select 3 as "3" -e commit
// BEGIN
// 3
// 3
Expand All @@ -809,15 +809,15 @@ func Example_sql() {
// postgres
// system
// t
// sql -e select 1; select 2
// sql -e select 1 as "1"; select 2 as "2"
// 1
// 1
// 2
// 2
// sql -e select 1; select 2 where false
// sql -e select 1 as "1"; select 2 as "@" where false
// 1
// 1
// 2
// @
// sql -e create table t.g1 (x int)
// CREATE TABLE
// sql -e create table t.g2 as select * from generate_series(1,10)
Expand All @@ -832,15 +832,15 @@ func Example_sql() {
// > SHOW USERS
// username
// root
// sql --set=errexit=0 -e select nonexistent -e select 123
// sql --set=errexit=0 -e select nonexistent -e select 123 as "123"
// pq: column "nonexistent" does not exist
// 123
// 123
// sql --set echo=true -e select 123
// > select 123
// sql --set echo=true -e select 123 as "123"
// > select 123 as "123"
// 123
// 123
// sql --set unknownoption -e select 123
// sql --set unknownoption -e select 123 as "123"
// invalid syntax: \set unknownoption. Try \? for help.
// invalid syntax
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/cli/interactive_tests/test_last_statement.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,13 @@ eexpect ":/# "
end_test

start_test "Check that a final comment after a final statement does not cause an error message. #9482"
send "printf 'select 1;-- final comment' | $argv sql | cat\r"
eexpect "1\r\n1\r\n:/# "
send "printf 'select 1 as woo;-- final comment' | $argv sql | cat\r"
eexpect "woo\r\n1\r\n:/# "
end_test

start_test "Check that a final comment does not cause an error message. #9243"
send "printf 'select 1;\\n-- final comment' | $argv sql | cat\r"
eexpect "1\r\n1\r\n:/# "
send "printf 'select 1 as woo;\\n-- final comment' | $argv sql | cat\r"
eexpect "woo\r\n1\r\n:/# "
end_test

# Finally terminate with Ctrl+C
Expand Down
10 changes: 5 additions & 5 deletions pkg/cli/interactive_tests/test_local_cmds.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ send "select 'hello\r"
eexpect " ->"
send "\\?\r"
eexpect " ->"
send "world';\r"
eexpect "hello\\\\nworld"
send "world' as woo;\r"
eexpect "hello\r\nworld"
eexpect "Time"
eexpect root@
end_test
Expand All @@ -126,8 +126,8 @@ start_test "Check that \\set can change the display of query times"
send "\\unset show_times\r\\set\r"
eexpect "show_times\tfalse"
eexpect root@
send "select 1;\r"
eexpect "1\r\n1\r\n"
send "select 1 as woo;\r"
eexpect "woo\r\n1\r\n"
expect {
"Time:" {
report "unexpected Time"
Expand Down Expand Up @@ -220,7 +220,7 @@ send "PS1=':''/# '\r"
eexpect ":/# "

start_test "Check that non-interactive built-in commands are only accepted at the start of a statement."
send "(echo '\\set check_syntax'; echo 'select '; echo '\\help'; echo '1;') | $argv sql\r"
send "(echo '\\set check_syntax'; echo 'select '; echo '\\help'; echo ';') | $argv sql\r"
eexpect "statement ignored"
eexpect ":/# "

Expand Down
12 changes: 6 additions & 6 deletions pkg/cli/interactive_tests/test_pretty.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ eexpect ":/# "
end_test

start_test "Check that tables are not pretty-printed when output is not a terminal and --format=pretty is not specified"
send "echo begin; echo 'select 1;' | $argv sql | cat\r"
eexpect "begin\r\n1\r\n1\r\n"
send "echo begin; echo 'select 1 as woo;' | $argv sql | cat\r"
eexpect "begin\r\nwoo\r\n1\r\n"
eexpect ":/# "
end_test

Expand All @@ -38,7 +38,7 @@ end_test

start_test "Check that the shell supports unicode input and that results display unicode characters."
send "select '☃';\r"
eexpect "U00002603"
eexpect "?column?"
eexpect ""
eexpect "+-*+\r\n*1 row"
eexpect root@
Expand All @@ -49,9 +49,9 @@ send "\\q\r"
eexpect ":/# "
send "$argv sql --format=tsv\r"
eexpect root@
send "select 42; select 1;\r"
eexpect "42\r\n42\r\n"
eexpect "1\r\n1\r\n"
send "select 42 as woo; select 1 as woo;\r"
eexpect "woo\r\n42\r\n"
eexpect "woo\r\n1\r\n"
eexpect root@
send "\\q\r"
end_test
Expand Down
Loading

0 comments on commit dde686a

Please sign in to comment.