Skip to content

sqlalchemy.ext.serializer fail with column_property in query condition #5086

Closed
@MikhailZhukov

Description

@MikhailZhukov

Deserialization of query with column_property fail with error

Traceback (most recent call last):
  File "/home/mzhukov/projects/test_sqa/test_sql/main.py", line 53, in <module>
    assert str(orig_req) == str(deserialized_obj)
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/orm/query.py", line 3379, in __str__
    return str(context.statement.compile(bind))
  File "<string>", line 1, in <lambda>
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/elements.py", line 462, in compile
    return self._compiler(dialect, bind=bind, **kw)
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/elements.py", line 468, in _compiler
    return dialect.statement_compiler(dialect, self, **kw)
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/compiler.py", line 571, in __init__
    Compiled.__init__(self, dialect, statement, **kwargs)
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/compiler.py", line 319, in __init__
    self.string = self.process(self.statement, **compile_kwargs)
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/compiler.py", line 350, in process
    return obj._compiler_dispatch(self, **kwargs)
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/visitors.py", line 92, in _compiler_dispatch
    return meth(self, **kw)
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/compiler.py", line 2140, in visit_select
    text, select, inner_columns, froms, byfrom, kwargs
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/compiler.py", line 2239, in _compose_select_body
    t = select._whereclause._compiler_dispatch(self, **kwargs)
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/visitors.py", line 92, in _compiler_dispatch
    return meth(self, **kw)
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/compiler.py", line 1299, in visit_binary
    return self._generate_generic_binary(binary, opstring, **kw)
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/compiler.py", line 1346, in _generate_generic_binary
    + binary.right._compiler_dispatch(
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/annotation.py", line 79, in _compiler_dispatch
    return self.__element.__class__._compiler_dispatch(self, visitor, **kw)
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/visitors.py", line 92, in _compiler_dispatch
    return meth(self, **kw)
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/compiler.py", line 836, in visit_label
    self, within_columns_clause=False, **kw
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/visitors.py", line 92, in _compiler_dispatch
    return meth(self, **kw)
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/dialects/sqlite/base.py", line 990, in visit_cast
    return super(SQLiteCompiler, self).visit_cast(cast, **kwargs)
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/compiler.py", line 1015, in visit_cast
    cast.clause._compiler_dispatch(self, **kwargs),
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/visitors.py", line 92, in _compiler_dispatch
    return meth(self, **kw)
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/compiler.py", line 1119, in visit_function
    ) % {"expr": self.function_argspec(func, **kwargs)}
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/compiler.py", line 1131, in function_argspec
    return func.clause_expr._compiler_dispatch(self, **kwargs)
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/visitors.py", line 92, in _compiler_dispatch
    return meth(self, **kw)
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/compiler.py", line 727, in visit_grouping
    return "(" + grouping.element._compiler_dispatch(self, **kwargs) + ")"
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/visitors.py", line 92, in _compiler_dispatch
    return meth(self, **kw)
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/compiler.py", line 983, in visit_clauselist
    c._compiler_dispatch(self, **kw) for c in clauselist.clauses
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/compiler.py", line 981, in <genexpr>
    s
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/compiler.py", line 983, in <genexpr>
    c._compiler_dispatch(self, **kw) for c in clauselist.clauses
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/visitors.py", line 92, in _compiler_dispatch
    return meth(self, **kw)
  File "/home/mzhukov/.local/share/virtualenvs/test_sqa-05qMtTeV/lib/python3.7/site-packages/sqlalchemy/sql/compiler.py", line 1503, in visit_bindparam
    % bindparam.key
sqlalchemy.exc.CompileError: Bind parameter '%(140081294316096 left)s' conflicts with unique bind parameter of the same name

I test this error with Python3.7 + SQLAlchemy 1.3.12
In Python2.7 + SQLAlchemy 1.1.18 deserialization does not work too, but with other exception

For reproducing this bug I make simple test file:

# -*- coding: utf-8 -*-
import pickle
import sys
import traceback

from sqlalchemy import create_engine, Integer, Column, String, exc as sa_exc
from sqlalchemy.ext import serializer
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.ext.hybrid import hybrid_property
from sqlalchemy.orm import sessionmaker, scoped_session, column_property
from sqlalchemy.sql import func

Base = declarative_base()
db_engine = create_engine('sqlite:///:memory:')
Session = scoped_session(sessionmaker(bind=db_engine))


class TestTable(Base):
    __tablename__ = 'test'

    id = Column(Integer, primary_key=True, autoincrement=True)
    _some_id = Column('some_id', String)
    some_primary_id = column_property(func.left(_some_id, 6).cast(Integer))


class SecondTestTable(Base):
    __tablename__ = 'test2'

    id = Column(Integer, primary_key=True, autoincrement=True)
    _some_id = Column('some_id', String)

    @hybrid_property
    def some_primary_id(self):
        return int(self._some_id[:6])

    @some_primary_id.expression
    def some_primary_id(cls):
        return func.left(cls._some_id, 6).cast(Integer)


#  Create tables and two records
Base.metadata.create_all(bind=db_engine)
db_session = Session()
db_session.add(TestTable(_some_id="1234567890"))
db_session.add(SecondTestTable(_some_id="1234567890"))
db_session.commit()

#  Query without column_property in condition work
orig_req = db_session.query(TestTable)
serialised_obj = serializer.dumps(orig_req, pickle.HIGHEST_PROTOCOL)
deserialized_obj = serializer.loads(serialised_obj, metadata=Base.metadata, scoped_session=Session)
assert str(orig_req) == str(deserialized_obj)

#  Query with column_property in condition fail
orig_req = db_session.query(TestTable).filter(TestTable.some_primary_id == 123456)
serialised_obj = serializer.dumps(orig_req, pickle.HIGHEST_PROTOCOL)
deserialized_obj = serializer.loads(serialised_obj, metadata=Base.metadata, scoped_session=Session)
try:
    assert str(orig_req) == str(deserialized_obj)
except sa_exc.SQLAlchemyError as exc:
    print('Error: %r' % exc)
    traceback.print_exc(file=sys.stdout)

#  Query with hybrid_property in condition work
orig_req = db_session.query(SecondTestTable).filter(SecondTestTable.some_primary_id == 123456)
serialised_obj = serializer.dumps(orig_req, pickle.HIGHEST_PROTOCOL)
deserialized_obj = serializer.loads(serialised_obj, metadata=Base.metadata, scoped_session=Session)
assert str(orig_req) == str(deserialized_obj)

Activity

added
bugSomething isn't working
sqlalchemy.extextension modules, most of which are ORM related
on Jan 10, 2020
zzzeek

zzzeek commented on Jan 10, 2020

@zzzeek
Member

hi there -

thanks for the clear test case.

unfortunately this issue is an illustration of a problem in the "serializer" extension that isn't really solvable in a generalized way. The issue here is that the query against TestTable, when deserialized, uses the TestTable class that's in the script as well as the TestTable.some_primary_id object when it renders columns clause of the query. However, the WHERE clause is using the deserialized version of TestTable.some_primary_id. The bound parameter, when presented with itself as well as it's cloned self, causes a conflict because they have the same name.

The bound parameter object could try to work around this by changing its "unique" name when deserialized, i'll probably just commit that, which does fix this specific issue and is:


diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py
index 422eb6220..54981375a 100644
--- a/lib/sqlalchemy/sql/elements.py
+++ b/lib/sqlalchemy/sql/elements.py
@@ -1386,6 +1386,13 @@ class BindParameter(roles.InElementRole, ColumnElement):
         d["value"] = v
         return d
 
+    def __setstate__(self, state):
+        if state.get('unique', False):
+            state['key'] = _anonymous_label(
+                "%%(%d %s)s" % (id(self), state.get('_orig_key', 'param'))
+            )
+        self.__dict__.update(state)
+
     def __repr__(self):
         return "BindParameter(%r, %r, type_=%r)" % (
             self.key,

I can't be sure what other unwanted effects this may have in other areas that deal with serialization but I will have to hope that there is very little serialize/deserialize of queries going on in real world applications in general.

In fact I hope to deprecate and remove the "serializer" extension in any case as there are a lot more things it doesn't really do correctly. Can I ask what your use case for the "serializer" is? AFAIK there aren't really any good uses for it and it should not have been added.

added this to the 1.3.x milestone on Jan 10, 2020
sqla-tester

sqla-tester commented on Jan 10, 2020

@sqla-tester
Collaborator

Mike Bayer has proposed a fix for this issue in the master branch:

Alter unique bound parameter key on deserialize https://gerrit.sqlalchemy.org/1660

sqla-tester

sqla-tester commented on Jan 10, 2020

@sqla-tester
Collaborator

Mike Bayer has proposed a fix for this issue in the rel_1_3 branch:

Alter unique bound parameter key on deserialize https://gerrit.sqlalchemy.org/1661

added a commit that references this issue on Jan 10, 2020
1561787
tribals

tribals commented on Jan 13, 2020

@tribals

I used serializer for implementing pagination for REST API. First request arrives, I build query, then serialize and cache it. When client want next chunk, I deserialize query, adjust limit/offset and execute it.

The reason why I chosen this way is that queries can be very complicated, and context (attributes and values, joins, etc) will be lost when requesting next chunk.

MikhailZhukov

MikhailZhukov commented on Jan 13, 2020

@MikhailZhukov
Author

Hi.

Thanks a lot for helping.

In fact I hope to deprecate and remove the "serializer" extension in any case as there are a lot more things it doesn't really do correctly. Can I ask what your use case for the "serializer" is? AFAIK there aren't really any good uses for it and it should not have been added.

Thanks for information about deprecating serializer. We will think about replacement it in our pagination implementation.

zzzeek

zzzeek commented on Jan 13, 2020

@zzzeek
Member

is it possible that instaed of caching the completed query, you cache the information from the REST request that is used to build up that query? this would be much more portable.

MikhailZhukov

MikhailZhukov commented on Jan 13, 2020

@MikhailZhukov
Author

yes, i think we will do that in nearest future. I replaced all column_property to hybrid_property now and it look like it work good enough while we will refactor code base

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingsqlalchemy.extextension modules, most of which are ORM related

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @zzzeek@tribals@sqla-tester@MikhailZhukov

        Issue actions

          sqlalchemy.ext.serializer fail with column_property in query condition · Issue #5086 · sqlalchemy/sqlalchemy