diff --git a/apps/cli-go/pkg/parser/state.go b/apps/cli-go/pkg/parser/state.go index 47775390d1..0a4bce6662 100644 --- a/apps/cli-go/pkg/parser/state.go +++ b/apps/cli-go/pkg/parser/state.go @@ -26,7 +26,12 @@ type ReadyState struct{} func (s *ReadyState) Next(r rune, data []byte) State { switch r { case '$': + // A $ after an identifier rune continues the identifier (pending$$foo$), not a + // dollar quote. A digit counts too (1$$), unlike PostgreSQL; valid SQL never has that. offset := len(data) - utf8.RuneLen(r) + if hasIdentifierRuneBefore(data, offset) { + return s + } return &TagState{offset: offset} case '\'': fallthrough @@ -42,42 +47,85 @@ func (s *ReadyState) Next(r rune, data []byte) State { // Emit token return nil case '(': - return &AtomicState{prev: s, delimiter: []byte{')'}} + return &ParenState{prev: s} case 'c': fallthrough case 'C': if isBeginAtomic(data) { - return &AtomicState{prev: s, delimiter: []byte(END_ATOMIC)} + return &AtomicState{prev: s, statementStart: len(data)} } } return s } func isBeginAtomic(data []byte) bool { - offset := len(data) - len(BEGIN_ATOMIC) - if offset < 0 || !strings.EqualFold(string(data[offset:]), BEGIN_ATOMIC) { + if !endsWithKeyword(data, BEGIN_ATOMIC) { return false } - if offset > 0 { - r, _ := utf8.DecodeLastRune(data[:offset]) - if isIdentifierRune(r) { - return false - } - } - prefix := bytes.TrimRightFunc(data[:offset], unicode.IsSpace) - offset = len(prefix) - len("BEGIN") - if offset < 0 || !strings.EqualFold(string(prefix[offset:]), "BEGIN") { + prefix := bytes.TrimRight(data[:len(data)-len(BEGIN_ATOMIC)], sqlWhitespace) + return endsWithKeyword(prefix, "BEGIN") +} + +// PostgreSQL's scan.l treats every byte at or above 0x80 as an identifier/dollar-tag +// character (ident_cont/dolq_cont), whatever its Unicode category. +func isIdentifierRune(r rune) bool { + return r >= utf8.RuneSelf || unicode.IsLetter(r) || unicode.IsDigit(r) || r == '_' || r == '$' +} + +func hasIdentifierRuneBefore(data []byte, offset int) bool { + if offset <= 0 { return false } - if offset == 0 { - return true + r, _ := utf8.DecodeLastRune(data[:offset]) + return isIdentifierRune(r) +} + +func endsWithKeyword(data []byte, keyword string) bool { + offset := len(data) - len(keyword) + if offset < 0 || !strings.EqualFold(string(data[offset:]), keyword) { + return false } - r, _ := utf8.DecodeLastRune(prefix[:offset]) - return !isIdentifierRune(r) + return !hasIdentifierRuneBefore(data, offset) } -func isIdentifierRune(r rune) bool { - return unicode.IsLetter(r) || unicode.IsDigit(r) || r == '_' || r == '$' +const sqlWhitespace = " \t\n\r\f\v" + +func isSqlWhitespace(b byte) bool { + return bytes.IndexByte([]byte(sqlWhitespace), b) >= 0 +} + +func isCommentsAndWhitespace(text []byte) bool { + for i := 0; i < len(text); { + switch { + case isSqlWhitespace(text[i]): + i++ + case bytes.HasPrefix(text[i:], []byte("--")): + newline := bytes.IndexByte(text[i+2:], '\n') + if newline == -1 { + return true + } + i += 2 + newline + 1 + case bytes.HasPrefix(text[i:], []byte("/*")): + // Match BlockState's sliding-window scan so both agree on overlapping delimiters. + depth := 1 + i += 2 + for i < len(text) && depth > 0 { + switch { + case bytes.HasPrefix(text[i-1:], []byte("/*")): + depth++ + case bytes.HasPrefix(text[i-1:], []byte("*/")): + depth-- + } + i++ + } + if depth > 0 { + return true + } + default: + return false + } + } + return true } // Opened a line comment @@ -173,7 +221,7 @@ func (s *TagState) Next(r rune, data []byte) State { return &dollar } // Valid tag: https://www.postgresql.org/docs/current/sql-syntax-lexical.html - if unicode.IsLetter(r) || unicode.IsDigit(r) || r == '_' { + if isIdentifierRune(r) { return s } // Break out of tag state @@ -188,22 +236,60 @@ func (s *EscapeState) Next(r rune, data []byte) State { return &ReadyState{} } +// Opened a parenthesis group +type ParenState struct { + prev State +} + +func (s *ParenState) Next(r rune, data []byte) State { + curr := s.prev.Next(r, data) + if curr == nil { + s.prev = &ReadyState{} + return s + } + s.prev = curr + if _, ok := s.prev.(*ReadyState); !ok { + return s + } + if r == ')' { + return &ReadyState{} + } + return s +} + // Opened BEGIN ATOMIC function body type AtomicState struct { - prev State - delimiter []byte + prev State + pendingEnd bool + statementStart int + statementHasContent bool } func (s *AtomicState) Next(r rune, data []byte) State { - // If we are in a quoted state, the current delimiter doesn't count. - if curr := s.prev.Next(r, data); curr != nil { - s.prev = curr - } - if _, ok := s.prev.(*ReadyState); ok { - window := data[len(data)-len(s.delimiter):] - // Treat delimiter as case insensitive - if strings.EqualFold(string(window), string(s.delimiter)) { - return &ReadyState{} + pendingEnd := s.pendingEnd + s.pendingEnd = false + if pendingEnd && !isIdentifierRune(r) { + return (&ReadyState{}).Next(r, data) + } + // An END inside a nested quote/comment doesn't count. + curr := s.prev.Next(r, data) + if curr == nil { + s.prev = &ReadyState{} + s.statementStart = len(data) + s.statementHasContent = false + return s + } + s.prev = curr + if _, ok := s.prev.(*ReadyState); !ok { + return s + } + // PostgreSQL requires each inner statement to end with ';', so the closing END is + // always the first token of a statement; a later END is expression text. + if !s.statementHasContent && endsWithKeyword(data, END_ATOMIC) { + if isCommentsAndWhitespace(data[s.statementStart : len(data)-len(END_ATOMIC)]) { + s.pendingEnd = true + } else { + s.statementHasContent = true } } return s diff --git a/apps/cli-go/pkg/parser/state_test.go b/apps/cli-go/pkg/parser/state_test.go index ad6db9d26a..921a33b852 100644 --- a/apps/cli-go/pkg/parser/state_test.go +++ b/apps/cli-go/pkg/parser/state_test.go @@ -77,6 +77,14 @@ func TestDollarQuote(t *testing.T) { checkSplit(t, sql) }) + t.Run("non-ASCII named tag", func(t *testing.T) { + for _, tag := range []string{"a²", "a😀", "á"} { + t.Run(tag, func(t *testing.T) { + checkSplit(t, []string{"$" + tag + "$ any ; END; string$" + tag + "$;", " SELECT 2;"}) + }) + } + }) + t.Run("anonymous tag", func(t *testing.T) { sql := []string{"$$\"Dane's horse\"$$"} checkSplit(t, sql) @@ -193,6 +201,81 @@ SELECT 1;`, } }) + t.Run("ignores end inside identifiers", func(t *testing.T) { + for _, name := range []string{"pending", "pending_change", "append", "legend", "𐐀end", "😀end", "́end", "²end", "pending$$foo$"} { + t.Run(name, func(t *testing.T) { + body := `CREATE FUNCTION f() RETURNS int LANGUAGE sql BEGIN ATOMIC SELECT 1 AS ` + name + `; END;` + checkSplit(t, []string{body, ` SELECT 2;`}) + }) + } + }) + + t.Run("ignores identifiers starting with end", func(t *testing.T) { + for _, name := range []string{"endpoint", "end_date", "ended_at", "end𐐀", "end😀", "end́", "end²", "end$$foo$"} { + t.Run(name, func(t *testing.T) { + body := `CREATE FUNCTION f() RETURNS int LANGUAGE sql BEGIN ATOMIC SELECT ` + name + `; SELECT 1; END;` + checkSplit(t, []string{body, ` SELECT 2;`}) + }) + } + }) + + t.Run("ignores end inside inner statements", func(t *testing.T) { + for _, expr := range []string{ + "CASE WHEN true THEN 1 ELSE 0 END", + "case when true then 1 end", + "CASE WHEN true THEN 1 END AS ended", + "CASE WHEN CASE WHEN true THEN true END THEN 1 END", + "(CASE WHEN (true) THEN 1 END)", + "coalesce(CASE WHEN length('a') > 0 THEN 1 END, 0)", + "CASE(1)WHEN 1 THEN 1 END", + "1 AS case", + "1 case", + "1 AS end", + "1 end", + "'end'", + } { + t.Run(expr, func(t *testing.T) { + body := `CREATE FUNCTION f() RETURNS int LANGUAGE sql BEGIN ATOMIC SELECT ` + expr + `; SELECT 1; END;` + checkSplit(t, []string{body, ` SELECT 2;`}) + }) + } + }) + + t.Run("closes at end preceded only by comments", func(t *testing.T) { + for _, comment := range []string{"-- note END\n", "/* note; */ ", "\n/* a /* b; */ */ -- c\n"} { + t.Run(comment, func(t *testing.T) { + body := `CREATE FUNCTION f() RETURNS int LANGUAGE sql BEGIN ATOMIC SELECT 1; ` + comment + `END;` + checkSplit(t, []string{body, ` SELECT 2;`}) + }) + } + }) + + t.Run("ignores non-ASCII begin atomic lookalikes", func(t *testing.T) { + // atomıc (dotless ı) case-folds to ATOMIC but is a plain identifier in SQL. + checkSplit(t, []string{"BEGIN atomıc;", " SELECT 'end';", " SELECT 2;"}) + }) + + t.Run("closes atomic body at end right after a positional parameter", func(t *testing.T) { + checkSplit(t, []string{"CREATE FUNCTION f(int) RETURNS int LANGUAGE sql BEGIN ATOMIC SELECT $1;END;", " SELECT 2;"}) + }) + + t.Run("requires sql whitespace between begin and atomic", func(t *testing.T) { + for _, gap := range []string{"\u00A0", "\uFEFF", "\u0085"} { + t.Run(gap, func(t *testing.T) { + checkSplit(t, []string{"BEGIN " + gap + " ATOMIC;", " SELECT 1;", " end;", " SELECT 2;"}) + }) + } + }) + + t.Run("closes nested atomic body inside parentheses", func(t *testing.T) { + checkSplit(t, []string{"DO (BEGIN ATOMIC SELECT 1; END; );", " SELECT 2;"}) + }) + + t.Run("ignores end after overlapping block comment", func(t *testing.T) { + body := `CREATE FUNCTION f() RETURNS int LANGUAGE sql BEGIN ATOMIC SELECT 1; /* a /*/ b */ SELECT 2 END; SELECT 3; END;` + checkSplit(t, []string{body, ` SELECT 4;`}) + }) + t.Run("does not treat schema-qualified atomic function names as begin atomic", func(t *testing.T) { sql := []string{`CREATE OR REPLACE FUNCTION public.atomic_example() RETURNS INTEGER diff --git a/apps/cli/src/command-internal/sql-split.ts b/apps/cli/src/command-internal/sql-split.ts index 0722d09c2b..6abc7c9c20 100644 --- a/apps/cli/src/command-internal/sql-split.ts +++ b/apps/cli/src/command-internal/sql-split.ts @@ -15,26 +15,75 @@ interface State { const BEGIN_ATOMIC = "ATOMIC"; const END_ATOMIC = "END"; -// `\p{Nd}` (decimal digits only), not `\p{N}` (all Unicode numbers): `\p{N}` would wrongly -// accept `No`/`Nl` runes like superscript-2 (`²`) as a valid identifier/dollar-tag character. -const isIdentifierRune = (rune: string): boolean => /[\p{L}\p{Nd}_$]/u.test(rune); +// PostgreSQL's scan.l treats every code point at or above 0x80 as an identifier/dollar-tag +// character (`ident_cont`/`dolq_cont`), whatever its Unicode category. +const isIdentifierRune = (rune: string): boolean => { + const codePoint = rune.codePointAt(0); + return codePoint !== undefined && (codePoint >= 0x80 || /[A-Za-z0-9_$]/u.test(rune)); +}; + +// A code point spans at most two UTF-16 units, so the last one before `offset` lies within +// the preceding two. +const hasIdentifierRuneBefore = (data: string, offset: number): boolean => { + if (offset <= 0) return false; + const rune = Array.from(data.slice(Math.max(0, offset - 2), offset)).at(-1); + return rune !== undefined && isIdentifierRune(rune); +}; + +const asciiUpper = (text: string): string => text.replace(/[a-z]/g, (c) => c.toUpperCase()); + +function endsWithKeyword(data: string, keyword: string): boolean { + const offset = data.length - keyword.length; + if (offset < 0 || asciiUpper(data.slice(offset)) !== keyword) return false; + return !hasIdentifierRuneBefore(data, offset); +} + +const isSqlWhitespace = (rune: string): boolean => " \t\n\r\f\v".includes(rune); function isBeginAtomic(data: string): boolean { - let offset = data.length - BEGIN_ATOMIC.length; - if (offset < 0 || data.slice(offset).toUpperCase() !== BEGIN_ATOMIC) return false; - if (offset > 0 && isIdentifierRune(data[offset - 1]!)) return false; - const prefix = data.slice(0, offset).replace(/\s+$/u, ""); - offset = prefix.length - "BEGIN".length; - if (offset < 0 || prefix.slice(offset).toUpperCase() !== "BEGIN") return false; - if (offset === 0) return true; - return !isIdentifierRune(prefix[offset - 1]!); + if (!endsWithKeyword(data, BEGIN_ATOMIC)) return false; + let end = data.length - BEGIN_ATOMIC.length; + while (end > 0 && isSqlWhitespace(data[end - 1]!)) end -= 1; + return endsWithKeyword(data.slice(0, end), "BEGIN"); +} + +function isCommentsAndWhitespace(text: string): boolean { + let i = 0; + while (i < text.length) { + if (isSqlWhitespace(text[i]!)) { + i += 1; + } else if (text.startsWith("--", i)) { + const newline = text.indexOf("\n", i + 2); + if (newline === -1) return true; + i = newline + 1; + } else if (text.startsWith("/*", i)) { + // Match `BlockState`'s sliding-window scan so both agree on overlapping delimiters. + let depth = 1; + i += 2; + while (i < text.length && depth > 0) { + const window = text.slice(i - 1, i + 1); + if (window === "/*") depth += 1; + else if (window === "*/") depth -= 1; + i += 1; + } + if (depth > 0) return true; + } else { + return false; + } + } + return true; } class ReadyState implements State { next(rune: string, data: string): State | null { switch (rune) { - case "$": - return new TagState(data.length - rune.length); + case "$": { + // A `$` after an identifier rune continues the identifier (`pending$$foo$`), not a + // dollar quote. A digit counts too (`1$$`), unlike PostgreSQL; valid SQL never has that. + const offset = data.length - rune.length; + if (hasIdentifierRuneBefore(data, offset)) return this; + return new TagState(offset); + } case "'": case '"': return new QuoteState(rune); @@ -47,10 +96,10 @@ class ReadyState implements State { case ";": return null; case "(": - return new AtomicState(new ReadyState(), ")"); + return new ParenState(new ReadyState()); case "c": case "C": - if (isBeginAtomic(data)) return new AtomicState(new ReadyState(), END_ATOMIC); + if (isBeginAtomic(data)) return new AtomicState(new ReadyState(), data.length); return this; default: return this; @@ -112,9 +161,7 @@ class TagState implements State { constructor(private readonly offset: number) {} next(rune: string, data: string): State | null { if (rune === "$") return new DollarState(data.slice(this.offset)); - // Valid dollar-tag characters — see `isIdentifierRune`'s comment on why `\p{Nd}`, - // not `\p{N}`. - if (/[\p{L}\p{Nd}_]/u.test(rune)) return this; + if (isIdentifierRune(rune)) return this; return new ReadyState().next(rune, data); } } @@ -125,18 +172,54 @@ class EscapeState implements State { } } +class ParenState implements State { + constructor(private prev: State) {} + next(rune: string, data: string): State | null { + const curr = this.prev.next(rune, data); + if (curr === null) { + this.prev = new ReadyState(); + return this; + } + this.prev = curr; + if (!(this.prev instanceof ReadyState)) return this; + return rune === ")" ? new ReadyState() : this; + } +} + class AtomicState implements State { + private pendingEnd = false; + private statementStart: number; + private statementHasContent = false; constructor( private prev: State, - private readonly delimiter: string, - ) {} + start: number, + ) { + this.statementStart = start; + } next(rune: string, data: string): State | null { - // A delimiter inside a nested quote/comment doesn't count. + const pendingEnd = this.pendingEnd; + this.pendingEnd = false; + if (pendingEnd && !isIdentifierRune(rune)) return new ReadyState().next(rune, data); + // An `END` inside a nested quote/comment doesn't count. const curr = this.prev.next(rune, data); - if (curr !== null) this.prev = curr; - if (this.prev instanceof ReadyState) { - const window = data.slice(-this.delimiter.length); - if (window.toUpperCase() === this.delimiter.toUpperCase()) return new ReadyState(); + if (curr === null) { + this.prev = new ReadyState(); + this.statementStart = data.length; + this.statementHasContent = false; + return this; + } + this.prev = curr; + if (!(this.prev instanceof ReadyState)) return this; + // PostgreSQL requires each inner statement to end with `;`, so the closing `END` is + // always the first token of a statement; a later `END` is expression text. + if (!this.statementHasContent && endsWithKeyword(data, END_ATOMIC)) { + if ( + isCommentsAndWhitespace(data.slice(this.statementStart, data.length - END_ATOMIC.length)) + ) { + this.pendingEnd = true; + } else { + this.statementHasContent = true; + } } return this; } diff --git a/apps/cli/src/command-internal/sql-split.unit.test.ts b/apps/cli/src/command-internal/sql-split.unit.test.ts index 6db8a583ca..5ec3f74615 100644 --- a/apps/cli/src/command-internal/sql-split.unit.test.ts +++ b/apps/cli/src/command-internal/sql-split.unit.test.ts @@ -32,11 +32,9 @@ describe("splitAndTrim", () => { ]); }); - it("treats a non-decimal Unicode digit as an invalid dollar-tag character, like Go's unicode.IsDigit", () => { - // "a²" (U+00B2, category No) is not a valid dollar-tag character, so the tag falls back - // and the embedded `;` becomes a real boundary. - const sql = "CREATE FUNCTION f() AS $a²$foo; bar$a²$ LANGUAGE sql;"; - expect(splitAndTrim(sql)).toEqual(["CREATE FUNCTION f() AS $a²$foo", "bar$a²$ LANGUAGE sql"]); + it.each(["a²", "a😀", "á"])("respects non-ASCII dollar tag $%s$", (tag) => { + const statement = `CREATE FUNCTION f() AS $${tag}$foo; END; bar$${tag}$ LANGUAGE sql`; + expect(splitAndTrim(`${statement}; SELECT 2;`)).toEqual([statement, "SELECT 2"]); }); it("respects named dollar tags", () => { @@ -66,6 +64,102 @@ describe("splitAndTrim", () => { "SELECT 3", ]); }); + + it.each([ + "pending", + "pending_change", + "append", + "legend", + "𐐀end", + "😀end", + "́end", + "²end", + "pending$$foo$", + ])("does not close a BEGIN ATOMIC body inside %s", (identifier) => { + const body = `CREATE FUNCTION f() RETURNS int LANGUAGE sql BEGIN ATOMIC SELECT 1 AS ${identifier}; END`; + expect(splitAndTrim(`${body}; SELECT 2;`)).toEqual([body, "SELECT 2"]); + }); + + it.each(["endpoint", "end_date", "ended_at", "end𐐀", "end😀", "end́", "end²", "end$$foo$"])( + "does not close a BEGIN ATOMIC body at the start of %s", + (identifier) => { + const body = `CREATE FUNCTION f() RETURNS int LANGUAGE sql BEGIN ATOMIC SELECT ${identifier}; SELECT 1; END`; + expect(splitAndTrim(`${body}; SELECT 2;`)).toEqual([body, "SELECT 2"]); + }, + ); + + it.each([ + "CASE WHEN true THEN 1 ELSE 0 END", + "case when true then 1 end", + "CASE WHEN true THEN 1 END AS ended", + "CASE WHEN CASE WHEN true THEN true END THEN 1 END", + "(CASE WHEN (true) THEN 1 END)", + "coalesce(CASE WHEN length('a') > 0 THEN 1 END, 0)", + "CASE(1)WHEN 1 THEN 1 END", + "1 AS case", + "1 case", + "1 AS end", + "1 end", + "'end'", + ])("does not close a BEGIN ATOMIC body at an END inside %s", (expression) => { + const body = `CREATE FUNCTION f() RETURNS int LANGUAGE sql BEGIN ATOMIC SELECT ${expression}; SELECT 1; END`; + expect(splitAndTrim(`${body}; SELECT 2;`)).toEqual([body, "SELECT 2"]); + }); + + it.each(["-- note END\n", "/* note; */ ", "\n/* a /* b; */ */ -- c\n"])( + "closes a BEGIN ATOMIC body at an END preceded only by comments (%s)", + (comment) => { + const body = `CREATE FUNCTION f() RETURNS int LANGUAGE sql BEGIN ATOMIC SELECT 1; ${comment}END`; + expect(splitAndTrim(`${body}; SELECT 2;`)).toEqual([body, "SELECT 2"]); + }, + ); + + it("does not treat a BEGIN keyword followed by a non-ASCII identifier as BEGIN ATOMIC", () => { + // `atomıc` (dotless ı) uppercases to `ATOMIC` in JS but is a plain identifier in SQL. + expect(splitAndTrim("BEGIN atomıc; SELECT 'end'; SELECT 2;")).toEqual([ + "BEGIN atomıc", + "SELECT 'end'", + "SELECT 2", + ]); + }); + + it("closes a BEGIN ATOMIC body at an END confirmed by a newline", () => { + const body = "CREATE FUNCTION f() RETURNS int LANGUAGE sql BEGIN ATOMIC SELECT 1; END"; + expect(splitAndTrim(`${body}\n; SELECT 2;`)).toEqual([body, "SELECT 2"]); + }); + + it("keeps a BEGIN ATOMIC body whose END sits at EOF", () => { + expect(splitAndTrim("begin atomic; select 'end'; end")).toEqual([ + "begin atomic; select 'end'; end", + ]); + }); + + it("closes a BEGIN ATOMIC body at an END right after a positional parameter's ;", () => { + const body = "CREATE FUNCTION f(int) RETURNS int LANGUAGE sql BEGIN ATOMIC SELECT $1;END"; + expect(splitAndTrim(`${body}; SELECT 2;`)).toEqual([body, "SELECT 2"]); + }); + + it.each(["\u00A0", "\uFEFF", "\u0085"])( + "does not treat BEGIN %s ATOMIC as the keyword pair", + (gap) => { + // Only PostgreSQL's own whitespace separates the keywords; these are identifier runes. + const sql = `BEGIN ${gap} ATOMIC; SELECT 1; end; SELECT 2;`; + expect(splitAndTrim(sql)).toEqual([`BEGIN ${gap} ATOMIC`, "SELECT 1", "end", "SELECT 2"]); + }, + ); + + it("closes a nested BEGIN ATOMIC body inside parentheses", () => { + expect(splitAndTrim("DO (BEGIN ATOMIC SELECT 1; END; ); SELECT 2;")).toEqual([ + "DO (BEGIN ATOMIC SELECT 1; END; )", + "SELECT 2", + ]); + }); + + it("does not close a BEGIN ATOMIC body at an END after an overlapping block comment", () => { + const body = + "CREATE FUNCTION f() RETURNS int LANGUAGE sql BEGIN ATOMIC SELECT 1; /* a /*/ b */ SELECT 2 END; SELECT 3; END"; + expect(splitAndTrim(`${body}; SELECT 4;`)).toEqual([body, "SELECT 4"]); + }); }); describe("splitSql", () => {