From 9906545e4b5056a82947809c8825d3a15594e1b4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 04:43:38 +0000 Subject: [PATCH 1/3] Match the pinned build's parser error messages over the negative corpus Milestone 6, error fidelity: every must-reject corpus case now renders the oracle's recorded first line verbatim, gated by the new TestCorpusErrorMessages in errors_test.go alongside hand-written position (Offset/Line/Column) pins. Message fixes to reach zero divergence: - Whole-query UTF-8 validation before parsing ("Invalid UTF-8 in query (byte sequence mismatch)"), via a new port of Utf8Proc::Analyze that classifies byte mismatches vs invalid codepoints and, like upstream, accepts overlong encodings; escape-string literals reuse it for the "byte mismatch"/"invalid unicode" suffixes. - Column-without-type errors quote the name per KeywordHelper::WriteOptionallyQuoted (keywords and non-plain identifiers quoted, plain ones bare). - Chained-comparison ban transforms its operands first so deeper errors (e.g. Empty subscript '[]') win, and uses upstream's wording. - Window frames validate the start bound before the end bound. - Interval range specifiers name the range (YEAR TO MONTH is not supported) instead of a generic message. - RESET options name the last offending option; PIVOT-on-subquery renders the expression's source text; plus upstream wording for CREATE TABLE AS, OR REPLACE/IF NOT EXISTS, OR REPLACE|IGNORE with ON CONFLICT, COMMENT ON qualification depth, and NEAREST counts (which now also reject non-positive literals, retiring the last nearest_errors todo). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01QQHouvcnCnGT1MuHvwPJrj --- parser/dml_test.go | 2 +- parser/errors_test.go | 193 ++++++++++++++++++ parser/parser.go | 13 ++ .../nearest/nearest_errors.test.metadata.json | 5 - parser/transform_ddl.go | 15 +- parser/transform_dml.go | 2 +- parser/transform_expr.go | 11 +- parser/transform_func.go | 26 ++- parser/transform_misc.go | 25 ++- parser/transform_single.go | 23 +-- parser/transform_tableref.go | 18 +- parser/utf8.go | 69 +++++++ 12 files changed, 362 insertions(+), 40 deletions(-) create mode 100644 parser/errors_test.go delete mode 100644 parser/testdata/corpus/sql/join/nearest/nearest_errors.test.metadata.json create mode 100644 parser/utf8.go diff --git a/parser/dml_test.go b/parser/dml_test.go index 2aa6db1..81975d0 100644 --- a/parser/dml_test.go +++ b/parser/dml_test.go @@ -97,7 +97,7 @@ func TestInsertOnConflict(t *testing.T) { if ins.OnConflict == nil || ins.OnConflict.Action != ast.OnConflictReplace { t.Errorf("OR REPLACE on-conflict = %#v", ins.OnConflict) } - if msg := parseErr(t, "INSERT OR REPLACE INTO t VALUES (1) ON CONFLICT DO NOTHING"); !strings.Contains(msg, "not compatible") { + if msg := parseErr(t, "INSERT OR REPLACE INTO t VALUES (1) ON CONFLICT DO NOTHING"); !strings.Contains(msg, "can not provide both OR REPLACE|IGNORE") { t.Errorf("shorthand+clause error = %q", msg) } } diff --git a/parser/errors_test.go b/parser/errors_test.go new file mode 100644 index 0000000..69c171c --- /dev/null +++ b/parser/errors_test.go @@ -0,0 +1,193 @@ +// Error message and position fidelity (milestone 6): darkwing's rendered +// parse errors must match the pinned DuckDB build's. The corpus records +// the first line of the oracle's message for every must-reject case; +// TestCorpusErrorMessages holds darkwing to it exactly. Positions are not +// recorded in the corpus (the CLI renders them as a LINE/caret display +// below the first line), so TestErrorPositions pins them with hand-written +// cases instead. +package parser + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/sqlc-dev/darkwing/internal/testfile" +) + +// rejectError parses sql and returns the *Error it must fail with. +func rejectError(t *testing.T, sql string) *Error { + t.Helper() + _, err := Parse(context.Background(), strings.NewReader(sql)) + if err == nil { + t.Fatalf("Parse(%q): expected error", sql) + } + var pe *Error + if !errors.As(err, &pe) { + t.Fatalf("Parse(%q): got %T (%v), want *parser.Error", sql, err, err) + } + return pe +} + +// TestCorpusErrorMessages is the negative-corpus message gate: for every +// must-reject case, darkwing's error must render the oracle's recorded +// first line verbatim. +func TestCorpusErrorMessages(t *testing.T) { + for _, path := range corpusFiles(t) { + rel := strings.TrimPrefix(path, "testdata/") + t.Run(rel, func(t *testing.T) { + t.Parallel() + file, err := testfile.Read(path) + if err != nil { + t.Fatal(err) + } + meta, err := testfile.ReadMetadata(path) + if err != nil { + t.Fatal(err) + } + for i := range file.Cases { + c := &file.Cases[i] + key := c.Key() + if !c.Reject { + continue + } + if _, skip := meta.Skip[key]; skip { + continue + } + if _, todo := meta.Todo[key]; todo { + continue + } + got := classify(nil, c.SQL) + if !got.reject { + // TestCorpus reports accept/reject disagreements + continue + } + // the oracle records only the first line; darkwing's + // multi-line messages (e.g. the PIVOT hints) match beyond it + firstLine, _, _ := strings.Cut(got.detail, "\n") + if firstLine != c.Error { + t.Errorf("case %s: error message diverges from the oracle\nSQL: %s\nwant: %s\ngot: %s", + key, c.SQL, c.Error, firstLine) + } + } + }) + } +} + +// TestErrorPositions pins Offset/Line/Column on representative failures. +// Column counts bytes from 1, as the pinned CLI's caret display does. +func TestErrorPositions(t *testing.T) { + tests := []struct { + sql string + msg string // rendered first line + offset int + line, col int + }{ + { + // SELCT itself parses as a bare expression statement (a + // column reference), so the failure lands on the 1 + sql: "SELCT 1", + msg: `Parser Error: syntax error at or near "1"`, + offset: 6, line: 1, col: 7, + }, + { + sql: "SELECT 1 2", + msg: `Parser Error: syntax error at or near "2"`, + offset: 9, line: 1, col: 10, + }, + { + sql: "SELECT 1;\nSELCT 2;", + msg: `Parser Error: syntax error at or near "2"`, + offset: 16, line: 2, col: 7, + }, + { + sql: "SELECT 1\nFROM t\nWHERE (", + msg: `Parser Error: syntax error at or near "("`, + offset: 22, line: 3, col: 7, + }, + { + // whole-query UTF-8 validation, positioned at the bad byte + sql: "SELECT\n 42\nFR\xffOM t;", + msg: "Parser Error: Invalid UTF-8 in query (byte sequence mismatch)", + offset: 14, line: 3, col: 3, + }, + { + // a well-formed sequence encoding a surrogate is the other + // UnicodeInvalidReason + sql: "SELECT '\xed\xa0\x80'", + msg: "Parser Error: Invalid UTF-8 in query (invalid unicode)", + offset: 8, line: 1, col: 9, + }, + } + for _, tt := range tests { + pe := rejectError(t, tt.sql) + firstLine, _, _ := strings.Cut(pe.Error(), "\n") + if firstLine != tt.msg { + t.Errorf("Parse(%q): message = %q, want %q", tt.sql, firstLine, tt.msg) + } + if pe.Offset != tt.offset || pe.Line != tt.line || pe.Column != tt.col { + t.Errorf("Parse(%q): position = offset %d line %d col %d, want offset %d line %d col %d", + tt.sql, pe.Offset, pe.Line, pe.Column, tt.offset, tt.line, tt.col) + } + } +} + +// TestErrorMessages pins messages that only fire on inputs the corpus +// does not cover. +func TestErrorMessages(t *testing.T) { + tests := []struct{ sql, msg string }{ + // escape-literal UTF-8 classification: raw invalid byte vs + // well-formed surrogate sequence + {`SELECT e'\xFF'`, "Parser Error: Invalid UTF-8 in escape string literal at byte offset 0: byte mismatch"}, + {`SELECT e'a\xed\xa0\x80'`, "Parser Error: Invalid UTF-8 in escape string literal at byte offset 1: invalid unicode"}, + // KeywordHelper::WriteOptionallyQuoted quotes keywords and + // non-plain identifiers, and leaves plain ones bare + {"CREATE TABLE t (foo)", "Parser Error: Column foo must have a type or be defined as a GENERATED column."}, + {"CREATE TABLE t (name)", `Parser Error: Column "name" must have a type or be defined as a GENERATED column.`}, + {`CREATE TABLE t ("A b")`, `Parser Error: Column "A b" must have a type or be defined as a GENERATED column.`}, + // upstream names the last offending RESET option + {"ALTER TABLE t RESET (a='1', b='2')", `Parser Error: Reset option "b" cannot set any value. Did you mean to use SET?`}, + {"ALTER TABLE t RESET (a='1', b)", `Parser Error: Reset option "a" cannot set any value. Did you mean to use SET?`}, + // a deeper operand error beats the chained-comparison ban + {"SELECT 1 < 2 < [3][]", "Parser Error: Empty subscript '[]' is not allowed"}, + {"SELECT 1 < 2 < 3", "Parser Error: Chained comparisons are not supported, use AND to combine comparisons"}, + } + for _, tt := range tests { + pe := rejectError(t, tt.sql) + firstLine, _, _ := strings.Cut(pe.Error(), "\n") + if firstLine != tt.msg { + t.Errorf("Parse(%q): message = %q, want %q", tt.sql, firstLine, tt.msg) + } + } +} + +// TestAnalyzeUTF8 pins the Utf8Proc::Analyze port's classification and +// positions, including the deliberate divergence from the standard +// library: overlong encodings pass. +func TestAnalyzeUTF8(t *testing.T) { + tests := []struct { + in string + reason utf8Reason + pos int + }{ + {"", utf8Valid, -1}, + {"plain ascii", utf8Valid, -1}, + {"caf\xc3\xa9", utf8Valid, -1}, + {"\xf0\x9f\xa6\x86", utf8Valid, -1}, // U+1F986 (duck) + {"\xff", utf8ByteMismatch, 0}, + {"a\x80", utf8ByteMismatch, 1}, // stray continuation byte + {"a\xc3", utf8ByteMismatch, 1}, // truncated 2-byte sequence + {"ab\xe0\xa0", utf8ByteMismatch, 2}, // truncated 3-byte sequence + {"\xc3(", utf8ByteMismatch, 0}, // bad continuation byte + {"\xed\xa0\x80", utf8InvalidUnicode, 0}, // surrogate U+D800 + {"a\xf4\x90\x80\x80", utf8InvalidUnicode, 1}, // > U+10FFFF + {"\xc0\xaf", utf8Valid, -1}, // overlong '/', accepted like upstream + } + for _, tt := range tests { + reason, pos := analyzeUTF8(tt.in) + if reason != tt.reason || pos != tt.pos { + t.Errorf("analyzeUTF8(%q) = (%v, %d), want (%v, %d)", tt.in, reason, pos, tt.reason, tt.pos) + } + } +} diff --git a/parser/parser.go b/parser/parser.go index 0f15ba9..cca23d3 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -67,6 +67,9 @@ func ParseExpr(ctx context.Context, sql string) (expr ast.Expr, err error) { if err != nil { return nil, err } + if ue := validateQueryUTF8(sql); ue != nil { + return nil, ue + } tokens, err := lexer.Tokenize(sql) if err != nil { return nil, newError(sql, err) @@ -92,6 +95,10 @@ func parseString(ctx context.Context, src string) (stmts []ast.Stmt, err error) if err != nil { return nil, err } + // the pinned build validates the whole query's UTF-8 before parsing + if ue := validateQueryUTF8(src); ue != nil { + return nil, ue + } tokens, err := lexer.Tokenize(src) if err != nil { return nil, newError(src, err) @@ -204,6 +211,12 @@ func newError(src string, err error) error { default: msg = err.Error() } + return errorAt(src, msg, offset) +} + +// errorAt builds an *Error at a byte offset in src, deriving line and +// column (offset -1 means no position). +func errorAt(src, msg string, offset int) *Error { pe := &Error{Msg: msg, Offset: offset, Line: 1, Column: 1} if offset >= 0 { line, col := 1, 1 diff --git a/parser/testdata/corpus/sql/join/nearest/nearest_errors.test.metadata.json b/parser/testdata/corpus/sql/join/nearest/nearest_errors.test.metadata.json deleted file mode 100644 index de0d2fb..0000000 --- a/parser/testdata/corpus/sql/join/nearest/nearest_errors.test.metadata.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "todo": { - "07e88975ac7f20a0": "oracle's Parser Error comes from outside the parse pipeline (bind-time setting/function/constraint validation or nested SQL parsing); darkwing accepts by design - revisit with milestone 6 error-fidelity work" - } -} diff --git a/parser/transform_ddl.go b/parser/transform_ddl.go index 8612e33..fdc3e71 100644 --- a/parser/transform_ddl.go +++ b/parser/transform_ddl.go @@ -68,7 +68,7 @@ func (tc *transformContext) transformCreate(n tnode) ast.Stmt { base.Temporary = temporary if base.OnConflict == "" || onConflict == ast.CreateReplace { if base.OnConflict == ast.CreateIgnore && onConflict == ast.CreateReplace { - raise("cannot combine OR REPLACE with IF NOT EXISTS") + raise("Cannot specify both OR REPLACE and IF NOT EXISTS within single create statement") } if onConflict != ast.CreateError || base.OnConflict == "" { if base.OnConflict == "" { @@ -114,7 +114,7 @@ func (tc *transformContext) transformCreateTable(n tnode) ast.CreateInfo { stmt := tc.transformStatement(def.child(4)) sel, ok := stmt.(*ast.SelectStatement) if !ok { - raise("CREATE TABLE AS requires a SELECT statement") + raise("CREATE TABLE AS requires a SELECT clause") } info.Query = sel case "CreateColumnList": @@ -164,7 +164,7 @@ func (tc *transformContext) transformColumnDefinition(n tnode) ast.ColumnDef { } } if col.Type == nil && col.Generated == "" { - raise("Column %s must have a type or be defined as a GENERATED column.", strings.Join(col.Names, ".")) + raise("Column %s must have a type or be defined as a GENERATED column.", optionallyQuoted(strings.Join(col.Names, "."))) } if col.Generated != "" { name := col.Names[len(col.Names)-1] @@ -926,17 +926,22 @@ func (tc *transformContext) transformAlterTableOption(n tnode) ast.AlterInfo { info := &ast.GenericAlterInfo{Kind: "RESET_OPTIONS"} info.SetSpan(alt.span()) // RESET (...) options cannot carry values (a bare NULL passes, - // mirroring upstream's null-constant default) + // mirroring upstream's null-constant default); upstream checks + // the whole list and names the last offending option + offender := "" for _, o := range alt.child(1).parens().listElems() { // RelOption <- RelOptionName RelOptionArgumentOpt? if arg, ok := o.child(1).opt(); ok { // RelOptionArgumentOpt <- '=' DefArg _, def := arg.child(1).sole().choice() if def.name() != "DefArgNull" { - raise("Reset option \"%s\" cannot set any value. Did you mean to use SET?", relOptionName(o.child(0))) + offender = relOptionName(o.child(0)) } } } + if offender != "" { + raise("Reset option \"%s\" cannot set any value. Did you mean to use SET?", offender) + } return info } shapeError(alt, "unknown alter table option") diff --git a/parser/transform_dml.go b/parser/transform_dml.go index fcfeca2..1685578 100644 --- a/parser/transform_dml.go +++ b/parser/transform_dml.go @@ -83,7 +83,7 @@ func (tc *transformContext) transformInsert(n tnode) ast.Stmt { } if oc, ok := n.child(8).opt(); ok { if stmt.OnConflict != nil { - raise("OR REPLACE|IGNORE is not compatible with ON CONFLICT") + raise("You can not provide both OR REPLACE|IGNORE and an ON CONFLICT clause, please remove the first if you want to have more granular control") } stmt.OnConflict = tc.transformOnConflict(oc) } diff --git a/parser/transform_expr.go b/parser/transform_expr.go index 27a2971..f7dac98 100644 --- a/parser/transform_expr.go +++ b/parser/transform_expr.go @@ -229,8 +229,13 @@ func (tc *transformContext) transformComparison(n tnode) ast.Expr { return expr } if len(tails) > 1 { - // upstream bans chained comparisons in the transformer - raise("Chained comparisons (e.g. a < b < c) are not supported") + // upstream bans chained comparisons in the transformer, but only + // after transforming the operands - a deeper operand error (e.g. + // an empty subscript) takes precedence over the chain ban + for _, tail := range tails { + tc.transformBetweenInLike(tail.child(2)) + } + raise("Chained comparisons are not supported, use AND to combine comparisons") } tail := tails[0] _, opAlt := tail.child(0).sole().choice() @@ -954,7 +959,7 @@ func (tc *transformContext) transformSlice(base ast.Expr, n tnode) ast.Expr { endBound, hasEnd := bound.child(1).opt() if !hasEnd { if start == nil { - raise("empty subscript is not allowed") + raise("Empty subscript '[]' is not allowed") } return opExpr(sp, ast.ArrayExtract, base, start) } diff --git a/parser/transform_func.go b/parser/transform_func.go index bf7edc8..6022845 100644 --- a/parser/transform_func.go +++ b/parser/transform_func.go @@ -433,17 +433,18 @@ func (tc *transformContext) transformFrameClause(w *ast.WindowExpression, n tnod case "BetweenFrameExtent": start, startExpr := tc.transformFrameBound(extent.child(1), kind, true) end, endExpr := tc.transformFrameBound(extent.child(3), kind, false) - if end == ast.WindowUnboundedPreceding { - raise("Frame end cannot be UNBOUNDED PRECEDING") - } w.FrameStart, w.StartExpr = start, startExpr w.FrameEnd, w.EndExpr = end, endExpr default: shapeError(extent, "unknown frame extent") } + // upstream validates the start bound before the end bound if w.FrameStart == ast.WindowUnboundedFollowing { raise("Frame start cannot be UNBOUNDED FOLLOWING") } + if w.FrameEnd == ast.WindowUnboundedPreceding { + raise("Frame end cannot be UNBOUNDED PRECEDING") + } if excl, ok := n.child(2).opt(); ok { _, e := excl.child(1).sole().choice() switch e.name() { @@ -1004,6 +1005,18 @@ var intervalFuncs = map[string]string{ "MillenniumKeyword": "to_millennia", } +// intervalRangeSpecifiers spells the IntervalToInterval alternatives the +// way upstream's error message does. +var intervalRangeSpecifiers = map[string]string{ + "YearToMonth": "YEAR TO MONTH", + "DayToHour": "DAY TO HOUR", + "DayToMinute": "DAY TO MINUTE", + "DayToSecond": "DAY TO SECOND", + "HourToMinute": "HOUR TO MINUTE", + "HourToSecond": "HOUR TO SECOND", + "MinuteToSecond": "MINUTE TO SECOND", +} + // IntervalLiteral <- 'INTERVAL' IntervalParameter Interval? func (tc *transformContext) transformIntervalLiteral(n tnode) ast.Expr { sp := n.span() @@ -1031,7 +1044,12 @@ func (tc *transformContext) transformIntervalLiteral(n tnode) ast.Expr { } _, unit := unitNode.sole().choice() if unit.name() == "IntervalToInterval" { - raise("interval range specifiers (e.g. YEAR TO MONTH) are not supported in interval literals") + _, r := unit.sole().choice() + spec, ok := intervalRangeSpecifiers[r.name()] + if !ok { + shapeError(r, "unknown interval range specifier") + } + raise("%s is not supported", spec) } fname, ok := intervalFuncs[unit.name()] if !ok { diff --git a/parser/transform_misc.go b/parser/transform_misc.go index 5eae438..a92ed6b 100644 --- a/parser/transform_misc.go +++ b/parser/transform_misc.go @@ -22,6 +22,7 @@ import ( "strings" "github.com/sqlc-dev/darkwing/ast" + "github.com/sqlc-dev/darkwing/token" ) func init() { @@ -753,6 +754,28 @@ func (tc *transformContext) transformComment(n tnode) ast.Stmt { return stmt } +// optionallyQuoted renders an identifier for an error message the way +// upstream's KeywordHelper::WriteOptionallyQuoted does: wrapped in double +// quotes (embedded quotes doubled) when it is empty, is a keyword, or is +// not a plain lowercase identifier. +func optionallyQuoted(s string) string { + needs := s == "" || token.IsKeyword(s) + for i := 0; i < len(s) && !needs; i++ { + c := s[i] + switch { + case c >= 'a' && c <= 'z' || c == '_': + case c >= '0' && c <= '9': + needs = i == 0 + default: + needs = true + } + } + if !needs { + return s + } + return `"` + strings.ReplaceAll(s, `"`, `""`) + `"` +} + // qualifiedNameFromParts splits a dotted path into catalog/schema/name, // mirroring upstream's QualifiedName::StringToQualifiedName. func qualifiedNameFromParts(parts []string) (catalog, schema, name string) { @@ -764,7 +787,7 @@ func qualifiedNameFromParts(parts []string) (catalog, schema, name string) { case 3: return parts[0], parts[1], parts[2] } - raise("Too many qualifications for name \"%s\"", strings.Join(parts, ".")) + raise("Too many qualifications found - expected [catalog.schema.name] or [schema.name]") return "", "", "" } diff --git a/parser/transform_single.go b/parser/transform_single.go index 95d1241..aa76ee7 100644 --- a/parser/transform_single.go +++ b/parser/transform_single.go @@ -9,7 +9,6 @@ import ( "math" "strconv" "strings" - "unicode/utf8" "github.com/sqlc-dev/darkwing/ast" "github.com/sqlc-dev/darkwing/internal/matcher" @@ -191,9 +190,12 @@ func (tc *transformContext) stringConstant(sp ast.Span, s string, kind matcher.S if strings.IndexByte(unescaped, 0) >= 0 { raise("Null character not permitted in escape string literal") } - if !utf8.ValidString(unescaped) { - raise("Invalid UTF-8 in escape string literal at byte offset %d: invalid unicode codepoint", - invalidUTF8Offset(unescaped)) + if reason, off := analyzeUTF8(unescaped); reason != utf8Valid { + kind := "byte mismatch" + if reason == utf8InvalidUnicode { + kind = "invalid unicode" + } + raise("Invalid UTF-8 in escape string literal at byte offset %d: %s", off, kind) } return constExpr(sp, varcharValue(unescaped)) default: @@ -250,19 +252,6 @@ func unescapeString(s string) string { return sb.String() } -// invalidUTF8Offset finds the first invalid byte offset of a non-UTF-8 -// string (for the escape-literal error message). -func invalidUTF8Offset(s string) int { - for i := 0; i < len(s); { - r, size := utf8.DecodeRuneInString(s[i:]) - if r == utf8.RuneError && size == 1 { - return i - } - i += size - } - return len(s) -} - func isHexDigit(c byte) bool { return c >= '0' && c <= '9' || c >= 'a' && c <= 'f' || c >= 'A' && c <= 'F' } diff --git a/parser/transform_tableref.go b/parser/transform_tableref.go index d1892c1..07103fa 100644 --- a/parser/transform_tableref.go +++ b/parser/transform_tableref.go @@ -511,8 +511,8 @@ func (tc *transformContext) transformNearestJoin(left ast.TableRef, n tnode) ast } if num, ok := alt.child(5).opt(); ok { v, err := strconv.ParseInt(num.number(), 10, 64) - if err != nil { - raise("invalid NEAREST count") + if err != nil || v <= 0 { + raise("NEAREST expects a positive integer literal, got \"%s\"", num.number()) } join.NearestCount = v } @@ -795,7 +795,7 @@ func (tc *transformContext) transformPivotStatement(n tnode) ast.QueryNode { raise("Cannot pivot on constant value \"%s\"", constantText(e)) } if hasSubquery(e) { - raise("Cannot pivot on subquery \"%s\"", constantText(e)) + raise("Cannot pivot on subquery \"%s\"", tc.exprSourceText(e)) } } } @@ -898,6 +898,18 @@ func constantText(e ast.Expr) string { return "?" } +// exprSourceText renders an expression for an error message by slicing +// the original source at its span — a best-effort stand-in for upstream's +// ToString renderer, which darkwing does not have. Falls back to +// constantText when the span is invalid. +func (tc *transformContext) exprSourceText(e ast.Expr) string { + start, end := e.Pos(), e.End() + if start >= 0 && start < end && end <= len(tc.src) { + return tc.src[start:end] + } + return constantText(e) +} + // PivotColumnEntry <- PivotColumnSubquery / PivotValueList / PivotColumnExpression func (tc *transformContext) transformPivotColumnEntry(n tnode) ast.PivotColumn { _, alt := n.sole().choice() diff --git a/parser/utf8.go b/parser/utf8.go new file mode 100644 index 0000000..1ca97f1 --- /dev/null +++ b/parser/utf8.go @@ -0,0 +1,69 @@ +package parser + +// utf8Reason classifies why a byte string is not valid UTF-8 — the port +// of upstream's UnicodeInvalidReason. +type utf8Reason int + +const ( + utf8Valid utf8Reason = iota + // utf8ByteMismatch is a malformed byte sequence (bad start byte or + // missing/invalid continuation byte). + utf8ByteMismatch + // utf8InvalidUnicode is a well-formed sequence encoding an invalid + // codepoint (surrogate or beyond U+10FFFF). + utf8InvalidUnicode +) + +// analyzeUTF8 walks s the way upstream's Utf8Proc::Analyze does and +// returns the first invalid sequence's classification and start offset +// (utf8Valid and -1 when the whole string is valid). Unlike the standard +// library's utf8.ValidString it accepts overlong encodings, matching the +// pinned build's effective behavior. +func analyzeUTF8(s string) (utf8Reason, int) { + for i := 0; i < len(s); { + c := s[i] + if c&0x80 == 0 { + i++ + continue + } + start := i + var cont int + var cp int32 + switch { + case c&0xE0 == 0xC0: + cont, cp = 1, int32(c&0x1F) + case c&0xF0 == 0xE0: + cont, cp = 2, int32(c&0x0F) + case c&0xF8 == 0xF0: + cont, cp = 3, int32(c&0x07) + default: + return utf8ByteMismatch, start + } + i++ + for j := 0; j < cont; j++ { + if i >= len(s) || s[i]&0xC0 != 0x80 { + return utf8ByteMismatch, start + } + cp = cp<<6 | int32(s[i]&0x3F) + i++ + } + if cp > 0x10FFFF || (cp >= 0xD800 && cp <= 0xDFFF) { + return utf8InvalidUnicode, start + } + } + return utf8Valid, -1 +} + +// validateQueryUTF8 raises the pinned build's whole-query UTF-8 error +// when src is not valid UTF-8, positioned at the invalid sequence. +func validateQueryUTF8(src string) *Error { + reason, off := analyzeUTF8(src) + if reason == utf8Valid { + return nil + } + kind := "byte sequence mismatch" + if reason == utf8InvalidUnicode { + kind = "invalid unicode" + } + return errorAt(src, "Invalid UTF-8 in query ("+kind+")", off) +} From 7591c8c5e1e010456d7a9635c18ae0a5ddbe5d64 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 04:49:35 +0000 Subject: [PATCH 2/3] Add milestone 6 benchmarks, fuzz target and parallel-parse race gate - bench_test.go: tokenize + parse throughput on simple/complex/mixed statement workloads, plus upstream's pathological unmatched-parens packrat case as both a benchmark and a fast-failure regression test (19 and 40 parens complete in milliseconds thanks to memoization). - fuzz_test.go: FuzzParse holds Parse to its public invariants on any input - errors are *parser.Error with in-range offset and 1-based line/column, successful parses return non-nil statements whose spans tile the input. Seeds cover the dialect's corners (FROM-first, PIVOT, list comprehensions, dollar quotes, invalid UTF-8, chained comparisons); a plain test keeps the seeds honest in ordinary runs (3-minute local fuzz run: 2.4M execs, no failures). - TestParallelParse hammers the shared compiled engine from concurrent goroutines so the -race dev loop and CI gate the engine's immutability and per-call packrat state. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01QQHouvcnCnGT1MuHvwPJrj --- parser/bench_test.go | 112 ++++++++++++++++++++++++++++++++++++++ parser/fuzz_test.go | 124 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 236 insertions(+) create mode 100644 parser/bench_test.go create mode 100644 parser/fuzz_test.go diff --git a/parser/bench_test.go b/parser/bench_test.go new file mode 100644 index 0000000..682d298 --- /dev/null +++ b/parser/bench_test.go @@ -0,0 +1,112 @@ +// Milestone 6 benchmarks: parse throughput on representative statements +// and upstream's pathological-backtracking case. Upstream's motivating +// packrat example — 19 unmatched '(' going from 10.6 s to 1 ms — is both +// a benchmark and a regression test here. +package parser + +import ( + "context" + "strings" + "testing" + + "github.com/sqlc-dev/darkwing/lexer" +) + +const benchSimpleSelect = "SELECT a, b + 1 FROM t WHERE c = 'x' ORDER BY a LIMIT 10" + +const benchComplexSelect = ` +WITH sales AS ( + SELECT region, product, amount, sold_at + FROM raw_sales + WHERE sold_at >= DATE '2024-01-01' +), ranked AS ( + SELECT region, product, + sum(amount) AS total, + rank() OVER (PARTITION BY region ORDER BY sum(amount) DESC) AS r + FROM sales + GROUP BY ALL +) +SELECT r.region, r.product, r.total, + t.name AS region_name, + CASE WHEN r.total > 1000 THEN 'big' ELSE 'small' END AS bucket +FROM ranked r +JOIN regions t ON r.region = t.id +WHERE r.r <= 3 +QUALIFY row_number() OVER (ORDER BY r.total DESC) <= 100 +ORDER BY ALL` + +// benchMixed exercises the non-SELECT transformers. +var benchMixed = []string{ + "INSERT INTO t (a, b) VALUES (1, 'x'), (2, 'y') ON CONFLICT (a) DO UPDATE SET b = excluded.b RETURNING a", + "UPDATE t SET b = b + 1 FROM u WHERE t.a = u.a", + "DELETE FROM t USING u WHERE t.a = u.a RETURNING *", + "CREATE TABLE IF NOT EXISTS t (a INTEGER PRIMARY KEY, b VARCHAR DEFAULT 'x', c STRUCT(x INT, y MAP(TEXT, DOUBLE[])))", + "COPY t TO 'out.parquet' (FORMAT parquet, COMPRESSION zstd)", + "PIVOT sales ON month IN (jan, feb) USING sum(amount) GROUP BY region", + "SELECT * EXCLUDE (a) REPLACE (b + 1 AS b) FROM t USING SAMPLE 10%", + "ATTACH 'other.db' AS other (READ_ONLY)", + "PREPARE q AS SELECT * FROM t WHERE a = $1 AND b = $name", +} + +func benchmarkParse(b *testing.B, sql string) { + b.Helper() + // prime the engine compile outside the timed loop + if _, err := Parse(context.Background(), strings.NewReader(sql)); err != nil { + b.Fatalf("Parse(%q): %v", sql, err) + } + b.SetBytes(int64(len(sql))) + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + if _, err := parseString(context.Background(), sql); err != nil { + b.Fatal(err) + } + } +} + +func BenchmarkParseSimpleSelect(b *testing.B) { benchmarkParse(b, benchSimpleSelect) } +func BenchmarkParseComplexSelect(b *testing.B) { benchmarkParse(b, benchComplexSelect) } + +func BenchmarkParseMixed(b *testing.B) { + all := strings.Join(benchMixed, ";\n") + ";" + benchmarkParse(b, all) +} + +func BenchmarkTokenize(b *testing.B) { + b.SetBytes(int64(len(benchComplexSelect))) + b.ReportAllocs() + for i := 0; i < b.N; i++ { + if _, err := lexer.Tokenize(benchComplexSelect); err != nil { + b.Fatal(err) + } + } +} + +// unmatchedParens is upstream's pathological backtracking input: without +// packrat memoization each unmatched '(' multiplies the work (10.6 s at +// 19 parens in upstream's pre-memoization measurement). +func unmatchedParens(n int) string { + return "SELECT " + strings.Repeat("(", n) + "1" +} + +func BenchmarkUnmatchedParens19(b *testing.B) { + sql := unmatchedParens(19) + b.ReportAllocs() + for i := 0; i < b.N; i++ { + if _, err := parseString(context.Background(), sql); err == nil { + b.Fatal("expected a syntax error") + } + } +} + +// TestPathologicalBacktracking is the packrat regression gate: well past +// upstream's 19-paren case must still fail fast. If memoization breaks, +// this does not finish. +func TestPathologicalBacktracking(t *testing.T) { + for _, n := range []int{19, 40} { + pe := rejectError(t, unmatchedParens(n)) + if !strings.Contains(pe.Msg, "syntax error") { + t.Errorf("unmatchedParens(%d): unexpected message %q", n, pe.Msg) + } + } +} diff --git a/parser/fuzz_test.go b/parser/fuzz_test.go new file mode 100644 index 0000000..04d557c --- /dev/null +++ b/parser/fuzz_test.go @@ -0,0 +1,124 @@ +// Milestone 6 hardening: native fuzzing of the public Parse entry point +// and a shared-engine parallel parse test (run under -race by the dev +// loop and CI). +package parser + +import ( + "context" + "errors" + "strings" + "sync" + "testing" + + "github.com/sqlc-dev/darkwing/ast" +) + +// checkParseInvariants parses src and enforces what Parse promises for +// any input: no panic other than the controlled internal kinds, errors +// are *parser.Error with sane positions, and on success the statement +// spans tile the input. +func checkParseInvariants(t *testing.T, src string) { + t.Helper() + stmts, err := Parse(context.Background(), strings.NewReader(src)) + if err != nil { + var pe *Error + if !errors.As(err, &pe) { + t.Fatalf("Parse(%q): error is %T (%v), want *parser.Error", src, err, err) + } + if pe.Offset < -1 || pe.Offset > len(src) { + t.Fatalf("Parse(%q): error offset %d out of range", src, pe.Offset) + } + if pe.Line < 1 || pe.Column < 1 { + t.Fatalf("Parse(%q): error position line %d col %d", src, pe.Line, pe.Column) + } + return + } + // spans tile the input: the first statement starts at 0, each next + // one starts where the previous ended, the last ends at len(src) + prev := 0 + for i, stmt := range stmts { + if stmt == nil { + t.Fatalf("Parse(%q): statement %d is nil", src, i) + } + sp := ast.Span{Start: stmt.Pos(), End: stmt.End()} + if sp.Start != prev { + t.Fatalf("Parse(%q): statement %d starts at %d, want %d", src, i, sp.Start, prev) + } + if sp.End < sp.Start || sp.End > len(src) { + t.Fatalf("Parse(%q): statement %d span [%d,%d) out of range", src, i, sp.Start, sp.End) + } + prev = sp.End + } + if len(stmts) > 0 && prev != len(src) { + t.Fatalf("Parse(%q): last statement ends at %d, want %d", src, prev, len(src)) + } +} + +var fuzzSeeds = []string{ + "", + ";;", + "SELECT 1", + "SELECT * FROM t WHERE a = $1 AND b = ? AND c = $name", + "FROM t SELECT x: a + 1", + "WITH RECURSIVE c AS (SELECT 1) SELECT * FROM c UNION ALL BY NAME SELECT 2", + "INSERT OR REPLACE INTO t BY NAME SELECT 1 RETURNING *", + "CREATE TABLE t (a INT PRIMARY KEY, b STRUCT(x INT[], y MAP(TEXT, UNION(i INT, s TEXT))))", + "PIVOT sales ON month USING sum(amount)", + "SELECT [x + 1 FOR x IN [1,2,3] IF x > 1], l[1:3:2], r['k'], s.*.x", + "SELECT interval '2 10' years to months", + "SELECT e'\\x41\\n' || $tag$body$tag$ || 'lit''eral'", + "SELECT a -> '$.b' ->> 'c' FROM j -- comment\n/* block /* nested */ */", + "ATTACH 'f.db' AS d (READ_ONLY); USE d; SET threads = 4;", + "SELECT (((((((1", + "SELCT 1 2 3 (", + "SELECT '\xff'", + "select 1 = 1 = 1", + "[1,2,3][];", +} + +func FuzzParse(f *testing.F) { + for _, seed := range fuzzSeeds { + f.Add(seed) + } + f.Fuzz(func(t *testing.T, src string) { + if len(src) > 1<<14 { + // long adversarial inputs stress the matcher superlinearly; + // throughput is covered by the benchmarks + t.Skip() + } + checkParseInvariants(t, src) + }) +} + +// TestParseInvariantsOnSeeds keeps the fuzz seeds honest in ordinary +// test runs (go test does not execute the fuzz engine). +func TestParseInvariantsOnSeeds(t *testing.T) { + for _, seed := range fuzzSeeds { + checkParseInvariants(t, seed) + } +} + +// TestParallelParse hammers the shared engine from many goroutines — the +// race-clean gate for the immutable compiled grammar and the per-call +// packrat state (meaningful under -race, as the dev loop and CI run it). +func TestParallelParse(t *testing.T) { + inputs := append([]string(nil), fuzzSeeds...) + inputs = append(inputs, benchComplexSelect, unmatchedParens(19)) + var wg sync.WaitGroup + for g := 0; g < 8; g++ { + wg.Add(1) + go func(g int) { + defer wg.Done() + for i := 0; i < 20; i++ { + src := inputs[(g+i)%len(inputs)] + _, err := Parse(context.Background(), strings.NewReader(src)) + var pe *Error + if err != nil && !errors.As(err, &pe) { + t.Errorf("Parse(%q): unexpected error type %T", src, err) + return + } + } + }(g) + } + wg.Wait() +} From bc44d51adfd929bc67324558bd05e4106ede4d70 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 05:26:18 +0000 Subject: [PATCH 3/3] Add cmd/difftest, the mutation differential fuzzer, and fix what it found Milestone 6, differential loop: cmd/difftest mutates corpus seed statements at token-span boundaries (delete, duplicate, swap adjacent, cross-seed replace, truncate, splice) and compares darkwing's verdict - and, for syntax errors, the message first line - against the pinned DuckDB CLI. Deterministic per -seed, parallel oracle workers, bounded by -n or -duration; a nightly GitHub Actions lane runs it (plus a native fuzz pass) while the DuckDB nightly artifact still matches the pin, and skips green once the artifact drifts. Known oracle imprecisions are filtered, not reported: nested-SQL functions whose bind-time inner parse raises Parser Error, bind-time ParserExceptions outside the parse pipeline (the corpus todo category), statements the CLI shell intercepts (column-0 '#' comments, dot commands), and multi-statement mutants where a post-parse error aborts the batch CLI before later statements parse - duckdbsrc.Verdict now records that with PostParseError. Real divergences it caught, all fixed with regression tests: - SyntaxError rendered token spellings through Go %q escaping; upstream puts the raw spelling between plain quotes (embedded quotes and backslashes verbatim). - COLLATE rejected constant collation names; upstream accepts constants via Value::ToString ('x' COLLATE 'nocase', even COLLATE 3) and only fails lookup at bind time, and raises NotImplementedException (post- parse) for other expression shapes, so darkwing now accepts those too. - Number tokens whose text has no value ("2.03.0") raised a parser error; upstream accepts the token and throws InvalidInputException while constructing the value, so darkwing records the raw text as a cast and accepts. - Dollar-quoted string tokens record the rewritten literal's length (upstream behavior pinned by query_location_length in the serialize goldens), which could push a statement span past the end of the input; consumedEnd now clamps so statement spans keep their tiling contract. Validated with ~24k mutants across seven RNG seeds (zero disagreements) and a corpus-wide serialize sweep against the live pinned CLI (25,982 statements, zero mismatches). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01QQHouvcnCnGT1MuHvwPJrj --- .github/workflows/difftest.yml | 47 +++++ CLAUDE.md | 4 + README.md | 42 ++-- cmd/difftest/main.go | 355 ++++++++++++++++++++++++++++++++ internal/duckdbsrc/duckdbsrc.go | 31 ++- internal/matcher/engine.go | 5 +- parser/errors_test.go | 4 + parser/fuzz_test.go | 1 + parser/misc_test.go | 39 ++++ parser/parser.go | 9 +- parser/transform_expr.go | 81 +++++++- parser/transform_single.go | 22 +- 12 files changed, 607 insertions(+), 33 deletions(-) create mode 100644 .github/workflows/difftest.yml create mode 100644 cmd/difftest/main.go diff --git a/.github/workflows/difftest.yml b/.github/workflows/difftest.yml new file mode 100644 index 0000000..b6e0d6c --- /dev/null +++ b/.github/workflows/difftest.yml @@ -0,0 +1,47 @@ +name: difftest + +# The milestone-6 cron lane: differential fuzzing of darkwing against the +# pinned DuckDB CLI, plus a native fuzz run of the public Parse API. The +# nightly artifact only matches the pin until upstream moves it, so the +# lane verifies the version first and skips (green) once it drifts — +# advancing the pin is its own workflow (cmd/regenerate), not this one. + +on: + schedule: + - cron: "17 5 * * *" + workflow_dispatch: + +permissions: + contents: read + +jobs: + difftest: + runs-on: ubuntu-latest + timeout-minutes: 60 + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + - name: Download the nightly DuckDB CLI + run: | + curl -sSL -o duckdb.zip https://artifacts.duckdb.org/latest/duckdb-binaries-linux-amd64.zip + unzip -o -q duckdb.zip duckdb_cli-linux-amd64.zip + unzip -o -q duckdb_cli-linux-amd64.zip + ./duckdb --version + - name: Check the nightly against the pin + id: pin + run: | + pin=$(sed -n 's/.*PinnedVersion = "\(.*\)"/\1/p' internal/duckdbsrc/duckdbsrc.go) + version=$(./duckdb --version) + if echo "$version" | grep -qF "$pin"; then + echo "matches=true" >> "$GITHUB_OUTPUT" + else + echo "matches=false" >> "$GITHUB_OUTPUT" + echo "nightly is $version, pin is $pin - skipping until the pin advances" + fi + - name: Differential fuzzing vs the pinned CLI + if: steps.pin.outputs.matches == 'true' + run: DARKWING_DUCKDB=$PWD/duckdb go run ./cmd/difftest -seed $(date +%s) -duration 20m + - name: Native fuzzing of Parse + run: go test ./parser -run '^$' -fuzz FuzzParse -fuzztime 10m diff --git a/CLAUDE.md b/CLAUDE.md index b35d5bd..ffb3fc9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -77,6 +77,10 @@ Two corpus gates run under `go test ./parser`: pinned DuckDB binary parses it. Since milestone 3 the classification runs the full Parse pipeline, so transformer-raised Parser Errors count alongside the matcher's syntax errors. +- **Error fidelity** (`TestCorpusErrorMessages`, since milestone 6): + every rejection renders the oracle's recorded first line verbatim. + Positions and messages the corpus can't pin are covered by unit tests + in `parser/errors_test.go`. - **Tree shape** (`TestSerializeGoldens`): `internal/serialize` output must match vendored `json_serialize_sql` goldens for a corpus sample (see `parser/testdata/serialize/README.md`). diff --git a/README.md b/README.md index 7939a27..4a28c5b 100644 --- a/README.md +++ b/README.md @@ -20,28 +20,34 @@ loader, and matcher (with packrat memoization) — to Go. ## Status -Milestone 2 (corpus + accept/reject conformance) is in place: the corpus -under `parser/testdata/` is extracted from DuckDB's own test suite and -classified by the pinned DuckDB CLI (`cmd/regenerate`); `go test ./parser` -enforces *darkwing accepts iff pinned DuckDB parses*, with remaining -disagreements tracked in todo metadata (`cmd/next-test`). - -Milestone 1 (engine) is complete: - -- `token/` — token kinds and DuckDB's keyword categories -- `lexer/` — port of the parsing tokenizer -- `internal/grammar/` — vendored grammar (pinned upstream commit recorded in - `internal/grammar/README.md`) and the grammar loader -- `internal/matcher/` — matcher tree, packrat memoization, rule overrides, - furthest-failure error reporting -- `cmd/debug-parse` — dump tokens or the raw parse tree for SQL on the - command line - -Next: the typed AST and transformer core (milestone 3). +Milestones 1–6 are complete: the engine (tokenizer, grammar loader, +matcher with packrat memoization), the corpus conformance gates, the full +typed AST with a transformer for every statement, and the hardening pass. +`go test ./parser` enforces, corpus-wide against the pinned DuckDB CLI: + +- **Accept/reject** — darkwing accepts a statement iff the pinned binary + parses it (`TestCorpus`), including transformer-raised parser errors. +- **Error fidelity** — every rejection renders the oracle's error message + verbatim (`TestCorpusErrorMessages`), with positions pinned by unit + tests. +- **Tree shape** — `internal/serialize` output matches vendored + `json_serialize_sql` goldens for the SELECT subset + (`TestSerializeGoldens`), with AST snapshots covering the rest. + +Hardening: `FuzzParse` holds the public API to its invariants on +arbitrary input, benchmarks pin parse throughput and upstream's +pathological-backtracking case (19 unmatched parens: milliseconds, thanks +to packrat), and `cmd/difftest` mutates corpus seeds and diffs darkwing's +verdicts against the pinned CLI (run nightly in CI while the nightly +artifact matches the pin). + +Next: advancing the pin to the v2.0.0 tag when it lands (milestone 7), +then sqlc integration. ``` $ go run ./cmd/debug-parse 'SELECT 1' $ go run ./cmd/debug-parse -tokens 'SELECT * FROM t' +$ go run ./cmd/debug-parse -ast 'FROM t SELECT x' ``` ## License diff --git a/cmd/difftest/main.go b/cmd/difftest/main.go new file mode 100644 index 0000000..02db11c --- /dev/null +++ b/cmd/difftest/main.go @@ -0,0 +1,355 @@ +// Command difftest is the milestone-6 differential fuzzer: it mutates +// corpus seed statements at token boundaries and compares darkwing's +// accept/reject verdict (and, for syntax errors, the message) against the +// pinned DuckDB CLI — the strongest check on the matcher's ordered-choice +// fidelity, cheap to run for hours in a cron lane. +// +// Usage: +// +// difftest [-n 500] [-duration 0] [-seed 1] [-jobs N] [-corpus dir] +// +// Deterministic for a given -seed/-corpus pair (a fixed -n; -duration +// bounds wall clock instead). Exit status 1 when any disagreement is +// found. The oracle binary is located via $DARKWING_DUCKDB (see +// internal/duckdbsrc). +// +// Verdict policy mirrors the corpus harness: darkwing must reject iff +// the oracle raises a Parser Error. Two known oracle imprecisions are +// filtered rather than reported: mutants that mention the nested-SQL +// functions (query, query_table, json_serialize_sql, ...) whose +// bind-time inner parse also raises Parser Error, and oracle rejects +// whose message is not a syntax error (bind-time ParserException raised +// outside the parse pipeline — the corpus todo category). Oracle +// timeouts and crashes leave a mutant unverified and are skipped. +package main + +import ( + "context" + "errors" + "flag" + "fmt" + "io/fs" + "math/rand" + "os" + "path/filepath" + "runtime" + "strings" + "sync" + "sync/atomic" + "time" + + "github.com/sqlc-dev/darkwing/internal/duckdbsrc" + "github.com/sqlc-dev/darkwing/internal/testfile" + "github.com/sqlc-dev/darkwing/lexer" + "github.com/sqlc-dev/darkwing/parser" + "github.com/sqlc-dev/darkwing/token" +) + +func main() { + n := flag.Int("n", 500, "number of mutants to test (ignored with -duration)") + duration := flag.Duration("duration", 0, "run for this long instead of a fixed -n") + seed := flag.Int64("seed", 1, "mutation RNG seed") + jobs := flag.Int("jobs", runtime.NumCPU(), "concurrent oracle processes") + corpusDir := flag.String("corpus", filepath.Join("parser", "testdata", "corpus"), "corpus directory for seed statements") + maxPrint := flag.Int("max-print", 25, "max disagreements to print in detail") + verbose := flag.Bool("v", false, "print every mutant tested") + flag.Parse() + + bin, err := duckdbsrc.Find() + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + seeds, err := loadSeeds(*corpusDir) + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + if len(seeds) == 0 { + fmt.Fprintf(os.Stderr, "no seed statements under %s (run cmd/regenerate first)\n", *corpusDir) + os.Exit(1) + } + fmt.Printf("difftest: %d seeds, seed=%d, jobs=%d\n", len(seeds), *seed, *jobs) + + // one generator goroutine keeps mutation deterministic for a given + // -seed; workers race only for oracle execution + mutants := make(chan string, *jobs) + go func() { + defer close(mutants) + rng := rand.New(rand.NewSource(*seed)) + deadline := time.Time{} + if *duration > 0 { + deadline = time.Now().Add(*duration) + } + for i := 0; *duration > 0 || i < *n; i++ { + if !deadline.IsZero() && time.Now().After(deadline) { + return + } + mutants <- mutate(rng, seeds) + } + }() + + var ( + mu sync.Mutex + disagreements int + tested atomic.Int64 + unverified atomic.Int64 + softSkipped atomic.Int64 + ) + var wg sync.WaitGroup + for w := 0; w < *jobs; w++ { + wg.Add(1) + go func() { + defer wg.Done() + oracle := &duckdbsrc.Oracle{Binary: bin} + for sql := range mutants { + if nestedParseSQL(sql) || shellArtifact(sql) { + softSkipped.Add(1) + continue + } + verdict, err := oracle.Run(sql) + if err != nil { + fmt.Fprintf(os.Stderr, "oracle: %v\n", err) + os.Exit(1) + } + if verdict.TimedOut || verdict.Crashed { + unverified.Add(1) + continue + } + got := classify(sql) + if !verdict.Reject && got.reject && verdict.PostParseError && multiStatement(sql) { + // the batch-mode CLI stops at the first failing + // statement: a post-parse error on an earlier + // statement leaves the rest unparsed, so the + // oracle's accept does not cover darkwing's reject + unverified.Add(1) + continue + } + tested.Add(1) + if *verbose { + fmt.Printf("mutant: %q oracle-reject=%v darkwing-reject=%v\n", sql, verdict.Reject, got.reject) + } + if diff := compare(verdict, got); diff != "" { + if verdict.Reject && !got.reject && !isSyntaxError(verdict.Error) { + // bind-time ParserException outside the parse + // pipeline: the corpus todo category, not a bug + softSkipped.Add(1) + continue + } + mu.Lock() + disagreements++ + if disagreements <= *maxPrint { + fmt.Printf("DISAGREEMENT (%s)\nSQL: %s\noracle: %s\ndarkwing: %s\n\n", + diff, sql, describe(verdict.Reject, verdict.Error), describe(got.reject, got.detail)) + } + mu.Unlock() + } + } + }() + } + wg.Wait() + + fmt.Printf("tested %d mutants: %d disagreements (%d unverified, %d skipped as bind-time/nested-parse)\n", + tested.Load(), disagreements, unverified.Load(), softSkipped.Load()) + if disagreements > 0 { + os.Exit(1) + } +} + +// loadSeeds collects every corpus statement that is not skip-listed — +// including must-rejects, whose mutants probe the error paths. +func loadSeeds(root string) ([]string, error) { + var seeds []string + err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() || !strings.HasSuffix(path, ".test") { + return nil + } + file, err := testfile.Read(path) + if err != nil { + return err + } + meta, err := testfile.ReadMetadata(path) + if err != nil { + return err + } + for i := range file.Cases { + c := &file.Cases[i] + if _, skip := meta.Skip[c.Key()]; skip { + continue + } + if _, ok := lexer.Tokenize(c.SQL); ok == nil { + seeds = append(seeds, c.SQL) + } + } + return nil + }) + return seeds, err +} + +// verdict mirrors the corpus harness's classification: tokenizer, +// matcher and transformer parse errors are rejects. +type verdict struct { + reject bool + detail string +} + +func classify(sql string) (v verdict) { + defer func() { + if r := recover(); r != nil { + v = verdict{reject: false, detail: fmt.Sprintf("transformer panic: %v", r)} + } + }() + _, err := parser.Parse(context.Background(), strings.NewReader(sql)) + if err == nil { + return verdict{} + } + var pe *parser.Error + if errors.As(err, &pe) { + return verdict{reject: true, detail: pe.Error()} + } + return verdict{reject: false, detail: "internal error: " + err.Error()} +} + +func isSyntaxError(msg string) bool { + return strings.HasPrefix(msg, "Parser Error: syntax error at or near") +} + +// compare returns a non-empty disagreement kind when darkwing diverges +// from the oracle on the mutant. +func compare(oracle duckdbsrc.Verdict, got verdict) string { + if !got.reject && got.detail != "" { + return "internal error" + } + if oracle.Reject != got.reject { + if oracle.Reject { + return "oracle rejects, darkwing accepts" + } + return "darkwing rejects, oracle accepts" + } + // message fidelity: pinned only for syntax errors — a bind-time + // oracle reject can coincide with a darkwing reject for a different + // (legitimate) reason + if oracle.Reject && isSyntaxError(oracle.Error) { + // the oracle records the trimmed first line of a message; a token + // spelling with an embedded newline (unterminated dollar quote) + // spills across lines, so darkwing's line is trimmed to match + firstLine, _, _ := strings.Cut(got.detail, "\n") + if strings.TrimSpace(firstLine) != oracle.Error { + return "message mismatch" + } + } + return "" +} + +func describe(reject bool, detail string) string { + if !reject { + if detail != "" { + return "accept (" + detail + ")" + } + return "accept" + } + return "reject (" + detail + ")" +} + +// shellArtifact reports mutants the CLI shell intercepts before the +// parser sees them: a line starting with '#' in column 0 is a shell +// comment (an indented '#' reaches the parser as an operator) and a +// line starting with '.' is a dot command. +func shellArtifact(sql string) bool { + for _, line := range strings.Split(sql, "\n") { + if strings.HasPrefix(line, "#") || strings.HasPrefix(line, ".") { + return true + } + } + return false +} + +// multiStatement reports whether sql contains a statement separator with +// content after it (a ';' inside a string or comment also matches — the +// check only widens the unverified skip, never a reported disagreement). +func multiStatement(sql string) bool { + return strings.Contains(strings.TrimRight(sql, "; \t\r\n"), ";") +} + +// nestedParseSQL reports statements that mention the functions whose +// arguments the oracle parses as SQL at bind time — their Parser Errors +// come from the inner parse the grammar never sees. +func nestedParseSQL(sql string) bool { + l := strings.ToLower(sql) + for _, fn := range []string{"query", "query_table", "json_serialize_sql", "nextval", "currval"} { + if strings.Contains(l, fn) { + return true + } + } + return false +} + +// mutate produces one mutant from the seed pool. All mutations cut and +// splice the original source at token span boundaries, preserving the +// seed's exact spelling (strings, comments, whitespace). +func mutate(rng *rand.Rand, seeds []string) string { + src := seeds[rng.Intn(len(seeds))] + toks := realTokens(src) + if len(toks) == 0 { + return src + } + switch rng.Intn(6) { + case 0: // delete a token + t := toks[rng.Intn(len(toks))] + return src[:t.Span.Start] + src[t.Span.End:] + case 1: // duplicate a token + t := toks[rng.Intn(len(toks))] + return src[:t.Span.End] + " " + src[t.Span.Start:t.Span.End] + src[t.Span.End:] + case 2: // swap two adjacent tokens + if len(toks) < 2 { + return src + } + i := rng.Intn(len(toks) - 1) + a, b := toks[i], toks[i+1] + return src[:a.Span.Start] + src[b.Span.Start:b.Span.End] + src[a.Span.End:b.Span.Start] + + src[a.Span.Start:a.Span.End] + src[b.Span.End:] + case 3: // replace a token with one drawn from another seed + t := toks[rng.Intn(len(toks))] + other := seeds[rng.Intn(len(seeds))] + otherToks := realTokens(other) + if len(otherToks) == 0 { + return src + } + o := otherToks[rng.Intn(len(otherToks))] + return src[:t.Span.Start] + other[o.Span.Start:o.Span.End] + src[t.Span.End:] + case 4: // truncate at a token boundary + t := toks[rng.Intn(len(toks))] + return src[:t.Span.Start] + default: // splice a prefix of one seed onto a suffix of another + other := seeds[rng.Intn(len(seeds))] + otherToks := realTokens(other) + if len(otherToks) == 0 { + return src + } + t := toks[rng.Intn(len(toks))] + o := otherToks[rng.Intn(len(otherToks))] + return src[:t.Span.End] + " " + other[o.Span.Start:] + } +} + +// realTokens tokenizes src, drops the end-of-input sentinel, and clamps +// spans to the source (dollar-quoted string tokens record the rewritten +// literal's length, which can overrun the source extent). +func realTokens(src string) []token.Token { + toks, err := lexer.Tokenize(src) + if err != nil { + return nil + } + for len(toks) > 0 && toks[len(toks)-1].Kind == token.EndOfInput { + toks = toks[:len(toks)-1] + } + for i := range toks { + if toks[i].Span.End > len(src) { + toks[i].Span.End = len(src) + } + } + return toks +} diff --git a/internal/duckdbsrc/duckdbsrc.go b/internal/duckdbsrc/duckdbsrc.go index fd7daad..634fd93 100644 --- a/internal/duckdbsrc/duckdbsrc.go +++ b/internal/duckdbsrc/duckdbsrc.go @@ -11,6 +11,7 @@ import ( "fmt" "os" "os/exec" + "regexp" "strings" "time" ) @@ -79,6 +80,12 @@ type Verdict struct { // Crashed records that the CLI died before printing a verdict; like // TimedOut, the statement is unverified. Crashed bool + // PostParseError records that the CLI reported a non-parser error + // (Binder Error, Catalog Error, ...). Still must-accept — but the + // batch-mode CLI stops at the first failing statement, so for + // multi-statement input everything after that statement went + // unparsed and the accept verdict does not cover it. + PostParseError bool } // Oracle runs statements through the pinned CLI against :memory:. @@ -126,6 +133,7 @@ func (o *Oracle) Run(sql string) (Verdict, error) { // execution ran long: parsing succeeded return Verdict{TimedOut: true}, nil } + postParse := hasPostParseError(stderr.String()) if err != nil { var exitErr *exec.ExitError if errors.As(err, &exitErr) { @@ -137,11 +145,30 @@ func (o *Oracle) Run(sql string) (Verdict, error) { } // plain error exit with a non-parser error on stderr: // post-parse failure, must-accept - return Verdict{}, nil + return Verdict{PostParseError: postParse}, nil } return Verdict{}, fmt.Errorf("running oracle: %w", err) } - return Verdict{}, nil + return Verdict{PostParseError: postParse}, nil +} + +// errorLine matches the CLI's non-parser error headers ("Binder Error:", +// "Catalog Error: ", "Invalid Input Error: ", ...). +var errorLine = regexp.MustCompile(`^[A-Za-z][A-Za-z ]*Error: `) + +// hasPostParseError reports whether stderr carries a non-parser error +// line. +func hasPostParseError(stderr string) bool { + for _, line := range strings.Split(stderr, "\n") { + line = strings.TrimSpace(line) + if strings.HasPrefix(line, "Parser Error") { + continue + } + if errorLine.MatchString(line) { + return true + } + } + return false } // classify scans CLI stderr for the first error line. Only "Parser Error" diff --git a/internal/matcher/engine.go b/internal/matcher/engine.go index 416a0ee..6ee2e86 100644 --- a/internal/matcher/engine.go +++ b/internal/matcher/engine.go @@ -17,7 +17,10 @@ type SyntaxError struct { } func (e *SyntaxError) Error() string { - return fmt.Sprintf("syntax error at or near %q", e.Token.Text) + // the token's raw spelling goes between plain double quotes with no + // escaping, exactly as upstream renders it (embedded quotes, + // backslashes and newlines stay verbatim) + return `syntax error at or near "` + e.Token.Text + `"` } // MatchError is a non-positional matching error, mirroring the diff --git a/parser/errors_test.go b/parser/errors_test.go index 69c171c..d11f9b8 100644 --- a/parser/errors_test.go +++ b/parser/errors_test.go @@ -152,6 +152,10 @@ func TestErrorMessages(t *testing.T) { // a deeper operand error beats the chained-comparison ban {"SELECT 1 < 2 < [3][]", "Parser Error: Empty subscript '[]' is not allowed"}, {"SELECT 1 < 2 < 3", "Parser Error: Chained comparisons are not supported, use AND to combine comparisons"}, + // the offending token's raw spelling goes between plain quotes + // with no escaping (found by cmd/difftest) + {`TABLE ALTER "other name" RENAME`, `Parser Error: syntax error at or near ""other name""`}, + {`SELECT E'\\' = '\' '\';`, `Parser Error: syntax error at or near "'\'"`}, } for _, tt := range tests { pe := rejectError(t, tt.sql) diff --git a/parser/fuzz_test.go b/parser/fuzz_test.go index 04d557c..8dfbeb6 100644 --- a/parser/fuzz_test.go +++ b/parser/fuzz_test.go @@ -67,6 +67,7 @@ var fuzzSeeds = []string{ "SELECT [x + 1 FOR x IN [1,2,3] IF x > 1], l[1:3:2], r['k'], s.*.x", "SELECT interval '2 10' years to months", "SELECT e'\\x41\\n' || $tag$body$tag$ || 'lit''eral'", + "SELECT $$'''$$", // rewritten literal longer than its source extent "SELECT a -> '$.b' ->> 'c' FROM j -- comment\n/* block /* nested */ */", "ATTACH 'f.db' AS d (READ_ONLY); USE d; SET threads = 4;", "SELECT (((((((1", diff --git a/parser/misc_test.go b/parser/misc_test.go index 0db456b..5d4a33a 100644 --- a/parser/misc_test.go +++ b/parser/misc_test.go @@ -469,3 +469,42 @@ func TestSequenceOptionChecks(t *testing.T) { } parseOne(t, "CREATE SEQUENCE s INCREMENT 2 MINVALUE 0 MAXVALUE 100 START 4 CYCLE") } + +// TestCollateNames pins the collationName port: identifier chains join +// with '.', constants render via Value::ToString, and anything else — +// where upstream throws NotImplementedException at bind time — still +// parses (found by cmd/difftest). +func TestCollateNames(t *testing.T) { + sel := func(sql string) *ast.CollateExpression { + t.Helper() + stmt := parseOne(t, sql).(*ast.SelectStatement) + node := stmt.Node.(*ast.SelectNode) + return node.SelectList[0].(*ast.CollateExpression) + } + for _, tt := range []struct{ sql, collation string }{ + {"SELECT 'x' COLLATE nocase", "nocase"}, + {"SELECT 'x' COLLATE a.b", "a.b"}, + {"SELECT 'x' COLLATE 'lit'", "lit"}, + {"SELECT 'x' COLLATE NULL", "NULL"}, + {"SELECT 'x' COLLATE 3.5", "3.5"}, + {"SELECT 'x' COLLATE true", "true"}, + } { + if c := sel(tt.sql); c.Collation != tt.collation { + t.Errorf("%s: collation = %q, want %q", tt.sql, c.Collation, tt.collation) + } + } + // upstream: NotImplementedException (post-parse), so these parse + parseOne(t, "SELECT strpos('HELLO WORLD' COLLATE NOCASE 'o w')") + parseOne(t, "SELECT 'x' COLLATE $1") +} + +// TestUnconvertibleNumberLiterals: a number token whose text has no +// value ("2.03.0") still parses — upstream throws InvalidInputException +// only while constructing the value, which is post-parse under the +// oracle's classification (found by cmd/difftest). +func TestUnconvertibleNumberLiterals(t *testing.T) { + parseOne(t, "SELECT 2.03.0") + parseOne(t, "SELECT list_position([1.0,2.03.0,]::varchar[], 'a')") + parseOne(t, "SELECT 12.5.5.5") + parseOne(t, "SELECT 999999999999999999999999999999999999999999.1.1") +} diff --git a/parser/parser.go b/parser/parser.go index cca23d3..7681156 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -155,12 +155,19 @@ func parseString(ctx context.Context, src string) (stmts []ast.Stmt, err error) // consumedEnd returns the byte offset just past the tokens consumed by // the match from pos to newPos (excluding the end-of-input sentinel). +// Dollar-quoted string tokens record the rewritten literal's length +// rather than the source extent (upstream behavior, pinned by the +// serialize goldens' query_location_length), so the end is clamped to +// keep statement spans inside the input they promise to tile. func consumedEnd(tokens []token.Token, pos, newPos, srcLen int) int { for i := newPos - 1; i >= pos; i-- { if tokens[i].Kind == token.EndOfInput { continue } - return tokens[i].Span.End + if end := tokens[i].Span.End; end < srcLen { + return end + } + return srcLen } return srcLen } diff --git a/parser/transform_expr.go b/parser/transform_expr.go index f7dac98..e99e86f 100644 --- a/parser/transform_expr.go +++ b/parser/transform_expr.go @@ -6,6 +6,8 @@ package parser import ( + "math/big" + "strconv" "strings" "github.com/sqlc-dev/darkwing/ast" @@ -710,7 +712,7 @@ func (tc *transformContext) transformCollate(n tnode) ast.Expr { expr := tc.transformAtTimeZone(n.child(0)) for _, tail := range n.child(1).repeat() { rhs := tc.transformAtTimeZone(tail.child(1)) - collation := collationName(rhs) + collation := tc.collationName(rhs) c := &ast.CollateExpression{Child: expr, Collation: collation} c.SetSpan(ast.Span{Start: n.span().Start, End: tail.span().End}) expr = c @@ -719,13 +721,78 @@ func (tc *transformContext) transformCollate(n tnode) ast.Expr { } // collationName extracts the collation name from the right-hand -// expression of COLLATE (a bare identifier chain). -func collationName(e ast.Expr) string { - ref, ok := e.(*ast.ColumnRefExpression) - if !ok { - raise("COLLATE expects a collation name") +// expression of COLLATE: an identifier chain joined with '.', or a +// constant rendered with Value::ToString semantics ('x' COLLATE 'nocase' +// and even COLLATE 3 parse; the name fails lookup at bind time). For any +// other expression upstream throws NotImplementedException ("Unexpected +// expression encountered for collate, ...") — a post-parse error under +// the oracle's classification — so darkwing accepts and records the +// source spelling. +func (tc *transformContext) collationName(e ast.Expr) string { + switch v := e.(type) { + case *ast.ColumnRefExpression: + return strings.Join(v.ColumnNames, ".") + case *ast.ConstantExpression: + return valueString(v.Value) + } + return tc.exprSourceText(e) +} + +// valueString ports Value::ToString for the constants the transformer +// produces. +func valueString(v ast.Value) string { + if v.IsNull { + return "NULL" + } + switch v.Kind { + case ast.ValueBool: + if v.Bool { + return "true" + } + return "false" + case ast.ValueInt64: + if v.Type.ID == "DECIMAL" { + return decimalValueString(strconv.FormatInt(v.Int64, 10), v.Type.Scale) + } + return strconv.FormatInt(v.Int64, 10) + case ast.ValueHugeint: + s := hugeintString(v.Hugeint) + if v.Type.ID == "DECIMAL" { + return decimalValueString(s, v.Type.Scale) + } + return s + case ast.ValueDouble: + return strconv.FormatFloat(v.Float64, 'g', -1, 64) + case ast.ValueString: + return v.Str } - return strings.Join(ref.ColumnNames, ".") + return "" +} + +// hugeintString renders the 128-bit integer in decimal. +func hugeintString(h ast.Hugeint) string { + n := new(big.Int).SetInt64(h.Upper) + n.Lsh(n, 64) + n.Add(n, new(big.Int).SetUint64(h.Lower)) + return n.String() +} + +// decimalValueString inserts the decimal point into an unscaled integer +// rendering, Decimal::ToString-style ("35" scale 1 -> "3.5", "5" scale 2 +// -> "0.05"). +func decimalValueString(digits string, scale int) string { + if scale <= 0 { + return digits + } + sign := "" + if strings.HasPrefix(digits, "-") { + sign, digits = "-", digits[1:] + } + for len(digits) <= scale { + digits = "0" + digits + } + cut := len(digits) - scale + return sign + digits[:cut] + "." + digits[cut:] } // AtTimeZoneExpression <- PrefixExpression AtTimeZoneExpressionTail* diff --git a/parser/transform_single.go b/parser/transform_single.go index aa76ee7..81f1875 100644 --- a/parser/transform_single.go +++ b/parser/transform_single.go @@ -6,6 +6,7 @@ package parser import ( "errors" + "fmt" "math" "strconv" "strings" @@ -295,13 +296,26 @@ func integerTypeFor(v int64) string { // INTEGER/BIGINT/HUGEINT (then DOUBLE), plain decimals become DECIMAL // with derived width/scale (up to width 38), exponent forms become // DOUBLE. +// unconvertibleNumber stands in for a number literal whose text has no +// value (e.g. "2.03.0", tokenized as a single number): upstream accepts +// the token and throws InvalidInputException while constructing the +// value ("Could not convert string ... to DECIMAL(5,1)") — post-parse +// under the oracle's classification — so darkwing records the raw text +// as a cast to the derived type and accepts (found by cmd/difftest). +func unconvertibleNumber(sp ast.Span, text, typeID string) ast.Expr { + child := constExpr(invalidSpan, varcharValue(text)) + cast := &ast.CastExpression{Child: child, ResolvedType: typeID} + cast.SetSpan(sp) + return cast +} + func numberConstant(sp ast.Span, text string) ast.Expr { clean := strings.ReplaceAll(text, "_", "") lower := strings.ToLower(clean) if strings.ContainsAny(lower, "e") { f, err := parseDouble(clean) if err != nil { - raise("invalid number literal \"%s\"", text) + return unconvertibleNumber(sp, clean, "DOUBLE") } return constExpr(sp, ast.Value{Type: ast.LogicalType{ID: "DOUBLE"}, Kind: ast.ValueDouble, Float64: f}) } @@ -320,7 +334,7 @@ func numberConstant(sp ast.Span, text string) ast.Expr { } f, err := parseDouble(clean) if err != nil { - raise("invalid number literal \"%s\"", text) + return unconvertibleNumber(sp, clean, "DOUBLE") } return constExpr(sp, ast.Value{Type: ast.LogicalType{ID: "DOUBLE"}, Kind: ast.ValueDouble, Float64: f}) } @@ -351,7 +365,7 @@ func decimalConstant(sp ast.Span, clean string, dot int) ast.Expr { if width > 38 { f, err := parseDouble(clean) if err != nil { - raise("invalid number literal \"%s\"", clean) + return unconvertibleNumber(sp, clean, "DOUBLE") } return constExpr(sp, ast.Value{Type: ast.LogicalType{ID: "DOUBLE"}, Kind: ast.ValueDouble, Float64: f}) } @@ -365,7 +379,7 @@ func decimalConstant(sp ast.Span, clean string, dot int) ast.Expr { } h, ok := parseHugeint(unscaled) if !ok { - raise("invalid number literal \"%s\"", clean) + return unconvertibleNumber(sp, clean, fmt.Sprintf("DECIMAL(%d,%d)", width, scale)) } return constExpr(sp, ast.Value{Type: typ, Kind: ast.ValueHugeint, Hugeint: h}) }