Skip to content

new stack trace when combining non-eq compare_values w/ SQL expression in ORM persist #3402

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 Apr 29, 2015 · 4 comments
Closed

new stack trace when combining non-eq compare_values w/ SQL expression in ORM persist #3402

sqlalchemy-bot opened this issue Apr 29, 2015 · 4 comments
Labels
bug Something isn't working orm
Milestone
1.0.3

Comments

@sqlalchemy-bot
Copy link
Collaborator

Migrated issue, originally created by Michael Bayer (@zzzeek)

this is a 1.0 regression, though I'm pretty curious to see what 0.9 is doing that it isn't hitting this exception:

from sqlalchemy import *
from sqlalchemy.orm import *
from sqlalchemy.ext.declarative import declarative_base

Base = declarative_base()

def comparator(a, b):
    return a > b

class A(Base):
    __tablename__ = 'a'
    id = Column(Integer, primary_key=True)
    data = Column(PickleType(comparator=comparator))


e = create_engine("sqlite://", echo=True)
Base.metadata.create_all(e)

s = Session(e)
s.add(A(data='some data'))
s.commit()

a1 = s.query(A).first()
a1.data = func.foo("im a SQL expression")
s.commit()




#!

Traceback (most recent call last):
  File "test.py", line 27, in <module>
    s.commit()
  File "/Users/classic/dev/sqlalchemy/lib/sqlalchemy/orm/session.py", line 790, in commit
    self.transaction.commit()
  File "/Users/classic/dev/sqlalchemy/lib/sqlalchemy/orm/session.py", line 392, in commit
    self._prepare_impl()
  File "/Users/classic/dev/sqlalchemy/lib/sqlalchemy/orm/session.py", line 372, in _prepare_impl
    self.session.flush()
  File "/Users/classic/dev/sqlalchemy/lib/sqlalchemy/orm/session.py", line 2004, in flush
    self._flush(objects)
  File "/Users/classic/dev/sqlalchemy/lib/sqlalchemy/orm/session.py", line 2122, in _flush
    transaction.rollback(_capture_exception=True)
  File "/Users/classic/dev/sqlalchemy/lib/sqlalchemy/util/langhelpers.py", line 60, in __exit__
    compat.reraise(exc_type, exc_value, exc_tb)
  File "/Users/classic/dev/sqlalchemy/lib/sqlalchemy/orm/session.py", line 2086, in _flush
    flush_context.execute()
  File "/Users/classic/dev/sqlalchemy/lib/sqlalchemy/orm/unitofwork.py", line 373, in execute
    rec.execute(self)
  File "/Users/classic/dev/sqlalchemy/lib/sqlalchemy/orm/unitofwork.py", line 532, in execute
    uow
  File "/Users/classic/dev/sqlalchemy/lib/sqlalchemy/orm/persistence.py", line 170, in save_obj
    mapper, table, update)
  File "/Users/classic/dev/sqlalchemy/lib/sqlalchemy/orm/persistence.py", line 613, in _emit_update_statements
    lambda rec: (
  File "/Users/classic/dev/sqlalchemy/lib/sqlalchemy/orm/persistence.py", line 456, in _collect_update_commands
    value, state.committed_state[propkey]):
  File "/Users/classic/dev/sqlalchemy/lib/sqlalchemy/sql/elements.py", line 2726, in __bool__
    raise TypeError("Boolean value of this clause is not defined")
TypeError: Boolean value of this clause is not defined
@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

well if we have a SQL expression we know it has to be run, so in that case we skip the comparison. Confirm this is what 0.9 is doing and add a test to test_unitofworkv2:

diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py
index 34c37da..c3974ed 100644
--- a/lib/sqlalchemy/orm/persistence.py
+++ b/lib/sqlalchemy/orm/persistence.py
@@ -452,12 +452,11 @@ def _collect_update_commands(
                 value = state_dict[propkey]
                 col = propkey_to_col[propkey]
 
-                if not state.manager[propkey].impl.is_equal(
+                if isinstance(value, sql.ClauseElement):
+                    value_params[col] = value
+                elif not state.manager[propkey].impl.is_equal(
                         value, state.committed_state[propkey]):
-                    if isinstance(value, sql.ClauseElement):
-                        value_params[col] = value
-                    else:
-                        params[col.key] = value
+                    params[col.key] = value
 
         if update_version_id is not None and \
                 mapper.version_id_col in mapper._cols_by_table[table]:

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

Here's the test case using something much more normal, occurs with boolean expressions, as the operator comes out as "is_()":

from sqlalchemy import *
from sqlalchemy.orm import *
from sqlalchemy.ext.declarative import declarative_base

Base = declarative_base()

class A(Base):
    __tablename__ = 'a'
    id = Column(Integer, primary_key=True)
    data = Column(Boolean)

e = create_engine("sqlite://", echo=True)
Base.metadata.create_all(e)

s = Session(e)
s.add(A(data=None))
s.commit()

a1 = s.query(A).first()
a1.data = false()
s.commit()

we might want to consider "is" and "isnot" OK for the __bool__ here also but I'm happy not getting into that for now.

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

  • Fixed regression within the flush process when an attribute were
    set to a SQL expression for an UPDATE, and the SQL expression when
    compared to the previous value of the attribute would produce a SQL
    comparison other than == or !=, the exception "Boolean value
    of this clause is not defined" would raise. The fix ensures that
    the unit of work will not interpret the SQL expression in this way.
    fixes new stack trace when combining non-eq compare_values w/ SQL expression in ORM persist #3402

→ b0be921

@sqlalchemy-bot
Copy link
Collaborator Author

Changes by Michael Bayer (@zzzeek):

  • changed status to closed

@sqlalchemy-bot sqlalchemy-bot added bug Something isn't working orm labels Nov 27, 2018
@sqlalchemy-bot sqlalchemy-bot added this to the 1.0.3 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 orm
Projects
None yet
Milestone
1.0.3
Development

No branches or pull requests
1 participant
@sqlalchemy-bot