Add a qualifying character to multi INSERT..VALUES parameter names

Changed the naming convention used when generating bound parameters
for a multi-VALUES insert statement, so that the numbered parameter
names don't conflict with the anonymized parameters of a WHERE clause,
as is now common in a PostgreSQL ON CONFLICT construct.

Change-Id: I3188d100fe4d322a47d344d6a63d3e40b915f228
Fixes: #3828

5 files changed, 154 insertions, 88 deletions

diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst
index 11d49d7a7..2d640c9e3 100644
--- a/doc/build/changelog/changelog_11.rst
+++ b/doc/build/changelog/changelog_11.rst
@@ -64,6 +64,16 @@
         created for a non-native enumerated type, but more critically the
         ENUM object on the PostgreSQL backend.
 
+
+    .. change::
+        :tags: bug, postgresql, sql
+        :tickets: 3828
+
+        Changed the naming convention used when generating bound parameters
+        for a multi-VALUES insert statement, so that the numbered parameter
+        names don't conflict with the anonymized parameters of a WHERE clause,
+        as is now common in a PostgreSQL ON CONFLICT construct.
+
 .. changelog::
     :version: 1.1.1
     :released: October 7, 2016
diff --git a/lib/sqlalchemy/sql/crud.py b/lib/sqlalchemy/sql/crud.py
index 642659c84..452fe5d9a 100644
--- a/lib/sqlalchemy/sql/crud.py
+++ b/lib/sqlalchemy/sql/crud.py
@@ -317,7 +317,7 @@ def _append_param_parameter(
             compiler, c, value, required=value is REQUIRED,
             name=_col_bind_name(c)
             if not stmt._has_multi_parameters
-            else "%s_0" % _col_bind_name(c),
+            else "%s_m0" % _col_bind_name(c),
             **kw
         )
     else:
@@ -396,7 +396,7 @@ def _create_update_prefetch_bind_param(compiler, c, process=True, name=None):
 
 class _multiparam_column(elements.ColumnElement):
     def __init__(self, original, index):
-        self.key = "%s_%d" % (original.key, index + 1)
+        self.key = "%s_m%d" % (original.key, index + 1)
         self.original = original
         self.default = original.default
         self.type = original.type
@@ -603,7 +603,7 @@ def _extend_values_for_multiparams(compiler, stmt, values, kw):
                 c,
                 (_create_bind_param(
                     compiler, c, row[c.key],
-                    name="%s_%d" % (c.key, i + 1)
+                    name="%s_m%d" % (c.key, i + 1)
                 ) if elements._is_literal(row[c.key])
                     else compiler.process(
                         row[c.key].self_group(), **kw))
diff --git a/test/dialect/postgresql/test_compiler.py b/test/dialect/postgresql/test_compiler.py
index ac8bb4815..52dd699fc 100644
--- a/test/dialect/postgresql/test_compiler.py
+++ b/test/dialect/postgresql/test_compiler.py
@@ -1208,6 +1208,28 @@ class InsertOnConflictTest(fixtures.TestBase, AssertsCompiledSQL):
                             "WHERE mytable.name > %(name_1)s "
                             'DO UPDATE SET name = excluded.name')
 
+    def test_do_update_index_elements_where_target_multivalues(self):
+        i = insert(
+            self.table1,
+            values=[dict(name='foo'), dict(name='bar'), dict(name='bat')])
+        i = i.on_conflict_do_update(
+            index_elements=self.goofy_index.expressions,
+            index_where=self.goofy_index.dialect_options[
+                'postgresql']['where'],
+            set_=dict(name=i.excluded.name)
+        )
+        self.assert_compile(
+            i,
+            "INSERT INTO mytable (name) "
+            "VALUES (%(name_m0)s), (%(name_m1)s), (%(name_m2)s) "
+            "ON CONFLICT (name) "
+            "WHERE mytable.name > %(name_1)s "
+            "DO UPDATE SET name = excluded.name",
+            checkparams={
+                'name_1': 'm', 'name_m0': 'foo',
+                'name_m1': 'bar', 'name_m2': 'bat'}
+        )
+
     def test_do_update_unnamed_index_target(self):
         i = insert(
             self.table1, values=dict(name='foo'))
diff --git a/test/dialect/postgresql/test_on_conflict.py b/test/dialect/postgresql/test_on_conflict.py
index eb0298bc2..154d3fe58 100644
--- a/test/dialect/postgresql/test_on_conflict.py
+++ b/test/dialect/postgresql/test_on_conflict.py
@@ -210,6 +210,40 @@ class OnConflictTest(fixtures.TablesTest):
                 [(10, "I'm a name")]
             )
 
+    def test_on_conflict_do_update_multivalues(self):
+        users = self.tables.users
+
+        with testing.db.connect() as conn:
+            conn.execute(users.insert(), dict(id=1, name='name1'))
+            conn.execute(users.insert(), dict(id=2, name='name2'))
+
+            i = insert(users)
+            i = i.on_conflict_do_update(
+                index_elements=users.primary_key.columns,
+                set_=dict(name="updated"),
+                where=(i.excluded.name != 'name12')
+            ).values([
+                dict(id=1, name='name11'),
+                dict(id=2, name='name12'),
+                dict(id=3, name='name13'),
+                dict(id=4, name='name14'),
+            ])
+
+            result = conn.execute(i)
+            eq_(result.inserted_primary_key, [None])
+            eq_(result.returned_defaults, None)
+
+            eq_(
+                conn.execute(
+                    users.select().order_by(users.c.id)).fetchall(),
+                [
+                    (1, "updated"),
+                    (2, "name2"),
+                    (3, "name13"),
+                    (4, "name14")
+                ]
+            )
+
     def _exotic_targets_fixture(self, conn):
         users = self.tables.users_xtra
 
diff --git a/test/sql/test_insert.py b/test/sql/test_insert.py
index 3a884643b..79de40e9c 100644
--- a/test/sql/test_insert.py
+++ b/test/sql/test_insert.py
@@ -640,11 +640,11 @@ class InsertImplicitReturningTest(
         self.assert_compile(
             ins,
             "INSERT INTO myothertable (othername) "
-            "VALUES (%(othername_0)s), "
-            "(%(othername_1)s)",
+            "VALUES (%(othername_m0)s), "
+            "(%(othername_m1)s)",
             checkparams={
-                'othername_1': 'bar',
-                'othername_0': 'foo'}
+                'othername_m1': 'bar',
+                'othername_m0': 'foo'}
         )
 
     def test_insert_multiple_values_return_defaults(self):
@@ -657,11 +657,11 @@ class InsertImplicitReturningTest(
         self.assert_compile(
             ins,
             "INSERT INTO myothertable (othername) "
-            "VALUES (%(othername_0)s), "
-            "(%(othername_1)s)",
+            "VALUES (%(othername_m0)s), "
+            "(%(othername_m1)s)",
             checkparams={
-                'othername_1': 'bar',
-                'othername_0': 'foo'}
+                'othername_m1': 'bar',
+                'othername_m0': 'foo'}
         )
 
     def test_insert_single_list_values(self):
@@ -671,8 +671,8 @@ class InsertImplicitReturningTest(
         self.assert_compile(
             ins,
             "INSERT INTO myothertable (othername) "
-            "VALUES (%(othername_0)s)",
-            checkparams={'othername_0': 'foo'}
+            "VALUES (%(othername_m0)s)",
+            checkparams={'othername_m0': 'foo'}
         )
 
     def test_insert_single_element_values(self):
@@ -769,15 +769,15 @@ class MultirowTest(_InsertTestBase, fixtures.TablesTest, AssertsCompiledSQL):
         ]
 
         checkparams = {
-            'myid_0': 1,
-            'myid_1': 2,
-            'myid_2': 3,
-            'name_0': 'a',
-            'name_1': 'c',
-            'name_2': 'e',
-            'description_0': 'b',
-            'description_1': 'd',
-            'description_2': 'f',
+            'myid_m0': 1,
+            'myid_m1': 2,
+            'myid_m2': 3,
+            'name_m0': 'a',
+            'name_m1': 'c',
+            'name_m2': 'e',
+            'description_m0': 'b',
+            'description_m1': 'd',
+            'description_m2': 'f',
         }
 
         dialect = default.DefaultDialect()
@@ -786,9 +786,9 @@ class MultirowTest(_InsertTestBase, fixtures.TablesTest, AssertsCompiledSQL):
         self.assert_compile(
             table1.insert().values(values),
             'INSERT INTO mytable (myid, name, description) VALUES '
-            '(:myid_0, :name_0, :description_0), '
-            '(:myid_1, :name_1, :description_1), '
-            '(:myid_2, :name_2, :description_2)',
+            '(:myid_m0, :name_m0, :description_m0), '
+            '(:myid_m1, :name_m1, :description_m1), '
+            '(:myid_m2, :name_m2, :description_m2)',
             checkparams=checkparams,
             dialect=dialect)
 
@@ -859,21 +859,21 @@ class MultirowTest(_InsertTestBase, fixtures.TablesTest, AssertsCompiledSQL):
         ]
 
         checkparams = {
-            'id_0': 1,
-            'id_1': 2,
-            'id_2': 3,
-            'data_0': 'data1',
-            'data_1': 'data2',
-            'data_2': 'data3',
-            'foo_1': 'plainfoo',
+            'id_m0': 1,
+            'id_m1': 2,
+            'id_m2': 3,
+            'data_m0': 'data1',
+            'data_m1': 'data2',
+            'data_m2': 'data3',
+            'foo_m1': 'plainfoo',
         }
 
         self.assert_compile(
             table.insert().values(values),
             'INSERT INTO sometable (id, data, foo) VALUES '
-            '(%(id_0)s, %(data_0)s, foobar()), '
-            '(%(id_1)s, %(data_1)s, %(foo_1)s), '
-            '(%(id_2)s, %(data_2)s, foobar())',
+            '(%(id_m0)s, %(data_m0)s, foobar()), '
+            '(%(id_m1)s, %(data_m1)s, %(foo_m1)s), '
+            '(%(id_m2)s, %(data_m2)s, foobar())',
             checkparams=checkparams,
             dialect=postgresql.dialect())
 
@@ -891,15 +891,15 @@ class MultirowTest(_InsertTestBase, fixtures.TablesTest, AssertsCompiledSQL):
         ]
 
         checkparams = {
-            'id_0': 1,
-            'id_1': 2,
-            'id_2': 3,
-            'data_0': 'data1',
-            'data_1': 'data2',
-            'data_2': 'data3',
+            'id_m0': 1,
+            'id_m1': 2,
+            'id_m2': 3,
+            'data_m0': 'data1',
+            'data_m1': 'data2',
+            'data_m2': 'data3',
             'foo': None,  # evaluated later
-            'foo_1': 15,
-            'foo_2': None  # evaluated later
+            'foo_m1': 15,
+            'foo_m2': None  # evaluated later
         }
 
         stmt = table.insert().values(values)
@@ -910,17 +910,17 @@ class MultirowTest(_InsertTestBase, fixtures.TablesTest, AssertsCompiledSQL):
                 for (k, v) in
                 stmt.compile(dialect=postgresql.dialect()).binds.items()]),
             {
-                'foo': Integer, 'data_2': String, 'id_0': Integer,
-                'id_2': Integer, 'foo_1': Integer, 'data_1': String,
-                'id_1': Integer, 'foo_2': Integer, 'data_0': String}
+                'foo': Integer, 'data_m2': String, 'id_m0': Integer,
+                'id_m2': Integer, 'foo_m1': Integer, 'data_m1': String,
+                'id_m1': Integer, 'foo_m2': Integer, 'data_m0': String}
         )
 
         self.assert_compile(
             stmt,
             'INSERT INTO sometable (id, data, foo) VALUES '
-            '(%(id_0)s, %(data_0)s, %(foo)s), '
-            '(%(id_1)s, %(data_1)s, %(foo_1)s), '
-            '(%(id_2)s, %(data_2)s, %(foo_2)s)',
+            '(%(id_m0)s, %(data_m0)s, %(foo)s), '
+            '(%(id_m1)s, %(data_m1)s, %(foo_m1)s), '
+            '(%(id_m2)s, %(data_m2)s, %(foo_m2)s)',
             checkparams=checkparams,
             dialect=postgresql.dialect())
 
@@ -938,15 +938,15 @@ class MultirowTest(_InsertTestBase, fixtures.TablesTest, AssertsCompiledSQL):
         ]
 
         checkparams = {
-            'id_0': 1,
-            'id_1': 2,
-            'id_2': 3,
-            'data_0': 'data1',
-            'data_1': 'data2',
-            'data_2': 'data3',
+            'id_m0': 1,
+            'id_m1': 2,
+            'id_m2': 3,
+            'data_m0': 'data1',
+            'data_m1': 'data2',
+            'data_m2': 'data3',
             'foo': None,  # evaluated later
-            'foo_1': 15,
-            'foo_2': None,  # evaluated later
+            'foo_m1': 15,
+            'foo_m2': None,  # evaluated later
         }
 
         stmt = table.insert().values(values)
@@ -956,17 +956,17 @@ class MultirowTest(_InsertTestBase, fixtures.TablesTest, AssertsCompiledSQL):
                 for (k, v) in
                 stmt.compile(dialect=postgresql.dialect()).binds.items()]),
             {
-                'foo': Integer, 'data_2': String, 'id_0': Integer,
-                'id_2': Integer, 'foo_1': Integer, 'data_1': String,
-                'id_1': Integer, 'foo_2': Integer, 'data_0': String}
+                'foo': Integer, 'data_m2': String, 'id_m0': Integer,
+                'id_m2': Integer, 'foo_m1': Integer, 'data_m1': String,
+                'id_m1': Integer, 'foo_m2': Integer, 'data_m0': String}
         )
 
         self.assert_compile(
             stmt,
             "INSERT INTO sometable (id, data, foo) VALUES "
-            "(%(id_0)s, %(data_0)s, %(foo)s), "
-            "(%(id_1)s, %(data_1)s, %(foo_1)s), "
-            "(%(id_2)s, %(data_2)s, %(foo_2)s)",
+            "(%(id_m0)s, %(data_m0)s, %(foo)s), "
+            "(%(id_m1)s, %(data_m1)s, %(foo_m1)s), "
+            "(%(id_m2)s, %(data_m2)s, %(foo_m2)s)",
             checkparams=checkparams,
             dialect=postgresql.dialect())
 
@@ -985,31 +985,31 @@ class MultirowTest(_InsertTestBase, fixtures.TablesTest, AssertsCompiledSQL):
             {"id": 5, "data": "bar", "foo": func.foob()},
         ]
         checkparams = {
-            'id_0': 1,
-            'data_0': 'foo',
+            'id_m0': 1,
+            'data_m0': 'foo',
 
-            'id_1': 2,
-            'data_1': 'bar',
+            'id_m1': 2,
+            'data_m1': 'bar',
 
-            'id_2': 3,
-            'data_2': 'bar',
+            'id_m2': 3,
+            'data_m2': 'bar',
 
-            'id_3': 4,
-            'data_3': 'bar',
-            'foo_3': 15,
+            'id_m3': 4,
+            'data_m3': 'bar',
+            'foo_m3': 15,
 
-            'id_4': 5,
-            'data_4': 'bar'
+            'id_m4': 5,
+            'data_m4': 'bar'
         }
 
         self.assert_compile(
             table.insert().values(values),
             "INSERT INTO sometable (id, data, foo) VALUES "
-            "(%(id_0)s, %(data_0)s, foob()), "
-            "(%(id_1)s, %(data_1)s, foob()), "
-            "(%(id_2)s, %(data_2)s, bar()), "
-            "(%(id_3)s, %(data_3)s, %(foo_3)s), "
-            "(%(id_4)s, %(data_4)s, foob())",
+            "(%(id_m0)s, %(data_m0)s, foob()), "
+            "(%(id_m1)s, %(data_m1)s, foob()), "
+            "(%(id_m2)s, %(data_m2)s, bar()), "
+            "(%(id_m3)s, %(data_m3)s, %(foo_m3)s), "
+            "(%(id_m4)s, %(data_m4)s, foob())",
             checkparams=checkparams,
             dialect=postgresql.dialect())
 
@@ -1027,20 +1027,20 @@ class MultirowTest(_InsertTestBase, fixtures.TablesTest, AssertsCompiledSQL):
         ]
 
         checkparams = {
-            'id_0': 1,
-            'id_1': 2,
-            'id_2': 3,
-            'data_0': 'data1',
-            'data_1': 'data2',
-            'data_2': 'data3',
+            'id_m0': 1,
+            'id_m1': 2,
+            'id_m2': 3,
+            'data_m0': 'data1',
+            'data_m1': 'data2',
+            'data_m2': 'data3',
         }
 
         self.assert_compile(
             table.insert().values(values),
             'INSERT INTO sometable (id, data) VALUES '
-            '(%(id_0)s, %(data_0)s), '
-            '(%(id_1)s, %(data_1)s), '
-            '(%(id_2)s, %(data_2)s)',
+            '(%(id_m0)s, %(data_m0)s), '
+            '(%(id_m1)s, %(data_m1)s), '
+            '(%(id_m2)s, %(data_m2)s)',
             checkparams=checkparams,
             dialect=postgresql.dialect())