Bindings: more positional values than placeholders is a TypeError, not a silently dropped value

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:

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]}.'
+    )

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:

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))

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]