From c72e9bd7533deb1232a84b11815772eca71bf66d Mon Sep 17 00:00:00 2001 From: Radu Berinde Date: Sat, 20 Apr 2019 19:55:14 -0400 Subject: [PATCH] sql: restrict statements that can be used as row sources We currently support all preparable statements as row sources using the `SELECT FROM [ ... ]` syntax. Many of these statements are not really useful here and each one will require work when we deprecate the heuristic planner. This change restricts the set of statements that can be used as row sources to DML statements, SHOW statements, and EXPLAIN. There were no specific tests for this functionality for any of the statements that were removed. If we find that we need any of them, we can add them back on a case-by-case basis. Release note (sql change): Only SELECT, INSERT, UPDATE, UPSERT, DELETE, SHOW, EXPLAIN are supported as data sources using the `SELECT ... FROM [ ... ]` syntax. Informs #34848. --- docs/generated/sql/bnf/stmt_block.bnf | 11 ++++++++++- docs/generated/sql/bnf/table_ref.bnf | 2 +- pkg/sql/explain_test.go | 20 -------------------- pkg/sql/opt/optbuilder/testdata/create_table | 6 ------ pkg/sql/parser/sql.y | 17 ++++++++++++++++- 5 files changed, 27 insertions(+), 29 deletions(-) diff --git a/docs/generated/sql/bnf/stmt_block.bnf b/docs/generated/sql/bnf/stmt_block.bnf index 2cd9a90dfe67..ec7ee3c1264a 100644 --- a/docs/generated/sql/bnf/stmt_block.bnf +++ b/docs/generated/sql/bnf/stmt_block.bnf @@ -1827,7 +1827,7 @@ table_ref ::= | '(' joined_table ')' opt_ordinality alias_clause | func_table opt_ordinality opt_alias_clause | 'LATERAL' func_table opt_ordinality opt_alias_clause - | '[' preparable_stmt ']' opt_ordinality opt_alias_clause + | '[' row_source_extension_stmt ']' opt_ordinality opt_alias_clause all_or_distinct ::= 'ALL' @@ -2055,6 +2055,15 @@ func_table ::= func_expr_windowless | 'ROWS' 'FROM' '(' rowsfrom_list ')' +row_source_extension_stmt ::= + delete_stmt + | explain_stmt + | insert_stmt + | select_stmt + | show_stmt + | update_stmt + | upsert_stmt + opt_column ::= 'COLUMN' | diff --git a/docs/generated/sql/bnf/table_ref.bnf b/docs/generated/sql/bnf/table_ref.bnf index 190348245d0d..dd24ece24f8e 100644 --- a/docs/generated/sql/bnf/table_ref.bnf +++ b/docs/generated/sql/bnf/table_ref.bnf @@ -6,4 +6,4 @@ table_ref ::= | '(' joined_table ')' ( 'WITH' 'ORDINALITY' | ) ( ( 'AS' table_alias_name ( '(' ( ( name ) ( ( ',' name ) )* ) ')' | ) | table_alias_name ( '(' ( ( name ) ( ( ',' name ) )* ) ')' | ) ) | ) | func_application ( 'WITH' 'ORDINALITY' | ) ( ( 'AS' table_alias_name ( '(' ( ( name ) ( ( ',' name ) )* ) ')' | ) | table_alias_name ( '(' ( ( name ) ( ( ',' name ) )* ) ')' | ) ) | ) | 'LATERAL' func_application ( 'WITH' 'ORDINALITY' | ) ( ( 'AS' table_alias_name ( '(' ( ( name ) ( ( ',' name ) )* ) ')' | ) | table_alias_name ( '(' ( ( name ) ( ( ',' name ) )* ) ')' | ) ) | ) - | '[' preparable_stmt ']' ( 'WITH' 'ORDINALITY' | ) ( ( 'AS' table_alias_name ( '(' ( ( name ) ( ( ',' name ) )* ) ')' | ) | table_alias_name ( '(' ( ( name ) ( ( ',' name ) )* ) ')' | ) ) | ) + | '[' row_source_extension_stmt ']' ( 'WITH' 'ORDINALITY' | ) ( ( 'AS' table_alias_name ( '(' ( ( name ) ( ( ',' name ) )* ) ')' | ) | table_alias_name ( '(' ( ( name ) ( ( ',' name ) )* ) ')' | ) ) | ) diff --git a/pkg/sql/explain_test.go b/pkg/sql/explain_test.go index e0d3a01bae09..b125678e212e 100644 --- a/pkg/sql/explain_test.go +++ b/pkg/sql/explain_test.go @@ -177,26 +177,6 @@ func TestStatementReuses(t *testing.T) { }) } }) - t.Run("SELECT * FROM ", func(t *testing.T) { - for _, test := range testData { - t.Run(test, func(t *testing.T) { - rows, err := db.Query("EXPLAIN SELECT * FROM [" + test + "]") - if err != nil { - if testutils.IsError(err, "statement source .* does not return any columns") { - // This error is acceptable and does not constitute a test failure. - return - } - t.Fatal(err) - } - defer rows.Close() - for rows.Next() { - } - if err := rows.Err(); err != nil { - t.Fatal(err) - } - }) - } - }) t.Run("PREPARE EXPLAIN", func(t *testing.T) { for _, test := range testData { t.Run(test, func(t *testing.T) { diff --git a/pkg/sql/opt/optbuilder/testdata/create_table b/pkg/sql/opt/optbuilder/testdata/create_table index 2549a072ca69..bec611280ff8 100644 --- a/pkg/sql/opt/optbuilder/testdata/create_table +++ b/pkg/sql/opt/optbuilder/testdata/create_table @@ -54,12 +54,6 @@ CREATE TABLE ab (a, b) AS SELECT 1, 2, 3 ---- error (42601): CREATE TABLE specifies 2 column names, but data source has 3 columns -# Try to use CREATE TABLE in FROM clause. -build -SELECT * FROM [CREATE TABLE ab (a, b) AS SELECT 1, 2] ----- -error (42703): statement source "CREATE TABLE ab (a, b) AS SELECT 1, 2" does not return any columns - # Non-existent column. build CREATE TABLE ab (a, b) AS SELECT a diff --git a/pkg/sql/parser/sql.y b/pkg/sql/parser/sql.y index 92605d4aaee5..43712a3a445b 100644 --- a/pkg/sql/parser/sql.y +++ b/pkg/sql/parser/sql.y @@ -685,6 +685,7 @@ func newNameFromStr(s string) *tree.Name { %type explain_stmt %type prepare_stmt %type preparable_stmt +%type row_source_extension_stmt %type export_stmt %type execute_stmt %type deallocate_stmt @@ -2503,6 +2504,20 @@ preparable_stmt: | update_stmt // EXTEND WITH HELP: UPDATE | upsert_stmt // EXTEND WITH HELP: UPSERT +// These are statements that can be used as a data source using the special +// syntax with brackets. These are a subset of preparable_stmt. +row_source_extension_stmt: + delete_stmt // EXTEND WITH HELP: DELETE +| explain_stmt // EXTEND WITH HELP: EXPLAIN +| insert_stmt // EXTEND WITH HELP: INSERT +| select_stmt // help texts in sub-rule + { + $$.val = $1.slct() + } +| show_stmt // help texts in sub-rule +| update_stmt // EXTEND WITH HELP: UPDATE +| upsert_stmt // EXTEND WITH HELP: UPSERT + explain_option_list: explain_option_name { @@ -6093,7 +6108,7 @@ table_ref: // will know from the unusual choice that something rather different // is going on and may be pushed by the unusual syntax to // investigate further in the docs. -| '[' preparable_stmt ']' opt_ordinality opt_alias_clause +| '[' row_source_extension_stmt ']' opt_ordinality opt_alias_clause { $$.val = &tree.AliasedTableExpr{Expr: &tree.StatementSource{ Statement: $2.stmt() }, Ordinality: $4.bool(), As: $5.aliasClause() } }