From 5c4692818cc430168fc70377d7cffe1a5f04c187 Mon Sep 17 00:00:00 2001 From: Taewoon Kim Date: Sun, 25 Oct 2026 18:04:48 +0900 Subject: [PATCH] Bindings: more positional values than placeholders is a TypeError, not 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 --- bindings/python/docs/api/database.md | 4 +- bindings/python/docs/development/testing.md | 1 + bindings/python/docs/guide/core/queries.md | 14 ++ .../python/src/arcadedb_embedded/_params.py | 48 +++++++ bindings/python/src/arcadedb_embedded/core.py | 8 +- .../python/tests/test_parameter_binding.py | 4 +- .../python/tests/test_positional_count.py | 130 ++++++++++++++++++ 7 files changed, 205 insertions(+), 4 deletions(-) create mode 100644 bindings/python/src/arcadedb_embedded/_params.py create mode 100644 bindings/python/tests/test_positional_count.py diff --git a/bindings/python/docs/api/database.md b/bindings/python/docs/api/database.md index dda0a4b19d2..300173b1d5c 100644 --- a/bindings/python/docs/api/database.md +++ b/bindings/python/docs/api/database.md @@ -159,7 +159,7 @@ Execute a query and return results. Queries are read-only and don't require a tr - `language` (str): Query language - `"sql"`, `"sqlscript"`, `"opencypher"` (or its alias `"cypher"`), `"graphql"` - `command` (str): Query string - `*args`: Optional positional parameters, or one mapping for named parameters - A single list or tuple on its own is the positional-parameter array, one element per `?`, so `query("sql", "... ?", [0.9, 0.1, 0.0])` binds `?` to `0.9`. To bind one list as one parameter (a query vector, say), pass a NumPy array or `to_java_float_array(...)`, use a named parameter, or wrap it: `[[0.9, 0.1, 0.0]]`. + A single list or tuple on its own is the positional-parameter array, one element per `?`, so `query("sql", "... = ? AND ... = ?", [0.9, 0.1])` binds `0.9` and `0.1` to two `?`. A call with more positional values than the SQL statement has placeholders raises `TypeError` (the engine would bind the first value and drop the rest without a word), so `query("sql", "... WHERE @rid IN ?", [rid_a, rid_b])` is refused instead of returning one row. To bind one list as one parameter (a query vector, say, or the list of an `IN`), pass a NumPy array or `to_java_float_array(...)`, use a named parameter (`"... IN :ids"` with `{"ids": [a, b]}`), or wrap it: `[[a, b]]`. The count is an upper bound taken from the `?`, `$` and `:` characters of the text, so a statement whose literals hold such characters is checked more loosely, and only SQL and SQLScript are checked. **Returns:** @@ -222,7 +222,7 @@ one. - `language` (str): Command language (usually `"sql"` or `"opencypher"`) - `command` (str): Command string - `*args`: Optional positional parameters, or one mapping for named parameters - A single list or tuple on its own is the positional-parameter array, one element per `?`, so `query("sql", "... ?", [0.9, 0.1, 0.0])` binds `?` to `0.9`. To bind one list as one parameter (a query vector, say), pass a NumPy array or `to_java_float_array(...)`, use a named parameter, or wrap it: `[[0.9, 0.1, 0.0]]`. + A single list or tuple on its own is the positional-parameter array, one element per `?`, so `query("sql", "... = ? AND ... = ?", [0.9, 0.1])` binds `0.9` and `0.1` to two `?`. A call with more positional values than the SQL statement has placeholders raises `TypeError` (the engine would bind the first value and drop the rest without a word), so `query("sql", "... WHERE @rid IN ?", [rid_a, rid_b])` is refused instead of returning one row. To bind one list as one parameter (a query vector, say, or the list of an `IN`), pass a NumPy array or `to_java_float_array(...)`, use a named parameter (`"... IN :ids"` with `{"ids": [a, b]}`), or wrap it: `[[a, b]]`. The count is an upper bound taken from the `?`, `$` and `:` characters of the text, so a statement whose literals hold such characters is checked more loosely, and only SQL and SQLScript are checked. **Returns:** diff --git a/bindings/python/docs/development/testing.md b/bindings/python/docs/development/testing.md index 97734cfac4b..0a7017ca9e9 100644 --- a/bindings/python/docs/development/testing.md +++ b/bindings/python/docs/development/testing.md @@ -55,6 +55,7 @@ asserts, so the list below stops at the file. | `test_python_function_interrupt.py` | A real SIGINT during a statement that calls a Python function always arrives as `KeyboardInterrupt` and leaves the database usable, including when JPype fails with `RuntimeError: Fatal error occurred` in its race with Ctrl-C (not collected on Windows) | | `test_cypher.py` | OpenCypher queries, path modes, and planner regressions | | `test_parameter_binding.py` | Positional and named parameters reach the statement on every entry point, including a lone `None` and the async executor, and scalars skip the conversion walk | +| `test_positional_count.py` | More positional values than the SQL statement has placeholders is refused with `TypeError` before it runs (a lone list, a tuple, several arguments; `query()` and `command()`; nothing is changed), and the forms that bind keep binding: one value per `?`, a wrapped list, a named list, `$n`, SQLScript, other languages (P-07) | | `test_percall_calls.py` | The per-call paths of `query`, `command`, `first`, and `get` give the same bound values, overload choice, errors, and result-set state as the plain path (#280) | | `test_typed_rows.py` | `to_list()` and `iter_dicts()` read rows through the typed-row transport and return the same Python type and value for every property type as the row-by-row path (#285) | | `test_open_read_only.py` | `open_database(read_only=True)` and `DatabaseFactory.open(read_only=True)` read and refuse writes and schema changes; the default stays writable (#383) | diff --git a/bindings/python/docs/guide/core/queries.md b/bindings/python/docs/guide/core/queries.md index 655225478a0..687fd996d09 100644 --- a/bindings/python/docs/guide/core/queries.md +++ b/bindings/python/docs/guide/core/queries.md @@ -178,6 +178,20 @@ Positional values bind one per `?`, from the extra arguments or from one list or (`db.query("sql", q, ["Alice", 25])`). `None` binds as null in `query()`, `command()`, and `async_executor()`. +More positional values than the statement has placeholders raise `TypeError` before the +statement runs. A list or tuple on its own is the positional list, so it never binds as +one parameter: `db.query("sql", "... WHERE @rid IN ?", [rid_a, rid_b])` is refused (the +engine would bind `rid_a`, ignore `rid_b`, and return one row, with no error). Bind the +list by name, or wrap it so it is the one positional value: + +```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 count is an upper bound (the `?`, `$` and `:` characters of the SQL text), so it +never refuses a call that binds. Fewer values than placeholders are not checked. + Parameters that are plain scalars (`int`, `float`, `str`, `bool`, `None`) or Java arrays (a vector from `to_java_float_array`) cross into the JVM in one call, named or positional. A collection, a `Decimal`, a date, or a numpy scalar is converted first and costs a few diff --git a/bindings/python/src/arcadedb_embedded/_params.py b/bindings/python/src/arcadedb_embedded/_params.py new file mode 100644 index 00000000000..469ff9a7ce0 --- /dev/null +++ b/bindings/python/src/arcadedb_embedded/_params.py @@ -0,0 +1,48 @@ +"""More positional values than a statement has placeholders is a wrong call, refused before it runs (P-07). + +``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), and the +bindings cannot ask it: the parsed statement's placeholder count is no public engine call, and the native library would need a new +entry point. 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/bindings/python/src/arcadedb_embedded/core.py b/bindings/python/src/arcadedb_embedded/core.py index 03ae0ce69d0..c5b28a54f88 100644 --- a/bindings/python/src/arcadedb_embedded/core.py +++ b/bindings/python/src/arcadedb_embedded/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/bindings/python/tests/test_parameter_binding.py b/bindings/python/tests/test_parameter_binding.py index 897c28a7001..f8a7a822fb7 100644 --- a/bindings/python/tests/test_parameter_binding.py +++ b/bindings/python/tests/test_parameter_binding.py @@ -151,7 +151,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/bindings/python/tests/test_positional_count.py b/bindings/python/tests/test_positional_count.py new file mode 100644 index 00000000000..3b68982c50b --- /dev/null +++ b/bindings/python/tests/test_positional_count.py @@ -0,0 +1,130 @@ +"""More positional values than the statement has placeholders is refused, not bound silently (P-07). + +``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), 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]