sqlparser: replace nested tokenizer with inline versioned comment scanning (#19725)

Signed-off-by: Arthur Schreiber <arthur@planetscale.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: arthurschreiber

go/vt/sqlparser/AGENTS.md

@@ -0,0 +1,9 @@
+# SQL Parser
+
+## Tokenizer
+
+The tokenizer (`token.go`) processes SQL input character-by-character. When
+modifying scan methods, watch out for **recursion and stack overflow**. Scan
+helper methods must not call back into `Scan()`. Crafted input with many
+consecutive constructs can cause unbounded stack growth. Instead, have helpers
+advance scanner state and return to `Scan`'s main `for` loop via `continue`.

go/vt/sqlparser/ast_funcs_test.go

@@ -254,3 +254,100 @@ func TestKeyspaceToNonQualifiedTable(t *testing.T) {
 	AddKeyspace(stmt, "ks2")
 	require.Equal(t, "select col, col + (select 1 from ks2.t4) from ks.t join ks2.t2 join (select 1 from ks2.t3) as x where t.id = t2.id and x.id = t.id", String(stmt))
 }
+
+// TestVersionedCommentParsing tests MySQL versioned comment handling.
+// All expectations validated against MySQL 8.4.
+func TestVersionedCommentParsing(t *testing.T) {
+	parser, err := New(Options{MySQLServerVersion: "8.1.20"})
+	require.NoError(t, err)
+
+	tests := []struct {
+		name     string
+		query    string
+		expected string // expected formatted output after parse
+		isError  bool
+		errMsg   string // if isError, the expected error message
+	}{
+		{
+			name:     "expanded (version matches)",
+			query:    "SELECT /*!80100 JSON_VALUE(col, '$.x') AS jval, */ id FROM t",
+			expected: "select json_value(col, '$.x') as jval, id from t",
+		},
+		{
+			name:     "discarded (version too new)",
+			query:    "SELECT /*!90000 UPPER('hidden'), */ 42",
+			expected: "select 42 from dual",
+		},
+		{
+			name:     "no version number (always expanded)",
+			query:    "SELECT /*! 1 + 1 */ FROM dual",
+			expected: "select 1 + 1 from dual",
+		},
+		{
+			name:     "nested regular comment inside version comment",
+			query:    "SELECT /*!80100 1 /* a comment */ + 2 */",
+			expected: "select 1 + 2 from dual",
+		},
+		{
+			name:     "nested version comment inside version comment treated as regular comment",
+			query:    "SELECT /*!80100 1 /*!99999 noise */ + 2 */",
+			expected: "select 1 + 2 from dual",
+		},
+		{
+			name:    "unclosed version comment",
+			query:   "SELECT /*!80100 1 + 2",
+			isError: true,
+			errMsg:  "syntax error at position 22",
+		},
+		{
+			name:     "version number without whitespace after digits",
+			query:    "SELECT 1 + /*!801002*/",
+			expected: "select 1 + 2 from dual",
+		},
+		{
+			name:     "empty version comment",
+			query:    "SELECT 1 /*!80100 */",
+			expected: "select 1 from dual",
+		},
+		{
+			name:     "fewer than 5 version digits are treated as content",
+			query:    "SELECT /*!8010*/ FROM dual",
+			expected: "select 8010 from dual",
+		},
+		{
+			name:     "non-digits after /*! are treated as content",
+			query:    "SELECT /*! 1 + 2*/ FROM dual",
+			expected: "select 1 + 2 from dual",
+		},
+		{
+			name:     "no space before closing */",
+			query:    "SELECT /*!80100 42*/ FROM dual",
+			expected: "select 42 from dual",
+		},
+		{
+			name:     "nested comment inside skipped version comment",
+			query:    "SELECT /*!90000 1 /* nested */ + 2 */ 42",
+			expected: "select 42 from dual",
+		},
+		{
+			name:     "nested version comment inside skipped version comment",
+			query:    "SELECT /*!90000 1 /*!99999 nested */ + 2 */ 42",
+			expected: "select 42 from dual",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			stmt, err := parser.Parse(tt.query)
+			if tt.isError {
+				require.Error(t, err, "expected parse error for: %s", tt.query)
+				if tt.errMsg != "" {
+					assert.EqualError(t, err, tt.errMsg)
+				}
+				return
+			}
+			require.NoError(t, err, "unexpected error for: %s", tt.query)
+			assert.Equal(t, tt.expected, String(stmt))
+		})
+	}
+}

go/vt/sqlparser/comments.go

@@ -199,28 +199,6 @@ func hasCommentPrefix(sql string) bool {
 	return len(sql) > 1 && ((sql[0] == '/' && sql[1] == '*') || (sql[0] == '-' && sql[1] == '-'))
 }
 
-// ExtractMysqlComment extracts the version and SQL from a comment-only query
-// such as /*!50708 sql here */
-func ExtractMysqlComment(sql string) (string, string) {
-	sql = sql[3 : len(sql)-2]
-
-	digitCount := 0
-	endOfVersionIndex := strings.IndexFunc(sql, func(c rune) bool {
-		digitCount++
-		return !unicode.IsDigit(c) || digitCount == 6
-	})
-	if endOfVersionIndex < 0 {
-		return "", ""
-	}
-	if endOfVersionIndex < 5 {
-		endOfVersionIndex = 0
-	}
-	version := sql[0:endOfVersionIndex]
-	innerSQL := strings.TrimFunc(sql[endOfVersionIndex:], unicode.IsSpace)
-
-	return version, innerSQL
-}
-
 const commentDirectivePreamble = "/*vt+"
 
 // CommentDirectives is the parsed representation for execution directives

go/vt/sqlparser/comments_test.go

@@ -222,36 +222,6 @@ a`,
 	}
 }
 
-func TestExtractMysqlComment(t *testing.T) {
-	testCases := []struct {
-		input, outSQL, outVersion string
-	}{{
-		input:      "/*!50708SET max_execution_time=5000 */",
-		outSQL:     "SET max_execution_time=5000",
-		outVersion: "50708",
-	}, {
-		input:      "/*!50708 SET max_execution_time=5000*/",
-		outSQL:     "SET max_execution_time=5000",
-		outVersion: "50708",
-	}, {
-		input:      "/*!50708* from*/",
-		outSQL:     "* from",
-		outVersion: "50708",
-	}, {
-		input:      "/*! SET max_execution_time=5000*/",
-		outSQL:     "SET max_execution_time=5000",
-		outVersion: "",
-	}}
-	for _, testCase := range testCases {
-		gotVersion, gotSQL := ExtractMysqlComment(testCase.input)
-		assert.Equal(t, testCase.outVersion, gotVersion, "version mismatch")
-
-		if gotSQL != testCase.outSQL {
-			t.Errorf("test input: '%s', got SQL\n%+v, want\n%+v", testCase.input, gotSQL, testCase.outSQL)
-		}
-	}
-}
-
 func TestExtractCommentDirectives(t *testing.T) {
 	testCases := []struct {
 		input string

go/vt/sqlparser/testdata/select_cases.txt

@@ -5972,7 +5972,7 @@ INPUT
 select 1 + /*!00000 2 node_modules/ + 3 /*!99999 noise*/ + 4;
 END
 ERROR
-syntax error at position 57
+syntax error at position 36
 END
 INPUT
 select 1, st_touches(t.geom, p.geom) from tbl_polygon t, tbl_polygon p where t.id = 'POLY1' and p.id = 'POLY2';
@@ -20251,8 +20251,8 @@ END
 INPUT
 select 1/*!2*/;
 END
-OUTPUT
-select 1 from dual
+ERROR
+syntax error at position 13 near '2'
 END
 INPUT
 select a, t1.* as 'with_alias', b from t1;
@@ -22238,7 +22238,7 @@ INPUT
 select 1/*!000002*/;
 END
 ERROR
-syntax error at position 20 near '2'
+syntax error at position 18 near '2'
 END
 INPUT
 select 1, min(a) from t1i where a=99;