Skip to content

Commit 6c75a73

Browse files
committed
core: report the placeholders the analyzer used to walk past
Under SQLCEXPERIMENT=coreanalyzer several statement shapes lost their parameters, so the generated function took fewer arguments than the query has placeholders: - INSERT ... SELECT: the engines report the source query with an empty VALUES list rather than none, so the analyzer took the VALUES branch and walked nothing. The query is analyzed now, and a placeholder selected directly stands in for the column it lands in. - UPDATE ... LIMIT and DELETE ... LIMIT, and LIMIT/OFFSET on a UNION, INTERSECT or EXCEPT, were never looked at. - ON DUPLICATE KEY UPDATE assignments were never looked at; they bind the way SET does. - x IN (SELECT ...) on MySQL arrives wrapped in a sublink, which the IN node did not look through. - x COLLATE c: SQLite puts the expression in the node's other field and the collation's name where PostgreSQL puts the expression, so the placeholder underneath was neither found nor typed. - CALL: the compiler only sent SELECT, INSERT, UPDATE and DELETE through the core, and the schema package skipped CREATE PROCEDURE. A procedure is recorded now, with a void pseudo type as its result, and a CALL types each placeholder from the procedure's declared parameter, by position or by name, and names it after the parameter. The AST gains IsProcedure, which both parsers set, since a function with only OUT parameters also declares no return type. A placeholder on the left of an IN list takes its type from the members, the way the other operand of a comparison would. In the core replay context 12 more cases pass, with no case regressing. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XZcKb4GFiZQbeo9oVmyF3m
1 parent 2cd271a commit 6c75a73

13 files changed

Lines changed: 304 additions & 38 deletions

File tree

internal/compiler/parse_core.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func (c *Compiler) parseQueryCore(raw *ast.RawStmt, src string, pre *preprocess.
5454
var cols []*Column
5555
var params []Parameter
5656
switch raw.Stmt.(type) {
57-
case *ast.SelectStmt, *ast.InsertStmt, *ast.UpdateStmt, *ast.DeleteStmt:
57+
case *ast.SelectStmt, *ast.InsertStmt, *ast.UpdateStmt, *ast.DeleteStmt, *ast.CallStmt:
5858
res, err := coreanalyzer.Prepare(c.coreCatalog, raw)
5959
if err != nil {
6060
return nil, err

internal/core/analysis.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const (
77
CommandInsert Command = "INSERT"
88
CommandUpdate Command = "UPDATE"
99
CommandDelete Command = "DELETE"
10+
CommandCall Command = "CALL"
1011
)
1112

1213
type PrepareResult struct {

internal/core/analyzer/analyzer.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ func Prepare(cat *core.Catalog, stmt ast.Node) (core.PrepareResult, error) {
3737
return core.PrepareResult{}, err
3838
}
3939
a.command = core.CommandDelete
40+
case *ast.CallStmt:
41+
if err := a.analyzeCall(s); err != nil {
42+
return core.PrepareResult{}, err
43+
}
44+
a.command = core.CommandCall
4045
default:
4146
return core.PrepareResult{}, fmt.Errorf("analyzer: unsupported statement %T", stmt)
4247
}
@@ -174,7 +179,7 @@ func (a *analyzer) analyzeSelect(s *ast.SelectStmt) error {
174179
if err := a.analyzeSetOperation(s); err != nil {
175180
return err
176181
}
177-
return nil
182+
return a.typeLimits(s)
178183
}
179184

180185
sc, err := a.buildScope(s.FromClause)
@@ -234,6 +239,11 @@ func (a *analyzer) analyzeSelect(s *ast.SelectStmt) error {
234239
}
235240
}
236241
}
242+
return a.typeLimits(s)
243+
}
244+
245+
// typeLimits types a SELECT's LIMIT and OFFSET.
246+
func (a *analyzer) typeLimits(s *ast.SelectStmt) error {
237247
for _, n := range []ast.Node{s.LimitCount, s.LimitOffset} {
238248
if err := a.typeLimit(n); err != nil {
239249
return fmt.Errorf("limit: %w", err)

internal/core/analyzer/dml.go

Lines changed: 108 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,95 @@ func (a *analyzer) analyzeInsert(s *ast.InsertStmt) error {
2424
if err := a.bindInsertValues(s.SelectStmt, rel, targets); err != nil {
2525
return err
2626
}
27+
if s.OnDuplicateKeyUpdate != nil {
28+
if err := a.bindAssignments(rel, s.OnDuplicateKeyUpdate.TargetList); err != nil {
29+
return fmt.Errorf("on duplicate key update: %w", err)
30+
}
31+
}
2732
return a.projectReturning(s.ReturningList)
2833
}
2934

35+
// bindAssignments types the values a SET-style list assigns to the
36+
// relation's columns.
37+
func (a *analyzer) bindAssignments(rel scopeRel, targets *ast.List) error {
38+
for _, item := range listItems(targets) {
39+
rt, ok := item.(*ast.ResTarget)
40+
if !ok || rt.Name == nil {
41+
continue
42+
}
43+
col, ok := findColumn(rel, *rt.Name)
44+
if !ok {
45+
return fmt.Errorf("unknown column %q", *rt.Name)
46+
}
47+
if err := a.bindValue(rel, &col, rt.Val); err != nil {
48+
return fmt.Errorf("set %s: %w", *rt.Name, err)
49+
}
50+
}
51+
return nil
52+
}
53+
54+
// analyzeCall types the arguments of a CALL from the procedure's declared
55+
// parameters: a placeholder takes the parameter's type and name, whether
56+
// it is passed by position or by name.
57+
func (a *analyzer) analyzeCall(s *ast.CallStmt) error {
58+
if s.FuncCall == nil {
59+
return fmt.Errorf("call: missing procedure")
60+
}
61+
name := funcCallName(s.FuncCall)
62+
overloads, err := a.cat.FindProcs(name, nil)
63+
if err != nil {
64+
return err
65+
}
66+
args := listItems(s.FuncCall.Args)
67+
var procs []core.ProcOverload
68+
for _, p := range overloads {
69+
if p.Kind == "p" {
70+
procs = append(procs, p)
71+
}
72+
}
73+
if len(procs) == 0 {
74+
return fmt.Errorf("unknown procedure %q", name)
75+
}
76+
proc := procs[0]
77+
for _, p := range procs {
78+
if len(p.ArgTypes) == len(args) {
79+
proc = p
80+
break
81+
}
82+
}
83+
params, err := a.cat.ProcArgs(proc.OID)
84+
if err != nil {
85+
return err
86+
}
87+
for i, arg := range args {
88+
var param *core.ProcArg
89+
value := arg
90+
if named, ok := arg.(*ast.NamedArgExpr); ok {
91+
value = named.Arg
92+
if named.Name != nil {
93+
for j := range params {
94+
if params[j].Name == *named.Name {
95+
param = &params[j]
96+
break
97+
}
98+
}
99+
}
100+
} else if i < len(params) {
101+
param = &params[i]
102+
}
103+
pr, ok := value.(*ast.ParamRef)
104+
if !ok || param == nil {
105+
if _, err := a.typeExpr(value); err != nil {
106+
return err
107+
}
108+
continue
109+
}
110+
a.inferParam(pr.Number, exprType{typeOID: param.TypeOID})
111+
a.nameParam(pr.Number, param.Name)
112+
}
113+
return nil
114+
}
115+
30116
func (a *analyzer) analyzeUpdate(s *ast.UpdateStmt) error {
31117
if err := a.bindCTEs(s.WithClause); err != nil {
32118
return err
@@ -57,6 +143,9 @@ func (a *analyzer) analyzeUpdate(s *ast.UpdateStmt) error {
57143
return fmt.Errorf("where: %w", err)
58144
}
59145
}
146+
if err := a.typeLimit(s.LimitCount); err != nil {
147+
return fmt.Errorf("limit: %w", err)
148+
}
60149
return a.projectReturning(s.ReturningList)
61150
}
62151

@@ -75,6 +164,9 @@ func (a *analyzer) analyzeDelete(s *ast.DeleteStmt) error {
75164
return fmt.Errorf("where: %w", err)
76165
}
77166
}
167+
if err := a.typeLimit(s.LimitCount); err != nil {
168+
return fmt.Errorf("limit: %w", err)
169+
}
78170
return a.projectReturning(s.ReturningList)
79171
}
80172

@@ -140,10 +232,22 @@ func (a *analyzer) bindInsertValues(n ast.Node, rel scopeRel, targets []core.Cla
140232
return fmt.Errorf("insert: unsupported source %T", n)
141233
}
142234
// INSERT ... SELECT inserts whatever the query returns. The rows are not
143-
// the statement's result, but the query still holds placeholders.
144-
if sel.ValuesLists == nil {
145-
_, err := a.subqueryColumns(sel)
146-
return err
235+
// the statement's result, but the query still holds placeholders, and a
236+
// placeholder selected directly stands in for the column it lands in.
237+
if len(listItems(sel.ValuesLists)) == 0 {
238+
if _, err := a.subqueryColumns(sel); err != nil {
239+
return err
240+
}
241+
for i, item := range listItems(sel.TargetList) {
242+
rt, ok := item.(*ast.ResTarget)
243+
if !ok || i >= len(targets) {
244+
continue
245+
}
246+
if pr, ok := rt.Val.(*ast.ParamRef); ok {
247+
a.inferParam(pr.Number, columnType(rel, targets[i]))
248+
}
249+
}
250+
return nil
147251
}
148252
for _, row := range listItems(sel.ValuesLists) {
149253
values, ok := row.(*ast.List)

internal/core/analyzer/expr.go

Lines changed: 74 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func (a *analyzer) typeExpr(n ast.Node) (exprType, error) {
7878
return a.typeSubLink(e)
7979

8080
case *ast.CollateExpr:
81-
return a.typeExpr(e.Arg)
81+
return a.typeExpr(collated(e))
8282

8383
case *ast.IntervalExpr:
8484
// A dialect with no interval type of its own leaves it untyped.
@@ -232,6 +232,18 @@ func (a *analyzer) inferParam(number int, t exprType) {
232232
a.params[number] = cur
233233
}
234234

235+
// nameParam gives a placeholder a name when nothing has named it yet.
236+
func (a *analyzer) nameParam(number int, name string) {
237+
if name == "" {
238+
return
239+
}
240+
cur := a.params[number]
241+
if cur.Name == "" && cur.Source == nil {
242+
cur.Name = name
243+
a.params[number] = cur
244+
}
245+
}
246+
235247
// nameParamAfter names a placeholder compared with a function call after
236248
// the function, the way a placeholder compared with a column is named after
237249
// the column.
@@ -297,12 +309,12 @@ func (a *analyzer) typeAExpr(e *ast.A_Expr) (exprType, error) {
297309
return exprType{}, err
298310
}
299311

300-
if pr, ok := e.Lexpr.(*ast.ParamRef); ok && rightT.typeOID != 0 {
312+
if pr, ok := bareParam(e.Lexpr); ok && rightT.typeOID != 0 {
301313
a.inferParam(pr.Number, rightT)
302314
a.nameParamAfter(pr.Number, e.Rexpr)
303315
leftT = rightT
304316
}
305-
if pr, ok := e.Rexpr.(*ast.ParamRef); ok && leftT.typeOID != 0 {
317+
if pr, ok := bareParam(e.Rexpr); ok && leftT.typeOID != 0 {
306318
a.inferParam(pr.Number, leftT)
307319
a.nameParamAfter(pr.Number, e.Lexpr)
308320
rightT = leftT
@@ -352,10 +364,8 @@ func (a *analyzer) typePredicateList(e *ast.A_Expr) (exprType, error) {
352364
return exprType{}, err
353365
}
354366
if l, ok := e.Rexpr.(*ast.List); ok {
355-
for _, item := range listItems(l) {
356-
if err := a.typeOperands(item, leftT); err != nil {
357-
return exprType{}, err
358-
}
367+
if err := a.typeMembers(e.Lexpr, leftT, listItems(l)); err != nil {
368+
return exprType{}, err
359369
}
360370
return a.boolType(false)
361371
}
@@ -365,21 +375,51 @@ func (a *analyzer) typePredicateList(e *ast.A_Expr) (exprType, error) {
365375
return a.boolType(false)
366376
}
367377

378+
// typeMembers types the members of an IN list against the expression on
379+
// the left. When that expression is a bare placeholder, the members type
380+
// it instead, the way the other operand of a comparison would.
381+
func (a *analyzer) typeMembers(left ast.Node, leftT exprType, members []ast.Node) error {
382+
for _, item := range members {
383+
if err := a.typeOperands(item, leftT); err != nil {
384+
return err
385+
}
386+
}
387+
pr, ok := bareParam(left)
388+
if !ok || leftT.typeOID != 0 {
389+
return nil
390+
}
391+
for _, item := range members {
392+
t, err := a.typeExpr(item)
393+
if err != nil {
394+
return err
395+
}
396+
if t.typeOID != 0 {
397+
a.inferParam(pr.Number, t)
398+
a.nameParamAfter(pr.Number, item)
399+
return nil
400+
}
401+
}
402+
return nil
403+
}
404+
368405
// typeIn types the IN node the engines that have one report, where the values
369406
// compared against are held apart from the expression.
370407
func (a *analyzer) typeIn(e *ast.In) (exprType, error) {
371408
leftT, err := a.typeExpr(e.Expr)
372409
if err != nil {
373410
return exprType{}, err
374411
}
375-
for _, item := range e.List {
376-
if err := a.typeOperands(item, leftT); err != nil {
377-
return exprType{}, err
378-
}
412+
if err := a.typeMembers(e.Expr, leftT, e.List); err != nil {
413+
return exprType{}, err
379414
}
380415
// "x IN (SELECT ...)" compares x against the subquery's column, and the
381-
// subquery's own placeholders are reported with the rest.
382-
if sel, ok := e.Sel.(*ast.SelectStmt); ok {
416+
// subquery's own placeholders are reported with the rest. An engine may
417+
// report the subquery wrapped in a sublink.
418+
subselect := e.Sel
419+
if sl, ok := subselect.(*ast.SubLink); ok {
420+
subselect = sl.Subselect
421+
}
422+
if sel, ok := subselect.(*ast.SelectStmt); ok {
383423
cols, err := a.subqueryColumns(sel)
384424
if err != nil {
385425
return exprType{}, err
@@ -583,10 +623,30 @@ func (a *analyzer) typeNullIf(e *ast.A_Expr) (exprType, error) {
583623
return leftT, nil
584624
}
585625

626+
// collated is the expression a COLLATE clause applies to. Engines disagree
627+
// on which field holds it: PostgreSQL puts the expression in Arg, SQLite
628+
// puts it in Xpr and the collation's name in Arg.
629+
func collated(e *ast.CollateExpr) ast.Node {
630+
if _, isName := e.Arg.(*ast.String); e.Arg == nil || isName {
631+
return e.Xpr
632+
}
633+
return e.Arg
634+
}
635+
636+
// bareParam reports whether n is a placeholder, looking through a COLLATE
637+
// clause, which changes how a value compares and not what it is.
638+
func bareParam(n ast.Node) (*ast.ParamRef, bool) {
639+
if c, ok := n.(*ast.CollateExpr); ok {
640+
n = collated(c)
641+
}
642+
pr, ok := n.(*ast.ParamRef)
643+
return pr, ok
644+
}
645+
586646
// typeOperands types a node standing opposite one of known type, giving a bare
587647
// placeholder that type.
588648
func (a *analyzer) typeOperands(n ast.Node, other exprType) error {
589-
if pr, ok := n.(*ast.ParamRef); ok {
649+
if pr, ok := bareParam(n); ok {
590650
if other.typeOID != 0 || other.typeName != "" {
591651
a.inferParam(pr.Number, other)
592652
}

internal/core/catalogdb/query.sql.go

Lines changed: 41 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/core/catalogdef/query.sql

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,11 @@ INSERT INTO sql_proc
201201
return_type_oid, return_set, return_nullable, strict, variadic_kind)
202202
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?);
203203

204+
-- name: ProcArgs :many
205+
SELECT name, type_oid, mode, has_default FROM sql_proc_arg
206+
WHERE proc_oid = ?
207+
ORDER BY ord;
208+
204209
-- name: CreateProcArg :exec
205210
INSERT INTO sql_proc_arg (proc_oid, ord, name, type_oid, mode, has_default)
206211
VALUES (?, ?, ?, ?, ?, ?);

0 commit comments

Comments
 (0)