Merge cockroachdb#93754

93754: sql: deprecate ordinal column references r=mgartner a=mgartner

#### sql: deprecate ordinal column references

This commit disallows ordinal column references by default. Any
statements using them will result in an error. The session setting
`allow_ordinal_column_references` can be set to true to revert to
previous behavior.

Note that numeric tuple indexing (e.g., `SELECT ((1,2,3)).`@2`)` is still
allowed.

This deprecation is motivated by the inconsistent and surprising
behavior that ordinal column references provide. Ordinal column
references (e.g., `SELECT `@1` FROM t`) are a vestigial and undocumented
feature originally motivated to aid the heuristic planner. The PR that
added them, cockroachdb#10729, discourages their use by users because "they are not
robust against schema changes". It also points out a subtle difference
between ordinal column references and SQL standard ordinals that could
confuse users. As an example, the following statements are not
equivalent:

```sql
SELECT `@2` FROM t ORDER BY `@1;`

SELECT `@2` FROM t ORDER BY 1;
```

In the current implementation, an ordinal column reference ``@n`` resolves
to the n-th column in the current scope's slice of columns. This has
several implications:

1. Ordinal columns can refer to any column, not just columns of data
   sources in `FROM` clauses, as was originally intended. This makes it
   hard to reason about the resolution of an ordinal column reference.
   For example, it's somewhat surprising that the result of the `SELECT`
   below is not `(10, 1)`.

```
CREATE TABLE t (a INT, b INT);
INSERT INTO t VALUES (1, 10);

SELECT `@2,` `@1` FROM (SELECT `@2,` `@1` FROM t);
  ?column? | ?column?
-----------+-----------
         1 |       10
(1 row)
```

2. The ordering of columns in the scope's slice is not guaranteed to be
   consistent, so any reasonable ordinal column resolution occurs
   more-or-less by chance. As an example of unexpected behavior,
   consider:

```
CREATE TABLE t (a INT PRIMARY KEY, INDEX ((a+10)));
INSERT INTO t(a) VALUES (1);
ALTER TABLE t ADD COLUMN b INT;

SELECT `@1,` `@2` FROM t;
  ?column? | ?column?
-----------+-----------
         1 |       11
(1 row)
```

Most users would expect the result of the `SELECT` statement to be
`(1, NULL)` because ``@1`` should resolve to `a` and ``@2`` should resolve
to `b`. Instead, the ordinal column reference ``@2`` circumvents logic
that keeps inaccessible columns from being referenced, and resolves to
the virtual computed column backing the secondary index.

Epic: None

Release note (sql change): Ordinal column references (e.g.,
`SELECT `@1,` `@2` FROM t`) are now deprecated. By default, statements using
this syntax will now result in an error. They can be allowed using the
session setting `SET allow_ordinal_column_references=true`. Support for
ordinal column references is scheduled to be removed in version 23.2.


Co-authored-by: Marcus Gartner <[email protected]>

pkg/ccl/backupccl/testdata/backup-restore/user-defined-functions

@@ -50,7 +50,7 @@ db1 sc2 tbl2 table false
 db1 sc2 f2 function false
 
 query-sql
-SELECT @2 FROM [SHOW CREATE FUNCTION sc1.f1]
+SELECT create_statement FROM [SHOW CREATE FUNCTION sc1.f1]
 ----
 CREATE FUNCTION sc1.f1(IN a db1.sc1.enum1)
 	RETURNS INT8
@@ -85,7 +85,7 @@ USE db1_new
 # 2. db name in qualified name is rewritten.
 # 3. sequence id is rewritten so that sequence name is deserialized correctly.
 query-sql
-SELECT @2 FROM [SHOW CREATE FUNCTION sc1.f1]
+SELECT create_statement FROM [SHOW CREATE FUNCTION sc1.f1]
 ----
 CREATE FUNCTION sc1.f1(IN a db1_new.sc1.enum1)
 	RETURNS INT8
@@ -200,7 +200,7 @@ db1 sc2 tbl2 table true
 db1 sc2 f2 function true
 
 query-sql
-SELECT @2 FROM [SHOW CREATE FUNCTION sc1.f1]
+SELECT create_statement FROM [SHOW CREATE FUNCTION sc1.f1]
 ----
 CREATE FUNCTION sc1.f1(IN a db1.sc1.enum1)
 	RETURNS INT8
@@ -236,7 +236,7 @@ USE db1
 # 2. db name in qualified name is rewritten.
 # 3. sequence id is rewritten so that sequence name is deserialized correctly.
 query-sql
-SELECT @2 FROM [SHOW CREATE FUNCTION sc1.f1]
+SELECT create_statement FROM [SHOW CREATE FUNCTION sc1.f1]
 ----
 CREATE FUNCTION sc1.f1(IN a db1.sc1.enum1)
 	RETURNS INT8

pkg/ccl/logictestccl/testdata/logic_test/partitioning_hash_sharded_index

@@ -102,7 +102,7 @@ CREATE TABLE t_to_be_hashed (
 );
 
 query T
-SELECT @2 FROM [SHOW CREATE TABLE t_to_be_hashed];
+SELECT create_statement FROM [SHOW CREATE TABLE t_to_be_hashed];
 ----
 CREATE TABLE public.t_to_be_hashed (
   a INT8 NOT NULL,
@@ -120,7 +120,7 @@ statement ok
 CREATE INDEX ON t_to_be_hashed (c) USING HASH;
 
 query T
-SELECT @2 FROM [SHOW CREATE TABLE t_to_be_hashed];
+SELECT create_statement FROM [SHOW CREATE TABLE t_to_be_hashed];
 ----
 CREATE TABLE public.t_to_be_hashed (
   a INT8 NOT NULL,
@@ -140,7 +140,7 @@ statement ok
 CREATE UNIQUE INDEX ON t_to_be_hashed (c) USING HASH;
 
 query T
-SELECT @2 FROM [SHOW CREATE TABLE t_to_be_hashed];
+SELECT create_statement FROM [SHOW CREATE TABLE t_to_be_hashed];
 ----
 CREATE TABLE public.t_to_be_hashed (
   a INT8 NOT NULL,
@@ -161,7 +161,7 @@ statement ok
 ALTER TABLE t_to_be_hashed ALTER PRIMARY KEY USING COLUMNS (a) USING HASH;
 
 query T
-SELECT @2 FROM [SHOW CREATE TABLE t_to_be_hashed];
+SELECT create_statement FROM [SHOW CREATE TABLE t_to_be_hashed];
 ----
 CREATE TABLE public.t_to_be_hashed (
   a INT8 NOT NULL,
@@ -192,7 +192,7 @@ CREATE TABLE t_idx_pk_hashed_1 (
 );
 
 query T
-SELECT @2 FROM [SHOW CREATE TABLE t_idx_pk_hashed_1];
+SELECT create_statement FROM [SHOW CREATE TABLE t_idx_pk_hashed_1];
 ----
 CREATE TABLE public.t_idx_pk_hashed_1 (
   crdb_internal_a_shard_16 INT8 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(a)), 16:::INT8)) VIRTUAL,
@@ -223,7 +223,7 @@ CREATE TABLE t_idx_pk_hashed_2 (
 );
 
 query T
-SELECT @2 FROM [SHOW CREATE TABLE t_idx_pk_hashed_2];
+SELECT create_statement FROM [SHOW CREATE TABLE t_idx_pk_hashed_2];
 ----
 CREATE TABLE public.t_idx_pk_hashed_2 (
   a INT8 NOT NULL,

pkg/ccl/logictestccl/testdata/logic_test/regional_by_row_hash_sharded_index

@@ -44,7 +44,7 @@ CREATE TABLE t_to_be_hashed (
 ) LOCALITY REGIONAL BY ROW;
 
 query T
-SELECT @2 FROM [SHOW CREATE TABLE t_to_be_hashed];
+SELECT create_statement FROM [SHOW CREATE TABLE t_to_be_hashed];
 ----
 CREATE TABLE public.t_to_be_hashed (
                             pk INT8 NOT NULL,
@@ -58,7 +58,7 @@ statement ok
 CREATE INDEX ON t_to_be_hashed (b) USING HASH;
 
 query T
-SELECT @2 FROM [SHOW CREATE TABLE t_to_be_hashed];
+SELECT create_statement FROM [SHOW CREATE TABLE t_to_be_hashed];
 ----
 CREATE TABLE public.t_to_be_hashed (
                             pk INT8 NOT NULL,
@@ -74,7 +74,7 @@ statement ok
 CREATE UNIQUE INDEX ON t_to_be_hashed (b) USING HASH;
 
 query T
-SELECT @2 FROM [SHOW CREATE TABLE t_to_be_hashed];
+SELECT create_statement FROM [SHOW CREATE TABLE t_to_be_hashed];
 ----
 CREATE TABLE public.t_to_be_hashed (
                             pk INT8 NOT NULL,
@@ -91,7 +91,7 @@ statement ok
 ALTER TABLE t_to_be_hashed ALTER PRIMARY KEY USING COLUMNS (pk) USING HASH;
 
 query T
-SELECT @2 FROM [SHOW CREATE TABLE t_to_be_hashed];
+SELECT create_statement FROM [SHOW CREATE TABLE t_to_be_hashed];
 ----
 CREATE TABLE public.t_to_be_hashed (
                             pk INT8 NOT NULL,
@@ -114,7 +114,7 @@ CREATE TABLE t_regional_pk_hashed_1 (
 ) LOCALITY REGIONAL BY ROW;
 
 query T
-SELECT @2 FROM [SHOW CREATE TABLE t_regional_pk_hashed_1];
+SELECT create_statement FROM [SHOW CREATE TABLE t_regional_pk_hashed_1];
 ----
 CREATE TABLE public.t_regional_pk_hashed_1 (
                             crdb_internal_pk_shard_16 INT8 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(pk)), 16:::INT8)) VIRTUAL,
@@ -137,7 +137,7 @@ CREATE TABLE t_regional_pk_hashed_2 (
 ) LOCALITY REGIONAL BY ROW;
 
 query T
-SELECT @2 FROM [SHOW CREATE TABLE t_regional_pk_hashed_2];
+SELECT create_statement FROM [SHOW CREATE TABLE t_regional_pk_hashed_2];
 ----
 CREATE TABLE public.t_regional_pk_hashed_2 (
                             pk INT8 NOT NULL,

pkg/cli/clisqlshell/sql_test.go

@@ -57,7 +57,7 @@ func Example_sql() {
 	// growing capacity starting at 1 (the batch sizes will be 1, 2, 4, ...),
 	// and with the query below the division by zero error will occur after the
 	// first batch consisting of 1 row has been returned to the client.
-	c.RunWithArgs([]string{`sql`, `-e`, `select 1/(@1-2) from generate_series(1,3)`})
+	c.RunWithArgs([]string{`sql`, `-e`, `select 1/(i-2) from generate_series(1,3) g(i)`})
 	c.RunWithArgs([]string{`sql`, `-e`, `SELECT '20:01:02+03:04:05'::timetz AS regression_65066`})
 	c.RunWithArgs([]string{`sql`, `-e`, `CREATE USER my_user WITH CREATEDB; GRANT admin TO my_user;`})
 	c.RunWithArgs([]string{`sql`, `-e`, `\du my_user`})
@@ -116,7 +116,7 @@ func Example_sql() {
 	// CREATE TABLE
 	// x
 	// sql -e copy t.f from stdin
-	// sql -e select 1/(@1-2) from generate_series(1,3)
+	// sql -e select 1/(i-2) from generate_series(1,3) g(i)
 	// ?column?
 	// -1.0000000000000000000
 	// (error encountered after some results were delivered)

pkg/cli/interactive_tests/test_notice.tcl

@@ -8,7 +8,7 @@ spawn $argv demo --no-line-editor --no-example-database
 eexpect root@
 
 start_test "Test that notices always appear at the end after all results."
-send "SELECT IF(@1=4,crdb_internal.notice('hello'),@1) AS MYRES FROM generate_series(1,10);\r"
+send "SELECT IF(i=4,crdb_internal.notice('hello'),i) AS MYRES FROM generate_series(1,10) g(i);\r"
 eexpect myres
 eexpect 1
 eexpect 10

pkg/cli/interactive_tests/test_url_db_override.tcl

@@ -50,25 +50,25 @@ end_test
 
 
 start_test "Check that the database flag overrides the db if URL is already set."
-spawn $argv sql --url "postgresql://root@localhost:26257/system?sslmode=disable"  -e "select length(@1) as l, @1 as db from \[show database\]" --format=csv
+spawn $argv sql --url "postgresql://root@localhost:26257/system?sslmode=disable"  -e "select length(database) as l, database as db from \[show database\]" --format=csv
 eexpect "l,db"
 eexpect "6,system"
 eexpect eof
 
-spawn $argv sql --url "postgresql://root@localhost:26257/system?sslmode=disable" --database test -e "select length(@1) as l, @1 as db from \[show database\]" --format=csv
+spawn $argv sql --url "postgresql://root@localhost:26257/system?sslmode=disable" --database test -e "select length(database) as l, database as db from \[show database\]" --format=csv
 eexpect "l,db"
 eexpect "4,test"
 eexpect eof
 end_test
 
 start_test "Check that the database flag does override the database if none was present in the URL."
 # Use empty path.
-spawn $argv sql --url "postgresql://root@localhost:26257?sslmode=disable" --database system -e "select length(@1) as l, @1 as db from \[show database\]" --format=csv
+spawn $argv sql --url "postgresql://root@localhost:26257?sslmode=disable" --database system -e "select length(database) as l, database as db from \[show database\]" --format=csv
 eexpect "l,db"
 eexpect "6,system"
 eexpect eof
 # Use path = /
-spawn $argv sql --url "postgresql://root@localhost:26257/?sslmode=disable" --database system -e "select length(@1) as l, @1 as db from \[show database\]" --format=csv
+spawn $argv sql --url "postgresql://root@localhost:26257/?sslmode=disable" --database system -e "select length(database) as l, database as db from \[show database\]" --format=csv
 eexpect "l,db"
 eexpect "6,system"
 eexpect eof
@@ -84,12 +84,12 @@ end_test
 set ::env(COCKROACH_INSECURE) "false"
 
 start_test "Check that the user flag  override the user if URL is already set."
-spawn $argv sql --url "postgresql://test@localhost:26257?sslmode=disable" -e "select length(@1) as l, @1 as u from \[show session_user\]" --format=csv
+spawn $argv sql --url "postgresql://test@localhost:26257?sslmode=disable" -e "select length(session_user) as l, session_user as u from \[show session_user\]" --format=csv
 eexpect "l,u"
 eexpect "4,test"
 eexpect eof
 
-spawn $argv sql --url "postgresql://root@localhost:26257?sslmode=disable" --user test -e "select length(@1) as l, @1 as u from \[show session_user\]" --format=csv
+spawn $argv sql --url "postgresql://root@localhost:26257?sslmode=disable" --user test -e "select length(session_user) as l, session_user as u from \[show session_user\]" --format=csv
 eexpect "l,u"
 eexpect "4,test"
 eexpect eof

pkg/sql/delegate/show_syntax.go

@@ -27,17 +27,17 @@ import (
 func (d *delegator) delegateShowSyntax(n *tree.ShowSyntax) (tree.Statement, error) {
 	// Construct an equivalent SELECT query that produces the results:
 	//
-	// SELECT @1 AS field, @2 AS message
+	// SELECT f AS field, m AS message
 	//   FROM (VALUES
 	//           ('file',     'foo.go'),
 	//           ('line',     '123'),
 	//           ('function', 'blix()'),
 	//           ('detail',   'some details'),
-	//           ('hint',     'some hints'))
+	//           ('hint',     'some hints')) v(f, m)
 	//
 	var query bytes.Buffer
 	fmt.Fprintf(
-		&query, "SELECT @1 AS %s, @2 AS %s FROM (VALUES ",
+		&query, "SELECT f AS %s, m AS %s FROM (VALUES ",
 		colinfo.ShowSyntaxColumns[0].Name, colinfo.ShowSyntaxColumns[1].Name,
 	)
 
@@ -64,7 +64,7 @@ func (d *delegator) delegateShowSyntax(n *tree.ShowSyntax) (tree.Statement, erro
 		},
 		nil, /* reportErr */
 	)
-	query.WriteByte(')')
+	query.WriteString(") v(f, m)")
 
 	return parse(query.String())
 }

pkg/sql/exec_util.go

@@ -3381,6 +3381,10 @@ func (m *sessionDataMutator) SetExperimentalHashGroupJoinEnabled(val bool) {
 	m.data.ExperimentalHashGroupJoinEnabled = val
 }
 
+func (m *sessionDataMutator) SetAllowOrdinalColumnReference(val bool) {
+	m.data.AllowOrdinalColumnReferences = val
+}
+
 // Utility functions related to scrubbing sensitive information on SQL Stats.
 
 // quantizeCounts ensures that the Count field in the

pkg/sql/logictest/testdata/logic_test/alter_primary_key

@@ -1394,7 +1394,7 @@ statement ok
 ALTER TABLE t ALTER PRIMARY KEY USING COLUMNS (k);
 
 query T
-SELECT @2 FROM [SHOW CREATE TABLE t];
+SELECT create_statement FROM [SHOW CREATE TABLE t];
 ----
 CREATE TABLE public.t (
   a INT8 NOT NULL,
@@ -1430,7 +1430,7 @@ statement ok
 ALTER TABLE t ALTER PRIMARY KEY USING COLUMNS (b, k);
 
 query T
-SELECT @2 FROM [SHOW CREATE TABLE t];
+SELECT create_statement FROM [SHOW CREATE TABLE t];
 ----
 CREATE TABLE public.t (
   a INT8 NOT NULL,
@@ -1474,7 +1474,7 @@ statement ok
 ALTER TABLE t ALTER PRIMARY KEY USING COLUMNS (a);
 
 query T
-SELECT @2 FROM [SHOW CREATE TABLE t];
+SELECT create_statement FROM [SHOW CREATE TABLE t];
 ----
 CREATE TABLE public.t (
   a INT8 NOT NULL,
@@ -1548,7 +1548,7 @@ statement ok
 ALTER TABLE t_test_param ALTER PRIMARY KEY USING COLUMNS (b) USING HASH WITH BUCKET_COUNT = 5;
 
 query T
-SELECT @2 FROM [SHOW CREATE TABLE t_test_param]
+SELECT create_statement FROM [SHOW CREATE TABLE t_test_param]
 ----
 CREATE TABLE public.t_test_param (
   a INT8 NOT NULL,
@@ -1578,7 +1578,7 @@ COMMENT ON CONSTRAINT i2 ON pkey_comment IS 'idx3';
 
 # Comment should exist inside the create statement, so filter it out
 query T
-SELECT substring(@2, strpos(@2, 'COMMENT')) FROM [SHOW CREATE pkey_comment];
+SELECT substring(create_statement, strpos(create_statement, 'COMMENT')) FROM [SHOW CREATE pkey_comment];
 ----
 COMMENT ON INDEX public.pkey_comment@pkey IS 'idx';
 COMMENT ON INDEX public.pkey_comment@i2 IS 'idx2';
@@ -1592,7 +1592,7 @@ ALTER TABLE pkey_comment ALTER PRIMARY KEY USING COLUMNS (b);
 # No comment exists inside the CREATE statement
 skipif config local-legacy-schema-changer
 query T
-SELECT substring(@2, strpos(@2, 'COMMENT')) FROM [SHOW CREATE pkey_comment];
+SELECT substring(create_statement, strpos(create_statement, 'COMMENT')) FROM [SHOW CREATE pkey_comment];
 ----
 COMMENT ON INDEX public.pkey_comment@pkey IS 'idx';
 COMMENT ON INDEX public.pkey_comment@i2 IS 'idx2';

pkg/sql/logictest/testdata/logic_test/array

@@ -333,7 +333,7 @@ a
 
 # From a column ordinal.
 query T
-SELECT @1[1] FROM (SELECT ARRAY['a','b','c'] AS a)
+SELECT a[1] FROM (SELECT ARRAY['a','b','c'] AS a)
 ----
 a
 

pkg/sql/logictest/testdata/logic_test/create_index

@@ -387,7 +387,7 @@ statement ok
 CREATE UNIQUE INDEX idx_t_hash_b ON t_hash (b) USING HASH WITH BUCKET_COUNT = 5;
 
 query T
-SELECT @2 FROM [SHOW CREATE TABLE t_hash]
+SELECT create_statement FROM [SHOW CREATE TABLE t_hash]
 ----
 CREATE TABLE public.t_hash (
   pk INT8 NOT NULL,

pkg/sql/logictest/testdata/logic_test/create_table

@@ -847,7 +847,7 @@ CREATE TABLE t_good_hash_indexes_1 (
 );
 
 query T
-SELECT @2 FROM [SHOW CREATE TABLE t_good_hash_indexes_1];
+SELECT create_statement FROM [SHOW CREATE TABLE t_good_hash_indexes_1];
 ----
 CREATE TABLE public.t_good_hash_indexes_1 (
   crdb_internal_a_shard_5 INT8 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(a)), 5:::INT8)) VIRTUAL,
@@ -866,7 +866,7 @@ CREATE TABLE t_good_hash_indexes_2 (
 );
 
 query T
-SELECT @2 FROM [SHOW CREATE TABLE t_good_hash_indexes_2];
+SELECT create_statement FROM [SHOW CREATE TABLE t_good_hash_indexes_2];
 ----
 CREATE TABLE public.t_good_hash_indexes_2 (
   a INT8 NOT NULL,

pkg/sql/logictest/testdata/logic_test/hash_sharded_index

@@ -1,5 +1,5 @@
 # Tests for creating a hash sharded primary key
-statement ok 
+statement ok
 CREATE TABLE sharded_primary (a INT PRIMARY KEY USING HASH WITH (bucket_count=10))
 
 query TT
@@ -769,7 +769,7 @@ statement ok
 $create_statement
 
 query T
-SELECT @2 FROM [SHOW CREATE TABLE t]
+SELECT create_statement FROM [SHOW CREATE TABLE t]
 ----
 CREATE TABLE public.t (
   crdb_internal_a_shard_8 INT8 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(a)), 8:::INT8)) VIRTUAL,
@@ -792,7 +792,7 @@ CREATE TABLE public.t (
 )
 
 query T
-SELECT @2 FROM [SHOW CREATE TABLE t]
+SELECT create_statement FROM [SHOW CREATE TABLE t]
 ----
 CREATE TABLE public.t (
   crdb_internal_a_shard_8 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(a)), 8:::INT8)) VIRTUAL,
@@ -856,7 +856,7 @@ CREATE TABLE t_default_bucket_16 (
 );
 
 query T
-SELECT @2 FROM [SHOW CREATE TABLE t_default_bucket_16]
+SELECT create_statement FROM [SHOW CREATE TABLE t_default_bucket_16]
 ----
 CREATE TABLE public.t_default_bucket_16 (
   crdb_internal_a_shard_16 INT8 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(a)), 16:::INT8)) VIRTUAL,
@@ -879,7 +879,7 @@ statement ok
 CREATE TABLE t_default_bucket_8 (a INT PRIMARY KEY USING HASH);
 
 query T
-SELECT @2 FROM [SHOW CREATE TABLE t_default_bucket_8]
+SELECT create_statement FROM [SHOW CREATE TABLE t_default_bucket_8]
 ----
 CREATE TABLE public.t_default_bucket_8 (
   crdb_internal_a_shard_8 INT8 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(a)), 8:::INT8)) VIRTUAL,

pkg/sql/logictest/testdata/logic_test/information_schema

@@ -4741,6 +4741,7 @@ WHERE
     );
 ----
 variable                                              value
+allow_ordinal_column_references                       off
 allow_prepare_as_opt_plan                             off
 alter_primary_region_super_region_override            off
 application_name                                      ·