Skip to content

'on conflict ... do update ...' where clause and implicit returning #3813

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
sqlalchemy-bot opened this issue Oct 4, 2016 · 8 comments
Closed

'on conflict ... do update ...' where clause and implicit returning #3813

sqlalchemy-bot opened this issue Oct 4, 2016 · 8 comments
Labels
bug Something isn't working postgresql
Milestone
1.1

Comments

@sqlalchemy-bot
Copy link
Collaborator

Migrated issue, originally created by Pawel (@pszynk)

When using Postgres 9.5 INSERT ... ON CONFLICT ... DO UPDATE ... I get an error:

  File ".../sqlalchemy/orm/scoping.py", line 157, in do
    return getattr(self.registry(), name)(*args, **kwargs)
  File ".../sqlalchemy/orm/session.py", line 1044, in execute
    bind, close_with_result=True).execute(clause, params or {})
  File ".../sqlalchemy/engine/base.py", line 947, in execute
    return meth(self, multiparams, params)
  File ".../sqlalchemy/sql/elements.py", line 262, in _execute_on_connection
    return connection._execute_clauseelement(self, multiparams, params)
  File ".../sqlalchemy/engine/base.py", line 1055, in _execute_clauseelement
    compiled_sql, distilled_params
  File ".../sqlalchemy/engine/base.py", line 1204, in _execute_context
    result = context._setup_crud_result_proxy()
  File ".../sqlalchemy/engine/default.py", line 862, in _setup_crud_result_proxy
    self._setup_ins_pk_from_implicit_returning(row)
  File ".../sqlalchemy/engine/default.py", line 927, in _setup_ins_pk_from_implicit_returning
    for col in table.primary_key
TypeError: 'NoneType' object has no attribute '__getitem__'

I have hard time recreating issue with test (sorry I'm a bit new to the project), but i think I know what conditions cause this error.

  1. INSERT must give a CONFLICT
  2. ON UPDATE must have a WHERE with condition that does not allow for update
  3. There should be only one data set for the INSERT VALUES (so that the implicit returning is True

so sth like this should give an error:

#!python            
conn.execute(users.insert(), dict(id=1, name='name1'))
i = insert(users)
i = i.on_conflict_do_update(
  index_elements=[users.c.id],
  set_=dict(name=i.excluded.name),
  where=(i.excluded.name == 'other_name''))
)

conn.execute(u, [dict(id=1, name='name2')])

This test passes but _implicit_returning here is False. In my code I use session and implicit_returning is True (is it because of session?)

Then in file: sqlalchemy/engine/default.py:862 function: _setup_crud_result_proxy
we go inside both if's and row = result.fetchone() sets row to None because of the where clause I think (nothing will be inserted or updated). Then the call self._setup_ins_pk_from_implicit_returning(row) with row == None give an error.

if self.isinsert:
  if self._is_implicit_returning:  # when implicit_returning is True!
    # row == None
    row = result.fetchone()
    self.returned_defaults = row
    # if row == None we get an error in this method!
    self._setup_ins_pk_from_implicit_returning(row)
    result._soft_close(_autoclose_connection=False)
    result._metadata = None
  elif not self._is_explicit_returning:
    result._soft_close(_autoclose_connection=False)
    result._metadata = None
@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

It is very likely this is a dupe of #3807. Please confirm this issue still exists with latest master, thanks.

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

and if so, please produce the SQL being emitted. Also note that the "implicit_returning" flags that are accessible via the API in create_engine() and Table should likely not be used, as the upsert feature is tested only against the default RETURNING behavior.

@sqlalchemy-bot
Copy link
Collaborator Author

Pawel (@pszynk) wrote:

Yeah I though that #3807 could resolve this (I was the one that asked for help on google groups), but I can confirm that this problem still exists, I tested it on master. As you said, turning the flag implicit_returning to False in create_engine() helped.

I think I found out how to recreate this error. The key is not to put primary key into VALUES so the INSERT (with implicit_returning == True) will try to return primary key that 'would' inserted/updated.

I wrote this test to replicate it: (in sqlalchemy/test/dialect/postgresql/test_on_conflict.py)

    def test_on_conflict_do_update_exotic_none(self):
        users = self.tables.users_xtra

        with testing.db.connect() as conn:
            self._exotic_targets_fixture(conn)
            i = insert(users)
            i = i.on_conflict_do_update(
                index_elements=[users.c.login_email],
                set_=dict(name='new_name'),
                where=(i.excluded.name == 'other_name')
            )
            conn.execute(i, dict(name='name2', login_email='name1@gmail.com'))

            eq_(
                conn.execute(users.select().where(users.c.id == 1)).fetchall(),
                [(1, 'name2', 'name1@gmail.com', 'not')]
            )

and here is the SQL:

INSERT INTO users_xtra (name, login_email) VALUES (%(name)s, %(login_email)s) 
  ON CONFLICT (login_email) 
  DO UPDATE 
    SET name = %(param_1)s 
    WHERE excluded.name = %(name_1)s 
  RETURNING users_xtra.id

{'name_1': 'other_name', 'param_1': 'new_name', 'login_email': 'name1@gmail.com', 'name': 'name2'}

@sqlalchemy-bot
Copy link
Collaborator Author

Pawel (@pszynk) wrote:

ofc this assertion is wrong, just wanted to check for the exception.

Proper test would be

    def test_on_conflict_do_update_exotic_none(self):
        users = self.tables.users_xtra

        with testing.db.connect() as conn:
            self._exotic_targets_fixture(conn)
            i = insert(users)
            i = i.on_conflict_do_update(
                index_elements=[users.c.login_email],
                set_=dict(name='new_name'),
                where=(i.excluded.name == 'other_name')
            )
            conn.execute(i, dict(name='name2', login_email='name1@gmail.com'))

            eq_(
                conn.execute(users.select()).fetchall(),
                [(1, 'name1', 'name1@gmail.com', 'not'), (2, 'name2', 'name2@gmail.com', 'not')]
            )

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

perfect, thanks very much!

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

so here is your workaround:

i = insert(users, inline=True)

which is the same thing that insert from select does.

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

Check row for None with implicit returning PK to accommodate ON CONFLICT

An adjustment to ON CONFLICT such that the "inserted_primary_key"
logic is able to accommodate the case where there's no INSERT or
UPDATE and there's no net change. The value comes out as None
in this case, rather than failing on an exception.

Change-Id: I0794e95c3ca262cb1ab2387167d96b8984225fce
Fixes: #3813

→ 20384e8

@sqlalchemy-bot
Copy link
Collaborator Author

Changes by Michael Bayer (@zzzeek):

  • changed status to closed

@sqlalchemy-bot sqlalchemy-bot added postgresql bug Something isn't working labels Nov 27, 2018
@sqlalchemy-bot sqlalchemy-bot added this to the 1.1 milestone Nov 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working postgresql
Projects
None yet
Milestone
1.1
Development

No branches or pull requests
1 participant
@sqlalchemy-bot