Repository navigation
Conversation
Since 4.0.4, inline parameters render every Python sequence as an ARRAY literal, so the documented `x IN %(ids)s` pattern produces `x IN ARRAY(1,2,3)`, which Spark rejects with PARSE_SYNTAX_ERROR. A sequence whose marker directly follows IN (or NOT IN) now renders as a parenthesised list again. Everywhere else, including `IN (%(p)s)` where the caller wrote the parentheses, it stays an ARRAY literal, so inline writes to ARRAY columns keep working. Because `x IN ARRAY(...)` never parses, no query that runs today renders differently. Co-authored-by: Isaac <[email protected]> Signed-off-by: cathleen.yan <[email protected]>
Follow-ups from review of the inline IN fix: - Decide per marker instead of per parameter name. A name used both directly after IN and elsewhere now keeps its ARRAY rendering at the other markers. Before, every occurrence became `(...)`, which could break a query that runs on main, e.g. one with a commented-out filter: `arr = %(tags)s -- AND tag IN %(tags)s`. - Probe only sequence values and give every other marker its real escaped value, so a `%d` or `%(n)d` marker elsewhere in the query no longer turns the fix off. Probe tokens carry a random nonce so that a string value cannot pass for one. - Leave escape_args exactly as it is on main and move the logic into ParamEscaper.render_inline, which Cursor._prepare_inline_parameters now calls. Co-authored-by: Isaac <[email protected]> Signed-off-by: Cathleen Yan <[email protected]>
cathleeny
requested review from
aakash-saravanan-db,
rahuls-db and
vuanhphung
November 6, 2026 01:34
vuanhphung
reviewed
Nov 6, 2026
Follow-ups from review: - Inside an IN list a tuple now renders as a row, so a multi-column `(a, b) IN %(pairs)s` with `[(1, 2), (3, 4)]` renders as `(a, b) IN ((1,2),(3,4))`, as it did in 3.x. Lists still render as ARRAY literals so ARRAY columns can be matched. Spark does not widen the field types of a row, so each value must have its column's type; docs/parameters.md says so. - When no parameter is a sequence, render_inline skips the probe pass and renders exactly as escape_args and inject_parameters do on main. Co-authored-by: Isaac <[email protected]> Signed-off-by: Cathleen Yan <[email protected]>
cathleeny
had a problem deploying
to
azure-prod
November 6, 2026 21:54 — with 
GitHub Actions
Failure
GitHub Actions
Failure
![peco-review-bot[bot]](https://avatars.githubusercontent.com/in/3801177?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: 1 Low
Looks good overall, with one low-severity edge case in IN detection. The probe-based per-marker approach is correct for named, positional, %% and %d markers, and it keeps the code path main uses when no parameter is a sequence. Nit (not posted inline): the new render_inline / _sequence_markers_following_in signatures leave parameters/escaped untyped, unlike the typed helpers next to them.
Review bot follow-up: a `--` comment line that ends with the word IN
made the marker on the next line render as a list. For example
WHERE arr = -- tags we filter IN
%(tags)s
rendered as `arr = ('a','b')` instead of `arr = ARRAY('a','b')`, which
breaks a query that runs on main. A match is now ignored when a `--`
comes before its IN on the same line and the marker is on a later line.
The probe pass also replaces escaped strings with '' (numbers stay, so
%d still formats), so a value containing `--` cannot hide a real IN.
Co-authored-by: Isaac <[email protected]>
Signed-off-by: Cathleen Yan <[email protected]>
There was a problem hiding this comment.
Verdict: 1 Nit
Looks good. I found no correctness issues. The probe-based detection uses the same %-formatting as the real substitution, so marker numbering lines up for named, positional and %% cases. Errors still surface the same way as before, and queries without a sequence parameter render exactly as they do on main. The only note is a nit: a CHANGELOG entry is missing.
Other findings
- ⚪ Nit — Consider adding an entry under
# Unreleasedin CHANGELOG.md. This is a user-visible fix for a 4.0.4 regression (#629) that blocks users migrating from 3.x.
Up to 4.0.3 the inline escaper rendered any iterable as `(...)`, so a set after IN worked. 4.0.4 switched to Sequence for ARRAY rendering and sets started raising ProgrammingError. A set, frozenset or dict key view is accepted again when every marker it fills directly follows IN. Its items are sorted so the SQL text is stable, and a set of tuples renders as rows. Anywhere else a set raises the same ProgrammingError as before, since a set has no order to keep in an ARRAY. Also adds a CHANGELOG entry for the inline IN fix. Co-authored-by: Isaac <[email protected]> Signed-off-by: Cathleen Yan <[email protected]>
There was a problem hiding this comment.
Verdict: 1 Low
Looks good. The marker detection works by formatting the query once with probe tokens, and I traced it through named and positional params, %%, %d/%c, bare %s with dict params, missing keys and the set placeholder handling: each path produces the same output or raises the same error as escape_args. I have one low-severity note on how deterministic the sorted output of a set is.
Review bot follow-up: the comments and docs described comment handling as if it were settled. They now say that IN is found by matching the SQL text rather than parsing it, which case is handled (an IN that ends a `--` comment line) and which is not (an IN inside a string literal or block comment directly before a marker). Co-authored-by: Isaac <[email protected]> Signed-off-by: Cathleen Yan <[email protected]>
Review bot follow-up: sorting a set's items does not give a stable
order when items compare inconsistently, such as float("nan") among
other numbers. The SQL still matches the same rows, but the docstring
and docs/parameters.md no longer say the query text is always the same.
Co-authored-by: Isaac <[email protected]>
Signed-off-by: Cathleen Yan <[email protected]>
cathleeny
had a problem deploying
to
azure-prod
November 6, 2026 23:44 — with
GitHub Actions
Failure
GitHub Actions
Failure
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
BUG FIX: Since 4.0.4 (#559), inline parameters (
use_inline_params=True) render every Python sequence as anARRAY(...)literal. TheWHERE x IN %(ids)spattern documented indocs/parameters.mdtherefore renders asx IN ARRAY(1,2,3), which Spark rejects withPARSE_SYNTAX_ERROR. That blocks users moving from 3.x.Change: A sequence at a marker that directly follows
INorNOT INrenders as a parenthesised list,(1,2,3), again. Everywhere else it stays anARRAY(...)literal, includingIN (%(p)s)where the caller wrote the parentheses, so the inline writes toARRAYcolumns added in #559 keep working. Inside the list, a tuple renders as a row and a list as anARRAY(...)literal, so a multi-column(a, b) IN %(pairs)swith[(1, 2), (3, 4)]renders as(a, b) IN ((1,2),(3,4)), as on 3.x.Scope:
Cursor._prepare_inline_parametersbefore backend dispatch, so the fix covers the Thrift, SEA and kernel backends.main.1is anINTand doesn't match aBIGINTcolumn.docs/parameters.mdsays so.IN (), which Spark rejects withINVALID_SQL_SYNTAX.EMPTY_IN_PREDICATE, the same as on 3.x.INby a comment keeps theARRAYrendering.(...). That would break the inlineARRAYinserts covered bytest_primitive_single[INLINE-ARRAYS]andtest_inline_recursive_complex_type, so this change only targets theINposition.How is this tested?
Unit:
-m "not realkernel"), andblack --check srcandmypy srcare clean.E2E:
mainid IN %(ids)swith[1, 3]PARSE_SYNTAX_ERRORid NOT IN %(ids)swith(1, 3)PARSE_SYNTAX_ERROR[2]PARSE_SYNTAX_ERROR'and\PARSE_SYNTAX_ERRORINand inarray_containsPARSE_SYNTAX_ERRORIN %(ids)s ... LIMIT %(n)dPARSE_SYNTAX_ERRORPIVOT (... FOR k IN %(ks)s)PARSE_SYNTAX_ERRORposition('b' IN %(s)s)with["abc"]DATATYPE_MISMATCH[[1, 2], [3]]PARSE_SYNTAX_ERRORNULLPARSE_SYNTAX_ERRORDecimal("1.50")PARSE_SYNTAX_ERRORPARSE_SYNTAX_ERROREMPTY_IN_PREDICATE, as on 3.x(id, tag) IN %(pairs)swith[(1, "a"), (2, "x"), (3, "c")]PARSE_SYNTAX_ERROR(a, b) NOT IN %swith((1, 2), (3, 4))PARSE_SYNTAX_ERRORRowobjects fromfetchall()PARSE_SYNTAX_ERRORINTliterals againstBIGINTcolumnsPARSE_SYNTAX_ERRORDATATYPE_MISMATCH(Spark doesn't widen row types)The unchanged renderings,
array_contains(%(p)s, 2)andarr IN (%(p)s), still return the expected results.Related Tickets & Documents
Fixes #629