__eq__ operator appears to not be working correctly for decorated types

Issue #2734 resolved
Anonymous created an issue

So from the documentation it makes it appear that by defining process_bind_param and process_result_value I should have all the workings to transparently perform a translation. However the == operator doesn't perform the translation before sending the query to the db. Here's my example:

{{{#!python
    class NullableString(TypeDecorator):
        **Turns None into the string "Null" and back in order to prevent Null!=Null issues**
        impl = String
        def process_bind_param(self, value, dialect):
            return "NONE" if value is None else value
        def process_result_value(self, value, dialect):
            return None if value == "NONE" else value

}}} If I have a class SomeType and I perform the following:

{{{#!python
    session.query(SomeType).filter(SomeType.nullableStringCol == None).all()

}}} I get no results but if I do:

{{{#!python
    session.query(SomeType).filter(SomeType.nullableStringCol == "NONE").all()

}}} I get my results.

Now I can get around this by declaring the instance of the NullableString to use a custom comparator in SomeType but that seems like a broken paradigm because that comparison behavior should be captive within my new type.

Am I misunderstanding the mechanism for the comparison? I apologize if this isn't a bug but it seems like it's broken (the example of typedecorators in the docs would presumably suffer the same problem when comparing unicode strings)

Comments (8)

  1. Michael Bayer repo owner

    unfortunately this is a special use case that requires additional logic, because SQLAlchemy coerces any "obj == None" into "obj IS NULL" - if you turn on SQL echoing you will see this, and it occurs before the type is consulted. Because it's a lexical change to the statement, there's not really any way to move this consultation to a later point.

    So the __eq__() (and probably __ne__() also) here must be overridden specifically:

    from sqlalchemy import *
    from sqlalchemy.orm import *
    from sqlalchemy.ext.declarative import declarative_base
    
    class NullableString(TypeDecorator):
    
        impl = String
    
        class comparator_factory(String.comparator_factory):
            def __eq__(self, other):
                if other is None:
                    return self.expr == "NONE"
                else:
                    return String.comparator_factory.__eq__(self, other)
    
        def process_bind_param(self, value, dialect):
            return "NONE" if value is None else value
    
        def process_result_value(self, value, dialect):
            return None if value == "NONE" else value
    
    Base = declarative_base()
    
    
    class A(Base):
        __tablename__ = 'a'
    
        id = Column(Integer, primary_key=True)
        value = Column(NullableString, nullable=False)
    
    e = create_engine("sqlite://", echo=True)
    Base.metadata.create_all(e)
    
    sess = Session(e)
    
    sess.add_all([A(value='x'), A(value=None)])
    sess.commit()
    
    
    assert sess.query(A).filter(A.value == 'x').one().id == 1
    assert sess.query(A).filter(A.value == None).one().id == 2
    
  2. Michael Bayer repo owner

    the patch in #2744 allows this workaround to be performed more simply:

    class NullableString(TypeDecorator):
        **Turns None into the string "Null" and back in order to prevent Null!=Null issues**
    
        impl = String
    
        coerce_to_is_types = ()
    
        def process_bind_param(self, value, dialect):
            return "NONE" if value is None else value
    
        def process_result_value(self, value, dialect):
            return None if value == "NONE" else value
    
  3. Diggsey

    This workaround doesn't work - although the expression is no longer converted to an IS clause, the process_bind_param method is not called, so the SQL ends up looking like 'column_name == NULL' instead of 'column_name == "NONE"'. The original fix (with a custom comparator factory) does work.

  4. Michael Bayer repo owner

    process_bind_param is always called, you'd need to provide specifics.

    here's the test above (with the formatting fixed, it got broke in an issue migration) using the "coerce_to_is_types" version:

    from sqlalchemy import *
    from sqlalchemy.orm import *
    from sqlalchemy.ext.declarative import declarative_base
    
    class NullableString(TypeDecorator):
    
        impl = String
    
        coerce_to_is_types = ()
    
        def process_bind_param(self, value, dialect):
            return "NONE" if value is None else value
    
        def process_result_value(self, value, dialect):
            return None if value == "NONE" else value
    
    Base = declarative_base()
    
    
    class A(Base):
        __tablename__ = 'a'
    
        id = Column(Integer, primary_key=True)
        value = Column(NullableString, nullable=False)
    
    e = create_engine("sqlite://", echo=True)
    Base.metadata.create_all(e)
    
    sess = Session(e)
    
    sess.add_all([A(value='x'), A(value=None)])
    sess.commit()
    
    
    assert sess.query(A).filter(A.value == 'x').one().id == 1
    assert sess.query(A).filter(A.value == None).one().id == 2
    

    output at the end shows 'NONE':

    SELECT a.id AS a_id, a.value AS a_value 
    FROM a 
    WHERE a.value = ?
    2017-05-04 11:31:35,210 INFO sqlalchemy.engine.base.Engine ('x',)
    2017-05-04 11:31:35,211 INFO sqlalchemy.engine.base.Engine SELECT a.id AS a_id, a.value AS a_value 
    FROM a 
    WHERE a.value = ?
    2017-05-04 11:31:35,211 INFO sqlalchemy.engine.base.Engine ('NONE',)
    
  5. Diggsey

    My custom types look like this:

    class PortableJSONB(types.TypeDecorator):
        impl = db.UnicodeText
    
        def __init__(self):
            super(PortableJSONB, self).__init__(length=0xFFFFFFFF)
    
        def load_dialect_impl(self, dialect):
            if dialect.name == 'postgresql':
                return dialect.type_descriptor(PostgresJSONB())
            else:
                return dialect.type_descriptor(self.impl)
    
        def process_bind_param(self, value, dialect):
            return value
    
        def process_result_value(self, value, dialect):
            return value
    
        def compare_against_backend(self, dialect, other):
            if dialect.name == 'mysql':
                return isinstance(other, mysql.LONGTEXT)
            else:
                raise NotImplementedError()
    
        def coerce_compared_value(self, op, value):
            return self.impl.coerce_compared_value(op, value)
    
    
    class CustomJSON(PortableJSONB):
        coerce_to_is_types = ()
    
        def __init__(self, encoder=None, decoder=None):
            PortableJSONB.__init__(self)
            self.encoder = encoder
            self.decoder = decoder
    
        def process_bind_param(self, value, dialect):
            value = json.dumps(value, cls=self.encoder, sort_keys=True, ensure_ascii=False)
    
            value = PortableJSONB.process_bind_param(self, value, dialect)
            return value
    
        def process_result_value(self, value, dialect):
            value = PortableJSONB.process_result_value(self, value, dialect)
            if value is not None:
                value = json.loads(value, cls=self.decoder)
            return value
    
    
    class SchemaJSON(CustomJSON):
        def __init__(self, schema=None):
            self.schema = schema
            CustomJSON.__init__(self, json.JSONEncoder, json.JSONDecoder)
    
        def process_bind_param(self, value, dialect):
            if value is not None:
                value = serialize_to_simple_types(value, self.schema)
            value = CustomJSON.process_bind_param(self, value, dialect)
            return value
    
        def process_result_value(self, value, dialect):
            value = CustomJSON.process_result_value(self, value, dialect)
            if value is not None:
                value = deserialize_from_simple_types(value, self.schema)
            return value
    

    With a column of type "SchemaJSON", none of the process_bind_param methods are called in response to a query with filter(column != None), and with echo enabled shows it is substituting None rather than 'null' into the query.

  6. Michael Bayer repo owner

    this part:

       def coerce_compared_value(self, op, value):
            return self.impl.coerce_compared_value(op, value)
    

    is wrong. take that out, because it is preventing the == operator from treating the right-hand side as the SchemaJson type (it's forcing it to be treated as plain UnicodeText).

  7. Log in to comment