From 40101b5a52bf20680d2091c0f7cce91ede360414 Mon Sep 17 00:00:00 2001 From: Taewoon Kim Date: Sun, 25 Oct 2026 18:11:39 +0900 Subject: [PATCH] Refuse more positional values than the SQL statement has placeholders query("sql", "... WHERE @rid IN ?", [rid_a, rid_b]) reads the lone list as the positional-parameter list, one element per ?. The statement has one ?, so the engine binds rid_a, drops rid_b without a word, and returns one row where two were meant. The engine does not count positional values (ArcadeData/arcadedb#10721), so Database.query() and command() now raise TypeError before the statement runs when a SQL or SQLScript call has more positional values than the text can bind. The count is an upper bound taken from the ?, $ and : characters of the text, so a call that binds is never refused. The message names both ways out: wrap the list, [[a, b]], or bind it by name, IN :ids. test_parameter_binding.py passed values to SELECT 1 (no placeholder) and now uses SELECT ?, ?. The how-to page documents the behaviour. Port of humemai/arcadedb-embedded-python#536. Co-Authored-By: Claude Sonnet 5.5 --- .../pages/how-to/databases-and-queries.adoc | 10 ++ src/arcadedb/_params.py | 48 +++++++ src/arcadedb/core.py | 8 +- tests/test_parameter_binding.py | 4 +- tests/test_positional_count.py | 131 ++++++++++++++++++ 5 files changed, 199 insertions(+), 2 deletions(-) create mode 100644 src/arcadedb/_params.py create mode 100644 tests/test_positional_count.py diff --git a/docs/modules/ROOT/pages/how-to/databases-and-queries.adoc b/docs/modules/ROOT/pages/how-to/databases-and-queries.adoc index 6e47e77..f572269 100644 --- a/docs/modules/ROOT/pages/how-to/databases-and-queries.adoc +++ b/docs/modules/ROOT/pages/how-to/databases-and-queries.adoc @@ -210,6 +210,16 @@ Bind values as parameters instead of pasting them into the statement text. Ident * Named: `:name` in the statement and a dict as the one extra argument. A named parameter can hold a collection, as in `age IN :ages`. * `None` binds as null. +A list or tuple on its own is the positional list, so it never binds as one parameter. `db.query("sql", "SELECT FROM V WHERE @rid IN ?", [rid_a, rid_b])` has two values for one `?`: the engine would bind `rid_a`, drop `rid_b` without a word and return one row where two were meant. The bindings raise `TypeError` before the statement runs when a SQL or SQLScript call has more positional values than the statement can bind, because the engine does not count them (https://github.com/ArcadeData/arcadedb/issues/10721[ArcadeData/arcadedb#10721]). To bind a list as one parameter, bind it by name or wrap it so it is the one positional value: + +[source,python] +---- +db.query("sql", "SELECT FROM V WHERE @rid IN :ids", {"ids": [rid_a, rid_b]}) +db.query("sql", "SELECT FROM V WHERE @rid IN ?", [[rid_a, rid_b]]) +---- + +The check counts the `?`, `$` and `:` characters of the SQL text, so the count is an upper bound: a `?` inside a string literal counts as a placeholder, and a call that binds is never refused. Fewer values than placeholders are not checked, and the engine binds null for the missing ones. `db.query` and `db.command` run the check; the async executor does not. + === Results `db.query` returns a result set you can iterate. Two methods cover the usual needs: diff --git a/src/arcadedb/_params.py b/src/arcadedb/_params.py new file mode 100644 index 0000000..90940c9 --- /dev/null +++ b/src/arcadedb/_params.py @@ -0,0 +1,48 @@ +"""More positional values than a statement has placeholders is a wrong call, refused before it runs. + +``query("sql", "... WHERE @rid IN ?", [rid_a, rid_b])`` reads the lone list as the positional-parameter list (the documented form, +one element per ``?``). The statement holds one ``?``, the engine binds the first element, ignores the second without a word, and +the query returns fewer rows than the caller meant. The engine does not count (it binds null for a missing value too; +ArcadeData/arcadedb#10721), and the bindings cannot ask it: the parsed statement's placeholder count is no public engine call. +So the check is the one that never refuses a call that binds: an UPPER BOUND on the placeholders, the characters ``?``, ``$`` and +``:`` anywhere in the SQL text (a ``?`` in a string literal counts, so such a statement is checked more loosely, never refused +wrongly). A call with more values than that bound has a value no placeholder can take. +""" + +from functools import lru_cache + +_COUNTED_LANGUAGES = frozenset(("sql", "sqlscript")) + + +@lru_cache(maxsize=1024) +def _placeholder_bound(command: str) -> int: + """Upper bound on the positional placeholders of ``command``: ``?``, ``$1`` and ``:name`` characters, wherever they stand.""" + return command.count("?") + command.count("$") + command.count(":") + + +def refuse_surplus_positionals( + name: str, language: str, command: str, args: tuple +) -> None: + """Raise ``TypeError`` when ``args`` (the extra arguments of query()/command()) hold more positional values than ``command`` can bind. + + A lone list or tuple is the positional list itself, so its elements are the values; anything else counts one value per argument. + Fewer than two values never mismatch an upper bound of one or more, and a mapping alone is the named form: both return at once. + """ + if len(args) == 1: + values = args[0] + if not isinstance(values, (list, tuple)): + return + count = len(values) + else: + count = len(args) + if count < 2 or language.lower() not in _COUNTED_LANGUAGES: + return + bound = _placeholder_bound(command) + if count <= bound: + return + raise TypeError( + f"{name}() got {count} positional values, but the statement has at most {bound} placeholder{'' if bound == 1 else 's'}. " + "A list or tuple on its own is the positional-parameter list, one element per ?, so it does not bind one list to one ?. " + "To bind a list as ONE parameter, wrap it in another list, [[a, b]], or pass it by name: " + '"... WHERE x IN :ids" with {"ids": [a, b]}.' + ) diff --git a/src/arcadedb/core.py b/src/arcadedb/core.py index 76d2d77..ed9803c 100644 --- a/src/arcadedb/core.py +++ b/src/arcadedb/core.py @@ -14,6 +14,7 @@ import jpype from ._logging import get_logger +from ._params import refuse_surplus_positionals from .callbacks import ( EventHandle, FunctionRegistration, @@ -399,9 +400,12 @@ def query(self, language: str, command: str, *args) -> ResultSet: Parameters bind positionally (``?``) from the extra arguments, or by name (``:name``, ``$name``) from a single dict. A single list or tuple - is the positional list itself; ``None`` binds as null. + is the positional list itself; ``None`` binds as null. More positional + values than the SQL statement has placeholders raise ``TypeError``. """ self._check_not_closed() + if args: + refuse_surplus_positionals("query", language, command, args) clear_pending_interrupt() try: if args: @@ -427,6 +431,8 @@ def command(self, language: str, command: str, *args) -> Optional[ResultSet]: Parameters bind as in :meth:`query`. """ self._check_not_closed() + if args: + refuse_surplus_positionals("command", language, command, args) clear_pending_interrupt() try: if args: diff --git a/tests/test_parameter_binding.py b/tests/test_parameter_binding.py index 09ab7e9..1307b49 100644 --- a/tests/test_parameter_binding.py +++ b/tests/test_parameter_binding.py @@ -152,7 +152,9 @@ def as_python(value): return value def received(method, *args): - getattr(db, method)("sql", "SELECT 1", *args) + getattr(db, method)( + "sql", "SELECT ?, ?", *args + ) # two ?: the check refuses surplus values (arg,) = seen.pop() if isinstance(arg, java_map): return ("Map", as_python(arg)) diff --git a/tests/test_positional_count.py b/tests/test_positional_count.py new file mode 100644 index 0000000..83f5c55 --- /dev/null +++ b/tests/test_positional_count.py @@ -0,0 +1,131 @@ +"""More positional values than the statement has placeholders is refused, not bound silently. + +``query("sql", "... 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 bound the first element, dropped the second without a +word, and returned fewer rows than the caller meant. The engine does not check the count (it also lets a call with fewer values +than placeholders bind null; ArcadeData/arcadedb#10721), so the bindings refuse the form that cannot be right: more values than +the statement can hold. + +The check is an upper bound (placeholder characters counted anywhere in the text), so it never refuses a call that binds. +""" + +import pytest + + +@pytest.fixture +def db(temp_db): + temp_db.command("sql", "CREATE DOCUMENT TYPE T") + with temp_db.transaction(): + for k in range(4): + temp_db.command("sql", "INSERT INTO T SET k = ?, v = 0", k) + return temp_db + + +def _rids(db, *keys): + by_key = {row.get("k"): row.get_rid() for row in db.query("sql", "SELECT FROM T")} + return [by_key[k] for k in keys] + + +def _keys(rows): + return sorted(row["k"] for row in rows) + + +LIST_FORMS = [ + pytest.param(lambda a, b: ([a, b],), id="list"), + pytest.param(lambda a, b: ((a, b),), id="tuple"), + pytest.param(lambda a, b: (a, b), id="two-arguments"), +] + + +@pytest.mark.parametrize("form", LIST_FORMS) +def test_query_refuses_two_values_for_one_placeholder(db, form): + with pytest.raises(TypeError, match=r"2 positional values.*1 placeholder"): + db.query("sql", "SELECT FROM T WHERE k IN ?", *form(0, 1)) + + +@pytest.mark.parametrize("form", LIST_FORMS) +def test_rid_list_by_position_is_refused_not_cut_to_the_first(db, form): + rid_a, rid_b = _rids(db, 0, 1) + with pytest.raises(TypeError, match="IN :ids"): + db.query("sql", "SELECT FROM T WHERE @rid IN ?", *form(rid_a, rid_b)) + + +def test_command_refuses_and_changes_nothing(db): + with pytest.raises(TypeError, match="positional values"): + with db.transaction(): + db.command("sql", "UPDATE T SET v = 1 WHERE k = ?", [0, 1]) + assert {row["v"] for row in db.query("sql", "SELECT FROM T").to_list()} == {0} + + +def test_the_refusal_is_raised_before_the_statement_runs(db): + # not wrapped as "Query failed": it is a wrong call, not a failed statement + with pytest.raises(TypeError) as caught: + db.query("sql", "SELECT FROM T WHERE k = ?", 1, 2, 3) + assert not str(caught.value).startswith("Query failed") + + +def test_the_message_names_the_two_ways_out(db): + with pytest.raises(TypeError) as caught: + db.query("sql", "SELECT FROM T WHERE k IN ?", [0, 1]) + message = str(caught.value) + assert "[[0, 1]]" not in message # the message does not echo the data + assert "wrap" in message and "IN :ids" in message + + +# -- the documented forms keep binding + + +def test_a_list_wrapped_in_a_list_binds_as_one_parameter(db): + rows = db.query("sql", "SELECT FROM T WHERE k IN ?", [[0, 1]]).to_list() + assert _keys(rows) == [0, 1] + + +def test_a_rid_list_wrapped_in_a_list_binds_as_one_parameter(db): + rid_a, rid_b = _rids(db, 0, 1) + rows = db.query("sql", "SELECT FROM T WHERE @rid IN ?", [[rid_a, rid_b]]).to_list() + assert _keys(rows) == [0, 1] + + +def test_the_named_form_binds_the_whole_list(db): + rows = db.query( + "sql", "SELECT FROM T WHERE k IN :ids", {"ids": [0, 1, 2]} + ).to_list() + assert _keys(rows) == [0, 1, 2] + + +@pytest.mark.parametrize("form", LIST_FORMS) +def test_one_value_per_placeholder_still_binds(db, form): + rows = db.query("sql", "SELECT FROM T WHERE k = ? OR k = ?", *form(0, 3)).to_list() + assert _keys(rows) == [0, 3] + + +def test_one_value_one_placeholder_in_every_spelling(db): + for args in ((2,), ([2],), ((2,),)): + assert _keys(db.query("sql", "SELECT FROM T WHERE k = ?", *args).to_list()) == [ + 2 + ] + + +def test_a_placeholder_character_in_a_literal_does_not_refuse_a_binding_call(db): + # the bound is an upper bound: a "?" in a string counts as a placeholder, so this call (2 values, 2 counted) is not refused + rows = db.query("sql", "SELECT FROM T WHERE k = ? AND '?' = '?'", [1]).to_list() + assert _keys(rows) == [1] + + +def test_positional_dollar_placeholders_count(db): + db.query( + "sql", "SELECT FROM T WHERE k = $1 OR k = $2", [0, 1] + ).to_list() # two `$n`, two values: not a TypeError + + +def test_a_script_with_two_placeholders_binds_two_values(db): + rows = db.command( + "sqlscript", "SELECT FROM T WHERE k = ? OR k = ?;", [0, 1] + ).to_list() + assert _keys(rows) == [0, 1] + + +def test_other_languages_are_not_counted(db): + # openCypher has no "?" placeholders; the check is for SQL, whose list form is the documented one + rows = db.query("opencypher", "RETURN $a AS a", {"a": 1}).to_list() + assert [row["a"] for row in rows] == [1]