From 61c0d5ef79b7aea7c09104f862ca9b43a562a8be Mon Sep 17 00:00:00 2001 From: Rafi Shamim Date: Wed, 27 May 2026 00:55:23 -0400 Subject: [PATCH] sql/parser: require parens around resource group options The grammar for `with_resource_group_options` previously accepted both `WITH = , ...` and `WITH ( = , ...)`. Because `BUCKET_COUNT` is in the lexer's `WITH_LA` lookahead set, supporting the unparenthesized form required four grammar alternatives (the `WITH` and `WITH_LA` variants of each), and the generated BNF surfaced this as duplicate `'WITH' ...` lines in `stmt_block.bnf`. PR #170894 made the `Format` methods always emit the parenthesized form, so the unparenthesized grammar paths are no longer needed for the parse/format round-trip. Drop them and require parentheses, matching CRDB convention for arbitrary KV-style option lists (`CREATE TABLE` storage params, `BACKUP ... WITH OPTIONS (...)`, etc.). Resolves: #170788 Release note: None --- docs/generated/sql/bnf/stmt_block.bnf | 5 +- .../testdata/logic_test/resource_group | 61 +++++++++---------- pkg/sql/parser/sql.y | 14 +---- pkg/sql/parser/testdata/resource_group | 51 ++++++---------- 4 files changed, 48 insertions(+), 83 deletions(-) diff --git a/docs/generated/sql/bnf/stmt_block.bnf b/docs/generated/sql/bnf/stmt_block.bnf index c51c7c2eca6e..c7d909a75d4a 100644 --- a/docs/generated/sql/bnf/stmt_block.bnf +++ b/docs/generated/sql/bnf/stmt_block.bnf @@ -1946,10 +1946,7 @@ label_spec ::= | 'IF' 'NOT' 'EXISTS' string_or_placeholder with_resource_group_options ::= - 'WITH' resource_group_option_list - | 'WITH' resource_group_option_list - | 'WITH' '(' resource_group_option_list ')' - | 'WITH' '(' resource_group_option_list ')' + 'WITH' '(' resource_group_option_list ')' role_or_group_or_user ::= 'ROLE' diff --git a/pkg/sql/logictest/testdata/logic_test/resource_group b/pkg/sql/logictest/testdata/logic_test/resource_group index 8de1b20cf5f7..bac5d6e69c46 100644 --- a/pkg/sql/logictest/testdata/logic_test/resource_group +++ b/pkg/sql/logictest/testdata/logic_test/resource_group @@ -7,10 +7,10 @@ subtest disabled_by_default statement error pq: resource group SQL is disabled -CREATE RESOURCE GROUP g WITH cpu_weight = 100 +CREATE RESOURCE GROUP g WITH (cpu_weight = 100) statement error pq: resource group SQL is disabled -ALTER RESOURCE GROUP g WITH cpu_weight = 100 +ALTER RESOURCE GROUP g WITH (cpu_weight = 100) statement error pq: resource group SQL is disabled DROP RESOURCE GROUP g @@ -34,28 +34,32 @@ statement error at or near "EOF": syntax error CREATE RESOURCE GROUP missing_weight statement error pq: cpu_weight is required and must be a positive integer -CREATE RESOURCE GROUP only_max_cpu WITH max_cpu = true +CREATE RESOURCE GROUP only_max_cpu WITH (max_cpu = true) statement error pq: cpu_weight must be a positive integer -CREATE RESOURCE GROUP zero_weight WITH cpu_weight = 0 +CREATE RESOURCE GROUP zero_weight WITH (cpu_weight = 0) statement error pq: cpu_weight must be a positive integer -CREATE RESOURCE GROUP negative_weight WITH cpu_weight = -1 +CREATE RESOURCE GROUP negative_weight WITH (cpu_weight = -1) # Unknown options are rejected. statement error pq: unknown resource group option "bogus" -CREATE RESOURCE GROUP unknown_opt WITH cpu_weight = 1, bogus = 1 +CREATE RESOURCE GROUP unknown_opt WITH (cpu_weight = 1, bogus = 1) # Repeating an option is rejected (rather than silently last-writer-wins). statement error pq: cpu_weight specified multiple times -CREATE RESOURCE GROUP dup_weight WITH cpu_weight = 10, cpu_weight = 20 +CREATE RESOURCE GROUP dup_weight WITH (cpu_weight = 10, cpu_weight = 20) statement error pq: max_cpu specified multiple times -CREATE RESOURCE GROUP dup_max_cpu WITH cpu_weight = 1, max_cpu = true, max_cpu = false +CREATE RESOURCE GROUP dup_max_cpu WITH (cpu_weight = 1, max_cpu = true, max_cpu = false) + +# The no-parens form is rejected; parentheses are required around the option list. +statement error at or near "cpu_weight": syntax error +CREATE RESOURCE GROUP no_parens WITH cpu_weight = 100 # IDs < 16 are reserved; the sequence's first allocated id is 17. statement ok -CREATE RESOURCE GROUP high WITH cpu_weight = 100, max_cpu = true +CREATE RESOURCE GROUP high WITH (cpu_weight = 100, max_cpu = true) query ITIBF colnames SHOW RESOURCE GROUP high @@ -63,29 +67,22 @@ SHOW RESOURCE GROUP high id name cpu_weight max_cpu cpu_share_percent 17 high 100 true 100 -# The parens form is also accepted. -statement ok -CREATE RESOURCE GROUP parens WITH (cpu_weight = 50, max_cpu = false) - -statement ok -DROP RESOURCE GROUP parens - # Defaults: max_cpu omitted on CREATE -> false. statement ok -CREATE RESOURCE GROUP medium WITH cpu_weight = 50 +CREATE RESOURCE GROUP medium WITH (cpu_weight = 50) query ITIBF colnames SHOW RESOURCE GROUP medium ---- id name cpu_weight max_cpu cpu_share_percent -19 medium 50 false 33.33333333333333 +18 medium 50 false 33.33333333333333 # Duplicate name is rejected; IF NOT EXISTS swallows the conflict. statement error pq: resource group "high" already exists -CREATE RESOURCE GROUP high WITH cpu_weight = 999 +CREATE RESOURCE GROUP high WITH (cpu_weight = 999) statement ok -CREATE RESOURCE GROUP IF NOT EXISTS high WITH cpu_weight = 999 +CREATE RESOURCE GROUP IF NOT EXISTS high WITH (cpu_weight = 999) # The existing row was not modified. query I @@ -103,7 +100,7 @@ SHOW RESOURCE GROUPS ---- id name cpu_weight max_cpu cpu_share_percent 17 high 100 true 66.66666666666666 -19 medium 50 false 33.33333333333333 +18 medium 50 false 33.33333333333333 # SHOW RESOURCE GROUP errors when the named group does not exist. statement error pq: resource group "ghost" does not exist @@ -115,7 +112,7 @@ subtest alter # ALTER updates only specified options (partial update). statement ok -ALTER RESOURCE GROUP high WITH cpu_weight = 200 +ALTER RESOURCE GROUP high WITH (cpu_weight = 200) query ITIBF colnames SHOW RESOURCE GROUP high @@ -124,7 +121,7 @@ id name cpu_weight max_cpu cpu_share_percent 17 high 200 true 80 statement ok -ALTER RESOURCE GROUP high WITH max_cpu = false +ALTER RESOURCE GROUP high WITH (max_cpu = false) query ITIBF colnames SHOW RESOURCE GROUP high @@ -134,14 +131,14 @@ id name cpu_weight max_cpu cpu_share_percent # ALTER on a missing group: error without IF EXISTS, no-op with. statement error pq: resource group "ghost" does not exist -ALTER RESOURCE GROUP ghost WITH cpu_weight = 1 +ALTER RESOURCE GROUP ghost WITH (cpu_weight = 1) statement ok -ALTER RESOURCE GROUP IF EXISTS ghost WITH cpu_weight = 1 +ALTER RESOURCE GROUP IF EXISTS ghost WITH (cpu_weight = 1) # Repeating an option is rejected on ALTER as well. statement error pq: cpu_weight specified multiple times -ALTER RESOURCE GROUP high WITH cpu_weight = 10, cpu_weight = 20 +ALTER RESOURCE GROUP high WITH (cpu_weight = 10, cpu_weight = 20) subtest end @@ -179,13 +176,13 @@ SELECT cpu_share_percent FROM [SHOW RESOURCE GROUP high] 100 statement ok -ALTER RESOURCE GROUP high WITH cpu_weight = 20 +ALTER RESOURCE GROUP high WITH (cpu_weight = 20) statement ok -CREATE RESOURCE GROUP a WITH cpu_weight = 30 +CREATE RESOURCE GROUP a WITH (cpu_weight = 30) statement ok -CREATE RESOURCE GROUP b WITH cpu_weight = 50 +CREATE RESOURCE GROUP b WITH (cpu_weight = 50) query TF rowsort SELECT name, cpu_share_percent FROM [SHOW RESOURCE GROUPS] @@ -211,7 +208,7 @@ statement ok DROP RESOURCE GROUP a statement ok -ALTER RESOURCE GROUP high WITH cpu_weight = 200 +ALTER RESOURCE GROUP high WITH (cpu_weight = 200) subtest end @@ -220,10 +217,10 @@ subtest non_admin user testuser statement error pq: only users with the admin role may use resource group statements -CREATE RESOURCE GROUP unauth WITH cpu_weight = 1 +CREATE RESOURCE GROUP unauth WITH (cpu_weight = 1) statement error pq: only users with the admin role may use resource group statements -ALTER RESOURCE GROUP high WITH cpu_weight = 1 +ALTER RESOURCE GROUP high WITH (cpu_weight = 1) statement error pq: only users with the admin role may use resource group statements DROP RESOURCE GROUP high diff --git a/pkg/sql/parser/sql.y b/pkg/sql/parser/sql.y index 5b9b3be406b3..ee21af96da09 100644 --- a/pkg/sql/parser/sql.y +++ b/pkg/sql/parser/sql.y @@ -4285,19 +4285,7 @@ drop_external_connection_stmt: | DROP EXTERNAL CONNECTION error // SHOW HELP: DROP EXTERNAL CONNECTION with_resource_group_options: - WITH resource_group_option_list - { - $$.val = $2.kvOptions() - } -| WITH_LA resource_group_option_list - { - $$.val = $2.kvOptions() - } -| WITH '(' resource_group_option_list ')' - { - $$.val = $3.kvOptions() - } -| WITH_LA '(' resource_group_option_list ')' + WITH '(' resource_group_option_list ')' { $$.val = $3.kvOptions() } diff --git a/pkg/sql/parser/testdata/resource_group b/pkg/sql/parser/testdata/resource_group index 880e848153df..f878d02d7a84 100644 --- a/pkg/sql/parser/testdata/resource_group +++ b/pkg/sql/parser/testdata/resource_group @@ -15,15 +15,6 @@ CREATE RESOURCE GROUP my_group WITH (cpu_weight = (100), max_cpu = (false)) -- f CREATE RESOURCE GROUP my_group WITH (cpu_weight = _, max_cpu = _) -- literals removed CREATE RESOURCE GROUP _ WITH (cpu_weight = 100, max_cpu = false) -- identifiers removed -# No-parens form is also accepted; the canonical form adds them. -parse -CREATE RESOURCE GROUP my_group WITH cpu_weight = 100, max_cpu = false ----- -CREATE RESOURCE GROUP my_group WITH (cpu_weight = 100, max_cpu = false) -- normalized! -CREATE RESOURCE GROUP my_group WITH (cpu_weight = (100), max_cpu = (false)) -- fully parenthesized -CREATE RESOURCE GROUP my_group WITH (cpu_weight = _, max_cpu = _) -- literals removed -CREATE RESOURCE GROUP _ WITH (cpu_weight = 100, max_cpu = false) -- identifiers removed - parse CREATE RESOURCE GROUP IF NOT EXISTS my_group WITH (cpu_weight = 1, max_cpu = true) ---- @@ -32,7 +23,6 @@ CREATE RESOURCE GROUP IF NOT EXISTS my_group WITH (cpu_weight = (1), max_cpu = ( CREATE RESOURCE GROUP IF NOT EXISTS my_group WITH (cpu_weight = _, max_cpu = _) -- literals removed CREATE RESOURCE GROUP IF NOT EXISTS _ WITH (cpu_weight = 1, max_cpu = true) -- identifiers removed -# Ensure BUCKET_COUNT (which triggers WITH_LA in the lexer) works correctly. parse CREATE RESOURCE GROUP my_group WITH (bucket_count = 10) ---- @@ -41,14 +31,6 @@ CREATE RESOURCE GROUP my_group WITH (bucket_count = (10)) -- fully parenthesized CREATE RESOURCE GROUP my_group WITH (bucket_count = _) -- literals removed CREATE RESOURCE GROUP _ WITH (bucket_count = 10) -- identifiers removed -parse -CREATE RESOURCE GROUP my_group WITH bucket_count = 10 ----- -CREATE RESOURCE GROUP my_group WITH (bucket_count = 10) -- normalized! -CREATE RESOURCE GROUP my_group WITH (bucket_count = (10)) -- fully parenthesized -CREATE RESOURCE GROUP my_group WITH (bucket_count = _) -- literals removed -CREATE RESOURCE GROUP _ WITH (bucket_count = 10) -- identifiers removed - parse ALTER RESOURCE GROUP my_group WITH (cpu_weight = 200) ---- @@ -57,15 +39,6 @@ ALTER RESOURCE GROUP my_group WITH (cpu_weight = (200)) -- fully parenthesized ALTER RESOURCE GROUP my_group WITH (cpu_weight = _) -- literals removed ALTER RESOURCE GROUP _ WITH (cpu_weight = 200) -- identifiers removed -# No-parens form is also accepted; the canonical form adds them. -parse -ALTER RESOURCE GROUP my_group WITH cpu_weight = 200 ----- -ALTER RESOURCE GROUP my_group WITH (cpu_weight = 200) -- normalized! -ALTER RESOURCE GROUP my_group WITH (cpu_weight = (200)) -- fully parenthesized -ALTER RESOURCE GROUP my_group WITH (cpu_weight = _) -- literals removed -ALTER RESOURCE GROUP _ WITH (cpu_weight = 200) -- identifiers removed - parse ALTER RESOURCE GROUP IF EXISTS my_group WITH (max_cpu = true) ---- @@ -74,14 +47,24 @@ ALTER RESOURCE GROUP IF EXISTS my_group WITH (max_cpu = (true)) -- fully parenth ALTER RESOURCE GROUP IF EXISTS my_group WITH (max_cpu = _) -- literals removed ALTER RESOURCE GROUP IF EXISTS _ WITH (max_cpu = true) -- identifiers removed -# Ensure BUCKET_COUNT (which triggers WITH_LA in the lexer) works correctly. -parse -ALTER RESOURCE GROUP my_group WITH bucket_count = 10 +# The no-parens form is rejected; parentheses are required. +error +CREATE RESOURCE GROUP my_group WITH cpu_weight = 100 +---- +at or near "cpu_weight": syntax error +DETAIL: source SQL: +CREATE RESOURCE GROUP my_group WITH cpu_weight = 100 + ^ +HINT: try \h CREATE RESOURCE GROUP + +error +ALTER RESOURCE GROUP my_group WITH cpu_weight = 200 ---- -ALTER RESOURCE GROUP my_group WITH (bucket_count = 10) -- normalized! -ALTER RESOURCE GROUP my_group WITH (bucket_count = (10)) -- fully parenthesized -ALTER RESOURCE GROUP my_group WITH (bucket_count = _) -- literals removed -ALTER RESOURCE GROUP _ WITH (bucket_count = 10) -- identifiers removed +at or near "cpu_weight": syntax error +DETAIL: source SQL: +ALTER RESOURCE GROUP my_group WITH cpu_weight = 200 + ^ +HINT: try \h ALTER RESOURCE GROUP parse DROP RESOURCE GROUP my_group