issue with custom data types between schemas on same connection

[defg]

• asyncpg version: 0.8.4
• PostgreSQL version: PostgreSQL 9.6.0
• Python version: Python 3.5.2
• Platform: Ubuntu 14.04.2 LTS
• Do you use pgbouncer?: No
• Do you use the wheel package or built locally
  (if locally, which version of Cython was used)?: pypi pip pkg
• Can you the issue be reproduced when running under
  uvloop?: Yes

I'm receiving the exception
"asyncpg.exceptions.FeatureNotSupportedError: cached plan must not change result type"
when I try to use a custom enum type across 2 schemas in the same connection. This happens even when I'm using independent transactions. This behavior doesn't happen when I issue the SQL in postgres directly.

I've written a sample python script along with some SQL to reproduce the issue.

https://gist.github.com/defg/8fd9e97efd1ba5195052a0faa1457dcc

Maybe something is cached w/o regard for the schema?

I'm working around this issue for the time being by making a connection per schema, but it would be much easier in the future if I could access different schemas on the same connection w/o error.

[elprans]

Yes, asyncpg caches query plans by default. In your case you are changing the default schema, but since the query text remains the same, asyncpg erroneously tries to use the original plan. This is a genuine asyncpg bug.

My best guess for the fix would be to have connection object drop the plan cache on every SET command.

[vitaly-burovoy]

on every SET command.

Searching SET in queries can be complex enough and therefore down speed (SET can be inside a multicommand string which is supported by execute without args).
The error can occurs not only after SET command. It raises when schema objects are changed (even without transactions).
E.g.:

>>> await conn.execute("CREATE TABLE IF NOT EXISTS abc(i int)")
>>> await conn.fetch("SELECT * FROM abc")
>>> await conn.execute("ALTER TABLE abc ALTER COLUMN i TYPE bigint")
>>> await conn.fetch("SELECT * FROM abc")

asyncpg.exceptions.FeatureNotSupportedError: cached plan must not change result type

Note that DDL can be done in a stored procedure or even via another connection (e.g. by a migration script).

The easiest way is to intercept the exception, recreate prepared statement and run it again. Unfortunately, it does not work in transactions because a single error ruins the whole transaction at once (or up to the last savepoint[1]).

In my project at work I just limited all other code from interacting inside transactions (a wrapper has all queries to be run in a transaction before starts it), and when such exception raises (or connection is lost), the wrapper drops that prepared statement (or waits a new connection is established) and runs those queries in another transaction.

My best guess would be:

• to catch the exception if a query is not in a transaction;
• give people a handle to get the best option for themselves:
• • use caching and let them catch the exception (leave the current behaviour);
• • let asyncpg send SAVEPOINT command before each query until the end of a transaction (and send "ROLLBACK TO SAVEPOINT" on error);
• • turn caching off.

P.S.: of course there is an edge case which can lead to an infinite loop:

>>> await conn.execute(
...     "CREATE TABLE a(i int);"
...     "PREPARE p AS SELECT * FROM a;"
...     "EXECUTE p;"
...     "ALTER TABLE a ALTER COLUMN i TYPE bigint;"
...     "EXECUTE p;"
... )

or another one:

>>> await conn.execute("DROP TABLE IF EXISTS abc")
>>> await conn.execute("CREATE TABLE IF NOT EXISTS abc(i int)")
>>> await conn.fetch("PREPARE p AS SELECT * FROM abc")
>>> await conn.fetch("EXECUTE p;")
>>> await conn.execute("ALTER TABLE abc ALTER COLUMN i TYPE bigint")
>>> await conn.fetch("EXECUTE p;")

Nothing changes if you recreate a prepared statement for the last command (prepared statement for prepared statement) or even do not cache it at all. The reason is in user's statement, not in asyncpg's.
Therefore attempts should be counted and the exception be reraised after specified threshold.

[1]https://www.postgresql.org/docs/current/static/sql-savepoint.html

[elprans]

Actually, the documentation states the following:

Although the main point of a prepared statement is to avoid repeated parse analysis and planning of the statement, PostgreSQL will force re-analysis and re-planning of the statement before using it whenever database objects used in the statement have undergone definitional (DDL) changes since the previous use of the prepared statement. Also, if the value of search_path changes from one use to the next, the statement will be re-parsed using the new search_path. (This latter behavior is new as of PostgreSQL 9.3.)

Which makes this error a bit odd, as prepared statement submission should survive both the schema change and DDL. I'll need to look into this more.

[elprans]

So, on a bit of further investigation it appears that this is a limitation of Postgres' current plan cache implementation [1]. The prepared plan is replanned, but the query must return the tuple of the same type, which is what the error message is saying.

I'm now inclined to leave the current asyncpg behaviour as is, but provide an explicit API to deallocate all prepared statements on a connection.

[vitaly-burovoy]

prepared statement submission should survive both the schema change and DDL

It survives until result type of all "columns" (which type information has been already sent to a client side) are the same.
Here is mine modified example from above:

>>> await conn.execute("CREATE TABLE IF NOT EXISTS abc(i int)")
>>> await conn.fetch("SELECT * FROM abc")
>>> await conn.execute("ALTER TABLE abc ALTER COLUMN i TYPE bigint")
>>> await conn.execute("ALTER TABLE abc ALTER COLUMN i TYPE int")  # change it back
>>> await conn.fetch("SELECT * FROM abc")
>>> print("OK")  # check it is OK

OK

[vitaly-burovoy]

but provide an explicit API to deallocate all prepared statements on a connection.

Whether users will have to explicitly wrap execute/fetch calls and run API method to deallocate prepared statements asyncpg has made implicitly?

[elprans]

Probably a "prepare threshold" setting on a connection, like JDBC does. Setting it to 0 would disable automatic plan caching altogether. Arguably, this issue is an edge case.

[1st1]

I'm now inclined to leave the current asyncpg behaviour as is, but provide an explicit API to deallocate all prepared statements on a connection.

-1. APIs like this are hard to use and usually lead to error prone code.

Two solutions for OP's problem:

1. Use a dedicated connection/pool per schema.
2. Instantiate Connection with statement_cache_size set to 0 to disable the cache.

[vitaly-burovoy]

Setting it to 0 would disable automatic plan caching altogether.

Debatable. According to POLA, 0 for "prepare threshold" just should prevent catching an exception even once and pass it as is (just "don't repeat", not "don't cache").
statement_cache_size mentioned by @1st1 is much better for such thing.

this issue is an edge case.

I agree it is true for changing current schema name between queries.
But I mentioned above - DDL caused by a schema migration tool is not uncommon. Since asyncpg implicitly uses prepare statements, users can found their systems in a terrible state at a bad time.

[1st1]

But I mentioned above - DDL caused by a schema migration tool is not uncommon. Since asyncpg implicitly uses prepare statements, users can found their systems in a terrible state at a bad time.

Alternatively, we can reset the cache and auto-retry if we catch this very exception cached plan must not change result type. But since this is the first and only bug report on the matter, I'd simply keep the status quo for now.

[vitaly-burovoy]

we can reset the cache and auto-retry

I wrote about possible ways and corner- and edge-cases to be covered.

But since this is the first and only bug report on the matter

As the library becomes more popular, more users will impact this issue. The primary reason why the current report appears is not in Postgres, it is because asyncpg uses prepare statements implicitly.

Happily I impacted this issue long time ago (and not in production) and in my current project I implemented workaround in a wrapper (it does many other things besides discussed one). But since I have that experience (and a wrapper for it), I am not a person who would be an author of a similar report here.

But many people do not have such experience and/or use libraries which handles that issue (@elprans mentioned JDBC does something). I think sooner or later it will be fixed in asyncpg.

[elprans]

Debatable. According to POLA, 0 for "prepare threshold" just should prevent catching an exception even once and pass it as is (just "don't repeat", not "don't cache").
statement_cache_size mentioned by @1st1 is much better for such thing.

Yes. I meant the same thing. See jdbc docs on preparedStatementCacheQueries and preparedStatementCacheSizeMiB ("prepared threshold" was something I just remembered JDBC had around controlling the prepared statements).

Setting statement_cache_size to 0 would fix the @defg's issue.

We probably need to put a note in the docs mentioning that for connections that fiddle with DDL and/or search_path, statement_cache_size should be set to zero to avoid the cached plan error. Other than that, I see no need to change the current behaviour of asyncpg.

[1st1]

@vitaly-burovoy

I wrote about possible ways and corner- and edge-cases to be covered.

Right, we understand the actual issue here. Speaking about possible solutions you proposed:

• to catch the exception if a query is not in a transaction;

The behaviour inside and outside transactions should be the same. [1]

• give people a handle to get the best option for themselves:
  • use caching and let them catch the exception (leave the current behaviour);

We think that you're right and the problem needs to be addressed. More on that below.

• let asyncpg send SAVEPOINT command before each query until the end of a transaction (and send "ROLLBACK TO SAVEPOINT" on error);

This would be super inefficient, we cannot do this, so [1] can't be implemented either.

• turn caching off.

This is the solution for the OP problem.

---

The solution we arrived at is:

1. Document the problem. For some cases, explicitly disabling the cache is an acceptable solution.

2. For systems which need to support online migrations, we can do the following:

• Subscribe to a special internal notification channel (see NOTIFY) in all connections; reset the cache when a notification arrives.

• Write an event trigger for ddl_command_end to send that special notification on schema changes. Since creating event triggers requires superuser privileges, this will be a manual step for users who need this feature.