named: Port parameter style validation to SQL (#504)

kyleconroy · web-flow · commit ddaa211a3dd4 · 2020-05-18T10:04:06.000-07:00
diff --git a/internal/compiler/compile.go b/internal/compiler/compile.go
@@ -55,8 +55,8 @@ func enumValueName(value string) string {
 }
 
 // end copypasta
-func parseCatalog(p Parser, c *catalog.Catalog, schema []string) error {
-	files, err := sqlpath.Glob(schema)
+func parseCatalog(p Parser, c *catalog.Catalog, schemas []string) error {
+	files, err := sqlpath.Glob(schemas)
 	if err != nil {
 		return err
 	}
@@ -86,6 +86,34 @@ func parseCatalog(p Parser, c *catalog.Catalog, schema []string) error {
 	return nil
 }
 
+func parseQueries(p Parser, c *catalog.Catalog, queries []string) (*Result, error) {
+	merr := multierr.New()
+	files, err := sqlpath.Glob(queries)
+	if err != nil {
+		return nil, err
+	}
+	for _, filename := range files {
+		blob, err := ioutil.ReadFile(filename)
+		if err != nil {
+			merr.Add(filename, "", 0, err)
+			continue
+		}
+		source := string(blob)
+		stmts, err := p.Parse(strings.NewReader(source))
+		if err != nil {
+			merr.Add(filename, source, 0, err)
+			continue
+		}
+		for _, stmt := range stmts {
+			fmt.Println(stmt)
+		}
+	}
+	if len(merr.Errs()) > 0 {
+		return nil, merr
+	}
+	return &Result{}, nil
+}
+
 func buildResult(c *catalog.Catalog) (*Result, error) {
 	var structs []dinosql.GoStruct
 	var enums []dinosql.GoEnum
@@ -130,7 +158,6 @@ func buildResult(c *catalog.Catalog) (*Result, error) {
 			}
 		}
 	}
-
 	if len(structs) > 0 {
 		sort.Slice(structs, func(i, j int) bool { return structs[i].Name < structs[j].Name })
 	}
diff --git a/internal/compiler/engine.go b/internal/compiler/engine.go
@@ -44,7 +44,7 @@ func (e *Engine) ParseCatalog(schema []string) error {
 }
 
 func (e *Engine) ParseQueries(queries []string, opts dinosql.ParserOpts) error {
-	r, err := buildResult(e.catalog)
+	r, err := parseQueries(e.parser, e.catalog, e.conf.Queries)
 	if err != nil {
 		return err
 	}
diff --git a/internal/compiler/runtime/parse.go b/internal/compiler/runtime/parse.go
@@ -0,0 +1,51 @@
+package runtime
+
+import (
+	"errors"
+
+	"github.com/kyleconroy/sqlc/internal/source"
+	"github.com/kyleconroy/sqlc/internal/sql/ast"
+	"github.com/kyleconroy/sqlc/internal/sql/ast/pg"
+	"github.com/kyleconroy/sqlc/internal/sql/catalog"
+	"github.com/kyleconroy/sqlc/internal/sql/validate"
+)
+
+type Query struct {
+}
+
+var ErrUnsupportedStatementType = errors.New("parseQuery: unsupported statement type")
+
+func parseQuery(c *catalog.Catalog, stmt ast.Node, src string, rewriteParameters bool) (*Query, error) {
+	if err := validate.ParamStyle(stmt); err != nil {
+		return nil, err
+	}
+	if err := validate.ParamRef(stmt); err != nil {
+		return nil, err
+	}
+	raw, ok := stmt.(*ast.RawStmt)
+	if !ok {
+		return nil, errors.New("node is not a statement")
+	}
+	switch n := raw.Stmt.(type) {
+	case *pg.SelectStmt:
+	case *pg.DeleteStmt:
+	case *pg.InsertStmt:
+		if err := validate.InsertStmt(n); err != nil {
+			return nil, err
+		}
+	case *pg.TruncateStmt:
+	case *pg.UpdateStmt:
+	default:
+		return nil, ErrUnsupportedStatementType
+	}
+
+	rawSQL, err := source.Pluck(src, raw.StmtLocation, raw.StmtLen)
+	if err != nil {
+		return nil, err
+	}
+	if rawSQL == "" {
+		return nil, errors.New("missing semicolon at end of file")
+	}
+
+	return &Query{}, nil
+}
diff --git a/internal/dinosql/parser.go b/internal/dinosql/parser.go
@@ -148,24 +148,24 @@ func ParseQueries(c core.Catalog, queriesPaths []string, opts ParserOpts) (*Resu
 			merr.Add(filename, "", 0, err)
 			continue
 		}
-		source := string(blob)
-		tree, err := pg.Parse(source)
+		src := string(blob)
+		tree, err := pg.Parse(src)
 		if err != nil {
-			merr.Add(filename, source, 0, err)
+			merr.Add(filename, src, 0, err)
 			continue
 		}
 		for _, stmt := range tree.Statements {
-			query, err := parseQuery(c, stmt, source, opts.UsePositionalParameters)
+			query, err := parseQuery(c, stmt, src, opts.UsePositionalParameters)
 			if err == errUnsupportedStatementType {
 				continue
 			}
 			if err != nil {
-				merr.Add(filename, source, location(stmt), err)
+				merr.Add(filename, src, location(stmt), err)
 				continue
 			}
 			if query.Name != "" {
 				if _, exists := set[query.Name]; exists {
-					merr.Add(filename, source, location(stmt), fmt.Errorf("duplicate query name: %s", query.Name))
+					merr.Add(filename, src, location(stmt), fmt.Errorf("duplicate query name: %s", query.Name))
 					continue
 				}
 				set[query.Name] = struct{}{}
@@ -198,12 +198,6 @@ func location(node nodes.Node) int {
 	return 0
 }
 
-func pluckQuery(source string, n nodes.RawStmt) (string, error) {
-	head := n.StmtLocation
-	tail := n.StmtLocation + n.StmtLen
-	return source[head:tail], nil
-}
-
 func rangeVars(root nodes.Node) []nodes.RangeVar {
 	var vars []nodes.RangeVar
 	find := ast.VisitorFunc(func(node nodes.Node) {
@@ -303,7 +297,7 @@ func validateCmd(n nodes.Node, name, cmd string) error {
 
 var errUnsupportedStatementType = errors.New("parseQuery: unsupported statement type")
 
-func parseQuery(c core.Catalog, stmt nodes.Node, source string, rewriteParameters bool) (*Query, error) {
+func parseQuery(c core.Catalog, stmt nodes.Node, src string, rewriteParameters bool) (*Query, error) {
 	if err := validate.ParamStyle(stmt); err != nil {
 		return nil, err
 	}
@@ -327,7 +321,7 @@ func parseQuery(c core.Catalog, stmt nodes.Node, source string, rewriteParameter
 		return nil, errUnsupportedStatementType
 	}
 
-	rawSQL, err := pluckQuery(source, raw)
+	rawSQL, err := source.Pluck(src, raw.StmtLocation, raw.StmtLen)
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/dinosql/parser_test.go b/internal/dinosql/parser_test.go
@@ -37,7 +37,7 @@ func TestPluck(t *testing.T) {
 	for i, stmt := range tree.Statements {
 		switch n := stmt.(type) {
 		case nodes.RawStmt:
-			q, err := pluckQuery(pluck, n)
+			q, err := source.Pluck(pluck, n.StmtLocation, n.StmtLen)
 			if err != nil {
 				t.Error(err)
 				continue
diff --git a/internal/postgresql/parse.go b/internal/postgresql/parse.go
@@ -179,7 +179,11 @@ func (p *Parser) Parse(r io.Reader) ([]ast.Statement, error) {
 			return nil, fmt.Errorf("unexpected nil node")
 		}
 		stmts = append(stmts, ast.Statement{
-			Raw: &ast.RawStmt{Stmt: n},
+			Raw: &ast.RawStmt{
+				Stmt:         n,
+				StmtLocation: raw.StmtLocation,
+				StmtLen:      raw.StmtLen,
+			},
 		})
 	}
 	return stmts, nil
diff --git a/internal/source/code.go b/internal/source/code.go
@@ -37,3 +37,9 @@ func LineNumber(source string, head int) (int, int) {
 	}
 	return line + 1, col
 }
+
+func Pluck(source string, location, length int) (string, error) {
+	head := location
+	tail := location + length
+	return source[head:tail], nil
+}
diff --git a/internal/sql/ast/ast.go b/internal/sql/ast/ast.go
@@ -13,11 +13,13 @@ func (n *Statement) Pos() int {
 }
 
 type RawStmt struct {
-	Stmt Node
+	Stmt         Node
+	StmtLocation int
+	StmtLen      int
 }
 
 func (n *RawStmt) Pos() int {
-	return 0
+	return n.StmtLocation
 }
 
 type TableName struct {
diff --git a/internal/sql/astutils/search.go b/internal/sql/astutils/search.go
@@ -0,0 +1,21 @@
+package astutils
+
+import "github.com/kyleconroy/sqlc/internal/sql/ast"
+
+type nodeSearch struct {
+	list  *ast.List
+	check func(ast.Node) bool
+}
+
+func (s *nodeSearch) Visit(node ast.Node) Visitor {
+	if s.check(node) {
+		s.list.Items = append(s.list.Items, node)
+	}
+	return s
+}
+
+func Search(root ast.Node, f func(ast.Node) bool) *ast.List {
+	ns := &nodeSearch{check: f, list: &ast.List{}}
+	Walk(ns, root)
+	return ns.list
+}
diff --git a/internal/sql/named/is.go b/internal/sql/named/is.go
@@ -0,0 +1,17 @@
+package named
+
+import (
+	"github.com/kyleconroy/sqlc/internal/sql/ast"
+	"github.com/kyleconroy/sqlc/internal/sql/ast/pg"
+	"github.com/kyleconroy/sqlc/internal/sql/astutils"
+)
+
+func IsParamFunc(node ast.Node) bool {
+	fun, ok := node.(*pg.FuncCall)
+	return ok && astutils.Join(fun.Funcname, ".") == "sqlc.arg"
+}
+
+func IsParamSign(node ast.Node) bool {
+	expr, ok := node.(*pg.A_Expr)
+	return ok && astutils.Join(expr.Name, ".") == "@"
+}
diff --git a/internal/sql/validate/insert_stmt.go b/internal/sql/validate/insert_stmt.go
@@ -0,0 +1,32 @@
+package validate
+
+import (
+	"github.com/kyleconroy/sqlc/internal/sql/ast/pg"
+	"github.com/kyleconroy/sqlc/internal/sql/sqlerr"
+)
+
+func InsertStmt(stmt *pg.InsertStmt) error {
+	sel, ok := stmt.SelectStmt.(*pg.SelectStmt)
+	if !ok {
+		return nil
+	}
+	if len(sel.ValuesLists) != 1 {
+		return nil
+	}
+
+	colsLen := len(stmt.Cols.Items)
+	valsLen := len(sel.ValuesLists[0])
+	switch {
+	case colsLen > valsLen:
+		return &sqlerr.Error{
+			Code:    "42601",
+			Message: "INSERT has more target columns than expressions",
+		}
+	case colsLen < valsLen:
+		return &sqlerr.Error{
+			Code:    "42601",
+			Message: "INSERT has more expressions than target columns",
+		}
+	}
+	return nil
+}
diff --git a/internal/sql/validate/param_ref.go b/internal/sql/validate/param_ref.go
@@ -0,0 +1,37 @@
+package validate
+
+import (
+	"fmt"
+
+	"github.com/kyleconroy/sqlc/internal/sql/ast"
+	"github.com/kyleconroy/sqlc/internal/sql/ast/pg"
+	"github.com/kyleconroy/sqlc/internal/sql/astutils"
+	"github.com/kyleconroy/sqlc/internal/sql/sqlerr"
+)
+
+func ParamRef(n ast.Node) error {
+	var allrefs []*pg.ParamRef
+
+	// Find all parameter references
+	astutils.Walk(astutils.VisitorFunc(func(node ast.Node) {
+		switch n := node.(type) {
+		case *pg.ParamRef:
+			allrefs = append(allrefs, n)
+		}
+	}), n)
+
+	seen := map[int]struct{}{}
+	for _, r := range allrefs {
+		seen[r.Number] = struct{}{}
+	}
+
+	for i := 1; i <= len(seen); i += 1 {
+		if _, ok := seen[i]; !ok {
+			return &sqlerr.Error{
+				Code:    "42P18",
+				Message: fmt.Sprintf("could not determine data type of parameter $%d", i),
+			}
+		}
+	}
+	return nil
+}
diff --git a/internal/sql/validate/param_style.go b/internal/sql/validate/param_style.go
@@ -0,0 +1,35 @@
+package validate
+
+import (
+	"github.com/kyleconroy/sqlc/internal/sql/ast"
+	"github.com/kyleconroy/sqlc/internal/sql/ast/pg"
+	"github.com/kyleconroy/sqlc/internal/sql/astutils"
+	"github.com/kyleconroy/sqlc/internal/sql/named"
+	"github.com/kyleconroy/sqlc/internal/sql/sqlerr"
+)
+
+// A query can use one (and only one) of the following formats:
+// - positional parameters           $1
+// - named parameter operator        @param
+// - named parameter function calls  sqlc.arg(param)
+func ParamStyle(n ast.Node) error {
+	positional := astutils.Search(n, func(node ast.Node) bool {
+		_, ok := node.(*pg.ParamRef)
+		return ok
+	})
+	namedFunc := astutils.Search(n, named.IsParamFunc)
+	namedSign := astutils.Search(n, named.IsParamSign)
+	for _, check := range []bool{
+		len(positional.Items) > 0 && len(namedSign.Items)+len(namedFunc.Items) > 0,
+		len(namedFunc.Items) > 0 && len(namedSign.Items)+len(positional.Items) > 0,
+		len(namedSign.Items) > 0 && len(positional.Items)+len(namedFunc.Items) > 0,
+	} {
+		if check {
+			return &sqlerr.Error{
+				Code:    "", // TODO: Pick a new error code
+				Message: "query mixes positional parameters ($1) and named parameters (sqlc.arg or @arg)",
+			}
+		}
+	}
+	return nil
+}

Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,7 @@ func (e *Engine) ParseCatalog(schema []string) error {`
`44`	`44`	`}`
`45`	`45`
`46`	`46`	`func (e *Engine) ParseQueries(queries []string, opts dinosql.ParserOpts) error {`
`47`		`- r, err := buildResult(e.catalog)`
	`47`	`+ r, err := parseQueries(e.parser, e.catalog, e.conf.Queries)`
`48`	`48`	`if err != nil {`
`49`	`49`	`return err`
`50`	`50`	`}`
Original file line number	Diff line number	Diff line change
`@@ -179,7 +179,11 @@ func (p *Parser) Parse(r io.Reader) ([]ast.Statement, error) {`
`179`	`179`	`return nil, fmt.Errorf("unexpected nil node")`
`180`	`180`	`}`
`181`	`181`	`stmts = append(stmts, ast.Statement{`
`182`		`- Raw: &ast.RawStmt{Stmt: n},`
	`182`	`+ Raw: &ast.RawStmt{`
	`183`	`+ Stmt: n,`
	`184`	`+ StmtLocation: raw.StmtLocation,`
	`185`	`+ StmtLen: raw.StmtLen,`
	`186`	`+ },`
`183`	`187`	`})`
`184`	`188`	`}`
`185`	`189`	`return stmts, nil`
Original file line number	Diff line number	Diff line change
`@@ -37,3 +37,9 @@ func LineNumber(source string, head int) (int, int) {`
`37`	`37`	`}`
`38`	`38`	`return line + 1, col`
`39`	`39`	`}`
	`40`	`+`
	`41`	`+func Pluck(source string, location, length int) (string, error) {`
	`42`	`+ head := location`
	`43`	`+ tail := location + length`
	`44`	`+ return source[head:tail], nil`
	`45`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -13,11 +13,13 @@ func (n *Statement) Pos() int {`
`13`	`13`	`}`
`14`	`14`
`15`	`15`	`type RawStmt struct {`
`16`		`- Stmt Node`
	`16`	`+ Stmt Node`
	`17`	`+ StmtLocation int`
	`18`	`+ StmtLen int`
`17`	`19`	`}`
`18`	`20`
`19`	`21`	`func (n *RawStmt) Pos() int {`
`20`		`- return 0`
	`22`	`+ return n.StmtLocation`
`21`	`23`	`}`
`22`	`24`
`23`	`25`	`type TableName struct {`