Skip to content

Commit 18a6990

Browse files
kyleconroyclaude
andcommitted
Handle USE DATABASE syntax and improve clientError detection
- Skip optional DATABASE keyword in USE statements (USE DATABASE d1 == USE d1) - Only skip DATABASE if followed by an identifier (not semicolon/EOF) - Track clientError annotations in splitStatements for proper handling - Handle clientError for statements with no explain file (runtime errors) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 317d037 commit 18a6990

File tree

5 files changed

+69
-32
lines changed

5 files changed

+69
-32
lines changed

parser/parser.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6270,6 +6270,13 @@ func (p *Parser) parseUse() *ast.UseQuery {
62706270

62716271
p.nextToken() // skip USE
62726272

6273+
// Skip optional DATABASE keyword (USE DATABASE dbname is equivalent to USE dbname)
6274+
// But only if DATABASE is followed by another identifier/keyword (not semicolon or EOF)
6275+
// e.g., "USE DATABASE d1" vs "USE database" where database is the db name
6276+
if p.currentIs(token.DATABASE) && !p.peekIs(token.SEMICOLON) && !p.peekIs(token.EOF) {
6277+
p.nextToken()
6278+
}
6279+
62736280
// Database name can be an identifier or a keyword like DEFAULT (can also start with number)
62746281
use.Database = p.parseIdentifierName()
62756282

parser/parser_test.go

Lines changed: 59 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,17 @@ type testMetadata struct {
2727
ParseError bool `json:"parse_error,omitempty"` // true if query is intentionally invalid SQL
2828
}
2929

30+
// statementInfo holds a parsed statement and its metadata
31+
type statementInfo struct {
32+
stmt string
33+
hasClientErr bool
34+
}
35+
3036
// splitStatements splits SQL content into individual statements.
31-
func splitStatements(content string) []string {
32-
var statements []string
37+
func splitStatements(content string) []statementInfo {
38+
var statements []statementInfo
3339
var current strings.Builder
40+
var currentHasClientErr bool
3441

3542
lines := strings.Split(content, "\n")
3643
for _, line := range lines {
@@ -41,6 +48,12 @@ func splitStatements(content string) []string {
4148
continue
4249
}
4350

51+
// Check for clientError annotation before stripping comment
52+
// Handles both "-- { clientError" and "--{clientError" formats
53+
if strings.Contains(trimmed, "clientError") {
54+
currentHasClientErr = true
55+
}
56+
4457
// Remove inline comments (-- comment at end of line)
4558
if idx := findCommentStart(trimmed); idx >= 0 {
4659
trimmed = strings.TrimSpace(trimmed[:idx])
@@ -60,17 +73,18 @@ func splitStatements(content string) []string {
6073
stmt := strings.TrimSpace(current.String())
6174
// Skip empty statements (just semicolons or empty)
6275
if stmt != "" && stmt != ";" {
63-
statements = append(statements, stmt)
76+
statements = append(statements, statementInfo{stmt: stmt, hasClientErr: currentHasClientErr})
6477
}
6578
current.Reset()
79+
currentHasClientErr = false
6680
}
6781
}
6882

6983
// Handle statement without trailing semicolon
7084
if current.Len() > 0 {
7185
stmt := strings.TrimSpace(current.String())
7286
if stmt != "" {
73-
statements = append(statements, stmt)
87+
statements = append(statements, statementInfo{stmt: stmt, hasClientErr: currentHasClientErr})
7488
}
7589
}
7690

@@ -170,9 +184,11 @@ func TestParser(t *testing.T) {
170184
}
171185

172186
// Test each statement as a subtest
173-
for i, stmt := range statements {
187+
for i, stmtInfo := range statements {
174188
stmtIndex := i + 1
175189
t.Run(fmt.Sprintf("stmt%d", stmtIndex), func(t *testing.T) {
190+
stmt := stmtInfo.stmt
191+
176192
// Determine explain file path: explain.txt for first, explain_N.txt for N >= 2
177193
var explainPath string
178194
if stmtIndex == 1 {
@@ -181,15 +197,6 @@ func TestParser(t *testing.T) {
181197
explainPath = filepath.Join(testDir, fmt.Sprintf("explain_%d.txt", stmtIndex))
182198
}
183199

184-
// For statements beyond the first, skip if no explain file exists
185-
// (these statements haven't been regenerated yet)
186-
if stmtIndex > 1 {
187-
if _, err := os.Stat(explainPath); os.IsNotExist(err) {
188-
t.Skipf("No explain_%d.txt file (run regenerate-explain to generate)", stmtIndex)
189-
return
190-
}
191-
}
192-
193200
// Skip statements marked in explain_todo (unless -check-explain is set)
194201
stmtKey := fmt.Sprintf("stmt%d", stmtIndex)
195202
isExplainTodo := metadata.ExplainTodo[stmtKey]
@@ -198,6 +205,41 @@ func TestParser(t *testing.T) {
198205
return
199206
}
200207

208+
// For statements beyond the first, check if explain file exists
209+
explainFileExists := true
210+
if stmtIndex > 1 {
211+
if _, err := os.Stat(explainPath); os.IsNotExist(err) {
212+
explainFileExists = false
213+
}
214+
}
215+
216+
// If no explain file and statement has clientError annotation, skip (no expected output for runtime errors)
217+
if !explainFileExists && stmtInfo.hasClientErr {
218+
// Remove from explain_todo if present
219+
if isExplainTodo && *checkExplain {
220+
delete(metadata.ExplainTodo, stmtKey)
221+
if len(metadata.ExplainTodo) == 0 {
222+
metadata.ExplainTodo = nil
223+
}
224+
updatedBytes, err := json.MarshalIndent(metadata, "", " ")
225+
if err != nil {
226+
t.Errorf("Failed to marshal updated metadata: %v", err)
227+
} else if err := os.WriteFile(metadataPath, append(updatedBytes, '\n'), 0644); err != nil {
228+
t.Errorf("Failed to write updated metadata.json: %v", err)
229+
} else {
230+
t.Logf("EXPLAIN PASSES NOW (clientError skip, no explain file) - removed explain_todo[%s] from: %s", stmtKey, entry.Name())
231+
}
232+
}
233+
t.Skipf("No explain_%d.txt file (clientError annotation - runtime error)", stmtIndex)
234+
return
235+
}
236+
237+
// For statements beyond the first without clientError, skip if no explain file exists
238+
if !explainFileExists {
239+
t.Skipf("No explain_%d.txt file (run regenerate-explain to generate)", stmtIndex)
240+
return
241+
}
242+
201243
// Create context with 1 second timeout
202244
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
203245
defer cancel()
@@ -242,9 +284,9 @@ func TestParser(t *testing.T) {
242284
if strings.HasSuffix(expected, "\nOK") {
243285
expected = strings.TrimSpace(expected[:len(expected)-3])
244286
}
245-
// Skip if expected is empty and statement has --{clientError annotation
287+
// Skip if expected is empty and statement has clientError annotation
246288
// (ClickHouse errors at runtime before producing EXPLAIN output)
247-
if expected == "" && strings.Contains(stmt, "--{clientError") {
289+
if expected == "" && stmtInfo.hasClientErr {
248290
// Also remove from explain_todo if present (this case is now handled)
249291
if isExplainTodo && *checkExplain {
250292
delete(metadata.ExplainTodo, stmtKey)
@@ -260,7 +302,7 @@ func TestParser(t *testing.T) {
260302
t.Logf("EXPLAIN PASSES NOW (clientError skip) - removed explain_todo[%s] from: %s", stmtKey, entry.Name())
261303
}
262304
}
263-
t.Skipf("Skipping: empty expected output with --{clientError annotation")
305+
t.Skipf("Skipping: empty expected output with clientError annotation")
264306
return
265307
}
266308
actual := strings.TrimSpace(parser.Explain(stmts[0]))
Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1 @@
1-
{
2-
"explain_todo": {
3-
"stmt239": true
4-
}
5-
}
1+
{}
Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1 @@
1-
{
2-
"explain_todo": {
3-
"stmt3": true
4-
}
5-
}
1+
{}
Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1 @@
1-
{
2-
"explain_todo": {
3-
"stmt5": true
4-
}
5-
}
1+
{}

0 commit comments

Comments
 (0)