diff --git a/.changes/unreleased/Fixed-20260815-120000.yaml b/.changes/unreleased/Fixed-20260815-120000.yaml new file mode 100644 index 00000000..46ac9fc2 --- /dev/null +++ b/.changes/unreleased/Fixed-20260815-120000.yaml @@ -0,0 +1,6 @@ +kind: Fixed +body: 'Arguments of a reused `sqlc.slice` are no longer misordered when the query also contains a backtick- or bracket-quoted identifier holding a `?` (SQLite family), and a slice whose name contains `%` now expands correctly on the MySQL drivers - its marker was doubled in the SQL constant but not in the replacement the generated code performs, so the expansion silently matched nothing.' +time: 2026-08-15T12:00:00.0000000Z +custom: + Author: Rayakame + PR: "257" diff --git a/.gitignore b/.gitignore index 659e13da..0b25a956 100644 --- a/.gitignore +++ b/.gitignore @@ -45,3 +45,8 @@ test/enums.py /sqlc-gen-better-python.wasm cover*.tmp .claude/ + +# JetBrains IDE state is machine-local (absolute paths, branch names, user +# ids); only the shared run configurations are tracked. +.idea/* +!.idea/runConfigurations/ diff --git a/CLAUDE.md b/CLAUDE.md index 12196f90..75425ff0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -127,8 +127,10 @@ generation pipeline lives in `internal/handler.go`: `FilterUnusedModels()`. `type.go` builds `PyType` and normalizes `SQLType` (lowercased once here; every downstream consumer relies on it). `psycopg_sql.go` and `mysql_sql.go` rewrite placeholders at IR build time - (psycopg: `$N` -> `%(pN)s`; MySQL: `?` -> `%s` with `%` doubled - small - SQL lexers matching each engine's rules). `plainParams` pre-reserves + (psycopg: `$N` -> `%(pN)s`; MySQL: `?` -> `%s` with `%` doubled). MySQL + lexes through `internal/sqllex`; psycopg keeps its own lexer, since + PostgreSQL has no `sqlc.slice` and therefore no second consumer to stay + in step with. `plainParams` pre-reserves every local the driver bodies emit (`conn`/`self`, `sql` for slice queries, psycopg's and MySQL's `sql_params`/`cur`/`row`/`_decode_hook`); a new local in a driver body needs a matching seed or a param can shadow @@ -189,9 +191,12 @@ buffer is emitted as an extra output file. returns -> converters). psycopg's loader registration follows the same policy (returned json/jsonb types only). - The MySQL drivers interpolate pyformat placeholders client-side: rewritten - SQL constants carry `%s`/`%%`, and the slice-expansion/placeholder-scanner - machinery in `internal/driver/common.go` must lex them exactly like the - rewriter emitted them. + SQL constants carry `%s`/`%%`. `internal/sqllex` owns every rule for + telling a bindable placeholder from one inside a string, identifier or + comment; the MySQL rewriter and the drivers' slice ordering both scan + through it, so the rules cannot drift apart. Callers pick a named dialect + (`MySQLRaw`, `MySQLPyformat`, `SQLite`) - the fields are unexported, so no + call site can assemble rules the producer of the text never used. - `speedups: true` swaps date/datetime decoding to `ciso8601` (sqlite converter bodies, turso inline decodes); the import resolver tracks which variant is emitted. diff --git a/internal/driver/common.go b/internal/driver/common.go index 7b53372f..567401fb 100644 --- a/internal/driver/common.go +++ b/internal/driver/common.go @@ -2,10 +2,12 @@ package driver import ( "fmt" + "slices" "strings" "github.com/rayakame/sqlc-gen-better-python/internal/config" "github.com/rayakame/sqlc-gen-better-python/internal/model" + "github.com/rayakame/sqlc-gen-better-python/internal/sqllex" "github.com/rayakame/sqlc-gen-better-python/internal/types" "github.com/rayakame/sqlc-gen-better-python/internal/writer" "github.com/sqlc-dev/plugin-sdk-go/metadata" @@ -62,82 +64,57 @@ func writeFuncSignature( // type natively. type wireConvertFunc func(sqlType string) (string, bool) -// placeholderStyle describes how bindable placeholders appear in a query's -// final SQL text. The sqlite-family drivers keep sqlc's native "?"; the -// MySQL drivers rewrite to pyformat "%s" at IR build time, which also -// changes the lexing rules for the surrounding text. +// placeholderStyle pairs the dialect a driver's SQL is written in with the +// expression that expands a sqlc.slice at call time. The sqlite family +// multiplies the single-character "?"; pyformat needs a tuple repeat because +// join would otherwise walk "%s" character by character. type placeholderStyle struct { - // token is one bindable placeholder as it appears in the SQL. - token string - // joinExpr is the Sprintf template (one %s verb: the sequence - // expression) for the runtime slice expansion - one comma-joined - // placeholder per element, "NULL" for an empty sequence. + dialect sqllex.Dialect joinExpr string - // numbered marks placeholders that may carry a digit suffix ("?2", - // sqlite only); the digits belong to the token. - numbered bool - // backslashEscapes marks '...' and "..." literals as honoring - // backslash escapes in addition to doubled quotes (MySQL). - backslashEscapes bool - // hashComments marks "#" as a line-comment introducer (MySQL). - hashComments bool - // dashCommentNeedsGap requires whitespace (or end of input) after "--" - // for it to start a comment (MySQL; "a--1" is arithmetic). - dashCommentNeedsGap bool - // backtickIdents marks `...` as quoted identifiers (MySQL). - backtickIdents bool - // versionComments marks /*! comment bodies as live SQL that can hold - // placeholders (MySQL; sqlc's parser emits parameters for them). - versionComments bool - // doubledToken is a non-placeholder escape sequence to skip as a unit - // ("%%" in pyformat text); empty when not applicable. - doubledToken string } var ( - questionPlaceholders = placeholderStyle{ - token: "?", + questionStyle = placeholderStyle{ //nolint:gochecknoglobals + dialect: sqllex.SQLite, joinExpr: `",".join("?" * len(%s)) or "NULL"`, - numbered: true, - } - pyformatPlaceholders = placeholderStyle{ - token: "%s", - // A tuple repeat, not a string repeat: join iterates strings - // per-character, which only works for one-char placeholders. - joinExpr: `",".join(("%%s",) * len(%s)) or "NULL"`, - backslashEscapes: true, - hashComments: true, - dashCommentNeedsGap: true, - backtickIdents: true, - versionComments: true, - doubledToken: "%%", + } + pyformatStyle = placeholderStyle{ //nolint:gochecknoglobals + dialect: sqllex.MySQLPyformat, + joinExpr: `",".join(("%%s",) * len(%s)) or "NULL"`, } ) +// querySlots scans a query's final SQL for its bindable positions. Both the +// argument ordering and the slice expansion read it, so a marker inside a +// string or comment can never be counted by one and skipped by the other. +func querySlots(query model.Query, ph placeholderStyle) []sqllex.Slot { + return sqllex.Slots(query.SQL, ph.dialect) +} + // expandParams returns the Python argument expressions for a query's parameters. // Bundled Params classes (query_parameter_limit) are expanded into their fields // ("params.a, params.b") so drivers receive positional values. :copyfrom params // are never passed through here - writeCopyFromBody builds its own records list. func expandParams(query model.Query) []string { - return expandParamsImpl(query, false, nil, questionPlaceholders) + return expandParamsImpl(query, false, nil, questionStyle) } // expandParamsFlattenSlices additionally star-unpacks sqlc.slice parameters // ("*ids"), so after runtime placeholder expansion every "?" binds one element. func expandParamsFlattenSlices(query model.Query) []string { - return expandParamsImpl(query, true, nil, questionPlaceholders) + return expandParamsImpl(query, true, nil, questionStyle) } -// expandParamsFlattenSlicesWire is expandParamsFlattenSlices for drivers that -// additionally convert parameters to their wire type inline. +// expandParamsFlattenSlicesWire is expandParamsFlattenSlices for the turso +// drivers, which additionally convert parameters to their wire type inline. func expandParamsFlattenSlicesWire(query model.Query, wire wireConvertFunc) []string { - return expandParamsImpl(query, true, wire, questionPlaceholders) + return expandParamsImpl(query, true, wire, questionStyle) } -// expandParamsPyformat is the MySQL variant: wire conversion plus the -// pyformat placeholder style of the rewritten SQL text. +// expandParamsPyformat is the MySQL variant: wire conversion over text whose +// placeholders were rewritten to pyformat. func expandParamsPyformat(query model.Query, wire wireConvertFunc) []string { - return expandParamsImpl(query, true, wire, pyformatPlaceholders) + return expandParamsImpl(query, true, wire, pyformatStyle) } func expandParamsImpl(query model.Query, flattenSlices bool, wire wireConvertFunc, ph placeholderStyle) []string { @@ -170,9 +147,15 @@ func expandParamsImpl(query model.Query, flattenSlices bool, wire wireConvertFun appendPart(param.Name, param.Type) } + // Only slice parameters consult the bind order, and the argument list of + // a query without one is already in text order. + var slots []sqllex.Slot + if slices.ContainsFunc(parts, func(p part) bool { return p.slice != "" }) { + slots = querySlots(query, ph) + } reused := false for _, p := range parts { - if p.slice != "" && sliceMarkerCount(query, p.slice, ph) > 1 { + if p.slice != "" && slotMarkerCount(slots, p.slice) > 1 { reused = true break @@ -199,7 +182,7 @@ func expandParamsImpl(query model.Query, flattenSlices bool, wire wireConvertFun starred[p.slice] = p.expr } } - if ordered, ok := orderByPlaceholders(query.SQL, plain, starred, ph); ok { + if ordered, ok := orderByPlaceholders(slots, plain, starred); ok { return ordered } @@ -208,7 +191,7 @@ func expandParamsImpl(query model.Query, flattenSlices bool, wire wireConvertFun out := make([]string, 0, len(parts)) for _, p := range parts { if p.slice != "" { - for range sliceMarkerCount(query, p.slice, ph) { + for range slotMarkerCount(slots, p.slice) { out = append(out, p.expr) } @@ -220,16 +203,17 @@ func expandParamsImpl(query model.Query, flattenSlices bool, wire wireConvertFun return out } -// orderByPlaceholders lines the flattened arguments up with the SQL text's -// placeholder sequence: plain expressions fill "?" slots in order, and every -// marker occurrence gets its slice's starred copy. Reports false when the SQL -// does not account for exactly the given arguments. -func orderByPlaceholders(sql string, plain []string, starred map[string]string, ph placeholderStyle) ([]string, bool) { - seq := placeholderSequence(sql, ph) - out := make([]string, 0, len(seq)) +// orderByPlaceholders lines the flattened arguments up with the query's bind +// order: plain expressions fill plain slots in order, and every marker +// occurrence gets its slice's starred copy. Reports false when the bind order +// does not account for exactly the given arguments - only reachable from +// hand-built IR, since transform derives SQL and its bind order together. +func orderByPlaceholders(slots []sqllex.Slot, plain []string, starred map[string]string) ([]string, bool) { + out := make([]string, 0, len(slots)) next := 0 used := make(map[string]struct{}, len(starred)) - for _, name := range seq { + for _, slot := range slots { + name := slot.Name if name == "" { if next >= len(plain) { return nil, false @@ -255,100 +239,6 @@ func orderByPlaceholders(sql string, plain []string, starred map[string]string, return out, true } -// placeholderSequence scans the SQL for bindable placeholders in text order: -// the raw slice name for a /*SLICE:name*/ marker, "" for a plain -// (possibly numbered) token. String literals, quoted identifiers, and -// comments are skipped, so a token inside them never counts as a -// placeholder. The lexing rules follow the style: MySQL text adds backslash -// escapes, backtick identifiers, "#" comments, the "--"+whitespace rule, -// and the "%%" literal escape. -func placeholderSequence(sql string, ph placeholderStyle) []string { - var seq []string - for i := 0; i < len(sql); { - rest := sql[i:] - switch { - case strings.HasPrefix(rest, "/*SLICE:"): - end := strings.Index(rest, "*/"+ph.token) - if end == -1 { - return seq - } - seq = append(seq, rest[len("/*SLICE:"):end]) - i += end + len("*/") + len(ph.token) - case ph.versionComments && strings.HasPrefix(rest, "/*!"): - // The body is live SQL: keep scanning it; the closing */ passes - // through the default case as ordinary text. - i += len("/*!") - case strings.HasPrefix(rest, "/*"): - end := strings.Index(rest[len("/*"):], "*/") - if end == -1 { - return seq - } - i += len("/*") + end + len("*/") - case strings.HasPrefix(rest, "--"): - if ph.dashCommentNeedsGap && len(rest) > 2 && rest[2] > ' ' { - // MySQL: "--x" is double unary minus, not a comment. Advance - // one byte, not two: in an odd-length dash run the comment - // starts mid-run, and the rewriter re-examines every - // position the same way. - i++ - - continue - } - end := strings.IndexByte(rest, '\n') - if end == -1 { - return seq - } - i += end + 1 - case ph.hashComments && rest[0] == '#': - end := strings.IndexByte(rest, '\n') - if end == -1 { - return seq - } - i += end + 1 - case rest[0] == '\'' || rest[0] == '"' || (ph.backtickIdents && rest[0] == '`'): - // Backslash escapes never apply inside backticks. - i = scanQuotedRegion(sql, i, ph.backslashEscapes && rest[0] != '`') - case ph.doubledToken != "" && strings.HasPrefix(rest, ph.doubledToken): - i += len(ph.doubledToken) - case strings.HasPrefix(rest, ph.token): - seq = append(seq, "") - i += len(ph.token) - if ph.numbered { - for i < len(sql) && sql[i] >= '0' && sql[i] <= '9' { - i++ - } - } - default: - i++ - } - } - - return seq -} - -// scanQuotedRegion returns the index just past the closing quote of the -// quoted region starting at sql[i]. A doubled quote is an escape; with -// escapes, a backslash escapes the following byte. An unterminated region -// consumes the rest of the input. -func scanQuotedRegion(sql string, i int, escapes bool) int { - quote := sql[i] - j := i + 1 - for j < len(sql) { - switch { - case escapes && sql[j] == '\\' && j+1 < len(sql): - j += 2 - case sql[j] != quote: - j++ - case j+1 < len(sql) && sql[j+1] == quote: - j += 2 - default: - return j + 1 - } - } - - return j -} - type sliceParam struct { // marker is the raw sqlc.slice name inside the /*SLICE:name*/? placeholder. marker string @@ -356,24 +246,40 @@ type sliceParam struct { expr string } -// sliceMarker returns the placeholder left in the SQL for a slice name: -// sqlc's raw marker for "?" styles, its rewritten form for pyformat. -func sliceMarker(name string, ph placeholderStyle) string { - return "/*SLICE:" + name + "*/" + ph.token -} - -// sliceMarkerCount reports how often a slice parameter's placeholder occurs in -// the query. sqlc merges same-named sqlc.slice uses into ONE parameter but +// slotMarkerCount reports how often a slice parameter's marker occurs in the +// scanned text. sqlc merges same-named sqlc.slice uses into ONE parameter but // keeps a marker per use site, so each occurrence needs its own expansion and -// its own copy of the arguments. Clamped to 1 for queries without the marker. -func sliceMarkerCount(query model.Query, name string, ph placeholderStyle) int { - if count := strings.Count(query.SQL, sliceMarker(name, ph)); count > 1 { +// its own copy of the arguments. Clamped to 1 for hand-built IR without SQL. +func slotMarkerCount(slots []sqllex.Slot, name string) int { + count := 0 + for _, slot := range slots { + if slot.Name == name { + count++ + } + } + if count > 1 { return count } return 1 } +// slotMarkerText returns a slice parameter's marker exactly as it appears in +// the SQL. Taking the scanned text rather than rebuilding it from the raw +// sqlc name is what keeps a name containing "%" - which the MySQL rewriter +// doubles - replaceable at runtime. A marker the scan never saw falls back to +// the reconstructed form: Python's str.replace("") PREPENDS its argument, so +// an empty target would corrupt the statement rather than leave it alone. +func slotMarkerText(slots []sqllex.Slot, name string, ph placeholderStyle) string { + for _, slot := range slots { + if slot.Name == name { + return slot.Marker + } + } + + return ph.dialect.SliceMarker(name) +} + // sliceParams collects the sqlc.slice parameters of a query, including fields // of a bundled Params class. func sliceParams(query model.Query) []sliceParam { @@ -515,15 +421,16 @@ func writeSliceExpansion(body *writer.CodeWriter, indent int, query model.Query, if len(params) == 0 { return query.ConstantName } + slots := querySlots(query, ph) src := query.ConstantName for _, param := range params { args := []string{ - writer.PyQuote(sliceMarker(param.marker, ph)), + writer.PyQuote(slotMarkerText(slots, param.marker, ph)), fmt.Sprintf(ph.joinExpr, param.expr), } // A reused slice has one marker per use site: replace them all, with // the flattening param expansion supplying a copy of the args for each. - if sliceMarkerCount(query, param.marker, ph) == 1 { + if slotMarkerCount(slots, param.marker) == 1 { args = append(args, "1") } body.WriteWrappedCall(indent, "sql = "+src+".replace(", args, ")") diff --git a/internal/driver/common_test.go b/internal/driver/common_test.go index b65b36a2..e003a88d 100644 --- a/internal/driver/common_test.go +++ b/internal/driver/common_test.go @@ -328,182 +328,6 @@ func TestExpandParamsFlattenSlices(t *testing.T) { } } -func TestPlaceholderSequence(t *testing.T) { - t.Parallel() - cases := []struct { - name string - sql string - want []string - }{ - { - name: "markers and plain placeholders in text order", - sql: "WHERE id IN (/*SLICE:ids*/?) AND name = ? AND ref_id IN (/*SLICE:ids*/?)", - want: []string{"ids", "", "ids"}, - }, - { - name: "question marks inside string literals do not count", - sql: "WHERE note LIKE 'what?%' AND s = 'it''s?' AND id IN (/*SLICE:ids*/?)", - want: []string{"ids"}, - }, - { - name: "quoted identifiers and comments are skipped", - sql: "SELECT \"weird?col\" FROM t /* really? */ WHERE a = ? -- trailing?\nAND b = ?2", - want: []string{"", ""}, - }, - { - name: "unterminated marker stops the scan", - sql: "WHERE a = ? AND id IN (/*SLICE:ids", - want: []string{""}, - }, - { - name: "unterminated comment stops the scan", - sql: "WHERE a = ? /* dangling", - want: []string{""}, - }, - { - name: "unterminated line comment stops the scan", - sql: "WHERE a = ? -- dangling", - want: []string{""}, - }, - { - name: "unterminated string swallows the rest", - sql: "WHERE a = ? AND s = 'open?", - want: []string{""}, - }, - // The question style must keep sqlite's lexing rules where they - // differ from the pyformat flags: -- comments need no gap, "#", - // backticks, and backslashes are ordinary text. - { - name: "dash dash glued to text is still a comment", - sql: "SELECT a--1 dead ?\nFROM t WHERE b = ?", - want: []string{""}, - }, - { - name: "hash is not a comment", - sql: "WHERE x = ? # not a comment ?", - want: []string{"", ""}, - }, - { - name: "backtick is ordinary text", - sql: "SELECT `weird?col` FROM t WHERE a = ?", - want: []string{"", ""}, - }, - { - name: "backslash does not escape a quote", - sql: `WHERE s = 'a\' AND b = ?`, - want: []string{""}, - }, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - if got := placeholderSequence(tc.sql, questionPlaceholders); !slices.Equal(got, tc.want) { - t.Errorf("placeholderSequence() = %q, want %q", got, tc.want) - } - }) - } -} - -func TestPlaceholderSequencePyformat(t *testing.T) { - t.Parallel() - cases := []struct { - name string - sql string - want []string - }{ - { - name: "percent-s slots in text order", - sql: "WHERE a = %s AND b = %s", - want: []string{"", ""}, - }, - { - name: "doubled percent is a literal, not a slot", - sql: "WHERE a %% 2 = 0 AND b = %s", - want: []string{""}, - }, - { - name: "lone percent is not a slot", - sql: "WHERE a % 2 = 0 AND b = %s", - want: []string{""}, - }, - { - name: "slice marker yields the name", - sql: "WHERE id IN (/*SLICE:ids*/%s)", - want: []string{"ids"}, - }, - { - name: "slots inside string literals do not count despite backslash escapes", - sql: "WHERE s = 'It\\'s %s' AND t = \"quote \\\" %s\" AND a = %s", - want: []string{""}, - }, - { - name: "backticked identifier swallows its slot", - sql: "SELECT `weird %s col` FROM t WHERE a = %s", - want: []string{""}, - }, - { - // If backslash escaped the closing backtick, the identifier would - // swallow the rest of the input and the slot with it. - name: "backslash is not an escape inside backticks", - sql: "SELECT `dir\\` FROM t WHERE a = %s", - want: []string{""}, - }, - { - name: "hash comment is dead to the newline", - sql: "WHERE a = %s # dead %s\nAND b = %s", - want: []string{"", ""}, - }, - { - name: "dash dash with whitespace starts a comment", - sql: "WHERE a = %s -- x %s\nAND b = %s", - want: []string{"", ""}, - }, - { - name: "dash dash without whitespace is arithmetic, slot stays live", - sql: "WHERE a = b--1 + %s", - want: []string{""}, - }, - { - name: "block comment hides its slot", - sql: "WHERE a = %s /* %s */ AND b = %s", - want: []string{"", ""}, - }, - { - name: "unterminated string swallows the rest", - sql: "WHERE a = %s AND s = 'open %s", - want: []string{""}, - }, - { - name: "unterminated hash comment swallows the rest", - sql: "WHERE a = %s # tail %s", - want: []string{""}, - }, - { - name: "version comment body is live", - sql: "SELECT id /*! WHERE a = %s */ AND b = %s", - want: []string{"", ""}, - }, - { - name: "odd dash run still starts a comment", - sql: "WHERE a = %s --------- don't edit\nAND id IN (/*SLICE:ids*/%s)", - want: []string{"", "ids"}, - }, - { - name: "numbered question placeholder is not a slot", - sql: "WHERE a = ?1 AND b = %s", - want: []string{""}, - }, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - if got := placeholderSequence(tc.sql, pyformatPlaceholders); !slices.Equal(got, tc.want) { - t.Errorf("placeholderSequence() = %q, want %q", got, tc.want) - } - }) - } -} - func TestSliceParams(t *testing.T) { t.Parallel() cases := []struct { @@ -966,46 +790,80 @@ func TestExpandParamsFlattenSlicesWire(t *testing.T) { } } -func TestSliceMarkerStyles(t *testing.T) { +func TestSlotMarkerHelpers(t *testing.T) { t.Parallel() - markers := []struct { - name string - ph placeholderStyle - want string - }{ - {name: "question marker keeps sqlc's raw form", ph: questionPlaceholders, want: "/*SLICE:ids*/?"}, - {name: "pyformat marker uses the rewritten token", ph: pyformatPlaceholders, want: "/*SLICE:ids*/%s"}, - } - for _, tc := range markers { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - if got := sliceMarker("ids", tc.ph); got != tc.want { - t.Errorf("sliceMarker(%q) = %q, want %q", "ids", got, tc.want) - } - }) - } - counts := []struct { - name string - sql string - want int + cases := []struct { + name string + sliceName string + sql string + style placeholderStyle + wantCount int + wantText string }{ { - name: "two pyformat markers count both", - sql: "WHERE id IN (/*SLICE:ids*/%s) OR ref_id IN (/*SLICE:ids*/%s)", - want: 2, - }, - { - name: "missing marker clamps to one", - sql: "WHERE id = %s", - want: 1, + name: "question marker keeps sqlc's raw form", + sliceName: "ids", + sql: "WHERE id IN (/*SLICE:ids*/?)", + style: questionStyle, + wantCount: 1, + wantText: "/*SLICE:ids*/?", + }, + { + name: "pyformat marker keeps the rewritten token", + sliceName: "ids", + sql: "WHERE id IN (/*SLICE:ids*/%s)", + style: pyformatStyle, + wantCount: 1, + wantText: "/*SLICE:ids*/%s", + }, + { + // The rewriter doubles a percent inside the marker, so the text + // scanned back out has to be doubled too or the generated + // replace misses. + name: "percent in a slice name keeps its doubled marker", + sliceName: "a%b", + sql: "WHERE id IN (/*SLICE:a%%b*/%s)", + style: pyformatStyle, + wantCount: 1, + wantText: "/*SLICE:a%%b*/%s", + }, + { + name: "two markers count both", + sliceName: "ids", + sql: "WHERE id IN (/*SLICE:ids*/%s) OR r IN (/*SLICE:ids*/%s)", + style: pyformatStyle, + wantCount: 2, + wantText: "/*SLICE:ids*/%s", + }, + { + // A marker inside a string literal is not a bind slot, so the + // text falls back to the reconstructed marker rather than the + // empty string, which str.replace would prepend. + name: "marker inside a string literal is invisible", + sliceName: "ids", + sql: "WHERE note = '/*SLICE:ids*/?'", + style: questionStyle, + wantCount: 1, + wantText: "/*SLICE:ids*/?", + }, + { + name: "missing marker clamps to one and rebuilds its text", + sliceName: "ids", + sql: "WHERE id = ?", + style: questionStyle, + wantCount: 1, + wantText: "/*SLICE:ids*/?", }, } - for _, tc := range counts { + for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { t.Parallel() - query := model.Query{SQL: tc.sql} - if got := sliceMarkerCount(query, "ids", pyformatPlaceholders); got != tc.want { - t.Errorf("sliceMarkerCount(%q) = %d, want %d", tc.sql, got, tc.want) + slots := querySlots(model.Query{SQL: tc.sql}, tc.style) + if got := slotMarkerCount(slots, tc.sliceName); got != tc.wantCount { + t.Errorf("slotMarkerCount(%q) = %d, want %d", tc.sliceName, got, tc.wantCount) + } + if got := slotMarkerText(slots, tc.sliceName, tc.style); got != tc.wantText { + t.Errorf("slotMarkerText(%q) = %q, want %q", tc.sliceName, got, tc.wantText) } }) } diff --git a/internal/driver/mysql_base.go b/internal/driver/mysql_base.go index a76b2f83..fcb3011d 100644 --- a/internal/driver/mysql_base.go +++ b/internal/driver/mysql_base.go @@ -187,7 +187,7 @@ func (mb *mysqlBase) WriteQueryFunc(body *writer.CodeWriter, config *config.Conf // line keeps the assignment from touching the nested def (ruff E306). sqlRef := query.ConstantName if query.Cmd != metadata.CmdMany { - sqlRef = writeSliceExpansion(body, indent, query, pyformatPlaceholders) + sqlRef = writeSliceExpansion(body, indent, query, pyformatStyle) } // Neither MySQL module has conn.execute: every body opens a cursor. The @@ -242,7 +242,7 @@ func (mb *mysqlBase) WriteQueryFunc(body *writer.CodeWriter, config *config.Conf case metadata.CmdMany: decodeHook := mb.rows.WriteDecodeHook(body, indent, query, mysqlResultType) - sqlRef = writeSliceExpansion(body, indent, query, pyformatPlaceholders) + sqlRef = writeSliceExpansion(body, indent, query, pyformatStyle) manyArgs := append([]string{conn, sqlRef, decodeHook}, parts...) // Deliberately unsubscripted: QueryResults[T](...) would go through // typing's _GenericAlias.__call__ on every invocation (~10x call diff --git a/internal/driver/sqlite_base.go b/internal/driver/sqlite_base.go index e3561cb1..a17fae18 100644 --- a/internal/driver/sqlite_base.go +++ b/internal/driver/sqlite_base.go @@ -196,7 +196,7 @@ func (sb *sqliteBase) WriteQueryFunc(body *writer.CodeWriter, config *config.Con // line keeps the assignment from touching the nested def (ruff E306). sqlRef := query.ConstantName if query.Cmd != metadata.CmdMany { - sqlRef = writeSliceExpansion(body, indent, query, questionPlaceholders) + sqlRef = writeSliceExpansion(body, indent, query, questionStyle) } // stmt builds the execute-statement head/tail with the correct await @@ -250,7 +250,7 @@ func (sb *sqliteBase) WriteQueryFunc(body *writer.CodeWriter, config *config.Con case metadata.CmdMany: decodeHook := sb.rows.WriteDecodeHook(body, indent, query, sqliteResultType) - sqlRef = writeSliceExpansion(body, indent, query, questionPlaceholders) + sqlRef = writeSliceExpansion(body, indent, query, questionStyle) manyArgs := append([]string{conn, sqlRef, decodeHook}, expandParamsFlattenSlices(query)...) // Deliberately unsubscripted: QueryResults[T](...) would go through // typing's _GenericAlias.__call__ on every invocation (~10x call diff --git a/internal/driver/sqlite_test.go b/internal/driver/sqlite_test.go index edd30735..4d88170e 100644 --- a/internal/driver/sqlite_test.go +++ b/internal/driver/sqlite_test.go @@ -550,6 +550,7 @@ func TestSqliteWriteQueryFunc(t *testing.T) { query: model.Query{ Cmd: metadata.CmdMany, ConstantName: "GET_ROWS", + SQL: "SELECT id, name FROM t WHERE id IN (/*SLICE:ids*/?)", FuncName: "get_rows", Params: []model.QueryValue{ {Name: "ids", Type: model.PyType{Type: "int", SQLType: "integer", IsList: true, SqlcSliceName: "ids"}}, @@ -572,6 +573,7 @@ func TestSqliteWriteQueryFunc(t *testing.T) { query: model.Query{ Cmd: metadata.CmdOne, ConstantName: "GET_ROW", + SQL: "SELECT id, name FROM t WHERE name = ? AND id IN (/*SLICE:ids*/?) AND note = ?", FuncName: "get_row", Params: []model.QueryValue{ {Name: "name", Type: model.PyType{Type: "str", SQLType: "text"}}, @@ -596,6 +598,7 @@ func TestSqliteWriteQueryFunc(t *testing.T) { query: model.Query{ Cmd: metadata.CmdExec, ConstantName: "DELETE_ROWS", + SQL: "DELETE FROM t WHERE id IN (/*SLICE:ids*/?) AND name IN (/*SLICE:names*/?)", FuncName: "delete_rows", Params: []model.QueryValue{ {Name: "ids", Type: model.PyType{Type: "int", SQLType: "integer", IsList: true, SqlcSliceName: "ids"}}, diff --git a/internal/driver/turso.go b/internal/driver/turso.go index 1e215ece..ccfd3d78 100644 --- a/internal/driver/turso.go +++ b/internal/driver/turso.go @@ -310,7 +310,7 @@ func (tb *tursoBase) WriteQueryFunc(body *writer.CodeWriter, config *config.Conf // line keeps the assignment from touching the nested def (ruff E306). sqlRef := query.ConstantName if query.Cmd != metadata.CmdMany { - sqlRef = writeSliceExpansion(body, indent, query, questionPlaceholders) + sqlRef = writeSliceExpansion(body, indent, query, questionStyle) } // stmt builds the execute-statement head/tail with the correct await @@ -365,7 +365,7 @@ func (tb *tursoBase) WriteQueryFunc(body *writer.CodeWriter, config *config.Conf case metadata.CmdMany: decodeHook := tb.rows.WriteDecodeHook(body, indent, query, tursoResultType) - sqlRef = writeSliceExpansion(body, indent, query, questionPlaceholders) + sqlRef = writeSliceExpansion(body, indent, query, questionStyle) manyArgs := append([]string{conn, sqlRef, decodeHook}, parts...) // Deliberately unsubscripted: QueryResults[T](...) would go through // typing's _GenericAlias.__call__ on every invocation (~10x call diff --git a/internal/sqllex/dialect.go b/internal/sqllex/dialect.go new file mode 100644 index 00000000..8b42e5b5 --- /dev/null +++ b/internal/sqllex/dialect.go @@ -0,0 +1,87 @@ +// Package sqllex lexes the SQL sqlc hands the plugin, and the pyformat text +// the MySQL rewriter produces from it, far enough to tell a bindable +// placeholder from a "?" that merely sits inside a string, an identifier or a +// comment. The rewriter and the drivers' argument ordering share it so the +// rules cannot drift apart. +package sqllex + +// Dialect is the rule set for one SQL text. Its fields are unexported and +// only the values below are exported: every caller picks a named dialect +// instead of assembling flags, so no call site can lex with rules the +// producer of the text never used. +type Dialect struct { + // placeholder is one bindable slot as it appears in the text. + placeholder string + // escaped is a placeholder lookalike that is a literal instead ("%%" in + // pyformat text); empty when the dialect has none. + escaped string + // numbered marks placeholders that carry a digit suffix (sqlite's ?N). + numbered bool + // backslashEscapes marks '...' and "..." as honoring backslash escapes + // in addition to doubled quotes. + backslashEscapes bool + // backtickIdents and bracketIdents mark `...` and [...] as quoted + // identifiers, whose contents never bind. + backtickIdents bool + bracketIdents bool + // hashComments marks "#" as a line-comment introducer. + hashComments bool + // dashNeedsGap requires whitespace (or end of input) after "--" for it + // to start a comment; "a--1" is double unary minus. + dashNeedsGap bool + // liveVersionComments marks the body of a /*! comment as executable SQL + // that can hold placeholders. + liveVersionComments bool +} + +// The three texts the plugin lexes. MySQLRaw and MySQLPyformat describe the +// same grammar either side of the rewrite, which is why they must be defined +// together: the rewriter reads the first and every consumer of its output +// reads the second. +var ( + // MySQLRaw is sqlc's MySQL output, before the pyformat rewrite. Only + // default sql_mode is supported: sqlc's dolphin parser lexes with + // backslash escapes on and treats "..." as a string, so a query that + // reached the plugin already parsed under those rules. + MySQLRaw = Dialect{ //nolint:gochecknoglobals + placeholder: "?", + escaped: "", + numbered: false, + backslashEscapes: true, + backtickIdents: true, + bracketIdents: false, + hashComments: true, + dashNeedsGap: true, + liveVersionComments: true, + } + + // MySQLPyformat is the same text after the rewrite, where placeholders + // are "%s" and a literal percent has been doubled. + MySQLPyformat = Dialect{ //nolint:gochecknoglobals + placeholder: "%s", + escaped: "%%", + numbered: false, + backslashEscapes: true, + backtickIdents: true, + bracketIdents: false, + hashComments: true, + dashNeedsGap: true, + liveVersionComments: true, + } + + // SQLite is sqlc's SQLite output, which the drivers execute unchanged. + // sqlc numbers named parameters (?1, ?2); "#" is not a comment and + // backslashes are not escapes, but both `...` and [...] quote + // identifiers. + SQLite = Dialect{ //nolint:gochecknoglobals + placeholder: "?", + escaped: "", + numbered: true, + backslashEscapes: false, + backtickIdents: true, + bracketIdents: true, + hashComments: false, + dashNeedsGap: false, + liveVersionComments: false, + } +) diff --git a/internal/sqllex/lex.go b/internal/sqllex/lex.go new file mode 100644 index 00000000..f7034c65 --- /dev/null +++ b/internal/sqllex/lex.go @@ -0,0 +1,243 @@ +package sqllex + +import "strings" + +// Kind classifies a scanned token. +type Kind uint8 + +const ( + // KindText is ordinary SQL: copied as-is, holds nothing bindable. + KindText Kind = iota + // KindSkipped is a string literal, quoted identifier or comment. It is + // copied as-is too, but is called out because a placeholder inside one + // does not bind. + KindSkipped + // KindPlaceholder is one bindable slot. + KindPlaceholder + // KindSliceMarker is a /*SLICE:name*/ marker and the placeholder it + // binds, which sqlc always emits adjacent. + KindSliceMarker +) + +// sliceMarkerPrefix opens the marker sqlc leaves for a sqlc.slice parameter. +const sliceMarkerPrefix = "/*SLICE:" + +// dashComment is the line-comment introducer both engines share. +const dashComment = "--" + +// Token is one scanned span of the input, [Start, End). +type Token struct { + Kind Kind + Start int + End int + // Name is the sqlc.slice name of a KindSliceMarker token. + Name string + // MarkerEnd splits a KindSliceMarker: [Start, MarkerEnd) is the comment, + // [MarkerEnd, End) the placeholder it binds. Zero for other kinds. + MarkerEnd int +} + +// Slot is a bindable position in the text, in text order. Name and Marker are +// empty for a plain placeholder; for a sqlc.slice they carry the raw name and +// the marker's exact text, which is what generated code has to replace. +type Slot struct { + Name string + Marker string +} + +// Scan splits sql into tokens under the given dialect. Unterminated strings, +// identifiers and comments consume the rest of the input, matching how sqlc's +// own parsers recover. +func Scan(sql string, d Dialect) []Token { + var tokens []Token + text := 0 + flush := func(end int) { + if end > text { + tokens = append(tokens, Token{Kind: KindText, Start: text, End: end, Name: "", MarkerEnd: 0}) + } + } + skip := func(start, end int) { + flush(start) + tokens = append(tokens, Token{Kind: KindSkipped, Start: start, End: end, Name: "", MarkerEnd: 0}) + text = end + } + + for i := 0; i < len(sql); { + rest := sql[i:] + switch { + case strings.HasPrefix(rest, sliceMarkerPrefix): + end := scanBlockComment(sql, i) + // Only an immediately following placeholder binds to the marker; + // anything else leaves an ordinary comment behind. A dialect + // without a placeholder can bind nothing at all. + token := end + len(d.placeholder) + if d.placeholder != "" && token <= len(sql) && sql[end:token] == d.placeholder { + flush(i) + tokens = append(tokens, Token{ + Kind: KindSliceMarker, + Start: i, + End: token, + Name: d.sliceName(sql[i+len(sliceMarkerPrefix) : end]), + MarkerEnd: end, + }) + i, text = token, token + + continue + } + skip(i, end) + i = end + case d.liveVersionComments && strings.HasPrefix(rest, "/*!"): + // The body is executable SQL, so only the opener is consumed and + // the closing */ falls through as ordinary text. + i += len("/*!") + case strings.HasPrefix(rest, "/*"): + end := scanBlockComment(sql, i) + skip(i, end) + i = end + case strings.HasPrefix(rest, dashComment) && (!d.dashNeedsGap || isCommentGap(sql, i+len(dashComment))): + end := scanLineEnd(sql, i) + skip(i, end) + i = end + case d.hashComments && rest[0] == '#': + end := scanLineEnd(sql, i) + skip(i, end) + i = end + case rest[0] == '\'' || rest[0] == '"' || + (d.backtickIdents && rest[0] == '`'): + // Backslash escapes never apply inside backticks. + end := scanQuoted(sql, i, rest[0], d.backslashEscapes && rest[0] != '`') + skip(i, end) + i = end + case d.bracketIdents && rest[0] == '[': + end := scanBracket(sql, i) + skip(i, end) + i = end + case d.escaped != "" && strings.HasPrefix(rest, d.escaped): + i += len(d.escaped) + // The guard is what stops a zero-value Dialect: an empty prefix + // matches everywhere, and the scan would never advance. + case d.placeholder != "" && strings.HasPrefix(rest, d.placeholder): + flush(i) + end := i + len(d.placeholder) + if d.numbered { + for end < len(sql) && sql[end] >= '0' && sql[end] <= '9' { + end++ + } + } + tokens = append(tokens, Token{Kind: KindPlaceholder, Start: i, End: end, Name: "", MarkerEnd: 0}) + i, text = end, end + default: + i++ + } + } + flush(len(sql)) + + return tokens +} + +// Placeholder returns one bindable placeholder as it appears in this +// dialect's text. Emitters use it so the token they write and the token the +// scanner looks for can never be two different literals. +func (d Dialect) Placeholder() string { + return d.placeholder +} + +// SliceMarker returns the marker sqlc leaves for a sqlc.slice parameter, +// together with the placeholder it binds. Scanning reports the marker's +// actual text; this rebuilds it for the one caller that has a name but no +// scanned text to point at. +func (d Dialect) SliceMarker(name string) string { + return sliceMarkerPrefix + name + "*/" + d.placeholder +} + +// Slots reports the bindable positions of sql in text order. +func Slots(sql string, d Dialect) []Slot { + var slots []Slot + for _, token := range Scan(sql, d) { + switch token.Kind { + case KindPlaceholder: + slots = append(slots, Slot{Name: "", Marker: ""}) + case KindSliceMarker: + slots = append(slots, Slot{Name: token.Name, Marker: sql[token.Start:token.End]}) + case KindText, KindSkipped: + } + } + + return slots +} + +// sliceName recovers the sqlc.slice name from a marker body. In text whose +// literals were doubled by the rewriter the name is doubled too, so the +// dialect that describes such text undoes it - the name has to match the +// parameter sqlc reported, not the escaped spelling. +func (d Dialect) sliceName(body string) string { + name := strings.TrimSuffix(body, "*/") + if d.escaped == "" { + return name + } + + return strings.ReplaceAll(name, d.escaped, d.escaped[:len(d.escaped)/2]) +} + +// isCommentGap reports whether the byte at i lets a preceding "--" start a +// comment: MySQL wants whitespace, a control character, or end of input. +func isCommentGap(sql string, i int) bool { + return i >= len(sql) || sql[i] <= ' ' +} + +// scanLineEnd returns the index of the \n ending a line comment, or the end +// of the input. The terminator is left for the caller: it is ordinary text. +// Both MySQL and SQLite end line comments only at \n, so a bare \r stays +// comment text. +func scanLineEnd(sql string, i int) int { + end := strings.IndexByte(sql[i:], '\n') + if end == -1 { + return len(sql) + } + + return i + end +} + +// scanQuoted returns the index after a quoted region starting at i. A doubled +// quote is always an escape; a backslash escapes the next byte only where the +// dialect says so. +func scanQuoted(sql string, i int, quote byte, escapes bool) int { + j := i + 1 + for j < len(sql) { + switch { + case escapes && sql[j] == '\\' && j+1 < len(sql): + j += 2 + case sql[j] != quote: + j++ + case j+1 < len(sql) && sql[j+1] == quote: + j += 2 + default: + return j + 1 + } + } + + return len(sql) +} + +// scanBracket returns the index after a [quoted identifier]. SQLite has no +// escape inside brackets: the first ] ends it. +func scanBracket(sql string, i int) int { + end := strings.IndexByte(sql[i:], ']') + if end == -1 { + return len(sql) + } + + return i + end + 1 +} + +// scanBlockComment returns the index after a /* */ comment starting at i. +// Neither engine nests them: the first */ wins. +func scanBlockComment(sql string, i int) int { + body := i + len("/*") + end := strings.Index(sql[body:], "*/") + if end == -1 { + return len(sql) + } + + return body + end + len("*/") +} diff --git a/internal/sqllex/lex_test.go b/internal/sqllex/lex_test.go new file mode 100644 index 00000000..59c20f80 --- /dev/null +++ b/internal/sqllex/lex_test.go @@ -0,0 +1,326 @@ +package sqllex_test + +import ( + "reflect" + "testing" + + "github.com/rayakame/sqlc-gen-better-python/internal/sqllex" +) + +// plain is a bind slot that is not a sqlc.slice marker. +var plain = sqllex.Slot{Name: "", Marker: ""} //nolint:gochecknoglobals + +func TestSlotsMySQLRaw(t *testing.T) { + t.Parallel() + cases := []struct { + name string + sql string + want []sqllex.Slot + }{ + {name: "no placeholders", sql: "SELECT 1"}, + { + name: "plain placeholders in text order", + sql: "SELECT a FROM t WHERE b = ? AND c = ?", + want: []sqllex.Slot{plain, plain}, + }, + { + name: "slice marker carries its text", + sql: "SELECT a FROM t WHERE id IN (/*SLICE:ids*/?)", + want: []sqllex.Slot{{Name: "ids", Marker: "/*SLICE:ids*/?"}}, + }, + { + // The ordering case the drivers depend on. + name: "reused marker interleaved with a plain slot", + sql: "SELECT a FROM t WHERE x IN (/*SLICE:ids*/?) AND y = ? OR z IN (/*SLICE:ids*/?)", + want: []sqllex.Slot{ + {Name: "ids", Marker: "/*SLICE:ids*/?"}, + plain, + {Name: "ids", Marker: "/*SLICE:ids*/?"}, + }, + }, + { + name: "placeholders inside strings are not slots", + sql: "SELECT '?', \"?\", `we?rd` FROM t WHERE a = ?", + want: []sqllex.Slot{plain}, + }, + { + name: "backslash escapes a quote", + sql: "SELECT 'a\\'?b' FROM t WHERE a = ?", + want: []sqllex.Slot{plain}, + }, + { + name: "doubled quotes and backticks escape", + sql: "SELECT 'a''?b', `bt``q?` FROM t WHERE a = ?", + want: []sqllex.Slot{plain}, + }, + { + name: "placeholders inside comments are not slots", + sql: "SELECT a -- ?\n# ?\n/* ? */ FROM t WHERE b = ?", + want: []sqllex.Slot{plain}, + }, + { + // "--x" is arithmetic, so the rest of the line stays live SQL. + name: "dash run without a gap keeps its slot", + sql: "SELECT a FROM t WHERE b = 5--? AND c = ?", + want: []sqllex.Slot{plain, plain}, + }, + { + name: "odd dash run still starts a comment", + sql: "SELECT a FROM t WHERE b = ? --------- don't edit\nAND c = ?", + want: []sqllex.Slot{plain, plain}, + }, + { + name: "bare carriage return stays inside a line comment", + sql: "SELECT a FROM t WHERE b = ? -- note ?\r more ?\nAND c = ?", + want: []sqllex.Slot{plain, plain}, + }, + { + name: "version comment body is live SQL", + sql: "SELECT a FROM t /*! WHERE b = ? */ AND c = ?", + want: []sqllex.Slot{plain, plain}, + }, + { + // MySQL has no ?N: the digits are ordinary text. + name: "digits after a placeholder are text", + sql: "SELECT a FROM t WHERE b = ?1", + want: []sqllex.Slot{plain}, + }, + { + name: "block comment that is not a marker hides its slot", + sql: "SELECT a FROM t /*SLICE ids*/ WHERE b = ?", + want: []sqllex.Slot{plain}, + }, + { + name: "detached marker binds nothing", + sql: "SELECT a FROM t WHERE id IN (/*SLICE:ids*/ ?)", + want: []sqllex.Slot{plain}, + }, + { + name: "brackets are ordinary text", + sql: "SELECT [a? FROM t WHERE b = ?", + want: []sqllex.Slot{plain, plain}, + }, + {name: "unterminated string swallows the rest", sql: "SELECT a FROM t WHERE b = 'open ?"}, + {name: "unterminated block comment swallows the rest", sql: "SELECT a /* open ? FROM t"}, + } + runSlotCases(t, cases, sqllex.MySQLRaw) +} + +func TestSlotsMySQLPyformat(t *testing.T) { + t.Parallel() + cases := []struct { + name string + sql string + want []sqllex.Slot + }{ + { + name: "rewritten placeholders in text order", + sql: "SELECT a FROM t WHERE b = %s AND c = %s", + want: []sqllex.Slot{plain, plain}, + }, + { + // A doubled percent is a literal, not a slot, and must be + // matched before the placeholder itself. + name: "doubled percent is not a slot", + sql: "SELECT a FROM t WHERE b LIKE '50%%' AND c = %s", + want: []sqllex.Slot{plain}, + }, + { + name: "lone percent is not a slot", + sql: "SELECT a %% 2 FROM t WHERE b = %s", + want: []sqllex.Slot{plain}, + }, + { + name: "rewritten marker carries its text", + sql: "SELECT a FROM t WHERE id IN (/*SLICE:ids*/%s)", + want: []sqllex.Slot{{Name: "ids", Marker: "/*SLICE:ids*/%s"}}, + }, + { + // The rewriter doubles a percent inside the marker; the name has + // to come back undoubled so it matches sqlc's parameter. + name: "doubled percent in a slice name is undone", + sql: "SELECT a FROM t WHERE id IN (/*SLICE:a%%b*/%s)", + want: []sqllex.Slot{{Name: "a%b", Marker: "/*SLICE:a%%b*/%s"}}, + }, + { + name: "reused marker interleaved with a plain slot", + sql: "SELECT a FROM t WHERE x IN (/*SLICE:ids*/%s) AND y = %s OR z IN (/*SLICE:ids*/%s)", + want: []sqllex.Slot{ + {Name: "ids", Marker: "/*SLICE:ids*/%s"}, + plain, + {Name: "ids", Marker: "/*SLICE:ids*/%s"}, + }, + }, + } + runSlotCases(t, cases, sqllex.MySQLPyformat) +} + +func TestSlotsSQLite(t *testing.T) { + t.Parallel() + cases := []struct { + name string + sql string + want []sqllex.Slot + }{ + { + // sqlc numbers SQLite parameters; the digits belong to the slot. + name: "numbered placeholders count once each", + sql: "SELECT a FROM t WHERE b = ?1 AND c = ?12", + want: []sqllex.Slot{plain, plain}, + }, + { + name: "slice marker carries its text", + sql: "SELECT a FROM t WHERE id IN (/*SLICE:ids*/?)", + want: []sqllex.Slot{{Name: "ids", Marker: "/*SLICE:ids*/?"}}, + }, + { + name: "reused marker interleaved with a numbered slot", + sql: "SELECT a FROM t WHERE x IN (/*SLICE:ids*/?) AND y = ?2 OR z IN (/*SLICE:ids*/?)", + want: []sqllex.Slot{ + {Name: "ids", Marker: "/*SLICE:ids*/?"}, + plain, + {Name: "ids", Marker: "/*SLICE:ids*/?"}, + }, + }, + { + // A "?" inside a quoted identifier is not a bind slot; counting + // it misorders a reused slice's arguments. + name: "backtick identifier hides its placeholder", + sql: "SELECT `we?rd`, `bt``q?` FROM t WHERE a = ?", + want: []sqllex.Slot{plain}, + }, + { + name: "bracket identifier hides its placeholder", + sql: "SELECT [br?k] FROM t WHERE a = ?", + want: []sqllex.Slot{plain}, + }, + { + // SQLite has no backslash escape: the literal ends at the quote. + name: "backslash does not escape a quote", + sql: "SELECT 'a\\' FROM t WHERE a = ?", + want: []sqllex.Slot{plain}, + }, + { + // SQLite needs no whitespace after the dashes. + name: "dash comment without a gap hides its slot", + sql: "SELECT a FROM t WHERE b = 5--?\nAND c = ?", + want: []sqllex.Slot{plain}, + }, + { + // "#" is not a comment in SQLite, so the slot after it is live. + name: "hash is ordinary text", + sql: "SELECT a FROM t WHERE b = ? # ?", + want: []sqllex.Slot{plain, plain}, + }, + { + // SQLite treats /*! as an ordinary comment, unlike MySQL. + name: "version comment is an ordinary comment", + sql: "SELECT a FROM t /*! AND b = ? */ WHERE c = ?", + want: []sqllex.Slot{plain}, + }, + {name: "unterminated bracket swallows the rest", sql: "SELECT [br?k FROM t WHERE a = ?"}, + } + runSlotCases(t, cases, sqllex.SQLite) +} + +// TestScanSpansCoverInput pins the property the MySQL rewriter relies on: +// the tokens tile the input exactly, so copying every span reproduces it. +func TestScanSpansCoverInput(t *testing.T) { + t.Parallel() + inputs := []string{ + "SELECT a FROM t WHERE b = ? AND c IN (/*SLICE:ids*/?)", + "SELECT '?' -- c\n# h\n/* b */ `id` /*! live ? */ FROM t WHERE x = ?", + "SELECT a FROM t WHERE b = 5--? AND c = ?", + "", + } + for _, sql := range inputs { + t.Run(sql, func(t *testing.T) { + t.Parallel() + at := 0 + for _, token := range sqllex.Scan(sql, sqllex.MySQLRaw) { + if token.Start != at { + t.Fatalf("token starts at %d, want %d (gap or overlap)", token.Start, at) + } + at = token.End + } + // The trailing flush covers whatever the loop left pending, so + // the spans must reach the end: bytes no token covers would be + // dropped from the rewritten SQL. + if at != len(sql) { + t.Fatalf("tokens cover %d bytes, want %d", at, len(sql)) + } + }) + } +} + +func runSlotCases(t *testing.T, cases []struct { + name string + sql string + want []sqllex.Slot +}, dialect sqllex.Dialect, +) { + t.Helper() + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + if got := sqllex.Slots(tc.sql, dialect); !reflect.DeepEqual(got, tc.want) { + t.Errorf("Slots() = %+v, want %+v", got, tc.want) + } + }) + } +} + +func TestDialectEmitters(t *testing.T) { + t.Parallel() + cases := []struct { + name string + dialect sqllex.Dialect + placeholder string + marker string + }{ + {name: "mysql raw", dialect: sqllex.MySQLRaw, placeholder: "?", marker: "/*SLICE:ids*/?"}, + {name: "mysql pyformat", dialect: sqllex.MySQLPyformat, placeholder: "%s", marker: "/*SLICE:ids*/%s"}, + {name: "sqlite", dialect: sqllex.SQLite, placeholder: "?", marker: "/*SLICE:ids*/?"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + if got := tc.dialect.Placeholder(); got != tc.placeholder { + t.Errorf("Placeholder() = %q, want %q", got, tc.placeholder) + } + if got := tc.dialect.SliceMarker("ids"); got != tc.marker { + t.Errorf("SliceMarker() = %q, want %q", got, tc.marker) + } + // A rebuilt marker has to scan back to the slot it describes. + want := []sqllex.Slot{{Name: "ids", Marker: tc.marker}} + if got := sqllex.Slots(tc.dialect.SliceMarker("ids"), tc.dialect); !reflect.DeepEqual(got, want) { + t.Errorf("Slots(SliceMarker()) = %+v, want %+v", got, want) + } + }) + } +} + +// A Dialect can be constructed empty even though its fields are unexported. +// It must degrade to "this text has no placeholders" rather than scanning +// forever, and its spans must still tile the input so a rewriter driven by +// them cannot drop bytes. +func TestScanZeroDialectTerminates(t *testing.T) { + t.Parallel() + const sql = "SELECT a FROM t WHERE b = ? AND c IN (/*SLICE:ids*/?)" + at := 0 + for _, token := range sqllex.Scan(sql, sqllex.Dialect{}) { + if token.Kind == sqllex.KindPlaceholder || token.Kind == sqllex.KindSliceMarker { + t.Errorf("Scan() reported a bindable token %+v", token) + } + if token.Start != at { + t.Fatalf("token starts at %d, want %d", token.Start, at) + } + at = token.End + } + if at != len(sql) { + t.Fatalf("tokens cover %d bytes, want %d", at, len(sql)) + } + if got := sqllex.Slots(sql, sqllex.Dialect{}); got != nil { + t.Errorf("Slots() = %+v, want none", got) + } +} diff --git a/internal/transform/mysql_sql.go b/internal/transform/mysql_sql.go index cb1c28d1..839ea7fa 100644 --- a/internal/transform/mysql_sql.go +++ b/internal/transform/mysql_sql.go @@ -2,126 +2,32 @@ package transform import ( "strings" + + "github.com/rayakame/sqlc-gen-better-python/internal/sqllex" ) // rewriteMySQLSQL converts sqlc's MySQL placeholders into pyformat style: // every ? becomes %s, and every literal % is doubled, since PyMySQL and // asyncmy interpolate the whole query text with Python %-formatting once -// parameters are passed - including string literals and comments. String -// literals, backtick identifiers, and comments are tracked so a ? inside -// them stays text. Only default sql_mode lexing is supported: sqlc's -// dolphin (TiDB) parser lexes with backslash escapes on and treats "..." -// as a string, so any query that reached the plugin already parsed under -// those rules; NO_BACKSLASH_ESCAPES and ANSI_QUOTES are deliberately -// unsupported. +// parameters are passed - including string literals and comments. The lexing +// itself lives in internal/sqllex so the drivers, which scan the text this +// produces, cannot disagree about which "?" was bindable. func rewriteMySQLSQL(sql string) string { var out strings.Builder out.Grow(len(sql) + len(sql)/8) - for i := 0; i < len(sql); { - c := sql[i] - switch { - case c == '?': - // MySQL has no ?N syntax (that is sqlite-only), so digits after - // ? are ordinary text. - out.WriteString("%s") - i++ - case c == '%': - out.WriteString("%%") - i++ - case c == '\'' || c == '"': - end := scanEscapedString(sql, i, c) - writeDoubled(&out, sql[i:end]) - i = end - case c == '`': - end := scanQuoted(sql, i, '`') - writeDoubled(&out, sql[i:end]) - i = end - case c == '#': - end := scanLineEnd(sql, i) - writeDoubled(&out, sql[i:end]) - i = end - case c == '-' && strings.HasPrefix(sql[i:], "--") && isMySQLLineComment(sql, i): - end := scanLineEnd(sql, i) - writeDoubled(&out, sql[i:end]) - i = end - case c == '/' && strings.HasPrefix(sql[i:], "/*!"): - // MySQL executes /*! version comments and sqlc's parser agrees: - // the body is live SQL and a ? inside it is a real parameter. - // Emit the opener and scan the body with the normal rules; the - // closing */ falls through the default case as ordinary text. - out.WriteString("/*!") - i += len("/*!") - case c == '/' && strings.HasPrefix(sql[i:], "/*"): - end := scanMySQLBlockComment(sql, i) - writeDoubled(&out, sql[i:end]) - i = end - default: - out.WriteByte(c) - i++ + for _, token := range sqllex.Scan(sql, sqllex.MySQLRaw) { + switch token.Kind { + case sqllex.KindPlaceholder: + out.WriteString(sqllex.MySQLPyformat.Placeholder()) + case sqllex.KindSliceMarker: + // The marker is comment text and doubles like any other; only + // the placeholder it binds is rewritten. + writeDoubled(&out, sql[token.Start:token.MarkerEnd]) + out.WriteString(sqllex.MySQLPyformat.Placeholder()) + case sqllex.KindText, sqllex.KindSkipped: + writeDoubled(&out, sql[token.Start:token.End]) } } return out.String() } - -// isMySQLLineComment reports whether the -- at i starts a comment. MySQL -// requires the second dash to be followed by whitespace, a control -// character, or end of input; "a--1" is double unary minus, not a comment. -// When it is not a comment the caller copies the dashes as ordinary text. -// (DEL needs no arm: sqlc's parser rejects a bare 0x7f at generate time.) -func isMySQLLineComment(sql string, i int) bool { - if i+2 >= len(sql) { - return true - } - - return sql[i+2] <= ' ' -} - -// scanLineEnd returns the index of the \n terminating a line comment at or -// after i, or end of input. MySQL and sqlc's dolphin parser end -- and # -// comments only at \n (a bare \r is comment text, unlike PostgreSQL). The -// terminator itself is not consumed; the caller copies it as ordinary text. -func scanLineEnd(sql string, i int) int { - end := strings.IndexByte(sql[i:], '\n') - if end == -1 { - return len(sql) - } - - return i + end -} - -// scanEscapedString returns the index after a string literal starting at i, -// honoring backslash escapes and quote doubling. MySQL applies these rules -// to both '...' and "..."; PostgreSQL to E'...' (via scanStringLiteral). An -// unterminated literal swallows the rest of the input. -func scanEscapedString(sql string, i int, quote byte) int { - j := i + 1 - for j < len(sql) { - switch { - case sql[j] == '\\': - j += 2 - case sql[j] != quote: - j++ - case j+1 < len(sql) && sql[j+1] == quote: - j += 2 - default: - return j + 1 - } - } - - return len(sql) -} - -// scanMySQLBlockComment returns the index after a /* */ comment starting at -// i. MySQL block comments do not nest: the first */ ends the comment. /*+ -// optimizer hints and sqlc's /*SLICE:name*/ markers scan the same way; /*! -// version comments never reach here (their body is live SQL). -func scanMySQLBlockComment(sql string, i int) int { - body := i + len("/*") - end := strings.Index(sql[body:], "*/") - if end == -1 { - return len(sql) - } - - return body + end + len("*/") -} diff --git a/internal/transform/psycopg_sql.go b/internal/transform/psycopg_sql.go index dc8f7169..312fa733 100644 --- a/internal/transform/psycopg_sql.go +++ b/internal/transform/psycopg_sql.go @@ -133,12 +133,28 @@ func isEscapeString(sql string, i int) bool { // scanStringLiteral returns the index after a single-quoted literal starting // at i, honoring quote doubling and, for escape strings, backslash escapes. +// PostgreSQL is the only dialect here that splits the two: it needs the E +// prefix, while MySQL applies backslash escapes to every literal (see +// internal/sqllex). func scanStringLiteral(sql string, i int, escapes bool) int { if !escapes { return scanQuoted(sql, i, '\'') } + j := i + 1 + for j < len(sql) { + switch { + case sql[j] == '\\': + j += 2 + case sql[j] != '\'': + j++ + case j+1 < len(sql) && sql[j+1] == '\'': + j += 2 + default: + return j + 1 + } + } - return scanEscapedString(sql, i, '\'') + return len(sql) } // scanQuoted returns the index after a quoted region starting at i, where a diff --git a/sqlc.yaml b/sqlc.yaml index fedbd639..84be385f 100644 --- a/sqlc.yaml +++ b/sqlc.yaml @@ -3,7 +3,7 @@ plugins: - name: python wasm: url: file://sqlc-gen-better-python.wasm - sha256: 02ddf238b6c48ecfb79e5da90cbec179e420456c201e5977cca4ac34b697a993 + sha256: 6b9df77e00051cc9128c70cd35dc645120c5b2eff1c56e6dc5ac99e34485c4ef sql: - schema: test/schema.sql queries: test/queries.sql diff --git a/test/driver_aiosqlite/sqlc-gen-better-python.wasm b/test/driver_aiosqlite/sqlc-gen-better-python.wasm index 818774af..bda78044 100644 Binary files a/test/driver_aiosqlite/sqlc-gen-better-python.wasm and b/test/driver_aiosqlite/sqlc-gen-better-python.wasm differ diff --git a/test/driver_aiosqlite/sqlc.yaml b/test/driver_aiosqlite/sqlc.yaml index a022bfb4..fdf3c6bc 100644 --- a/test/driver_aiosqlite/sqlc.yaml +++ b/test/driver_aiosqlite/sqlc.yaml @@ -3,7 +3,7 @@ plugins: - name: python wasm: url: file://sqlc-gen-better-python.wasm - sha256: 02ddf238b6c48ecfb79e5da90cbec179e420456c201e5977cca4ac34b697a993 + sha256: 6b9df77e00051cc9128c70cd35dc645120c5b2eff1c56e6dc5ac99e34485c4ef sql: - schema: schema.sql queries: queries.sql diff --git a/test/driver_asyncmy/sqlc-gen-better-python.wasm b/test/driver_asyncmy/sqlc-gen-better-python.wasm index 818774af..bda78044 100755 Binary files a/test/driver_asyncmy/sqlc-gen-better-python.wasm and b/test/driver_asyncmy/sqlc-gen-better-python.wasm differ diff --git a/test/driver_asyncmy/sqlc.yaml b/test/driver_asyncmy/sqlc.yaml index 46afffe8..fbd97814 100644 --- a/test/driver_asyncmy/sqlc.yaml +++ b/test/driver_asyncmy/sqlc.yaml @@ -3,7 +3,7 @@ plugins: - name: python wasm: url: file://sqlc-gen-better-python.wasm - sha256: 02ddf238b6c48ecfb79e5da90cbec179e420456c201e5977cca4ac34b697a993 + sha256: 6b9df77e00051cc9128c70cd35dc645120c5b2eff1c56e6dc5ac99e34485c4ef sql: - schema: schema.sql queries: diff --git a/test/driver_asyncpg/sqlc-gen-better-python.wasm b/test/driver_asyncpg/sqlc-gen-better-python.wasm index 818774af..bda78044 100644 Binary files a/test/driver_asyncpg/sqlc-gen-better-python.wasm and b/test/driver_asyncpg/sqlc-gen-better-python.wasm differ diff --git a/test/driver_asyncpg/sqlc.yaml b/test/driver_asyncpg/sqlc.yaml index 111db052..2ef920b6 100644 --- a/test/driver_asyncpg/sqlc.yaml +++ b/test/driver_asyncpg/sqlc.yaml @@ -3,7 +3,7 @@ plugins: - name: python wasm: url: file://sqlc-gen-better-python.wasm - sha256: 02ddf238b6c48ecfb79e5da90cbec179e420456c201e5977cca4ac34b697a993 + sha256: 6b9df77e00051cc9128c70cd35dc645120c5b2eff1c56e6dc5ac99e34485c4ef sql: - schema: schema.sql queries: diff --git a/test/driver_psycopg_async/sqlc-gen-better-python.wasm b/test/driver_psycopg_async/sqlc-gen-better-python.wasm index 818774af..bda78044 100644 Binary files a/test/driver_psycopg_async/sqlc-gen-better-python.wasm and b/test/driver_psycopg_async/sqlc-gen-better-python.wasm differ diff --git a/test/driver_psycopg_async/sqlc.yaml b/test/driver_psycopg_async/sqlc.yaml index 5ae17c55..a47e96c3 100644 --- a/test/driver_psycopg_async/sqlc.yaml +++ b/test/driver_psycopg_async/sqlc.yaml @@ -3,7 +3,7 @@ plugins: - name: python wasm: url: file://sqlc-gen-better-python.wasm - sha256: 02ddf238b6c48ecfb79e5da90cbec179e420456c201e5977cca4ac34b697a993 + sha256: 6b9df77e00051cc9128c70cd35dc645120c5b2eff1c56e6dc5ac99e34485c4ef sql: - schema: schema.sql queries: diff --git a/test/driver_psycopg_sync/sqlc-gen-better-python.wasm b/test/driver_psycopg_sync/sqlc-gen-better-python.wasm index 818774af..bda78044 100755 Binary files a/test/driver_psycopg_sync/sqlc-gen-better-python.wasm and b/test/driver_psycopg_sync/sqlc-gen-better-python.wasm differ diff --git a/test/driver_psycopg_sync/sqlc.yaml b/test/driver_psycopg_sync/sqlc.yaml index 93724605..2e69afd0 100644 --- a/test/driver_psycopg_sync/sqlc.yaml +++ b/test/driver_psycopg_sync/sqlc.yaml @@ -3,7 +3,7 @@ plugins: - name: python wasm: url: file://sqlc-gen-better-python.wasm - sha256: 02ddf238b6c48ecfb79e5da90cbec179e420456c201e5977cca4ac34b697a993 + sha256: 6b9df77e00051cc9128c70cd35dc645120c5b2eff1c56e6dc5ac99e34485c4ef sql: - schema: schema.sql queries: diff --git a/test/driver_pymysql/sqlc-gen-better-python.wasm b/test/driver_pymysql/sqlc-gen-better-python.wasm index 818774af..bda78044 100755 Binary files a/test/driver_pymysql/sqlc-gen-better-python.wasm and b/test/driver_pymysql/sqlc-gen-better-python.wasm differ diff --git a/test/driver_pymysql/sqlc.yaml b/test/driver_pymysql/sqlc.yaml index b2ae1b54..52ff05f4 100644 --- a/test/driver_pymysql/sqlc.yaml +++ b/test/driver_pymysql/sqlc.yaml @@ -3,7 +3,7 @@ plugins: - name: python wasm: url: file://sqlc-gen-better-python.wasm - sha256: 02ddf238b6c48ecfb79e5da90cbec179e420456c201e5977cca4ac34b697a993 + sha256: 6b9df77e00051cc9128c70cd35dc645120c5b2eff1c56e6dc5ac99e34485c4ef sql: - schema: schema.sql queries: diff --git a/test/driver_sqlite3/sqlc-gen-better-python.wasm b/test/driver_sqlite3/sqlc-gen-better-python.wasm index 818774af..bda78044 100644 Binary files a/test/driver_sqlite3/sqlc-gen-better-python.wasm and b/test/driver_sqlite3/sqlc-gen-better-python.wasm differ diff --git a/test/driver_sqlite3/sqlc.yaml b/test/driver_sqlite3/sqlc.yaml index 1d0294a9..de1108e8 100644 --- a/test/driver_sqlite3/sqlc.yaml +++ b/test/driver_sqlite3/sqlc.yaml @@ -3,7 +3,7 @@ plugins: - name: python wasm: url: file://sqlc-gen-better-python.wasm - sha256: 02ddf238b6c48ecfb79e5da90cbec179e420456c201e5977cca4ac34b697a993 + sha256: 6b9df77e00051cc9128c70cd35dc645120c5b2eff1c56e6dc5ac99e34485c4ef sql: - schema: schema.sql queries: diff --git a/test/driver_turso_async/sqlc-gen-better-python.wasm b/test/driver_turso_async/sqlc-gen-better-python.wasm index 818774af..bda78044 100755 Binary files a/test/driver_turso_async/sqlc-gen-better-python.wasm and b/test/driver_turso_async/sqlc-gen-better-python.wasm differ diff --git a/test/driver_turso_async/sqlc.yaml b/test/driver_turso_async/sqlc.yaml index bac8c429..0e6a5d81 100644 --- a/test/driver_turso_async/sqlc.yaml +++ b/test/driver_turso_async/sqlc.yaml @@ -3,7 +3,7 @@ plugins: - name: python wasm: url: file://sqlc-gen-better-python.wasm - sha256: 02ddf238b6c48ecfb79e5da90cbec179e420456c201e5977cca4ac34b697a993 + sha256: 6b9df77e00051cc9128c70cd35dc645120c5b2eff1c56e6dc5ac99e34485c4ef sql: - schema: schema.sql queries: queries.sql diff --git a/test/driver_turso_sync/sqlc-gen-better-python.wasm b/test/driver_turso_sync/sqlc-gen-better-python.wasm index 818774af..bda78044 100755 Binary files a/test/driver_turso_sync/sqlc-gen-better-python.wasm and b/test/driver_turso_sync/sqlc-gen-better-python.wasm differ diff --git a/test/driver_turso_sync/sqlc.yaml b/test/driver_turso_sync/sqlc.yaml index e2bea768..de9ce518 100644 --- a/test/driver_turso_sync/sqlc.yaml +++ b/test/driver_turso_sync/sqlc.yaml @@ -3,7 +3,7 @@ plugins: - name: python wasm: url: file://sqlc-gen-better-python.wasm - sha256: 02ddf238b6c48ecfb79e5da90cbec179e420456c201e5977cca4ac34b697a993 + sha256: 6b9df77e00051cc9128c70cd35dc645120c5b2eff1c56e6dc5ac99e34485c4ef sql: - schema: schema.sql queries: