fixup! Add support for binding parameters by index

comdb2/_ccdb2.pyx

@@ -223,15 +223,15 @@ cdef class _ParameterValue(object):
         exc_desc = _describe_exception(exc)
 
         if exc is not None:
-            errmsg = "Can't bind %s value %r for parameter '%s': %s" % (
+            errmsg = "Can't bind %s value %r for parameter %r: %s" % (
                 type(obj).__name__,
                 obj,
                 param_name,
                 exc_desc,
             )
             raise Error(lib.CDB2ERR_CONV_FAIL, errmsg) from exc
         else:
-            errmsg = "Can't map %s value %r for parameter '%s' to a Comdb2 type" % (
+            errmsg = "Can't map %s value %r for parameter %r to a Comdb2 type" % (
                 type(obj).__name__,
                 obj,
                 param_name,
@@ -401,26 +401,33 @@ cdef class Handle(object):
         param_guards = []
         try:
             if parameters is not None:
-                bind_by_index = isinstance(parameters, (list, tuple))
-                items = enumerate(parameters, 1) if bind_by_index \
-                        else parameters.items()
+                try:
+                    items = parameters.items()
+                    bind_by_index = False
+                except Exception:
+                    items = enumerate(parameters, 1)
+                    bind_by_index = True
                 for key, val in items:
-                    ckey = _string_as_bytes(key) if not bind_by_index else key
+                    ckey = key if bind_by_index else _string_as_bytes(key)
                     cval = _ParameterValue(val, key)
                     param_guards.append(ckey)
                     param_guards.append(cval)
                     if cval.list_size == -1:
                         if bind_by_index:
                             rc = lib.cdb2_bind_index(self.hndl, ckey,
-                                                    cval.type, cval.data, cval.size)
+                                                     cval.type, cval.data, cval.size)
                         else:
                             rc = lib.cdb2_bind_param(self.hndl, <char*>ckey,
-                                                    cval.type, cval.data, cval.size)
+                                                     cval.type, cval.data, cval.size)
                     else:
                         if bind_by_index:
-                            raise ValueError("Binding arrays by index is currently unsupported. Bind arrays by name.")
-                        # Bind Array if cval is an array
-                        rc = lib.cdb2_bind_array(self.hndl, <char*>ckey, cval.type, cval.data, cval.list_size, cval.size)
+                            raise ValueError(
+                                "Binding arrays by index is currently unsupported."
+                                " You must bind by name when binding arrays."
+                            )
+                        rc = lib.cdb2_bind_array(self.hndl, <char*>ckey,
+                                                 cval.type, cval.data,
+                                                 cval.list_size, cval.size)
                     _errchk(rc, self.hndl)
 
             with nogil:

comdb2/cdb2.py

@@ -99,14 +99,18 @@
 examples we make use of the `list` constructor to turn the iterable returned
 by `Handle.execute` into a list of result rows.
 
-You can also bind by index by providing a sequence
-instead of by name with placeholders specified using ``?`` in sequence.
-Note that binding an array by index is currently not supported. For example:
+You can bind parameters positionally rather than by name, by using ``?``
+for each placeholder and providing a list or tuple of parameter values.
+For example:
 
     >>> query = "select 25 between ? and ?"
     >>> print(list(hndl.execute(query, [20, 42])))
     [[1]]
 
+Note:
+
+    Arrays can currently only be bound by name, not positionally.
+
 Types
 -----
 
@@ -376,8 +380,10 @@ def execute(
         The ``sql`` string may have placeholders for parameters to be passed.
         This should always be the preferred method of parameterizing the SQL
         query, as it prevents SQL injection vulnerabilities and is faster.
-        Placeholders for named parameters must be in Comdb2's native format,
-        ``@param_name``, or with ``?`` for positional parameters.
+        Placeholders for named parameters must be in one of Comdb2's native
+        formats, either ``@param_name`` or ``:param_name``. Alternatively, you
+        can use ``?`` for each placeholder to bind parameters positionally
+        instead of by name.
 
         If ``column_types`` is provided and non-empty, it must be a sequence of
         members of the `ColumnType` enumeration. The database will coerce the
@@ -389,10 +395,12 @@ def execute(
 
         Args:
             sql (str): The SQL string to execute.
-            parameters (Mapping[str, Any]): An optional mapping from parameter
-                names to the values to be bound for them.
-                (Sequence[Any]): Can also use sequence with ``?`` with parameters executed in sequence.
-                Note that binding arrays by index is currently not supported. These must be bound by name.
+            parameters (Mapping[str, Any] | Sequence[Any]):
+                If the SQL statement has ``@param_name`` style placeholders,
+                you must pass a mapping from parameter name to value.
+                If the SQL statement has ``?`` style placeholders, you must
+                instead pass an ordered sequence of parameter values.
+                Note that arrays can currently only be bound by name.
             column_types (Sequence[int]): An optional sequence of types (values
                 of the `ColumnType` enumeration) which the columns of the
                 result set will be coerced to.
@@ -409,13 +417,12 @@ def execute(
 
         Example:
             >>> for row in hndl.execute("select 1, 2 UNION ALL select @x, @y",
-            ...                         {'x': 2, 'y': 4}):
+            ...                         {"x": 2, "y": 4}):
             ...     print(row)
             [1, 2]
             [2, 4]
 
-            >>> for row in hndl.execute("select 1, 2 UNION ALL select ?, ?",
-            ...                         [2, 4]):
+            >>> for row in hndl.execute("select 1, 2 UNION ALL select ?, ?", [2, 4]):
             ...     print(row)
             [1, 2]
             [2, 4]

comdb2/dbapi2.py

@@ -120,23 +120,26 @@
 When we run the same query with parameter ``b`` bound to ``23``, a ``0`` is
 returned instead, because ``20 <= 25 <= 23`` is false.
 
-Alternatively, you can also bind by index instead of by name, by providing a list/tuple
-with placeholders specified using ``?`` in the same order as the elements in the list/tuple.
-Note that binding an array by index is currently not supported, these must be bound by name.  For example:
+Note:
+    Because named parameters are bound using ``%(name)s``, other ``%`` signs in
+    a query must be escaped.  For example, ``WHERE name like 'M%'`` becomes
+    ``WHERE name LIKE 'M%%'``. Conversely, when named parameters are not being
+    used, ``%`` signs in the SQL statement must not be escaped.
+
+You can bind parameters positionally rather than by name, by using ``?``
+for each placeholder and providing a list or tuple of parameter values.
+For example:
 
     >>> query = "select 25 between ? and ?"
     >>> print(conn.cursor().execute(query, [20, 42]).fetchall())
     [[1]]
 
-In this example, we execute the query with the first ``?`` bound to 20 and the second
-``?`` bound to 42. Thus, a ``1`` is returned like in the previous example.
+In this example, we execute the query with the first ``?`` bound to 20 and the
+second ``?`` bound to 42, so a ``1`` is returned like in the previous example.
 
 Note:
-    Because parameters by name are bound using ``%(name)s``, other ``%`` signs in
-    a query must be escaped.  For example, ``WHERE name like 'M%'`` becomes
-    ``WHERE name LIKE 'M%%'``.
-    However, this does not apply when binding parameters by index. ``%`` does not
-    need to be escaped in this case, and only in this case.
+
+    Arrays can currently only be bound by name, not positionally.
 
 Types
 -----
@@ -316,20 +319,22 @@
 Comdb2's native placeholder format is ``@name``, but that cannot be used by
 this module because it's not an acceptable `DB-API 2.0 placeholder style
 <https://www.python.org/dev/peps/pep-0249/#paramstyle>`_.  This module uses
-``pyformat`` because it is the only DB-API 2.0 paramstyle that we can translate
-into Comdb2's placeholder format without needing a SQL parser.
+``pyformat`` for named parameters because it is the only DB-API 2.0 paramstyle
+that we can translate into Comdb2's placeholder format without needing
+a SQL parser. This module also supports the ``qmark`` parameter style for
+binding parameters positionally.
 
 Note:
     An int value is bound as ``%(my_int)s``, not as ``%(my_int)d`` - the last
     character is always ``s``.
 
 Note:
-    Because SQL strings for this module use the ``pyformat`` placeholder style,
-    any literal ``%`` characters in a query must be escaped by doubling them.
-    ``WHERE name like 'M%'`` becomes ``WHERE name LIKE 'M%%'``.
-
-This module also has support for ``qmark`` if binding by index.
-``%`` does not need to be escaped if binding by position.
+    When binding parameters by name, any ``%`` sign is recognized as the start
+    of a ``pyformat`` style placeholder, and so any literal ``%`` characters in
+    a query must be escaped by doubling. ``WHERE name like 'M%'`` becomes
+    ``WHERE name LIKE 'M%%'``. This is not necessary when binding parameters
+    positionally with ``?`` placeholders, nor when the literal ``%`` appears in
+    a parameter value as opposed to literally in the query.
 """
 
 _FIRST_WORD_OF_STMT = re.compile(
@@ -1018,11 +1023,13 @@ def execute(
     ) -> Cursor:
         """Execute a database operation (query or command).
 
-        The ``sql`` string must be provided as a Python format string, with
-        parameter placeholders represented as ``%(name)s`` and all other ``%``
-        signs escaped as ``%%``.
-        HOWEVER, if binding by index (parameter placeholders represented as ``?``),
-        ``%`` does not need to be escaped. This is the only time it does not need to be escaped.
+        The ``sql`` string may contain either named placeholders represented
+        as ``%(name)s`` or positionally ordered placeholders represented
+        as ``?``. When named placeholders are used, any literal ``%`` signs in
+        the statement must be escaped by doubling them, to distinguish them
+        from the start of a named placeholder. When no placeholders are used
+        or when positional ``?`` placeholders are used, literal ``%`` signs in
+        the SQL must not be escaped.
 
         Note:
             Using placeholders should always be the preferred method of
@@ -1045,10 +1052,12 @@ def execute(
 
         Args:
             sql (str): The SQL string to execute, as a Python format string.
-            parameters (Mapping[str, Any]): An optional mapping from parameter
-                names to the values to be bound for them.
-                (Sequence[Any]): Can also use sequence with ``?`` with parameters executed in sequence.
-                Note that binding arrays by index is currently not supported. These must be bound by name.
+            parameters (Mapping[str, Any] | Sequence[Any]):
+                If the SQL statement has ``%(param_name)s`` style placeholders,
+                you must pass a mapping from parameter name to value.
+                If the SQL statement has ``?`` style placeholders, you must
+                instead pass an ordered sequence of parameter values.
+                Note that arrays can currently only be bound by name.
             column_types (Sequence[int]): An optional sequence of types (values
                 of the `ColumnType` enumeration) which the columns of the
                 result set will be coerced to.
@@ -1070,8 +1079,7 @@ def execute(
             >>> cursor.fetchall()
             [[1, 2], [2, 4]]
 
-            >>> cursor.execute("select 1, 2 UNION ALL select ?, ?",
-            ...                [2, 4]])
+            >>> cursor.execute("select 1, 2 UNION ALL select ?, ?", [2, 4]])
             >>> cursor.fetchall()
             [[1, 2], [2, 4]]
         """
@@ -1103,10 +1111,10 @@ def executemany(
         Args:
             sql (str): The SQL string to execute, as a Python format string of
                 the format expected by `execute`.
-            seq_of_parameters (Sequence[Mapping[str, Any]] | Sequence[Sequence[Any]]): A sequence of
-                mappings from parameter names to the values to be bound for
-                them or a sequence of a sequence of parameter values if binding by index.
-                The ``sql`` statement will be run once per element in this sequence.
+            seq_of_parameters (Sequence[Mapping[str, Any]] | Sequence[Sequence[Any]]):
+                The ``sql`` statement will be executed once per element in this
+                sequence, using each successive element as the parameter values
+                for the corresponding call to `.execute`.
         """
         self._check_closed()
         for parameters in seq_of_parameters:
@@ -1131,7 +1139,11 @@ def _execute(self, operation, sql, parameters=None, *, column_types=None):
             # If variable interpolation fails, then translate the exception to
             # an InterfaceError to signal that it's a client-side problem.
             # If binding by index then no need to modify sql
-            if not isinstance(parameters, (list, tuple)):
+            try:
+                by_name = hasattr(parameters, "items")
+            except Exception:
+                by_name = False
+            if by_name:
                 sql = sql % {name: "@" + name for name in parameters}
         except KeyError as keyerr:
             msg = "No value provided for parameter %s" % keyerr

tests/test_cdb2.py

@@ -314,7 +314,8 @@ def test_binding_array_by_index():
         hndl.execute("select * from carray(?)", [[1, 2, 3]])
 
     assert exc.value.args[0] == (
-        "Binding arrays by index is currently unsupported. Bind arrays by name."
+        "Binding arrays by index is currently unsupported. "
+        "You must bind by name when binding arrays."
     )
 
 

tests/test_dbapi2.py

@@ -273,14 +273,14 @@ def test_unescaped_percent():
 def test_different_sequences():
     conn = connect("mattdb", "dev")
     cursor = conn.cursor()
-    cursor.execute("select ?, ?", [1,2])
+    cursor.execute("select ?, ?", [1, 2])
     assert cursor.fetchall() == [[1, 2]]
 
-    cursor.execute("select ?, ?", (1,2))
+    cursor.execute("select ?, ?", (1, 2))
     assert cursor.fetchall() == [[1, 2]]
 
-    with pytest.raises(AttributeError):
-        cursor.execute("select ?, ?", "hi")
+    cursor.execute("select ?, ?", "hi")
+    assert cursor.fetchall() == [["h", "i"]]
 
 
 def test_reading_and_writing_datetimes():
@@ -402,7 +402,7 @@ def test_all_datatypes_as_parameters():
     cursor.execute(
         "insert into all_datatypes(" + ", ".join(COLUMN_LIST) + ")"
         " values(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)",
-        list(v for k, v in params),
+        [v for _, v in params],
     )
 
     conn.commit()