Parse UPDATE/DELETE ORDER BY and LIMIT behind an option (#5)

Author: kyleconroy

CLAUDE.md

@@ -23,6 +23,12 @@ https://sqlite.org/lang.html.
   side: no nonterminal of the vendored grammar may go unmentioned.
 - Error messages must match SQLite's parser byte-for-byte
   (`near "X": syntax error`, `unrecognized token: "X"`, `incomplete input`).
+- Grammar that a build option turns on — `SQLITE_ENABLE_UPDATE_DELETE_LIMIT`
+  so far — is a `parser.Options` field, off by default: the corpus is
+  generated from the pinned build, so the default has to stay exactly where
+  that build is. Expectations for an option come from a SQLite compiled with
+  it, which takes a Lemon run rather than a `-D` on the amalgamation; the
+  recipe is at the top of `parser/options_test.go`.
 
 ## The corpus
 

PLAN.md

@@ -212,6 +212,16 @@ them — meyer implements exactly what that build accepts, including
 CONSTRAINT` forms if (and only if) the pinned build has them. When the pin
 advances, `cmd/regenerate` re-derives expectations and diffs are reviewed.
 
+A gate that real deployments turn on is a second matter, because the pinned
+build is not the only SQLite sqlc is ever pointed at. Those get a field on
+`parser.Options`, defaulting to the pinned build's answer so conformance is
+untouched, and are opt-in per parse:
+`SQLITE_ENABLE_UPDATE_DELETE_LIMIT` (`Options.UpdateDeleteLimit`) is the
+first — ORDER BY and LIMIT on UPDATE and DELETE. Each one has to be a fork
+in SQLite's own grammar, never a dialect meyer invents, and has to be
+verified the same way everything else is: against a build with the option
+on.
+
 ### 4. Positions are load-bearing (sqlc integration)
 
 teesql has no positions at all; doubleclick's `End()` is vestigial. meyer

README.md

@@ -75,6 +75,18 @@ _, err := parser.ParseString("SELECT FROM t")
 fmt.Println(err) // 1:8: near "FROM": syntax error
 ```
 
+Those entry points parse what the pinned SQLite build parses. Where SQLite's
+own grammar forks on a build option, `parser.Options` has the same entry
+points as methods, so a caller aimed at a database built differently can
+parse the SQL that database accepts:
+
+```go
+// ORDER BY and LIMIT on UPDATE and DELETE, as SQLITE_ENABLE_UPDATE_DELETE_LIMIT
+// compiles in. Without it -- the default, and the pinned build -- they are a
+// syntax error, which is what SQLite reports too.
+stmts, err := parser.Options{UpdateDeleteLimit: true}.ParseString("DELETE FROM t ORDER BY x LIMIT 1")
+```
+
 Every node embeds `ast.Span`, so `Pos()` and `End()` give byte offsets into
 the original input — sqlc slices the source with them to find `-- name:`
 comments and to report errors, so they are load-bearing rather than

ast/dml.go

@@ -90,10 +90,13 @@ func (n *SetPair) Children() []Node {
 //
 //	cmd ::= with UPDATE orconf xfullname indexed_opt SET setlist from
 //	        where_opt_ret.
+//	cmd ::= with UPDATE orconf xfullname indexed_opt SET setlist from
+//	        where_opt_ret orderby_opt limit_opt.
 //
-// The pinned build defines neither SQLITE_ENABLE_UPDATE_DELETE_LIMIT nor
-// SQLITE_UDL_CAPABLE_PARSER, so UPDATE has no ORDER BY or LIMIT at all: they
-// are a plain syntax error rather than a grammar-action message.
+// The second form is the one SQLITE_ENABLE_UPDATE_DELETE_LIMIT compiles in.
+// The pinned build defines neither it nor SQLITE_UDL_CAPABLE_PARSER, so
+// OrderBy and Limit are only ever set for a parse that asked for them with
+// parser.Options.UpdateDeleteLimit; without it they are a syntax error.
 type UpdateStmt struct {
 	Span
 	With       *With           `json:"with,omitempty"`
@@ -106,6 +109,8 @@ type UpdateStmt struct {
 	From       []*TableRef     `json:"from,omitempty"`
 	Where      Expr            `json:"where,omitempty"`
 	Returning  []*ResultColumn `json:"returning,omitempty"`
+	OrderBy    []*OrderingTerm `json:"orderBy,omitempty"`
+	Limit      *Limit          `json:"limit,omitempty"`
 }
 
 func (*UpdateStmt) stmtNode() {}
@@ -115,14 +120,19 @@ func (n *UpdateStmt) Children() []Node {
 	out = appendNodes(out, n.From)
 	out = append(out, nodes(n.Where)...)
 	out = appendNodes(out, n.Returning)
-	return out
+	out = appendNodes(out, n.OrderBy)
+	return append(out, nodes(n.Limit)...)
 }
 
 // DeleteStmt is a DELETE statement.
 //
 //	cmd ::= with DELETE FROM xfullname indexed_opt where_opt_ret.
+//	cmd ::= with DELETE FROM xfullname indexed_opt where_opt_ret orderby_opt
+//	        limit_opt.
 //
-// As for UPDATE, the pinned build's DELETE takes no ORDER BY or LIMIT.
+// As for UPDATE, the second form is the one
+// SQLITE_ENABLE_UPDATE_DELETE_LIMIT compiles in, and OrderBy and Limit are
+// set only for a parse made with parser.Options.UpdateDeleteLimit.
 type DeleteStmt struct {
 	Span
 	With       *With           `json:"with,omitempty"`
@@ -132,11 +142,14 @@ type DeleteStmt struct {
 	NotIndexed bool            `json:"notIndexed,omitempty"`
 	Where      Expr            `json:"where,omitempty"`
 	Returning  []*ResultColumn `json:"returning,omitempty"`
+	OrderBy    []*OrderingTerm `json:"orderBy,omitempty"`
+	Limit      *Limit          `json:"limit,omitempty"`
 }
 
 func (*DeleteStmt) stmtNode() {}
 func (n *DeleteStmt) Children() []Node {
 	out := nodes(n.With, n.Table, n.Alias, n.IndexedBy, n.Where)
 	out = appendNodes(out, n.Returning)
-	return out
+	out = appendNodes(out, n.OrderBy)
+	return append(out, nodes(n.Limit)...)
 }

ast/render.go

@@ -945,6 +945,7 @@ func writeUpdate(b *strings.Builder, t *UpdateStmt) {
 		write(b, t.Where)
 	}
 	writeReturning(b, t.Returning)
+	writeUpdateDeleteLimit(b, t.OrderBy, t.Limit)
 }
 
 func writeDelete(b *strings.Builder, t *DeleteStmt) {
@@ -964,6 +965,20 @@ func writeDelete(b *strings.Builder, t *DeleteStmt) {
 		write(b, t.Where)
 	}
 	writeReturning(b, t.Returning)
+	writeUpdateDeleteLimit(b, t.OrderBy, t.Limit)
+}
+
+// writeUpdateDeleteLimit renders the ORDER BY and LIMIT an UPDATE or DELETE
+// carries only when it was parsed with parser.Options.UpdateDeleteLimit. The
+// rendering is re-parseable under the same option, and under no other.
+func writeUpdateDeleteLimit(b *strings.Builder, order []*OrderingTerm, limit *Limit) {
+	if len(order) > 0 {
+		w{b}.kw("ORDER BY")
+		writeOrdering(b, order)
+	}
+	if limit != nil {
+		write(b, limit)
+	}
 }
 
 func writeIndexedBy(b *strings.Builder, by *Ident, notIndexed bool) {

cmd/debug-parse/main.go

@@ -10,6 +10,7 @@
 //	go run ./cmd/debug-parse 'SELECT 1'
 //	go run ./cmd/debug-parse -tokens 'SELECT 1'
 //	go run ./cmd/debug-parse -f query.sql
+//	go run ./cmd/debug-parse -update-delete-limit 'DELETE FROM t LIMIT 1'
 //	echo 'SELECT 1' | go run ./cmd/debug-parse
 //
 // A parse error is printed with a caret under the offending byte, in the
@@ -36,6 +37,8 @@ func main() {
 		tokens    = flag.Bool("tokens", false, "print the token stream instead of the tree")
 		render    = flag.Bool("render", false, "print the tree rendered back to SQL")
 		positions = flag.Bool("pos", true, "include byte spans in the tree")
+		udl       = flag.Bool("update-delete-limit", false,
+			"accept ORDER BY and LIMIT on UPDATE and DELETE, as SQLITE_ENABLE_UPDATE_DELETE_LIMIT does")
 	)
 	flag.Parse()
 
@@ -51,7 +54,7 @@ func main() {
 		return
 	}
 
-	stmts, err := parser.ParseString(src)
+	stmts, err := parser.Options{UpdateDeleteLimit: *udl}.ParseString(src)
 	if err != nil {
 		var pe *parser.Error
 		if errors.As(err, &pe) {

internal/roundtrip/roundtrip.go

@@ -32,9 +32,14 @@ type Result struct {
 // Equality is structural: byte spans and the Raw fields differ after a round
 // trip by construction, and the renderer promises nothing about them, so
 // dump.Structure leaves both out.
-func Check(stmts []ast.Stmt) Result {
+func Check(stmts []ast.Stmt) Result { return CheckWith(parser.Options{}, stmts) }
+
+// CheckWith is Check for a tree that was parsed with opts. The rendering is
+// re-parsed with the same options, because a clause an option unlocked --
+// an UPDATE's LIMIT, say -- can only be read back with that option on.
+func CheckWith(opts parser.Options, stmts []ast.Stmt) Result {
 	rendered := ast.Statements(stmts)
-	again, err := parser.ParseString(rendered)
+	again, err := opts.ParseString(rendered)
 	if err != nil {
 		return Result{Rendered: rendered, Reason: "rendered SQL does not parse: " + err.Error()}
 	}

parser/options_test.go

@@ -0,0 +1,199 @@
+package parser_test
+
+import (
+	"errors"
+	"strings"
+	"testing"
+
+	"github.com/sqlc-dev/meyer/ast"
+	"github.com/sqlc-dev/meyer/internal/roundtrip"
+	"github.com/sqlc-dev/meyer/parser"
+)
+
+// The expectations in this file are the oracle's, like every other error
+// expectation in the package, but they need an oracle the corpus tooling
+// does not build: SQLITE_ENABLE_UPDATE_DELETE_LIMIT selects grammar rules,
+// so defining it while compiling the released amalgamation changes nothing
+// -- the amalgamation ships a parse.c that Lemon already generated without
+// them. The build that produced these lines was, from the pinned release's
+// two artifacts:
+//
+//	cc -o lemon sqlite-src-<v>/tool/lemon.c
+//	./lemon -DSQLITE_ENABLE_UPDATE_DELETE_LIMIT -S src/parse.y
+//
+// with the resulting parse.c spliced into sqlite3.c over the region the
+// amalgamation marks "Begin file parse.c", and the whole compiled with
+// -DSQLITE_ENABLE_UPDATE_DELETE_LIMIT. Lemon assigns the same token numbers
+// either way -- the generated parse.h is byte-identical -- so the splice is
+// sound. cmd/difftest run against that build, with the option on, agreed
+// with meyer on 228,652 mutations of the whole corpus.
+
+// udlAccepted is SQL that a build with SQLITE_ENABLE_UPDATE_DELETE_LIMIT
+// parses and the pinned build rejects. Every case reaches SQLite's semantic
+// layer there ("no such table: t1"), which is how the oracle reports a
+// statement that parsed.
+var udlAccepted = []string{
+	`DELETE FROM t1 ORDER BY x`,
+	`DELETE FROM t1 WHERE x=1 ORDER BY x`,
+	`DELETE FROM t1 WHERE x>0 LIMIT 5`,
+	`DELETE FROM t1 WHERE x>0 ORDER BY x LIMIT 5 OFFSET 2`,
+	`DELETE FROM t1 LIMIT 2, 3`,
+	`DELETE FROM t1 INDEXED BY i1 WHERE x=1 LIMIT 1`,
+	`DELETE FROM t1 AS a WHERE a.x=1 ORDER BY a.x LIMIT 1`,
+	`DELETE FROM t1 ORDER BY x COLLATE nocase DESC NULLS LAST LIMIT 1`,
+	`WITH c(x) AS (SELECT 1) DELETE FROM t1 WHERE x IN (SELECT x FROM c) LIMIT 1`,
+	`UPDATE t1 SET y=1 LIMIT 5`,
+	`UPDATE t1 SET y=1 WHERE x=1 ORDER BY x`,
+	`UPDATE OR REPLACE t1 SET y=1 WHERE x=1 ORDER BY x DESC LIMIT 5 OFFSET 2`,
+	`UPDATE t1 SET y=1 FROM t2 WHERE t1.x=t2.x LIMIT 1`,
+	// where_opt_ret comes before orderby_opt, so RETURNING precedes LIMIT.
+	`UPDATE t1 SET y=1 WHERE x=1 RETURNING x, y, '|' LIMIT 5`,
+	`UPDATE t1 SET (a,b)=(SELECT 1,2) WHERE x=1 ORDER BY x LIMIT 1`,
+}
+
+// udlRejected is SQL both builds reject, with the message and offset the
+// UDL build reports. The clauses are only ever the tail of a top-level
+// UPDATE or DELETE: OFFSET needs its LIMIT, RETURNING belongs to
+// where_opt_ret and so cannot follow one, and trigger_cmd never grew either
+// clause, whatever the build.
+var udlRejected = []struct {
+	sql    string
+	msg    string
+	offset int
+}{
+	{`DELETE FROM t1 WHERE x=1 OFFSET 2`, `near "OFFSET": syntax error`, 25},
+	{`UPDATE t1 SET y=1 WHERE x=1 OFFSET 2`, `near "OFFSET": syntax error`, 28},
+	{`DELETE FROM t1 LIMIT 1, 2 OFFSET 3`, `near "OFFSET": syntax error`, 26},
+	{`UPDATE t1 SET y=1 LIMIT 5 RETURNING x`, `near "RETURNING": syntax error`, 26},
+	{`DELETE FROM t1 LIMIT 5 RETURNING x`, `near "RETURNING": syntax error`, 23},
+	{
+		`CREATE TRIGGER r AFTER INSERT ON t1 BEGIN DELETE FROM t1 LIMIT 1; END`,
+		`near "LIMIT": syntax error`, 57,
+	},
+	{
+		`CREATE TRIGGER r AFTER INSERT ON t1 BEGIN UPDATE t1 SET y=1 ORDER BY x LIMIT 1; END`,
+		`near "ORDER": syntax error`, 60,
+	},
+}
+
+// udlOff is what the pinned build makes of the same clauses: the statement
+// ended at the token before, so ORDER or LIMIT is a token no rule can
+// shift. These offsets are the corpus's own, from wherelimit.test.
+var udlOff = []struct {
+	sql    string
+	msg    string
+	offset int
+}{
+	{`DELETE FROM t1 ORDER BY x`, `near "ORDER": syntax error`, 15},
+	{`DELETE FROM t1 WHERE x=1 ORDER BY x`, `near "ORDER": syntax error`, 25},
+	{`DELETE FROM t1 WHERE x>0 LIMIT 5`, `near "LIMIT": syntax error`, 25},
+	{`UPDATE t1 SET y=1 WHERE x=1 ORDER BY x`, `near "ORDER": syntax error`, 28},
+	{`UPDATE t1 SET y=1 WHERE x=1 RETURNING x, y, '|' LIMIT 5`, `near "LIMIT": syntax error`, 48},
+	{`WITH c(x) AS (SELECT 1) DELETE FROM t1 WHERE x IN (SELECT x FROM c) LIMIT 1`, `near "LIMIT": syntax error`, 68},
+}
+
+var udl = parser.Options{UpdateDeleteLimit: true}
+
+// TestUpdateDeleteLimit checks that the option accepts the clauses SQLite
+// accepts with SQLITE_ENABLE_UPDATE_DELETE_LIMIT, that the tree survives a
+// round trip through the renderer, and that the same SQL is still a syntax
+// error without the option -- the corpus is generated from a build without
+// it, so the default has to stay where it is.
+func TestUpdateDeleteLimit(t *testing.T) {
+	for _, sql := range udlAccepted {
+		t.Run(sql, func(t *testing.T) {
+			stmts, err := udl.ParseString(sql)
+			if err != nil {
+				t.Fatalf("expected the input to parse with the option, got: %v", err)
+			}
+			if r := roundtrip.CheckWith(udl, stmts); !r.Ok {
+				t.Errorf("round trip: %s\nrendered: %s", r.Reason, r.Rendered)
+			}
+			if _, err := parser.ParseString(sql); err == nil {
+				t.Error("the default options accepted an UPDATE/DELETE LIMIT")
+			}
+		})
+	}
+}
+
+func TestUpdateDeleteLimitErrors(t *testing.T) {
+	for _, tt := range udlRejected {
+		t.Run(tt.sql, func(t *testing.T) {
+			checkError(t, udl, tt.sql, tt.msg, tt.offset)
+		})
+	}
+	for _, tt := range udlOff {
+		t.Run(tt.sql, func(t *testing.T) {
+			checkError(t, parser.Options{}, tt.sql, tt.msg, tt.offset)
+		})
+	}
+}
+
+func checkError(t *testing.T, opts parser.Options, sql, msg string, offset int) {
+	t.Helper()
+	_, err := opts.ParseString(sql)
+	var pe *parser.Error
+	if !errors.As(err, &pe) {
+		t.Fatalf("expected %q, got %v", msg, err)
+	}
+	if pe.Message != msg {
+		t.Errorf("message:\n  got:  %s\n  want: %s", pe.Message, msg)
+	}
+	if pe.Offset != offset {
+		t.Errorf("offset: got %d, want %d", pe.Offset, offset)
+	}
+}
+
+// TestUpdateDeleteLimitTree checks that the clauses land on the statement
+// rather than being consumed and dropped, which accept/reject cannot see.
+func TestUpdateDeleteLimitTree(t *testing.T) {
+	stmt, err := udl.ParseStatement(`DELETE FROM t1 WHERE x>0 ORDER BY y DESC LIMIT 5 OFFSET 2`)
+	if err != nil {
+		t.Fatalf("parse: %v", err)
+	}
+	del, ok := stmt.(*ast.DeleteStmt)
+	if !ok {
+		t.Fatalf("parsed to %T, want *ast.DeleteStmt", stmt)
+	}
+	if len(del.OrderBy) != 1 || del.OrderBy[0].Order != ast.SortDesc {
+		t.Errorf("ORDER BY is %+v, want one descending term", del.OrderBy)
+	}
+	if del.Limit == nil || del.Limit.Count == nil || del.Limit.Offset == nil {
+		t.Fatalf("LIMIT is %+v, want a count and an offset", del.Limit)
+	}
+	if got, want := ast.String(del), `DELETE FROM t1 WHERE x > 0 ORDER BY y DESC LIMIT 5 OFFSET 2`; got != want {
+		t.Errorf("rendered\n  got:  %s\n  want: %s", got, want)
+	}
+
+	// "LIMIT x, y" swaps the operands, as it does in a SELECT, and the
+	// renderer has to put them back the way they were written.
+	stmt, err = udl.ParseStatement(`UPDATE t1 SET y=1 LIMIT 2, 3`)
+	if err != nil {
+		t.Fatalf("parse: %v", err)
+	}
+	up := stmt.(*ast.UpdateStmt)
+	if up.Limit == nil || !up.Limit.Comma {
+		t.Fatalf("LIMIT is %+v, want the comma spelling", up.Limit)
+	}
+	if got, want := ast.String(up), `UPDATE t1 SET y = 1 LIMIT 2, 3`; got != want {
+		t.Errorf("rendered\n  got:  %s\n  want: %s", got, want)
+	}
+}
+
+// TestOptionsEntryPoints checks that the option reaches every entry point,
+// and that the package-level ones are unaffected by it.
+func TestOptionsEntryPoints(t *testing.T) {
+	const sql = `DELETE FROM t1 LIMIT 1`
+	if _, err := udl.ParseStatement(sql); err != nil {
+		t.Errorf("Options.ParseStatement: %v", err)
+	}
+	if _, err := udl.Parse(t.Context(), strings.NewReader(sql)); err != nil {
+		t.Errorf("Options.Parse: %v", err)
+	}
+	if _, err := parser.ParseStatement(sql); err == nil {
+		t.Error("ParseStatement accepted a DELETE ... LIMIT")
+	}
+	if _, err := (parser.Options{}).ParseString(sql); err == nil {
+		t.Error("the zero Options accepted a DELETE ... LIMIT")
+	}
+}