Repository navigation
Bindings: more positional values than placeholders is a TypeError, not a silently dropped value (P-07) - #536
Merged
Merged
Conversation
…t a silently dropped value (P-07) A lone list is the positional list, so IN ? with [a, b] bound a and dropped b. The check is an upper bound over the SQL text, so it never refuses a call that binds. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
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.
Backlog row P-07.
db.query("sql", "SELECT FROM V WHERE @rid IN ?", [rid_a, rid_b])reads the lone list as the positional-parameter list (the documented form, one element per?). The statement has one?, so the engine bindsrid_a, dropsrid_bwithout a word, and the query returns one row where two were meant. The engine does not count: it also ignores a second value for a?and binds null for a missing one (verified on the 26.10.2 release jars and on a 26.11.1-SNAPSHOT build through the Java API, upstream issue drafted separately). The documented example indocs/api/database.md(query("sql", "... ?", [0.9, 0.1, 0.0])binds?to0.9) was this silent case.Reproduced on both wheels before the change (rows returned against expected):
@rid IN ?with[rid_a, rid_b], a tuple, or two arguments returned 1 of 2;k IN ?with[0, 1, 2]returned 1 of 3; the wrapped form[[a, b]]and the named formIN :idsreturned all. Identical on the JVM wheel (main at78e97b1cff) and the native image (nnext).What changes: a call with more positional values than the SQL statement can bind raises
TypeErrorbefore the statement runs, naming both ways out (wrap the list,[[a, b]], or bind it by name,IN :ids). A new module_params.pyholds the check and both wheels call it fromquery()/command()(JVM) and_exec(native), so the behaviour is the same. The count is an upper bound taken from the?,$and:characters of the text, with SQL and SQLScript checked, not an exact count: the parsed statement's placeholder count is no public engine call, the native library would need a new entry point and an image rebuild, and named parameters carry positional numbers in the parser, so a count from the text that was exact would be a SQL tokenizer. The bound never refuses a call that binds (a?inside a string literal counts as a placeholder, so such a statement is checked more loosely). Fewer values than placeholders are not checked. The documented API is unchanged: a lone list is still the positional list, and a call with one value per placeholder binds as before. The only calls refused are those that dropped a value.One existing test changed:
test_each_parameter_shape_reaches_one_java_overloadpassed values toSELECT 1(no placeholder); it now usesSELECT ?, ?(it checks overload resolution only).Tests first:
test_positional_count.pywas red on both wheels (9 of 20 failed, none raised) and is green after. Docs:docs/api/database.md(query and command),docs/guide/core/queries.md,docs/development/testing.md.Checked: JVM wheel, full suite on the worktree source over the main wheel (1386 passed; the 5 failures and 2 errors are the subprocess tests that need the installed package,
requests, and a Ctrl-C test, none touching parameters); native imagennext, full suite (895 passed; the 2 failures needsetuptoolsin my throwaway environment). Not timed: the check costs oneisinstanceand a length compare on the one-value paths and a cachedstr.counton calls with two or more values; no timing claim is made.Overlap: touches the same functions as #524 and #525 (query/command and
_exec, one addedif args:call each) and the parameter paragraph ofdocs/api/database.mdthat #529, #530 and #531 also edit; expect a trivial rebase.🤖 Generated with Claude Code