Skip to content

Commit 8a6f99d

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 94fb2a5 commit 8a6f99d

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.
@@ -231,6 +231,18 @@ func (a *analyzer) inferParam(number int, t exprType) {
231231
a.params[number] = cur
232232
}
233233

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

299-
if pr, ok := e.Lexpr.(*ast.ParamRef); ok && rightT.typeOID != 0 {
311+
if pr, ok := bareParam(e.Lexpr); ok && rightT.typeOID != 0 {
300312
a.inferParam(pr.Number, rightT)
301313
a.nameParamAfter(pr.Number, e.Rexpr)
302314
leftT = rightT
303315
}
304-
if pr, ok := e.Rexpr.(*ast.ParamRef); ok && leftT.typeOID != 0 {
316+
if pr, ok := bareParam(e.Rexpr); ok && leftT.typeOID != 0 {
305317
a.inferParam(pr.Number, leftT)
306318
a.nameParamAfter(pr.Number, e.Lexpr)
307319
rightT = leftT
@@ -351,10 +363,8 @@ func (a *analyzer) typePredicateList(e *ast.A_Expr) (exprType, error) {
351363
return exprType{}, err
352364
}
353365
if l, ok := e.Rexpr.(*ast.List); ok {
354-
for _, item := range listItems(l) {
355-
if err := a.typeOperands(item, leftT); err != nil {
356-
return exprType{}, err
357-
}
366+
if err := a.typeMembers(e.Lexpr, leftT, listItems(l)); err != nil {
367+
return exprType{}, err
358368
}
359369
return a.boolType(false)
360370
}
@@ -364,21 +374,51 @@ func (a *analyzer) typePredicateList(e *ast.A_Expr) (exprType, error) {
364374
return a.boolType(false)
365375
}
366376

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

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

‎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)