improves mysql parsing error messages

- additional support for position in errors

- adds position expression for schema validation errors

Author: cmoog

internal/cmd/cmd.go

@@ -139,7 +139,13 @@ var genCmd = &cobra.Command{
 				q, err := mysql.GeneratePkg(name, pkg.Schema, pkg.Queries, settings)
 				if err != nil {
 					fmt.Fprintf(os.Stderr, "# package %s\n", name)
-					fmt.Fprintf(os.Stderr, "error parsing file: %s\n", err)
+					if parserErr, ok := err.(*dinosql.ParserErr); ok {
+						for _, fileErr := range parserErr.Errs {
+							fmt.Fprintf(os.Stderr, "%s:%d:%d: %s\n", fileErr.Filename, fileErr.Line, fileErr.Column, fileErr.Err)
+						}
+					} else {
+						fmt.Fprintf(os.Stderr, "error parsing schema: %s\n", err)
+					}
 					errored = true
 					continue
 				}

internal/mysql/errors.go

@@ -0,0 +1,34 @@
+package mysql
+
+import (
+	"fmt"
+	"regexp"
+	"strconv"
+)
+
+func locFromSyntaxErr(errMessage error) (int, error) {
+	matcher := regexp.MustCompile("position ([0-9]*)")
+	results := matcher.FindStringSubmatch(errMessage.Error())
+	if len(results) > 0 {
+		return strconv.Atoi(results[1])
+	}
+	return 0, fmt.Errorf("failed to find position integer in parser error message")
+}
+
+func nearStrFromSyntaxErr(errMessage error) (string, error) {
+	matcher := regexp.MustCompile("near '(.*)'")
+	results := matcher.FindStringSubmatch(errMessage.Error())
+	if len(results) > 0 {
+		return results[1], nil
+	}
+	return "", fmt.Errorf("failed to find parser 'near' message")
+}
+
+type PositionedErr struct {
+	Pos int
+	Err error
+}
+
+func (e PositionedErr) Error() string {
+	return e.Err.Error()
+}

internal/mysql/errors_test.go

@@ -0,0 +1,33 @@
+package mysql
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"vitess.io/vitess/go/vt/sqlparser"
+)
+
+func TestSyntaxErr(t *testing.T) {
+	tokenizer := sqlparser.NewStringTokenizer("SELEC T id FROM users;")
+	expectedLocation := 6
+	expectedNear := "SELEC"
+
+	_, parseErr := sqlparser.ParseNextStrictDDL(tokenizer)
+	if parseErr == nil {
+		t.Errorf("Tokenizer failed to error on invalid MySQL syntax")
+	}
+
+	location, err := locFromSyntaxErr(parseErr)
+	if err != nil {
+		t.Errorf("failed to parse location from sqlparser syntax error message: %v", err)
+	} else if location != expectedLocation {
+		t.Errorf("parsed incorrect location from sqlparser syntax error message: %v", cmp.Diff(expectedLocation, location))
+	}
+
+	near, err := nearStrFromSyntaxErr(parseErr)
+	if err != nil {
+		t.Errorf("failed to parse 'nearby' chars from sqlparser syntax error message: %v", err)
+	} else if near != expectedNear {
+		t.Errorf("parse incorrect 'nearby' chars from sqlparser syntax error message: %v", cmp.Diff(expectedNear, near))
+	}
+}

internal/mysql/parse.go

@@ -31,23 +31,35 @@ func parsePath(sqlPath string, inPkg string, s *Schema, settings dinosql.Generat
 		return nil, err
 	}
 
+	parseErrors := dinosql.ParserErr{}
+
 	parsedQueries := []*Query{}
 	for _, filename := range files {
 		blob, err := ioutil.ReadFile(filename)
 		if err != nil {
-			return nil, fmt.Errorf("Failed to read file [%v]: %w", filename, err)
+			parseErrors.Add(filename, "", 0, err)
 		}
 		contents := dinosql.RemoveRollbackStatements(string(blob))
 		if err != nil {
-			return nil, fmt.Errorf("Failed to read contents of file [%v]: %w", filename, err)
+			parseErrors.Add(filename, "", 0, err)
+			continue
 		}
 		queries, err := parseContents(filename, contents, s, settings)
 		if err != nil {
-			return nil, fmt.Errorf("Failed to parse contents of file [%v]: %w", filename, err)
+			if positionedErr, ok := err.(PositionedErr); ok {
+				parseErrors.Add(filename, contents, positionedErr.Pos, err)
+			} else {
+				parseErrors.Add(filename, contents, 0, err)
+			}
+			continue
 		}
 		parsedQueries = append(parsedQueries, queries...)
 	}
 
+	if len(parseErrors.Errs) > 0 {
+		return nil, &parseErrors
+	}
+
 	return &Result{
 		Queries:     parsedQueries,
 		Schema:      s,
@@ -64,12 +76,20 @@ func parseContents(filename, contents string, s *Schema, settings dinosql.Genera
 		if err == io.EOF {
 			break
 		} else if err != nil {
-			return nil, err
+			parsedLoc, locErr := locFromSyntaxErr(err)
+			if locErr != nil {
+				parsedLoc = start // next best guess of the error location
+			}
+			near, nearErr := nearStrFromSyntaxErr(err)
+			if nearErr != nil {
+				return nil, PositionedErr{parsedLoc, fmt.Errorf("syntax error")}
+			}
+			return nil, PositionedErr{parsedLoc, fmt.Errorf("syntax error at or near '%s'", near)}
 		}
 		query := contents[start : t.Position-1]
 		result, err := parseQueryString(q, query, s, settings)
 		if err != nil {
-			return nil, fmt.Errorf("Failed to parse query in filepath [%v]: %w", filename, err)
+			return nil, PositionedErr{start, err}
 		}
 		start = t.Position
 		if result == nil {
@@ -87,26 +107,25 @@ func parseQueryString(tree sqlparser.Statement, query string, s *Schema, setting
 	case *sqlparser.Select:
 		selectQuery, err := parseSelect(tree, query, s, settings)
 		if err != nil {
-			return nil, fmt.Errorf("Failed to parse SELECT query: %w", err)
+			return nil, err
 		}
 		parsedQuery = selectQuery
 	case *sqlparser.Insert:
 		insert, err := parseInsert(tree, query, s, settings)
 		if err != nil {
-			return nil, fmt.Errorf("Failed to parse INSERT query: %w", err)
+			return nil, err
 		}
 		parsedQuery = insert
 	case *sqlparser.Update:
 		update, err := parseUpdate(tree, query, s, settings)
 		if err != nil {
-			return nil, fmt.Errorf("Failed to parse UPDATE query: %w", err)
+			return nil, err
 		}
 		parsedQuery = update
 	case *sqlparser.Delete:
 		delete, err := parseDelete(tree, query, s, settings)
-		delete.SchemaLookup = nil
 		if err != nil {
-			return nil, fmt.Errorf("Failed to parse DELETE query: %w", err)
+			return nil, err
 		}
 		parsedQuery = delete
 	case *sqlparser.DDL:
@@ -118,28 +137,28 @@ func parseQueryString(tree sqlparser.Statement, query string, s *Schema, setting
 	}
 	paramsReplacedQuery, err := replaceParamStrs(sqlparser.String(tree), parsedQuery.Params)
 	if err != nil {
-		return nil, fmt.Errorf("Failed to replace param variables in query string: %w", err)
+		return nil, fmt.Errorf("failed to replace param variables in query string: %w", err)
 	}
 	parsedQuery.SQL = paramsReplacedQuery
 	return parsedQuery, nil
 }
 
 func (q *Query) parseNameAndCmd() error {
 	if q == nil {
-		return fmt.Errorf("Cannot parse name and cmd from null query")
+		return fmt.Errorf("cannot parse name and cmd from null query")
 	}
 	_, comments := sqlparser.SplitMarginComments(q.SQL)
 	err := q.parseLeadingComment(comments.Leading)
 	if err != nil {
-		return fmt.Errorf("Failed to parse leading comment %w", err)
+		return fmt.Errorf("failed to parse leading comment %w", err)
 	}
 	return nil
 }
 
 func parseSelect(tree *sqlparser.Select, query string, s *Schema, settings dinosql.GenerateSettings) (*Query, error) {
 	tableAliasMap, err := parseFrom(tree.From, false)
 	if err != nil {
-		return nil, fmt.Errorf("Failed to parse table name alias's: %w", err)
+		return nil, fmt.Errorf("failed to parse table name alias's: %w", err)
 	}
 	defaultTableName := getDefaultTable(tableAliasMap)
 
@@ -205,7 +224,7 @@ func parseFrom(from sqlparser.TableExprs, isLeftJoined bool) (FromTables, error)
 		case *sqlparser.AliasedTableExpr:
 			name, ok := v.Expr.(sqlparser.TableName)
 			if !ok {
-				return nil, fmt.Errorf("Failed to parse AliasedTableExpr name: %v", spew.Sdump(v))
+				return nil, fmt.Errorf("failed to parse AliasedTableExpr name: %v", spew.Sdump(v))
 			}
 			t := FromTable{
 				TrueName:     name.Name.String(),
@@ -232,7 +251,7 @@ func parseFrom(from sqlparser.TableExprs, isLeftJoined bool) (FromTables, error)
 			}
 			return right, nil
 		default:
-			return nil, fmt.Errorf("Failed to parse table expr: %v", spew.Sdump(v))
+			return nil, fmt.Errorf("failed to parse table expr: %v", spew.Sdump(v))
 		}
 	}
 	return tables, nil
@@ -251,7 +270,7 @@ func getDefaultTable(tableAliasMap FromTables) string {
 func parseUpdate(node *sqlparser.Update, query string, s *Schema, settings dinosql.GenerateSettings) (*Query, error) {
 	tableAliasMap, err := parseFrom(node.TableExprs, false)
 	if err != nil {
-		return nil, fmt.Errorf("Failed to parse table name alias's: %w", err)
+		return nil, fmt.Errorf("failed to parse table name alias's: %w", err)
 	}
 	defaultTable := getDefaultTable(tableAliasMap)
 	if err != nil {
@@ -267,7 +286,7 @@ func parseUpdate(node *sqlparser.Update, query string, s *Schema, settings dinos
 		}
 		colDfn, err := s.getColType(col, tableAliasMap, defaultTable)
 		if err != nil {
-			return nil, fmt.Errorf("Failed to determine type of a parameter's column: %w", err)
+			return nil, fmt.Errorf("failed to determine type of a parameter's column: %w", err)
 		}
 		originalParamName := string(newValue.Val)
 		param := Param{
@@ -280,7 +299,7 @@ func parseUpdate(node *sqlparser.Update, query string, s *Schema, settings dinos
 
 	whereParams, err := paramsInWhereExpr(node.Where.Expr, s, tableAliasMap, defaultTable, settings)
 	if err != nil {
-		return nil, fmt.Errorf("Failed to parse params from WHERE expression: %w", err)
+		return nil, fmt.Errorf("failed to parse params from WHERE expression: %w", err)
 	}
 
 	parsedQuery := Query{
@@ -342,7 +361,7 @@ func parseInsert(node *sqlparser.Insert, query string, s *Schema, settings dinos
 func parseDelete(node *sqlparser.Delete, query string, s *Schema, settings dinosql.GenerateSettings) (*Query, error) {
 	tableAliasMap, err := parseFrom(node.TableExprs, false)
 	if err != nil {
-		return nil, fmt.Errorf("Failed to parse table name alias's: %w", err)
+		return nil, fmt.Errorf("failed to parse table name alias's: %w", err)
 	}
 	defaultTableName := getDefaultTable(tableAliasMap)
 	if err != nil {
@@ -412,7 +431,7 @@ func parseSelectAliasExpr(exprs sqlparser.SelectExprs, s *Schema, tableAliasMap
 			case *sqlparser.ColName:
 				res, err := s.getColType(v, tableAliasMap, defaultTable)
 				if err != nil {
-					panic(fmt.Sprintf("Column not found in schema: %v", err))
+					return nil, err
 				}
 				if hasAlias {
 					res.Name = expr.As // applys the alias
@@ -458,11 +477,11 @@ func GeneratePkg(pkgName, schemaPath, querysPath string, settings dinosql.Genera
 	s := NewSchema()
 	_, err := parsePath(schemaPath, pkgName, s, settings)
 	if err != nil {
-		return nil, fmt.Errorf("schema failure: %w", err)
+		return nil, err
 	}
 	result, err := parsePath(querysPath, pkgName, s, settings)
 	if err != nil {
-		return nil, fmt.Errorf("query failure: %w", err)
+		return nil, err
 	}
 	return result, nil
 }

internal/mysql/schema.go

@@ -24,7 +24,7 @@ func (s *Schema) getColType(col *sqlparser.ColName, tableAliasMap FromTables, de
 	if !col.Qualifier.IsEmpty() {
 		realTable, ok := tableAliasMap[col.Qualifier.Name.String()]
 		if !ok {
-			return nil, fmt.Errorf("Column qualifier [%v] not found in table alias map", col.Qualifier.Name.String())
+			return nil, fmt.Errorf("column qualifier \"%v\" not found in query", col.Qualifier.Name.String())
 		}
 		colDfn, err := s.schemaLookup(realTable.TrueName, col.Name.String())
 		if err != nil {
@@ -37,7 +37,7 @@ func (s *Schema) getColType(col *sqlparser.ColName, tableAliasMap FromTables, de
 		return &colDfnCopy, nil
 	}
 	if defaultTableName == "" {
-		return nil, fmt.Errorf("Column reference [%v] is ambiguous -- Add a qualifier", col.Name.String())
+		return nil, fmt.Errorf("column reference \"%v\" is ambiguous, consider adding a qualifier", col.Name.String())
 	}
 	colDfn, err := s.schemaLookup(defaultTableName, col.Name.String())
 	if err != nil {
@@ -54,7 +54,7 @@ func (s *Schema) Add(ddl *sqlparser.DDL) {
 	case "create":
 		name := ddl.Table.Name.String()
 		if ddl.TableSpec == nil {
-			panic(fmt.Sprintf("Failed to parse table [%v] schema.", name))
+			panic(fmt.Sprintf("failed to parse table \"%s\" schema.", name))
 		}
 		s.tables[name] = ddl.TableSpec.Columns
 	}
@@ -63,7 +63,7 @@ func (s *Schema) Add(ddl *sqlparser.DDL) {
 func (s *Schema) schemaLookup(table string, col string) (*sqlparser.ColumnDefinition, error) {
 	cols, ok := s.tables[table]
 	if !ok {
-		return nil, fmt.Errorf("Table [%v] not found in Schema", table)
+		return nil, fmt.Errorf("table \"%s\" not found in schema", table)
 	}
 
 	for _, colDef := range cols {
@@ -72,5 +72,5 @@ func (s *Schema) schemaLookup(table string, col string) (*sqlparser.ColumnDefini
 		}
 	}
 
-	return nil, fmt.Errorf("Column [%v] not found in table [%v]", col, table)
+	return nil, fmt.Errorf("column \"%s\" not found in table \"%s\"", col, table)
 }