Add check for bind variables duplicated in DML returning statement.

Author: anthony-tuininga

Diff for: doc/src/release_notes.rst

@@ -16,6 +16,12 @@ Thin Mode Changes
 #)  Fixed bug when calling :meth:`~Connection.gettype()` with an object type
     name containing ``%ROWTYPE``
     (`issue 304 <https://github.com/oracle/python-oracledb/issues/304>`__).
+#)  Error ``DPY-2048: the bind variable placeholder ":{name}" cannot be used
+    both before and after the RETURNING clause in a DML RETURNING statement``
+    is now raised when the same bind variable placeholder name is used both
+    before and after the RETURNING clause in a
+    :ref:`DML RETURNING statement <dml-returning-bind>`. Previously, various
+    internal errors were raised.
 #)  Tightened up code looking for the end of a database request.
 #)  Packet output is now immediately flushed in order to avoid losing output
     due to buffering when multiple threads are running.

Diff for: doc/src/user_guide/bind.rst

@@ -291,6 +291,8 @@ UROWID can be bound with type :attr:`oracledb.DB_TYPE_ROWID`.
 See :ref:`querymetadatadiff`.
 
 
+.. _dml-returning-bind:
+
 DML RETURNING Bind Variables
 ============================
 
@@ -315,9 +317,28 @@ In the above example, since the WHERE clause matches only one row, the output
 contains a single item in the list. If the WHERE clause matched multiple rows,
 the output would contain as many items as there were rows that were updated.
 
-No duplicate binds are allowed in a DML statement with a RETURNING clause, and
-no duplication is allowed between bind variables in the DML section and the
-RETURNING section of the statement.
+The same bind variable placeholder name cannot be used both before and after
+the RETURNING clause. For example, if the ``:dept_name`` bind variable is used
+both before and after the RETURNING clause:
+
+.. code-block:: python
+
+    # a variable cannot be used for both input and output in a DML returning
+    # statement
+    dept_name_var = cursor.var(str)
+    dept_name_var.setvalue(0, 'Input Department')
+    cursor.execute("""
+            update departments set
+                department_name = :dept_name || ' EXTRA TEXT'
+            returning department_name into :dept_name""",
+            dept_name=dept_name_var)
+
+The above example will not update the bind variable as expected, but no error
+will be raised if you are using python-oracledb Thick mode. With
+python-oracledb Thin mode, the above example returns the following error::
+
+    DPY-2048: the bind variable placeholder ":dept_name" cannot be used
+    both before and after the RETURNING clause in a DML RETURNING statement
 
 
 LOB Bind Variables

Diff for: src/oracledb/errors.py

@@ -241,6 +241,7 @@ def _raise_err(
 ERR_INVALID_ARRAYSIZE = 2045
 ERR_CURSOR_HAS_BEEN_CLOSED = 2046
 ERR_INVALID_LOB_AMOUNT = 2047
+ERR_DML_RETURNING_DUP_BINDS = 2048
 
 # error numbers that result in NotSupportedError
 ERR_TIME_NOT_SUPPORTED = 3000
@@ -463,6 +464,10 @@ def _raise_err(
         "(actual: {actual_size}, maximum: {max_size})"
     ),
     ERR_DB_TYPE_NOT_SUPPORTED: 'database type "{name}" is not supported',
+    ERR_DML_RETURNING_DUP_BINDS: (
+        'the bind variable placeholder ":{name}" cannot be used both before '
+        "and after the RETURNING clause in a DML RETURNING statement"
+    ),
     ERR_DUPLICATED_PARAMETER: (
         '"{deprecated_name}" and "{new_name}" cannot be specified together'
     ),

Diff for: src/oracledb/impl/thin/statement.pyx

@@ -353,11 +353,16 @@ cdef class Statement:
         Add bind information to the statement by examining the passed SQL for
         bind variable names.
         """
-        cdef BindInfo info
+        cdef BindInfo info, orig_info
         if not self._is_plsql or name not in self._bind_info_dict:
             info = BindInfo(name, self._is_returning)
             self._bind_info_list.append(info)
             if info._bind_name in self._bind_info_dict:
+                if self._is_returning:
+                    orig_info = self._bind_info_dict[info._bind_name][-1]
+                    if not orig_info._is_return_bind:
+                        errors._raise_err(errors.ERR_DML_RETURNING_DUP_BINDS,
+                                          name=info._bind_name)
                 self._bind_info_dict[info._bind_name].append(info)
             else:
                 self._bind_info_dict[info._bind_name] = [info]

Diff for: tests/test_1600_dml_returning.py

@@ -27,6 +27,7 @@
 """
 
 import datetime
+import unittest
 
 import oracledb
 import test_env
@@ -525,6 +526,20 @@ def test_1622(self):
         self.cursor.execute(sql, in_val=25, out_val=out_val)
         self.assertEqual(out_val.getvalue(), [25])
 
+    @unittest.skipUnless(test_env.get_is_thin(), "cannot be checked")
+    def test_1623(self):
+        "1623 - execute DML returning with duplicated binds"
+        self.cursor.execute("truncate table TestTempTable")
+        str_val = self.cursor.var(str)
+        str_val.setvalue(0, "Test Data")
+        sql = """
+            insert into TestTempTable (IntCol, StringCol1)
+            values (:id_val, :str_val || ' (Additional String)')
+            returning StringCol1 into :str_val
+        """
+        with self.assertRaisesFullCode("DPY-2048"):
+            self.cursor.execute(sql, id_val=1, str_val=str_val)
+
 
 if __name__ == "__main__":
     test_env.run_test_cases()