Skip to content

Commit bc05612

Browse files
committed
fix(clickhouse): fix parser position and analyzer for working implementation
Key fixes: - Fix 1-indexed offset from doubleclick parser to 0-indexed for sqlc - Include comment lines in statement extraction for metadata parsing - Calculate StmtLen correctly by finding semicolon positions - Replace ? placeholders with NULL for database introspection - Skip INSERT/UPDATE/DELETE queries when getting column info (no return cols) - Pass experiment flag through process.go and vet.go - Remove trailing semicolons before appending LIMIT 0 for introspection 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 6f636ce commit bc05612

File tree

5 files changed

+115
-47
lines changed

5 files changed

+115
-47
lines changed

internal/cmd/process.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ func processQuerySets(ctx context.Context, rp ResultProcessor, conf *config.Conf
8888

8989
var name, lang string
9090
parseOpts := opts.Parser{
91-
Debug: debug.Debug,
91+
Debug: debug.Debug,
92+
Experiment: o.Env.Experiment,
9293
}
9394

9495
switch {

internal/cmd/vet.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ func Vet(ctx context.Context, dir, filename string, opts *Options) error {
150150
Stderr: stderr,
151151
OnlyManagedDB: e.Debug.OnlyManagedDatabases,
152152
Replacer: shfmt.NewReplacer(nil),
153+
Experiment: e.Experiment,
153154
}
154155
errored := false
155156
for _, sql := range conf.SQL {
@@ -389,6 +390,7 @@ type checker struct {
389390
Client dbmanager.Client
390391
clientOnce sync.Once
391392
Replacer *shfmt.Replacer
393+
Experiment opts.Experiment
392394
}
393395

394396
// isInMemorySQLite checks if a SQLite URI refers to an in-memory database
@@ -481,7 +483,8 @@ func (c *checker) checkSQL(ctx context.Context, s config.SQL) error {
481483

482484
var name string
483485
parseOpts := opts.Parser{
484-
Debug: debug.Debug,
486+
Debug: debug.Debug,
487+
Experiment: c.Experiment,
485488
}
486489

487490
result, failed := parse(ctx, name, c.Dir, s, combo, parseOpts, c.Stderr)
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
-- name: GetAuthor :one
2-
SELECT * FROM authors WHERE id = ?;
2+
SELECT id, name, bio FROM authors WHERE id = ?;
33

44
-- name: ListAuthors :many
5-
SELECT * FROM authors ORDER BY name;
5+
SELECT id, name, bio FROM authors ORDER BY name;
66

77
-- name: CreateAuthor :exec
88
INSERT INTO authors (id, name, bio) VALUES (?, ?, ?);

internal/engine/clickhouse/analyzer/analyze.go

Lines changed: 69 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -50,49 +50,54 @@ func (a *Analyzer) Analyze(ctx context.Context, n ast.Node, query string, migrat
5050

5151
var result core.Analysis
5252

53-
// For ClickHouse, we use EXPLAIN to get column information
54-
// First, try to prepare the query to get parameter count
55-
// ClickHouse uses {name:type} or $1 style placeholders
53+
// Check if this is a SELECT query that returns columns
54+
// INSERT, UPDATE, DELETE don't return columns
55+
isSelectQuery := isSelectStatement(query)
5656

57-
// Replace named parameters with positional ones for EXPLAIN
58-
preparedQuery := query
57+
if isSelectQuery {
58+
// For ClickHouse, we use DESCRIBE or LIMIT 0 to get column information
5959

60-
// Use DESCRIBE (query) to get column information
61-
describeQuery := fmt.Sprintf("DESCRIBE (%s)", preparedQuery)
62-
rows, err := a.conn.QueryContext(ctx, describeQuery)
63-
if err != nil {
64-
// If DESCRIBE fails, try executing with LIMIT 0
65-
limitQuery := addLimit0(preparedQuery)
66-
rows, err = a.conn.QueryContext(ctx, limitQuery)
60+
// Replace ? placeholders with NULL for introspection
61+
// This allows us to run the query to get column types
62+
preparedQuery := strings.ReplaceAll(query, "?", "NULL")
63+
64+
// Use DESCRIBE (query) to get column information
65+
describeQuery := fmt.Sprintf("DESCRIBE (%s)", preparedQuery)
66+
rows, err := a.conn.QueryContext(ctx, describeQuery)
6767
if err != nil {
68-
return nil, a.extractSqlErr(n, err)
68+
// If DESCRIBE fails, try executing with LIMIT 0
69+
limitQuery := addLimit0(preparedQuery)
70+
rows, err = a.conn.QueryContext(ctx, limitQuery)
71+
if err != nil {
72+
return nil, a.extractSqlErr(n, err)
73+
}
6974
}
70-
}
71-
defer rows.Close()
75+
defer rows.Close()
7276

73-
// Get column information from result set
74-
colTypes, err := rows.ColumnTypes()
75-
if err != nil {
76-
return nil, a.extractSqlErr(n, err)
77-
}
77+
// Get column information from result set
78+
colTypes, err := rows.ColumnTypes()
79+
if err != nil {
80+
return nil, a.extractSqlErr(n, err)
81+
}
7882

79-
for i, colType := range colTypes {
80-
name := colType.Name()
81-
dataType := colType.DatabaseTypeName()
82-
nullable, _ := colType.Nullable()
83+
for i, colType := range colTypes {
84+
name := colType.Name()
85+
dataType := colType.DatabaseTypeName()
86+
nullable, _ := colType.Nullable()
8387

84-
col := &core.Column{
85-
Name: name,
86-
OriginalName: name,
87-
DataType: normalizeType(dataType),
88-
NotNull: !nullable,
89-
}
88+
col := &core.Column{
89+
Name: name,
90+
OriginalName: name,
91+
DataType: normalizeType(dataType),
92+
NotNull: !nullable,
93+
}
9094

91-
// Try to detect if this is an aggregate function result
92-
// (ClickHouse doesn't always provide this info)
93-
_ = i
95+
// Try to detect if this is an aggregate function result
96+
// (ClickHouse doesn't always provide this info)
97+
_ = i
9498

95-
result.Columns = append(result.Columns, col)
99+
result.Columns = append(result.Columns, col)
100+
}
96101
}
97102

98103
// Detect parameters in the query
@@ -204,8 +209,11 @@ func (a *Analyzer) GetColumnNames(ctx context.Context, query string) ([]string,
204209
return nil, fmt.Errorf("database connection not initialized")
205210
}
206211

212+
// Replace ? placeholders with NULL for introspection
213+
preparedQuery := strings.ReplaceAll(query, "?", "NULL")
214+
207215
// Add LIMIT 0 to avoid fetching data
208-
limitQuery := addLimit0(query)
216+
limitQuery := addLimit0(preparedQuery)
209217

210218
rows, err := a.conn.QueryContext(ctx, limitQuery)
211219
if err != nil {
@@ -372,7 +380,32 @@ func addLimit0(query string) string {
372380
if strings.Contains(lower, "limit") {
373381
return query
374382
}
375-
return query + " LIMIT 0"
383+
384+
// Remove trailing semicolon and whitespace
385+
trimmed := strings.TrimRight(query, " \t\n\r;")
386+
387+
return trimmed + " LIMIT 0"
388+
}
389+
390+
// isSelectStatement checks if a query is a SELECT statement that returns columns.
391+
// It skips past comment lines to find the actual statement.
392+
func isSelectStatement(query string) bool {
393+
lines := strings.Split(query, "\n")
394+
for _, line := range lines {
395+
trimmed := strings.TrimSpace(line)
396+
// Skip empty lines
397+
if trimmed == "" {
398+
continue
399+
}
400+
// Skip comment lines
401+
if strings.HasPrefix(trimmed, "--") || strings.HasPrefix(trimmed, "#") {
402+
continue
403+
}
404+
// Check if this is a SELECT or WITH statement
405+
lower := strings.ToLower(trimmed)
406+
return strings.HasPrefix(lower, "select") || strings.HasPrefix(lower, "with")
407+
}
408+
return false
376409
}
377410

378411
// isNullable checks if a ClickHouse type is nullable.

internal/engine/clickhouse/parse.go

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"io"
7+
"strings"
78

89
"github.com/sqlc-dev/doubleclick/ast"
910
"github.com/sqlc-dev/doubleclick/parser"
@@ -22,31 +23,54 @@ func NewParser() *Parser {
2223

2324
// Parse parses ClickHouse SQL statements and converts them to sqlc's AST.
2425
func (p *Parser) Parse(r io.Reader) ([]sqlcast.Statement, error) {
26+
// Read the full source for position calculations
27+
srcBytes, err := io.ReadAll(r)
28+
if err != nil {
29+
return nil, err
30+
}
31+
src := string(srcBytes)
32+
2533
ctx := context.Background()
26-
stmts, err := parser.Parse(ctx, r)
34+
stmts, err := parser.Parse(ctx, strings.NewReader(src))
2735
if err != nil {
2836
return nil, err
2937
}
3038

3139
var result []sqlcast.Statement
40+
loc := 0 // Track from start of file or after previous statement
41+
3242
for _, stmt := range stmts {
3343
converted := p.convert(stmt)
3444
if converted == nil {
3545
continue
3646
}
3747
pos := stmt.Pos()
48+
// doubleclick uses 1-indexed offsets, convert to 0-indexed
49+
stmtStart := pos.Offset
50+
if stmtStart > 0 {
51+
stmtStart = stmtStart - 1
52+
}
53+
54+
// Find the semicolon that ends this statement
55+
semiPos := strings.Index(src[stmtStart:], ";")
56+
stmtEnd := stmtStart
57+
if semiPos >= 0 {
58+
stmtEnd = stmtStart + semiPos + 1 // Include the semicolon
59+
}
60+
61+
// The statement length includes everything from loc to the semicolon
62+
stmtLen := stmtEnd - loc
63+
3864
result = append(result, sqlcast.Statement{
3965
Raw: &sqlcast.RawStmt{
4066
Stmt: converted,
41-
StmtLocation: pos.Offset,
42-
StmtLen: 0,
67+
StmtLocation: loc,
68+
StmtLen: stmtLen,
4369
},
4470
})
45-
}
4671

47-
// Calculate statement lengths
48-
for i := 0; i < len(result)-1; i++ {
49-
result[i].Raw.StmtLen = result[i+1].Raw.StmtLocation - result[i].Raw.StmtLocation
72+
// Move loc past the semicolon for the next statement
73+
loc = stmtEnd
5074
}
5175

5276
return result, nil
@@ -337,6 +361,13 @@ func (p *Parser) convertExpr(expr ast.Expression) sqlcast.Node {
337361
Fields: &sqlcast.List{Items: []sqlcast.Node{&sqlcast.A_Star{}}},
338362
}
339363

364+
case *ast.Parameter:
365+
// ClickHouse uses ? for positional parameters
366+
return &sqlcast.ParamRef{
367+
Number: 0, // Will be assigned during query parsing
368+
Location: e.Position.Offset,
369+
}
370+
340371
default:
341372
return &sqlcast.A_Const{Val: &sqlcast.String{Str: ""}}
342373
}

0 commit comments

Comments
 (0)