diff options
| -rw-r--r-- | doc/build/changelog/changelog_10.rst | 25 | ||||
| -rw-r--r-- | lib/sqlalchemy/sql/compiler.py | 4 | ||||
| -rw-r--r-- | lib/sqlalchemy/sql/dml.py | 34 | ||||
| -rw-r--r-- | test/sql/test_insert.py | 109 | ||||
| -rw-r--r-- | test/sql/test_query.py | 7 |
5 files changed, 163 insertions, 16 deletions
diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst index 1cbbec3b3..3da2c63c2 100644 --- a/doc/build/changelog/changelog_10.rst +++ b/doc/build/changelog/changelog_10.rst @@ -17,6 +17,31 @@ :version: 1.0.0 .. change:: + :tags: bug, sql + :tickets: 3169 + + The INSERT...FROM SELECT construct now implies ``inline=True`` + on :class:`.Insert`. This helps to fix a bug where an + INSERT...FROM SELECT construct would inadvertently be compiled + as "implicit returning" on supporting backends, which would + cause breakage in the case of an INSERT that inserts zero rows + (as implicit returning expects a row), as well as arbitrary + return data in the case of an INSERT that inserts multiple + rows (e.g. only the first row of many). + A similar change is also applied to an INSERT..VALUES + with multiple parameter sets; implicit RETURNING will no longer emit + for this statement either. As both of these constructs deal + with varible numbers of rows, the + :attr:`.ResultProxy.inserted_primary_key` accessor does not + apply. Previously, there was a documentation note that one + may prefer ``inline=True`` with INSERT..FROM SELECT as some databases + don't support returning and therefore can't do "implicit" returning, + but there's no reason an INSERT...FROM SELECT needs implicit returning + in any case. Regular explicit :meth:`.Insert.returning` should + be used to return variable numbers of result rows if inserted + data is needed. + + .. change:: :tags: bug, orm :tickets: 3167 diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py index e45510aa4..fac4980b0 100644 --- a/lib/sqlalchemy/sql/compiler.py +++ b/lib/sqlalchemy/sql/compiler.py @@ -1981,11 +1981,13 @@ class SQLCompiler(Compiled): need_pks = self.isinsert and \ not self.inline and \ - not stmt._returning + not stmt._returning and \ + not stmt._has_multi_parameters implicit_returning = need_pks and \ self.dialect.implicit_returning and \ stmt.table.implicit_returning + if self.isinsert: implicit_return_defaults = (implicit_returning and stmt._return_defaults) diff --git a/lib/sqlalchemy/sql/dml.py b/lib/sqlalchemy/sql/dml.py index f7e033d85..72dd92c99 100644 --- a/lib/sqlalchemy/sql/dml.py +++ b/lib/sqlalchemy/sql/dml.py @@ -269,6 +269,13 @@ class ValuesBase(UpdateBase): .. versionadded:: 0.8 Support for multiple-VALUES INSERT statements. + .. versionchanged:: 1.0.0 an INSERT that uses a multiple-VALUES + clause, even a list of length one, + implies that the :paramref:`.Insert.inline` flag is set to + True, indicating that the statement will not attempt to fetch + the "last inserted primary key" or other defaults. The statement + deals with an arbitrary number of rows, so the + :attr:`.ResultProxy.inserted_primary_key` accessor does not apply. .. seealso:: @@ -434,8 +441,13 @@ class Insert(ValuesBase): dynamically render the VALUES clause at execution time based on the parameters passed to :meth:`.Connection.execute`. - :param inline: if True, SQL defaults will be compiled 'inline' into - the statement and not pre-executed. + :param inline: if True, no attempt will be made to retrieve the + SQL-generated default values to be provided within the statement; + in particular, + this allows SQL expressions to be rendered 'inline' within the + statement without the need to pre-execute them beforehand; for + backends that support "returning", this turns off the "implicit + returning" feature for the statement. If both `values` and compile-time bind parameters are present, the compile-time bind parameters override the information specified @@ -495,17 +507,12 @@ class Insert(ValuesBase): would normally raise an exception if these column lists don't correspond. - .. note:: - - Depending on backend, it may be necessary for the :class:`.Insert` - statement to be constructed using the ``inline=True`` flag; this - flag will prevent the implicit usage of ``RETURNING`` when the - ``INSERT`` statement is rendered, which isn't supported on a - backend such as Oracle in conjunction with an ``INSERT..SELECT`` - combination:: - - sel = select([table1.c.a, table1.c.b]).where(table1.c.c > 5) - ins = table2.insert(inline=True).from_select(['a', 'b'], sel) + .. versionchanged:: 1.0.0 an INSERT that uses FROM SELECT + implies that the :paramref:`.Insert.inline` flag is set to + True, indicating that the statement will not attempt to fetch + the "last inserted primary key" or other defaults. The statement + deals with an arbitrary number of rows, so the + :attr:`.ResultProxy.inserted_primary_key` accessor does not apply. .. note:: @@ -525,6 +532,7 @@ class Insert(ValuesBase): self._process_colparams(dict((n, Null()) for n in names)) self.select_names = names + self.inline = True self.select = _interpret_as_select(select) def _copy_internals(self, clone=_clone, **kw): diff --git a/test/sql/test_insert.py b/test/sql/test_insert.py index d59d79d89..d2fba5862 100644 --- a/test/sql/test_insert.py +++ b/test/sql/test_insert.py @@ -17,7 +17,7 @@ class _InsertTestBase(object): Column('name', String(30)), Column('description', String(30))) Table('myothertable', metadata, - Column('otherid', Integer), + Column('otherid', Integer, primary_key=True), Column('othername', String(30))) @@ -138,6 +138,23 @@ class InsertTest(_InsertTestBase, fixtures.TablesTest, AssertsCompiledSQL): dialect=default.DefaultDialect() ) + def test_insert_from_select_returning(self): + table1 = self.tables.mytable + sel = select([table1.c.myid, table1.c.name]).where( + table1.c.name == 'foo') + ins = self.tables.myothertable.insert().\ + from_select(("otherid", "othername"), sel).returning( + self.tables.myothertable.c.otherid + ) + self.assert_compile( + ins, + "INSERT INTO myothertable (otherid, othername) " + "SELECT mytable.myid, mytable.name FROM mytable " + "WHERE mytable.name = %(name_1)s RETURNING myothertable.otherid", + checkparams={"name_1": "foo"}, + dialect="postgresql" + ) + def test_insert_from_select_select(self): table1 = self.tables.mytable sel = select([table1.c.myid, table1.c.name]).where( @@ -230,7 +247,7 @@ class InsertTest(_InsertTestBase, fixtures.TablesTest, AssertsCompiledSQL): ) ins = mytable.insert().\ from_select( - [mytable.c.name, mytable.c.description], sel) + [mytable.c.name, mytable.c.description], sel) self.assert_compile( ins, "INSERT INTO mytable (name, description) " @@ -254,6 +271,94 @@ class InsertTest(_InsertTestBase, fixtures.TablesTest, AssertsCompiledSQL): ) +class InsertImplicitReturningTest( + _InsertTestBase, fixtures.TablesTest, AssertsCompiledSQL): + __dialect__ = postgresql.dialect(implicit_returning=True) + + def test_insert_select(self): + table1 = self.tables.mytable + sel = select([table1.c.myid, table1.c.name]).where( + table1.c.name == 'foo') + ins = self.tables.myothertable.insert().\ + from_select(("otherid", "othername"), sel) + self.assert_compile( + ins, + "INSERT INTO myothertable (otherid, othername) " + "SELECT mytable.myid, mytable.name FROM mytable " + "WHERE mytable.name = %(name_1)s", + checkparams={"name_1": "foo"} + ) + + def test_insert_select_return_defaults(self): + table1 = self.tables.mytable + sel = select([table1.c.myid, table1.c.name]).where( + table1.c.name == 'foo') + ins = self.tables.myothertable.insert().\ + from_select(("otherid", "othername"), sel).\ + return_defaults(self.tables.myothertable.c.otherid) + self.assert_compile( + ins, + "INSERT INTO myothertable (otherid, othername) " + "SELECT mytable.myid, mytable.name FROM mytable " + "WHERE mytable.name = %(name_1)s", + checkparams={"name_1": "foo"} + ) + + def test_insert_multiple_values(self): + ins = self.tables.myothertable.insert().values([ + {"othername": "foo"}, + {"othername": "bar"}, + ]) + self.assert_compile( + ins, + "INSERT INTO myothertable (othername) " + "VALUES (%(othername_0)s), " + "(%(othername_1)s)", + checkparams={ + 'othername_1': 'bar', + 'othername_0': 'foo'} + ) + + def test_insert_multiple_values_return_defaults(self): + # TODO: not sure if this should raise an + # error or what + ins = self.tables.myothertable.insert().values([ + {"othername": "foo"}, + {"othername": "bar"}, + ]).return_defaults(self.tables.myothertable.c.otherid) + self.assert_compile( + ins, + "INSERT INTO myothertable (othername) " + "VALUES (%(othername_0)s), " + "(%(othername_1)s)", + checkparams={ + 'othername_1': 'bar', + 'othername_0': 'foo'} + ) + + def test_insert_single_list_values(self): + ins = self.tables.myothertable.insert().values([ + {"othername": "foo"}, + ]) + self.assert_compile( + ins, + "INSERT INTO myothertable (othername) " + "VALUES (%(othername_0)s)", + checkparams={'othername_0': 'foo'} + ) + + def test_insert_single_element_values(self): + ins = self.tables.myothertable.insert().values( + {"othername": "foo"}, + ) + self.assert_compile( + ins, + "INSERT INTO myothertable (othername) " + "VALUES (%(othername)s) RETURNING myothertable.otherid", + checkparams={'othername': 'foo'} + ) + + class EmptyTest(_InsertTestBase, fixtures.TablesTest, AssertsCompiledSQL): __dialect__ = 'default' diff --git a/test/sql/test_query.py b/test/sql/test_query.py index a475b899f..2075bcecf 100644 --- a/test/sql/test_query.py +++ b/test/sql/test_query.py @@ -276,6 +276,13 @@ class QueryTest(fixtures.TestBase): r = t6.insert().values(manual_id=id).execute() eq_(r.inserted_primary_key, [12, 1]) + def test_implicit_id_insert_select(self): + stmt = users.insert().from_select( + (users.c.user_id, users.c.user_name), + users.select().where(users.c.user_id == 20)) + + testing.db.execute(stmt) + def test_row_iteration(self): users.insert().execute( {'user_id': 7, 'user_name': 'jack'}, |