From 96db7cdd53ee9004be66545989b4ac5632bb7ccf Mon Sep 17 00:00:00 2001
From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Fri, 2 Dec 2022 17:00:10 -0500
Subject: [PATCH] add spaces, leading underscore to oracle checks

Expand the test suite from #8708 which unfortunately did
not exercise the bound parameter codepaths completely.

Continued fixes for Oracle fix :ticket:`8708` released in 1.4.43 where
bound parameter names that start with underscores, which are disallowed by
Oracle, were still not being properly escaped in all circumstances.

Fixes: #8708
Change-Id: Ic389c09bd7c53b773e5de35f1a18ef20769b92a7
---
 doc/build/changelog/unreleased_14/8708.rst   |  9 +++
 lib/sqlalchemy/dialects/oracle/cx_oracle.py  |  8 +--
 lib/sqlalchemy/testing/suite/test_dialect.py | 66 +++++++++++++++++++-
 3 files changed, 76 insertions(+), 7 deletions(-)
 create mode 100644 doc/build/changelog/unreleased_14/8708.rst

diff --git a/doc/build/changelog/unreleased_14/8708.rst b/doc/build/changelog/unreleased_14/8708.rst
new file mode 100644
index 0000000000..61dcbf658e
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/8708.rst
@@ -0,0 +1,9 @@
+.. change::
+    :tags: bug, oracle
+    :tickets: 8708
+    :versions: 2.0.0b4
+
+    Continued fixes for Oracle fix :ticket:`8708` released in 1.4.43 where
+    bound parameter names that start with underscores, which are disallowed by
+    Oracle, were still not being properly escaped in all circumstances.
+
diff --git a/lib/sqlalchemy/dialects/oracle/cx_oracle.py b/lib/sqlalchemy/dialects/oracle/cx_oracle.py
index 0be309cd4b..8f80aed656 100644
--- a/lib/sqlalchemy/dialects/oracle/cx_oracle.py
+++ b/lib/sqlalchemy/dialects/oracle/cx_oracle.py
@@ -445,13 +445,13 @@ from ...sql._typing import is_sql_compiler
 _CX_ORACLE_MAGIC_LOB_SIZE = 131072
 
 
-_ORACLE_BIND_TRANSLATE_RE = re.compile(r"[%\(\):\[\]\.\/\?]")
+_ORACLE_BIND_TRANSLATE_RE = re.compile(r"[%\(\):\[\]\.\/\? ]")
 
 # Oracle bind names can't start with digits or underscores.
 # currently we rely upon Oracle-specific quoting of bind names in most cases.
 # however for expanding params, the escape chars are used.
 # see #8708
-_ORACLE_BIND_TRANSLATE_CHARS = dict(zip("%():[]./?", "PAZCCCCCCC"))
+_ORACLE_BIND_TRANSLATE_CHARS = dict(zip("%():[]./? ", "PAZCCCCCCCC"))
 
 
 class _OracleInteger(sqltypes.Integer):
@@ -729,11 +729,11 @@ class OracleCompiler_cx_oracle(OracleCompiler):
                     lambda m: _ORACLE_BIND_TRANSLATE_CHARS[m.group(0)],
                     name,
                 )
-                if new_name[0].isdigit():
+                if new_name[0].isdigit() or new_name[0] == "_":
                     new_name = "D" + new_name
                 kw["escaped_from"] = name
                 name = new_name
-            elif name[0].isdigit():
+            elif name[0].isdigit() or name[0] == "_":
                 new_name = "D" + name
                 kw["escaped_from"] = name
                 name = new_name
diff --git a/lib/sqlalchemy/testing/suite/test_dialect.py b/lib/sqlalchemy/testing/suite/test_dialect.py
index 01cec1fb06..945edef85b 100644
--- a/lib/sqlalchemy/testing/suite/test_dialect.py
+++ b/lib/sqlalchemy/testing/suite/test_dialect.py
@@ -370,7 +370,7 @@ class FutureWeCanSetDefaultSchemaWEventsTest(
 class DifficultParametersTest(fixtures.TestBase):
     __backend__ = True
 
-    @testing.combinations(
+    tough_parameters = testing.combinations(
         ("boring",),
         ("per cent",),
         ("per % cent",),
@@ -381,14 +381,26 @@ class DifficultParametersTest(fixtures.TestBase):
         ("_starts_with_underscore",),
         ("dot.s",),
         ("more :: %colons%",),
+        ("_name",),
+        ("___name",),
+        ("[BracketsAndCase]",),
+        ("42numbers",),
+        ("percent%signs",),
+        ("has spaces",),
         ("/slashes/",),
         ("more/slashes",),
         ("q?marks",),
         ("1param",),
         ("1col:on",),
-        argnames="name",
+        argnames="paramname",
     )
-    def test_round_trip(self, name, connection, metadata):
+
+    @tough_parameters
+    def test_round_trip_same_named_column(
+        self, paramname, connection, metadata
+    ):
+        name = paramname
+
         t = Table(
             "t",
             metadata,
@@ -422,6 +434,54 @@ class DifficultParametersTest(fixtures.TestBase):
 
         row = connection.execute(stmt).first()
 
+    @testing.fixture
+    def multirow_fixture(self, metadata, connection):
+        mytable = Table(
+            "mytable",
+            metadata,
+            Column("myid", Integer),
+            Column("name", String(50)),
+            Column("desc", String(50)),
+        )
+
+        mytable.create(connection)
+
+        connection.execute(
+            mytable.insert(),
+            [
+                {"myid": 1, "name": "a", "desc": "a_desc"},
+                {"myid": 2, "name": "b", "desc": "b_desc"},
+                {"myid": 3, "name": "c", "desc": "c_desc"},
+                {"myid": 4, "name": "d", "desc": "d_desc"},
+            ],
+        )
+        yield mytable
+
+    @tough_parameters
+    def test_standalone_bindparam_escape(
+        self, paramname, connection, multirow_fixture
+    ):
+        tbl1 = multirow_fixture
+        stmt = select(tbl1.c.myid).where(
+            tbl1.c.name == bindparam(paramname, value="x")
+        )
+        res = connection.scalar(stmt, {paramname: "c"})
+        eq_(res, 3)
+
+    @tough_parameters
+    def test_standalone_bindparam_escape_expanding(
+        self, paramname, connection, multirow_fixture
+    ):
+        tbl1 = multirow_fixture
+        stmt = (
+            select(tbl1.c.myid)
+            .where(tbl1.c.name.in_(bindparam(paramname, value=["a", "b"])))
+            .order_by(tbl1.c.myid)
+        )
+
+        res = connection.scalars(stmt, {paramname: ["d", "a"]}).all()
+        eq_(res, [1, 4])
+
 
 class ReturningGuardsTest(fixtures.TablesTest):
     """test that the various 'returning' flags are set appropriately"""
-- 
2.47.2