From a4860eeb2f9a282ac071293081f0884630a86d9a Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Sat, 9 Oct 2021 13:18:39 -0500 Subject: [PATCH] fix(compiler): Detect invalid INSERT expression Return an error if you're trying to insert too many columns for the given table. Before the same query would have paniced. --- internal/compiler/find_params.go | 29 +++++++++++++++---- internal/compiler/parse.go | 5 +++- .../insert_select_invalid/mysql/query.sql | 5 ++++ .../insert_select_invalid/mysql/sqlc.json | 12 ++++++++ .../insert_select_invalid/mysql/stderr.txt | 2 ++ .../postgresql/pgx/query.sql | 5 ++++ .../postgresql/pgx/sqlc.json | 13 +++++++++ .../postgresql/pgx/stderr.txt | 2 ++ .../postgresql/stdlib/query.sql | 5 ++++ .../postgresql/stdlib/sqlc.json | 12 ++++++++ .../postgresql/stdlib/stderr.txt | 2 ++ 11 files changed, 86 insertions(+), 6 deletions(-) create mode 100644 internal/endtoend/testdata/insert_select_invalid/mysql/query.sql create mode 100644 internal/endtoend/testdata/insert_select_invalid/mysql/sqlc.json create mode 100644 internal/endtoend/testdata/insert_select_invalid/mysql/stderr.txt create mode 100644 internal/endtoend/testdata/insert_select_invalid/postgresql/pgx/query.sql create mode 100644 internal/endtoend/testdata/insert_select_invalid/postgresql/pgx/sqlc.json create mode 100644 internal/endtoend/testdata/insert_select_invalid/postgresql/pgx/stderr.txt create mode 100644 internal/endtoend/testdata/insert_select_invalid/postgresql/stdlib/query.sql create mode 100644 internal/endtoend/testdata/insert_select_invalid/postgresql/stdlib/sqlc.json create mode 100644 internal/endtoend/testdata/insert_select_invalid/postgresql/stdlib/stderr.txt diff --git a/internal/compiler/find_params.go b/internal/compiler/find_params.go index f4d32b8ce5..7a0130d47e 100644 --- a/internal/compiler/find_params.go +++ b/internal/compiler/find_params.go @@ -1,15 +1,23 @@ package compiler import ( + "fmt" + "github.com/kyleconroy/sqlc/internal/sql/ast" "github.com/kyleconroy/sqlc/internal/sql/astutils" ) -func findParameters(root ast.Node) []paramRef { +func findParameters(root ast.Node) ([]paramRef, error) { refs := make([]paramRef, 0) - v := paramSearch{seen: make(map[int]struct{}), refs: &refs} + errors := make([]error, 0) + v := paramSearch{seen: make(map[int]struct{}), refs: &refs, errs: &errors} astutils.Walk(v, root) - return refs + if len(*v.errs) > 0 { + problems := *v.errs + return nil, problems[0] + } else { + return refs, nil + } } type paramRef struct { @@ -24,6 +32,7 @@ type paramSearch struct { rangeVar *ast.RangeVar refs *[]paramRef seen map[int]struct{} + errs *[]error // XXX: Gross state hack for limit limitCount ast.Node @@ -45,6 +54,10 @@ func (l *limitOffset) Pos() int { } func (p paramSearch) Visit(node ast.Node) astutils.Visitor { + if len(*p.errs) > 0 { + return p + } + switch n := node.(type) { case *ast.A_Expr: @@ -64,7 +77,10 @@ func (p paramSearch) Visit(node ast.Node) astutils.Visitor { if !ok { continue } - // TODO: Out-of-bounds panic + if len(n.Cols.Items) <= i { + *p.errs = append(*p.errs, fmt.Errorf("INSERT has more expressions than target columns")) + return p + } *p.refs = append(*p.refs, paramRef{parent: n.Cols.Items[i], ref: ref, rv: n.Relation}) p.seen[ref.Location] = struct{}{} } @@ -78,7 +94,10 @@ func (p paramSearch) Visit(node ast.Node) astutils.Visitor { if !ok { continue } - // TODO: Out-of-bounds panic + if len(n.Cols.Items) <= i { + *p.errs = append(*p.errs, fmt.Errorf("INSERT has more expressions than target columns")) + return p + } *p.refs = append(*p.refs, paramRef{parent: n.Cols.Items[i], ref: ref, rv: n.Relation}) p.seen[ref.Location] = struct{}{} } diff --git a/internal/compiler/parse.go b/internal/compiler/parse.go index 2c312f0837..de47b9cd68 100644 --- a/internal/compiler/parse.go +++ b/internal/compiler/parse.go @@ -79,7 +79,10 @@ func (c *Compiler) parseQuery(stmt ast.Node, src string, o opts.Parser) (*Query, raw, namedParams, edits := rewrite.NamedParameters(c.conf.Engine, raw, numbers, dollar) rvs := rangeVars(raw.Stmt) - refs := findParameters(raw.Stmt) + refs, err := findParameters(raw.Stmt) + if err != nil { + return nil, err + } if o.UsePositionalParameters { edits, err = rewriteNumberedParameters(refs, raw, rawSQL) if err != nil { diff --git a/internal/endtoend/testdata/insert_select_invalid/mysql/query.sql b/internal/endtoend/testdata/insert_select_invalid/mysql/query.sql new file mode 100644 index 0000000000..cfd90fe55d --- /dev/null +++ b/internal/endtoend/testdata/insert_select_invalid/mysql/query.sql @@ -0,0 +1,5 @@ +CREATE TABLE foo (bar text); + +-- name: InsertFoo :exec +INSERT INTO foo (bar) +SELECT 1, ?, ?; diff --git a/internal/endtoend/testdata/insert_select_invalid/mysql/sqlc.json b/internal/endtoend/testdata/insert_select_invalid/mysql/sqlc.json new file mode 100644 index 0000000000..0657f4db83 --- /dev/null +++ b/internal/endtoend/testdata/insert_select_invalid/mysql/sqlc.json @@ -0,0 +1,12 @@ +{ + "version": "1", + "packages": [ + { + "engine": "mysql", + "path": "go", + "name": "querytest", + "schema": "query.sql", + "queries": "query.sql" + } + ] +} diff --git a/internal/endtoend/testdata/insert_select_invalid/mysql/stderr.txt b/internal/endtoend/testdata/insert_select_invalid/mysql/stderr.txt new file mode 100644 index 0000000000..063b2a149a --- /dev/null +++ b/internal/endtoend/testdata/insert_select_invalid/mysql/stderr.txt @@ -0,0 +1,2 @@ +# package querytest +query.sql:4:1: INSERT has more expressions than target columns diff --git a/internal/endtoend/testdata/insert_select_invalid/postgresql/pgx/query.sql b/internal/endtoend/testdata/insert_select_invalid/postgresql/pgx/query.sql new file mode 100644 index 0000000000..41b0289d23 --- /dev/null +++ b/internal/endtoend/testdata/insert_select_invalid/postgresql/pgx/query.sql @@ -0,0 +1,5 @@ +CREATE TABLE foo (bar text); + +-- name: InsertFoo :exec +INSERT INTO foo (bar) +SELECT 1, $1, $2; diff --git a/internal/endtoend/testdata/insert_select_invalid/postgresql/pgx/sqlc.json b/internal/endtoend/testdata/insert_select_invalid/postgresql/pgx/sqlc.json new file mode 100644 index 0000000000..9403bd0279 --- /dev/null +++ b/internal/endtoend/testdata/insert_select_invalid/postgresql/pgx/sqlc.json @@ -0,0 +1,13 @@ +{ + "version": "1", + "packages": [ + { + "path": "go", + "engine": "postgresql", + "sql_package": "pgx/v4", + "name": "querytest", + "schema": "query.sql", + "queries": "query.sql" + } + ] +} diff --git a/internal/endtoend/testdata/insert_select_invalid/postgresql/pgx/stderr.txt b/internal/endtoend/testdata/insert_select_invalid/postgresql/pgx/stderr.txt new file mode 100644 index 0000000000..063b2a149a --- /dev/null +++ b/internal/endtoend/testdata/insert_select_invalid/postgresql/pgx/stderr.txt @@ -0,0 +1,2 @@ +# package querytest +query.sql:4:1: INSERT has more expressions than target columns diff --git a/internal/endtoend/testdata/insert_select_invalid/postgresql/stdlib/query.sql b/internal/endtoend/testdata/insert_select_invalid/postgresql/stdlib/query.sql new file mode 100644 index 0000000000..41b0289d23 --- /dev/null +++ b/internal/endtoend/testdata/insert_select_invalid/postgresql/stdlib/query.sql @@ -0,0 +1,5 @@ +CREATE TABLE foo (bar text); + +-- name: InsertFoo :exec +INSERT INTO foo (bar) +SELECT 1, $1, $2; diff --git a/internal/endtoend/testdata/insert_select_invalid/postgresql/stdlib/sqlc.json b/internal/endtoend/testdata/insert_select_invalid/postgresql/stdlib/sqlc.json new file mode 100644 index 0000000000..de427d069f --- /dev/null +++ b/internal/endtoend/testdata/insert_select_invalid/postgresql/stdlib/sqlc.json @@ -0,0 +1,12 @@ +{ + "version": "1", + "packages": [ + { + "engine": "postgresql", + "path": "go", + "name": "querytest", + "schema": "query.sql", + "queries": "query.sql" + } + ] +} diff --git a/internal/endtoend/testdata/insert_select_invalid/postgresql/stdlib/stderr.txt b/internal/endtoend/testdata/insert_select_invalid/postgresql/stdlib/stderr.txt new file mode 100644 index 0000000000..063b2a149a --- /dev/null +++ b/internal/endtoend/testdata/insert_select_invalid/postgresql/stdlib/stderr.txt @@ -0,0 +1,2 @@ +# package querytest +query.sql:4:1: INSERT has more expressions than target columns