Skip to content
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.

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

SQLAlchemy 1.4 statements with .in_ operator not compiled correctly #577

Open
Askaholic opened this issue Apr 17, 2021 · 5 comments · May be fixed by #940
Open

SQLAlchemy 1.4 statements with .in_ operator not compiled correctly #577

Askaholic opened this issue Apr 17, 2021 · 5 comments · May be fixed by #940
Milestone

Comments

@Askaholic
Copy link
Contributor

In SQLAlchemy version 1.4 they way statements using the .in_ operator are compiled was changed to enable better caching. However this means statements need to be compiled with additional compile args in order to render correctly. You can read about the change here:
https://docs.sqlalchemy.org/en/14/changelog/migration_14.html#change-4645

The issue this manifests itself as:

stmt = select([my_table]).where(my_table.c.column.in_([1, 2]))

async with database.acquire() as conn:
    result = await conn.execute(stmt)
    
pymysql.err.ProgrammingError: (1064, "You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '[POSTCOMPILE_column_1])
@flask-rabmq
Copy link

flask-rabmq commented Apr 30, 2021

I have the same problem again.
I use monkey patch to fix this problem.

async def execute(self, query, *multiparams, **params):
    cursor = await self._connection.cursor()
    dp = _distill_params(multiparams, params)
    if len(dp) > 1:
        return await self._executemany(query, dp, cursor)
    elif dp:
        dp = dp[0]

    result_map = None
    if isinstance(query, str):
        await cursor.execute(query, dp or None)
    elif isinstance(query, ClauseElement):
        if self._compiled_cache is not None:
            key = query
            compiled = self._compiled_cache.get(key)
            if not compiled:
                compiled = query.compile(dialect=self._dialect, compile_kwargs={"render_postcompile": True})
                if dp and dp.keys() == compiled.params.keys() \
                        or not (dp or compiled.params):
                    # we only want queries with bound params in cache
                    self._compiled_cache[key] = compiled
        else:
            compiled = query.compile(dialect=self._dialect, compile_kwargs={"render_postcompile": True})

        if not isinstance(query, DDLElement):
            post_processed_params = self._base_params(
                query,
                dp,
                compiled,
                isinstance(query, UpdateBase)
            )
            result_map = compiled._result_columns
        else:
            if dp:
                raise exc.ArgumentError("Don't mix sqlalchemy DDL clause "
                                        "and execution with parameters")
            post_processed_params = compiled.construct_params()
            result_map = None
        await cursor.execute(str(compiled), post_processed_params)
    else:
        raise exc.ArgumentError("sql statement should be str or "
                                "SQLAlchemy data "
                                "selection/modification clause")

    ret = await create_result_proxy(
        self, cursor, self._dialect, result_map
    )
    self._weak_results.add(ret)
    return ret

 SAConnection._execute=execute

Temporary solution, hope the author fix this problem.

@paulefoe
Copy link
Member

paulefoe commented May 1, 2021

The simple solution would be as @flask-rabmq mentioned to add render_postcompile flag here and here

I'm not sure how this is going to work with the cache. Maybe literal_binds should be used instead of a render_postcompile?

We should probably allow users to submit the desired value and have some sensible defaults.

@flask-rabmq
Copy link

@yurenchen000
Copy link

meet the same problem.

my temp workaroud in user code:

// try1: literal_binds

    query = db_devices.select().where(
        db_devices.c.device_id.in_(device_id_list)
    )
    query = query.compile(compile_kwargs={"literal_binds": True}) # compile to final sql with args filled
    query = str(query)

    print('query:', type(query), query)
    async with g.app.ctx.db.acquire() as conn:
        return await (await conn.execute(query)).fetchall()

// try2: render_postcompile + literal_execute // same effect

    query = db_devices.select().where(
        # db_devices.c.device_id.in_(device_id_list)
        db_devices.c.device_id.in_(
            sa.bindparam('list', device_id_list,  literal_execute=True) # effect on compile() with render_postcompile
        )
    )
    query = query.compile(compile_kwargs={"render_postcompile": True}) # compile to final sql with args filled
    query = str(query)

    print('query:', type(query), query)
    async with g.app.ctx.db.acquire() as conn:
        return await (await conn.execute(query)).fetchall()

@flask-rabmq 's fix in diff format

diff with v0.0.22/aiomysql/sa/connection.py

--- a/aiomysql/sa/connection.py
+++ b/aiomysql/sa/connection.py
@@ -148,13 +148,13 @@ class SAConnection:
                 key = query
                 compiled = self._compiled_cache.get(key)
                 if not compiled:
-                    compiled = query.compile(dialect=self._dialect)
+                    compiled = query.compile(dialect=self._dialect, compile_kwargs={"render_postcompile": True})
                     if dp and dp.keys() == compiled.params.keys() \
                             or not (dp or compiled.params):
                         # we only want queries with bound params in cache
                         self._compiled_cache[key] = compiled
             else:
-                compiled = query.compile(dialect=self._dialect)
+                compiled = query.compile(dialect=self._dialect, compile_kwargs={"render_postcompile": True})
 
             if not isinstance(query, DDLElement):
                 post_processed_params = self._base_params(

worked well

@cono
Copy link

cono commented Jun 29, 2022

Same issue here.
Seems like in aiopg its already fixed:
https://github.com/aio-libs/aiopg/blob/master/aiopg/sa/connection.py#L123

Nothing4You added a commit to Nothing4You/aiomysql that referenced this issue Sep 3, 2022
@Nothing4You Nothing4You added this to the 0.3 milestone Jun 11, 2023
@Nothing4You Nothing4You linked a pull request Jun 11, 2023 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants