Merge #147386

147386: sql/parser: remove no longer used keywords r=yuzefovich a=yuzefovich

**plpgsql/parser: rename bazel target name for the package**

For some reason, Go library bazel target for the PLpgSQL parser package
was named `plpgparser`. This creates a problem with an attempt to add
a test that lives in `parser` (as opposed to `parser_test`), so rename
the target to `parser` which follows the convention elsewhere. (I
eventually decided against adding a test but thought it'd be nice to
clean things up.)

**sql/parser: remove no longer used keywords**

This commit removes a handful of unreserved keywords that are no longer
used. Additionally, it refactors existing `reserved_keywords.awk` into
a shell script that is run via a unit test to assert that no longer used
keywords get removed promptly. (I tried applying the same script to
PLpgSQL and JSONPath parsers, and it found nothing for the latter and
produced many false positives for the former, so I decided to not
include the corresponding test.)

Epic: None
Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>

Author: craig[bot]

docs/generated/sql/bnf/stmt_block.bnf

@@ -1110,7 +1110,6 @@ unreserved_keyword ::=
 	| 'BIDIRECTIONAL'
 	| 'BINARY'
 	| 'BUCKET_COUNT'
-	| 'BUNDLE'
 	| 'BY'
 	| 'BYPASSRLS'
 	| 'CACHE'
@@ -1300,7 +1299,6 @@ unreserved_keyword ::=
 	| 'MINUTE'
 	| 'MINVALUE'
 	| 'MODIFYCLUSTERSETTING'
-	| 'MODIFYSQLCLUSTERSETTING'
 	| 'MULTILINESTRING'
 	| 'MULTILINESTRINGM'
 	| 'MULTILINESTRINGZ'
@@ -1431,7 +1429,6 @@ unreserved_keyword ::=
 	| 'RESTRICTIVE'
 	| 'RESUME'
 	| 'RETENTION'
-	| 'RETRY'
 	| 'RETURN'
 	| 'RETURNS'
 	| 'REVISION_HISTORY'
@@ -1559,9 +1556,7 @@ unreserved_keyword ::=
 	| 'VIEW'
 	| 'VIEWACTIVITY'
 	| 'VIEWACTIVITYREDACTED'
-	| 'VIEWCLUSTERMETADATA'
 	| 'VIEWCLUSTERSETTING'
-	| 'VIEWDEBUG'
 	| 'VIRTUAL_CLUSTER_NAME'
 	| 'VIRTUAL_CLUSTER'
 	| 'VISIBLE'
@@ -3960,7 +3955,6 @@ bare_label_keywords ::=
 	| 'BOTH'
 	| 'BOX2D'
 	| 'BUCKET_COUNT'
-	| 'BUNDLE'
 	| 'BY'
 	| 'BYPASSRLS'
 	| 'CACHE'
@@ -4211,7 +4205,6 @@ bare_label_keywords ::=
 	| 'MINVALUE'
 	| 'MODE'
 	| 'MODIFYCLUSTERSETTING'
-	| 'MODIFYSQLCLUSTERSETTING'
 	| 'MOVE'
 	| 'MULTILINESTRING'
 	| 'MULTILINESTRINGM'
@@ -4356,7 +4349,6 @@ bare_label_keywords ::=
 	| 'RESTRICTIVE'
 	| 'RESUME'
 	| 'RETENTION'
-	| 'RETRY'
 	| 'RETURN'
 	| 'RETURNS'
 	| 'REVISION_HISTORY'
@@ -4511,9 +4503,7 @@ bare_label_keywords ::=
 	| 'VIEW'
 	| 'VIEWACTIVITY'
 	| 'VIEWACTIVITYREDACTED'
-	| 'VIEWCLUSTERMETADATA'
 	| 'VIEWCLUSTERSETTING'
-	| 'VIEWDEBUG'
 	| 'VIRTUAL'
 	| 'VIRTUAL_CLUSTER_NAME'
 	| 'VIRTUAL_CLUSTER'

pkg/BUILD.bazel

@@ -2176,8 +2176,8 @@ GO_TARGETS = [
     "//pkg/sql/physicalplan:physicalplan",
     "//pkg/sql/physicalplan:physicalplan_test",
     "//pkg/sql/plpgsql/parser/lexbase:lexbase",
+    "//pkg/sql/plpgsql/parser:parser",
     "//pkg/sql/plpgsql/parser:parser_test",
-    "//pkg/sql/plpgsql/parser:plpgparser",
     "//pkg/sql/prep:prep",
     "//pkg/sql/prep:prep_test",
     "//pkg/sql/privilege:privilege",

pkg/sql/BUILD.bazel

@@ -468,7 +468,7 @@ go_library(
         "//pkg/sql/pgwire/pgwirecancel",
         "//pkg/sql/physicalplan",
         "//pkg/sql/physicalplan/replicaoracle",
-        "//pkg/sql/plpgsql/parser:plpgparser",
+        "//pkg/sql/plpgsql/parser",
         "//pkg/sql/prep",
         "//pkg/sql/privilege",
         "//pkg/sql/protoreflect",

pkg/sql/catalog/redact/BUILD.bazel

@@ -9,7 +9,7 @@ go_library(
         "//pkg/sql/catalog/catpb",
         "//pkg/sql/catalog/descpb",
         "//pkg/sql/parser",
-        "//pkg/sql/plpgsql/parser:plpgparser",
+        "//pkg/sql/plpgsql/parser",
         "//pkg/sql/schemachanger/scpb",
         "//pkg/sql/sem/tree",
         "@com_github_cockroachdb_errors//:errors",

pkg/sql/catalog/rewrite/BUILD.bazel

@@ -19,7 +19,7 @@ go_library(
         "//pkg/sql/parser",
         "//pkg/sql/pgwire/pgcode",
         "//pkg/sql/pgwire/pgerror",
-        "//pkg/sql/plpgsql/parser:plpgparser",
+        "//pkg/sql/plpgsql/parser",
         "//pkg/sql/schemachanger/scpb",
         "//pkg/sql/schemachanger/screl",
         "//pkg/sql/sem/catid",

pkg/sql/catalog/tabledesc/BUILD.bazel

@@ -45,7 +45,7 @@ go_library(
         "//pkg/sql/pgwire/pgcode",
         "//pkg/sql/pgwire/pgerror",
         "//pkg/sql/pgwire/pgnotice",
-        "//pkg/sql/plpgsql/parser:plpgparser",
+        "//pkg/sql/plpgsql/parser",
         "//pkg/sql/privilege",
         "//pkg/sql/rowenc",
         "//pkg/sql/schemachanger/scpb",

pkg/sql/opt/optbuilder/BUILD.bazel

@@ -81,7 +81,7 @@ go_library(
         "//pkg/sql/parser/statements",
         "//pkg/sql/pgwire/pgcode",
         "//pkg/sql/pgwire/pgerror",
-        "//pkg/sql/plpgsql/parser:plpgparser",
+        "//pkg/sql/plpgsql/parser",
         "//pkg/sql/privilege",
         "//pkg/sql/sem/asof",
         "//pkg/sql/sem/builtins/builtinsregistry",

pkg/sql/parser/BUILD.bazel

@@ -55,11 +55,16 @@ go_test(
         "parse_internal_test.go",
         "parse_test.go",
         "scanner_test.go",
+        "unused_keywords_test.go",
         ":gen-helpmap-test",  # keep
     ],
-    data = glob(["testdata/**"]),
+    data = glob(["testdata/**"]) + [
+        "sql.y",
+        "unused_keywords.sh",
+    ],
     embed = [":parser"],
     deps = [
+        "//pkg/build/bazel",
         "//pkg/sql/lexbase",
         "//pkg/sql/pgwire/pgerror",
         "//pkg/sql/randgen",

pkg/sql/parser/sql.y

@@ -975,7 +975,7 @@ func (u *sqlSymUnion) doBlockOption() tree.DoBlockOption {
 
 %token <str> BACKUP BACKUPS BACKWARD BATCH BEFORE BEGIN BETWEEN BIDIRECTIONAL BIGINT BIGSERIAL BINARY BIT
 %token <str> BUCKET_COUNT
-%token <str> BOOLEAN BOTH BOX2D BUNDLE BY BYPASSRLS
+%token <str> BOOLEAN BOTH BOX2D BY BYPASSRLS
 
 %token <str> CACHE CALL CALLED CANCEL CANCELQUERY CAPABILITIES CAPABILITY CASCADE CASE CAST CBRT CHANGEFEED CHAR
 %token <str> CHARACTER CHARACTERISTICS CHECK CHECK_FILES CLOSE
@@ -1027,7 +1027,7 @@ func (u *sqlSymUnion) doBlockOption() tree.DoBlockOption {
 %token <str> LINESTRING LINESTRINGM LINESTRINGZ LINESTRINGZM
 %token <str> LIST LOCAL LOCALITY LOCALTIME LOCALTIMESTAMP LOCKED LOGGED LOGICAL LOGICALLY LOGIN LOOKUP LOW LSHIFT
 
-%token <str> MATCH MATERIALIZED MERGE MINVALUE MAXVALUE METHOD MINUTE MODIFYCLUSTERSETTING MODIFYSQLCLUSTERSETTING MODE MONTH MOVE
+%token <str> MATCH MATERIALIZED MERGE MINVALUE MAXVALUE METHOD MINUTE MODIFYCLUSTERSETTING MODE MONTH MOVE
 %token <str> MULTILINESTRING MULTILINESTRINGM MULTILINESTRINGZ MULTILINESTRINGZM
 %token <str> MULTIPOINT MULTIPOINTM MULTIPOINTZ MULTIPOINTZM
 %token <str> MULTIPOLYGON MULTIPOLYGONM MULTIPOLYGONZ MULTIPOLYGONZM
@@ -1052,7 +1052,7 @@ func (u *sqlSymUnion) doBlockOption() tree.DoBlockOption {
 %token <str> RANGE RANGES READ REAL REASON REASSIGN RECURSIVE RECURRING REDACT REF REFERENCES REFERENCING REFRESH
 %token <str> REGCLASS REGION REGIONAL REGIONS REGNAMESPACE REGPROC REGPROCEDURE REGROLE REGTYPE REINDEX
 %token <str> RELATIVE RELOCATE REMOVE_PATH REMOVE_REGIONS RENAME REPEATABLE REPLACE REPLICATED REPLICATION
-%token <str> RELEASE RESET RESTART RESTORE RESTRICT RESTRICTED RESTRICTIVE RESUME RETENTION RETURNING RETURN RETURNS RETRY REVISION_HISTORY
+%token <str> RELEASE RESET RESTART RESTORE RESTRICT RESTRICTED RESTRICTIVE RESUME RETENTION RETURNING RETURN RETURNS REVISION_HISTORY
 %token <str> REVOKE RIGHT ROLE ROLES ROLLBACK ROLLUP ROUTINES ROW ROWS RSHIFT RULE RUNNING
 
 %token <str> SAVEPOINT SCANS SCATTER SCHEDULE SCHEDULES SCROLL SCHEMA SCHEMA_ONLY SCHEMAS SCRUB
@@ -1073,8 +1073,8 @@ func (u *sqlSymUnion) doBlockOption() tree.DoBlockOption {
 %token <str> UNBOUNDED UNCOMMITTED UNIDIRECTIONAL UNION UNIQUE UNKNOWN UNLISTEN UNLOGGED UNSAFE_RESTORE_INCOMPATIBLE_VERSION UNSPLIT
 %token <str> UPDATE UPDATES_CLUSTER_MONITORING_METRICS UPSERT UNSET UNTIL USE USER USERS USING UUID
 
-%token <str> VALID VALIDATE VALUE VALUES VARBIT VARCHAR VARIADIC VECTOR VERIFY_BACKUP_TABLE_DATA VIEW VARIABLES VARYING VIEWACTIVITY VIEWACTIVITYREDACTED VIEWDEBUG
-%token <str> VIEWCLUSTERMETADATA VIEWCLUSTERSETTING VIRTUAL VISIBLE INVISIBLE VISIBILITY VOLATILE VOTERS
+%token <str> VALID VALIDATE VALUE VALUES VARBIT VARCHAR VARIADIC VECTOR VERIFY_BACKUP_TABLE_DATA VIEW VARIABLES VARYING VIEWACTIVITY VIEWACTIVITYREDACTED
+%token <str> VIEWCLUSTERSETTING VIRTUAL VISIBLE INVISIBLE VISIBILITY VOLATILE VOTERS
 %token <str> VIRTUAL_CLUSTER_NAME VIRTUAL_CLUSTER
 
 %token <str> WHEN WHERE WINDOW WITH WITHIN WITHOUT WORK WRITE
@@ -18137,7 +18137,6 @@ unreserved_keyword:
 | BIDIRECTIONAL
 | BINARY
 | BUCKET_COUNT
-| BUNDLE
 | BY
 | BYPASSRLS
 | CACHE
@@ -18327,7 +18326,6 @@ unreserved_keyword:
 | MINUTE
 | MINVALUE
 | MODIFYCLUSTERSETTING
-| MODIFYSQLCLUSTERSETTING
 | MULTILINESTRING
 | MULTILINESTRINGM
 | MULTILINESTRINGZ
@@ -18458,7 +18456,6 @@ unreserved_keyword:
 | RESTRICTIVE
 | RESUME
 | RETENTION
-| RETRY
 | RETURN
 | RETURNS
 | REVISION_HISTORY
@@ -18586,9 +18583,7 @@ unreserved_keyword:
 | VIEW
 | VIEWACTIVITY
 | VIEWACTIVITYREDACTED
-| VIEWCLUSTERMETADATA
 | VIEWCLUSTERSETTING
-| VIEWDEBUG
 | VIRTUAL_CLUSTER_NAME
 | VIRTUAL_CLUSTER
 | VISIBLE
@@ -18648,7 +18643,6 @@ bare_label_keywords:
 | BOTH
 | BOX2D
 | BUCKET_COUNT
-| BUNDLE
 | BY
 | BYPASSRLS
 | CACHE
@@ -18899,7 +18893,6 @@ bare_label_keywords:
 | MINVALUE
 | MODE
 | MODIFYCLUSTERSETTING
-| MODIFYSQLCLUSTERSETTING
 | MOVE
 | MULTILINESTRING
 | MULTILINESTRINGM
@@ -19044,7 +19037,6 @@ bare_label_keywords:
 | RESTRICTIVE
 | RESUME
 | RETENTION
-| RETRY
 | RETURN
 | RETURNS
 | REVISION_HISTORY
@@ -19199,9 +19191,7 @@ bare_label_keywords:
 | VIEW
 | VIEWACTIVITY
 | VIEWACTIVITYREDACTED
-| VIEWCLUSTERMETADATA
 | VIEWCLUSTERSETTING
-| VIEWDEBUG
 | VIRTUAL
 | VIRTUAL_CLUSTER_NAME
 | VIRTUAL_CLUSTER

pkg/sql/parser/unreserved_keywords.awk

@@ -0,0 +1,53 @@
+#!/usr/bin/env bash
+
+# Copyright 2025 The Cockroach Authors.
+#
+# Use of this software is governed by the CockroachDB Software License
+# included in the /LICENSE file.
+
+# Usage: unused_keywords.sh pkg/*.y
+
+set -euo pipefail
+
+GRAMMAR_FILE="$1"
+
+if [ -z "$1" ]; then
+  echo "Usage: $0 <input_grammar_file>"
+  exit 1
+fi
+
+# Find the line in the grammar file where different keyword groups begin - we
+# don't want to count occurrences from that point.
+line=$(grep -n -m 1 "^unreserved_keyword:$" "$GRAMMAR_FILE" | cut -d: -f1)
+
+# Find all non-reserved keywords.
+awk -f <(cat <<'EOF'
+/^(cockroachdb_extra_)?reserved_keyword:/ {
+  keyword = 0
+  next
+}
+/^.*_keyword:/ {
+  keyword = 1
+  next
+}
+/^$/ {
+  keyword = 0
+}
+{
+  if (keyword && $NF != "") {
+    print $NF
+  }
+}
+EOF
+) < "$GRAMMAR_FILE" | while read -r kw; do
+  # Count number of times the non-reserved keyword appears before the keyword
+  # groups - if it's present only once, then it's not actually used and can be
+  # removed from the grammar.
+  #
+  # Note that this could produce false positives in case these keywords are used
+  # somewhere outside the grammar (for example, in the lexer).
+  count=$(head -n "$line" "$GRAMMAR_FILE" | grep -c "$kw")
+  if [ "$count" -le 1 ]; then
+    echo "$kw"
+  fi
+done

pkg/sql/parser/unused_keywords.sh

@@ -0,0 +1,32 @@
+// Copyright 2025 The Cockroach Authors.
+//
+// Use of this software is governed by the CockroachDB Software License
+// included in the /LICENSE file.
+
+package parser_test
+
+import (
+	"os/exec"
+	"testing"
+
+	"github.com/cockroachdb/cockroach/pkg/build/bazel"
+)
+
+func TestUnusedKeywords(t *testing.T) {
+	grammarPath := "./sql.y"
+	if bazel.BuiltWithBazel() {
+		var err error
+		grammarPath, err = bazel.Runfile("sql.y")
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+	cmd := exec.Command("./unused_keywords.sh", grammarPath)
+	output, err := cmd.CombinedOutput()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(output) != 0 {
+		t.Fatalf("found some unused keywords:\n%s", output)
+	}
+}

pkg/sql/parser/unused_keywords_test.go

@@ -25,7 +25,7 @@ genrule(
 )
 
 go_library(
-    name = "plpgparser",
+    name = "parser",
     srcs = [
         "lexer.go",
         "parse.go",
@@ -62,7 +62,7 @@ go_test(
     srcs = ["parser_test.go"],
     data = glob(["testdata/**"]),
     deps = [
-        ":plpgparser",
+        ":parser",
         "//pkg/sql/sem/plpgsqltree/utils",
         "//pkg/testutils/datapathutils",
         "//pkg/testutils/sqlutils",

pkg/sql/plpgsql/parser/BUILD.bazel

@@ -35,7 +35,7 @@ go_library(
         "//pkg/sql/parser",
         "//pkg/sql/pgwire/pgcode",
         "//pkg/sql/pgwire/pgerror",
-        "//pkg/sql/plpgsql/parser:plpgparser",
+        "//pkg/sql/plpgsql/parser",
         "//pkg/sql/privilege",
         "//pkg/sql/schemachanger/scbuild/internal/scbuildstmt",
         "//pkg/sql/schemachanger/scdecomp",

pkg/sql/schemachanger/scbuild/BUILD.bazel

@@ -6,7 +6,7 @@ go_library(
     importpath = "github.com/cockroachdb/cockroach/pkg/sql/sem/plpgsqltree/utils",
     visibility = ["//visibility:public"],
     deps = [
-        "//pkg/sql/plpgsql/parser:plpgparser",
+        "//pkg/sql/plpgsql/parser",
         "//pkg/sql/sem/plpgsqltree",
         "//pkg/sql/sqltelemetry",
     ],

pkg/sql/sem/plpgsqltree/utils/BUILD.bazel

@@ -23,7 +23,7 @@ go_library(
         "//pkg/sql/parser",
         "//pkg/sql/parser/statements",
         "//pkg/sql/pgwire/pgerror",
-        "//pkg/sql/plpgsql/parser:plpgparser",
+        "//pkg/sql/plpgsql/parser",
         "//pkg/sql/protoreflect",
         "//pkg/sql/sem/catconstants",
         "//pkg/sql/sem/tree",