Invalidate statement cache on schema changes affecting statement result. #103
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1st1
reviewed
Mar 30, 2017
asyncpg/connection.py
Outdated
| @@ -226,6 +226,9 @@ def transaction(self, *, isolation='read_committed', readonly=False, | |||
| if not state.closed: | |||
| return state | |||
|
|
|||
| return await self._prepare_statement(query, timeout, cache) | |||
There was a problem hiding this comment.
Don't split the _get_statement method please.
1st1
reviewed
Mar 30, 2017
asyncpg/exceptions/_base.py
Outdated
|
|
||
|
|
||
| class InvalidCachedStatementError(AsyncpgError): | ||
| """Cached statement is not longer valid.""" |
elprans
force-pushed
the
invalidate-cache-on-schema-change
branch
2 times, most recently
from
181f8a1 to
51a01e9
Compare
1st1
reviewed
Mar 30, 2017
asyncpg/prepared_stmt.py
Outdated
| if e.server_source_function == 'RevalidateCachedQuery': | ||
| # Handle query plan rejection due to schema or config changes. | ||
| # See comment in Connection._do_execute. | ||
| raise exceptions.InvalidCachedStatementError( |
There was a problem hiding this comment.
Can we move this logic to PostgresMessage.new()?
1st1
reviewed
Mar 30, 2017
asyncpg/protocol/protocol.pyx
Outdated
| @@ -121,7 +121,7 @@ cdef class BaseProtocol(CoreProtocol): | |||
| return self.settings | |||
|
|
|||
| def is_in_transaction(self): | |||
| return self.xact_status == PQTRANS_INTRANS | |||
| return self.xact_status in (PQTRANS_INTRANS, PQTRANS_INERROR) | |||
There was a problem hiding this comment.
Add a comment here why PQTRANS_INERROR?
elprans
force-pushed
the
invalidate-cache-on-schema-change
branch
from
51a01e9 to
33c592d
Compare
1st1
reviewed
Mar 30, 2017
asyncpg/connection.py
Outdated
| # connection belongs to (if any). | ||
| # | ||
| # Note that we specifically do not rely on the error | ||
| # message, as it is localizable. |
1st1
reviewed
Mar 30, 2017
| if self._protocol.is_in_transaction(): | ||
| raise | ||
| else: | ||
| stmt = await self._get_statement(query, timeout) |
There was a problem hiding this comment.
Add a comment explaining that we only do 1 retry.
elprans
force-pushed
the
invalidate-cache-on-schema-change
branch
2 times, most recently
from
9d14705 to
47ce1c4
Compare
1st1
reviewed
Mar 30, 2017
tests/test_cache_invalidation.py
Outdated
| finally: | ||
| await self.con.execute('DROP TABLE tab1') | ||
| await pool.release(con2) | ||
| await pool.release(con1) |
There was a problem hiding this comment.
You don't need to release connections if you're closing the pool
PostgreSQL will raise an exception when it detects that the result type of the query has changed from when the statement was prepared. This may happen, for example, after an ALTER TABLE or SET search_path. When this happens, and there is no transaction running, we can simply re-prepare the statement and try again. If the transaction _is_ running, this error will put it into an error state, and we have no choice but to raise an exception. The original error is somewhat cryptic, so we raise a custom InvalidCachedStatementError with the original server exception as context. In either case we clear the statement cache for this connection and all other connections of the pool this connection belongs to (if any). See #72 and #76 for discussion. Fixes: #72.
1st1
approved these changes
Mar 30, 2017
elprans
force-pushed
the
invalidate-cache-on-schema-change
branch
from
47ce1c4 to
baf5ce7
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PostgreSQL will raise an exception when it detects that the result type of the
query has changed from when the statement was prepared. This may happen, for
example, after an ALTER TABLE or SET search_path.
When this happens, and there is no transaction running, we can simply
re-prepare the statement and try again.
If the transaction is running, this error will put it into an error state,
and we have no choice but to raise an exception. The original error is
somewhat cryptic, so we raise a custom InvalidCachedStatementError with the
original server exception as context.
In either case we clear the statement cache for this connection and all other
connections of the pool this connection belongs to (if any).
See #72 and #76 for discussion.
Fixes: #72.