feat: configurable literal string quoting for SQL export#150
Conversation
Add LiteralQuote policy (QuoteLegacy, QuoteAlways, QuoteMinEscape) so LiteralFormatConfig can emit uniform single-quoted literals for SQL INSERT while preserving byte-identical default output via QuoteLegacy. Closes #143. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Code Review
This pull request introduces configurable string and bytes literal quoting policies for the literal preset, adding LiteralQuoteConfig with legacy, always, and minimum-escape quoting strategies. The feedback highlights a critical bug in the itoaUint8 helper that could cause a panic for values of 100 or greater, and suggests a performance optimization to correctly pre-allocate the strings.Builder capacity in floatSpecialCastLiteral to avoid extra allocations.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Group literal-preset-only options in LiteralFormatOptions.Quote so FormatConfig stays preset-agnostic at the top level; document that a future major may move literal config off FormatConfig entirely. Co-authored-by: Cursor <cursoragent@cursor.com>
Use strconv.Itoa for invalid enum String() values and preallocate floatSpecialCastLiteral with the full CAST wrapper overhead (12 bytes). Co-authored-by: Cursor <cursoragent@cursor.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces configurable literal quoting strategies (legacy, always, and min-escape) and preferred quote delimiters (single or double) for SQL-style string and bytes literals. It adds new preset constructors, a LiteralQuoteConfig struct, and updates formatting logic to respect the configured quote policy. The review feedback highlights a potential nil pointer dereference panic in literalQuoteForFormatter when dealing with typed nil pointers, and suggests a performance optimization in legacyQuote to early-return when the opposite quote character is encountered.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Return legacy quote policy for typed nil *FormatConfig formatters. Restore suitableQuote-style early exit in legacyQuote when the opposite delimiter appears in the payload. Co-authored-by: Cursor <cursoragent@cursor.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces configurable string and bytes literal quoting policies for the literal preset, allowing users to choose between legacy adaptive quoting, always quoting with a preferred delimiter, or minimizing escapes. It adds new constructors and options such as LiteralFormatConfigWithSingleQuotedLiterals and WithLiteralQuote. The internal formatting logic has been updated to propagate and apply these policies. Feedback on the changes is minimal, with a single recommendation to adjust a nested GoDoc link in doc.go from [FormatConfig.Literal.Quote] to [FormatConfig.Literal].Quote to ensure it resolves correctly on pkg.go.dev.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
apstndb
left a comment
There was a problem hiding this comment.
Reviewed the latest head. Implementation and tests look good overall; I left two inline godoc comments on the FormatConfig.Literal description so the documented scope and normalization timing match the implementation.
Use a valid pkg.go.dev link target for FormatConfig.Literal.Quote. Document FormatLiteralValue and FormatProtoAsCast as Quote readers. Clarify that enum normalization applies at option apply and format time. Co-authored-by: Cursor <cursoragent@cursor.com>
|
/gemini review |
apstndb
left a comment
There was a problem hiding this comment.
Reviewed the implementation against #143. Solid work — the fast≡slow equivalence test, NaN/±Inf legacy-single goldens (both FLOAT32 and FLOAT64), normalize-at-format-time, and the typed-nil *FormatConfig guard all line up with the design, and the four Gemini findings are addressed. Three non-blocking comments below (design/doc, maintainability, minor perf); none block merge.
One thing outside the diff: the PR description's Upgrading snippet uses Literal.Quote: as a struct-literal key, which does not compile. It should be Literal: spanvalue.LiteralFormatOptions{Quote: spanvalue.LiteralQuoteConfig{}}. Worth fixing before it becomes the release note.
There was a problem hiding this comment.
Code Review
This pull request introduces configurable string and bytes literal quoting policies for the literal preset, supporting legacy adaptive, always-quoted, and minimum-escape quoting strategies with customizable preferred quote delimiters. The changes update scalar formatting functions to respect these policies and add new configuration constructors. The review feedback suggests improving test reliability in literal_quote_test.go by constructing expected values independently of the internal implementation helpers to ensure the tests act as true oracles.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Map root quote enums explicitly in toInternalQuotePolicy. Apply quote policy once per nullable literal path; compute it per scalar arm in the GCV fast path. Replace internal test oracles with hardcoded expected strings. Contrast QuoteLegacy vs QuoteMinEscape in godoc. Rename floatSpecialCastLiteral to castQuotedLiteral and localize nanInfCastQuote to the NaN/±Inf branches only. Co-authored-by: Cursor <cursoragent@cursor.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces configurable string and bytes literal quoting policies (including legacy adaptive, always, and minimum escape strategies with preferred single or double quotes) for the literal preset. It updates scalar formatting functions, internal literal conversion helpers, and public configuration options to support these policies, and adds comprehensive tests. The review feedback suggests a minor improvement to extract redundant toInternalQuotePolicy(q) calls to a local variable at the beginning of formatGCVScalarLiteral to enhance readability and consistency.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Extract policy once in formatGCVScalarLiteral per review feedback. Rename castQuotedLiteral to sqlCastQuotedString and nanInfCastQuote to nonFiniteCastDelimiter with a v0.6 alignment note in godoc. Co-authored-by: Cursor <cursoragent@cursor.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces configurable string and bytes literal quoting policies for the literal preset, allowing users to customize outer delimiters (such as single vs. double quotes) and strategies (such as legacy adaptive, always, or minimum escapes). It updates formatting functions to propagate these policies and adds several new constructors, such as LiteralFormatConfigWithSingleQuotedLiterals. Feedback on the changes includes a correction to a GoDoc link in doc.go to prevent a broken link on pkg.go.dev, suggesting referencing the struct type itself rather than the field directly.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Link FormatConfig as the type symbol; Literal.Quote remains plain text so pkg.go.dev resolves the reference correctly. Co-authored-by: Cursor <cursoragent@cursor.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces customizable string and bytes literal quoting policies for the literal preset, allowing users to configure strategies like adaptive, always-quoted, or minimum-escape quoting with preferred single or double quotes. The review feedback suggests a performance optimization: refactoring the internal quote policy functions to be generic over a ~string | ~[]byte constraint. This change allows formatting string literals directly without converting them to byte slices, thereby avoiding unnecessary memory allocations.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Use unexported quoteForPayload[T bytesSeq] so ToStringLiteralPolicy scans strings without []byte(s). Add 10 MiB (Spanner max cell) format benchmarks and quote-policy benches for large STRING payloads. Co-authored-by: Cursor <cursoragent@cursor.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces customizable literal quoting configurations for the literal preset, allowing users to specify quoting strategies (legacy, always, or min-escape) and preferred delimiters (single or double quotes) for string and bytes literals. The feedback suggests optimizing the legacyQuote function in internal/quote_policy.go by replacing the manual byte-by-byte loop with highly optimized standard library functions (strings.IndexByte and bytes.IndexByte) to improve performance on large payloads, which also requires importing the bytes and strings packages.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Split quoteForPayload into string and []byte entry points so ToStringLiteralPolicy and ToReadableBytesLiteralPolicy avoid any() while legacyQuote uses strings.IndexByte / bytes.IndexByte. Add legacy-shape quote benchmarks at 10 MiB (no quotes, preferred only, opposite early). Allocs/op stay at zero on quote scan paths. Co-authored-by: Cursor <cursoragent@cursor.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces configurable quoting policies (such as legacy adaptive, always, or minimum escape strategies with preferred single or double quotes) for SQL-style string and bytes literals in the literal preset. It updates formatting functions, adds configuration options, and includes comprehensive tests and benchmarks. The review feedback suggests restricting the bytesSeq interface constraint in internal/quote_policy.go to string | []byte (removing the approximation ~) to prevent potential runtime panics in type switches that only handle exact types.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
This reverts commit 6bc0c75.
Use a union constraint without ~ so quote helpers accept only string or []byte at compile time. Co-authored-by: Cursor <cursoragent@cursor.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces customizable string and bytes literal quoting policies for the literal preset. It adds new configuration options (LiteralQuoteConfig, QuoteStrategy, PreferredQuote) and constructors (LiteralFormatConfigWithQuote, LiteralFormatConfigWithSingleQuotedLiterals, LiteralFormatConfigWithOptions) to allow users to control outer delimiters (e.g., forcing single quotes for SQL INSERT compatibility or using a minimum-escape strategy). The formatting logic across scalar and nullable paths has been updated to propagate these policies, supported by comprehensive unit tests and benchmarks. There are no review comments to address, and I have no additional feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Summary
FormatConfig.Literal.Quote(LiteralQuoteConfig) and quote strategies (QuoteLegacy,QuoteAlways,QuoteMinEscape) for the literal preset.LiteralFormatConfig()/ zeroLiteral.Quote) stays byte-identical to today'ssuitableQuoteoutput for STRING/BYTES/DATE/TIMESTAMP/NUMERIC and related literals.LiteralFormatConfigWithQuote,LiteralFormatConfigWithSingleQuotedLiterals,LiteralFormatConfigWithOptions, andWithLiteralQuote.QuoteLegacygeneralizes historical adaptive quoting while respectingPreferredQuote(zero value +PreferredDoubleQuote= current behavior).PreferredQuotehas no separate "unspecified" variant: omitting the field is the same asPreferredDoubleQuote.PreferredQuote).Closes #143.
Upgrading
1.
FormatConfiggains an exported fieldDownstream unkeyed composite literals may fail to compile:
LiteralFormatConfig()signature is unchanged (func() *FormatConfig). Existing call sites that use the constructor or clone presets need no change.2. Default behavior unchanged (v0.5)
LiteralFormatConfig(),FormatRowLiteral,FormatColumnLiteral, andSQLInsertWriterdefaults produce the same strings as before unless you opt into a new constructor.3. Single-quoted SQL INSERT (opt-in)
Other options:
LiteralFormatConfigWithQuote,LiteralFormatConfigWithOptions(WithLiteralQuote(...)), or assignLiteral.Quoteon a cloned config.4. NaN/±Inf CAST literals (v0.5 compat; planned v0.6 change)
In v0.5, non-finite float CAST output is intentionally special-cased:
QuoteLegacy(default)CAST('nan' AS FLOAT64)QuoteAlways/QuoteMinEscape+PreferredDoubleQuoteCAST("nan" AS FLOAT64)QuoteAlways/QuoteMinEscape+PreferredSingleQuoteThis differs from ordinary STRING/BYTES quoting under the same policy when the payload contains no quote characters.
Planned for v0.6: align NaN/±Inf CAST delimiters with the same
Literal.Quoterules used for other quoted literals (viaquoteForPayload), in one release. Strict preservation of the v0.5 NaN/±Inf exception is not a long-term compatibility goal. NoPreferredQuote"unspecified" variant will be added; zero / omitted preferred remainsPreferredDoubleQuote.Document the v0.6 output change in the GitHub Release at tag time when NaN/±Inf formatting is updated.
5. Performance
BenchmarkFormatPerType/(STRING|BYTES|DATE)/Literal/Directvsmain@ 3831066: geomean +0.19% sec/op, allocs/op identical (benchstat, n=5).Test plan
make checkliteral_quote_test.go)FormatProtoAsCast+SQLInsertWritersingle-quoted e2eExampleLiteralFormatConfigWithSingleQuotedLiterals