Skip to content

Commit

Permalink
Fix in_tx_dbver (#6857)
Browse files Browse the repository at this point in the history
Looks like we had in_tx_dbver since 2021 but never really used it.
The added test demonstrates an issue where we cached a query under
the wrong dbver in a transaction, and caused ISE in later calls.
  • Loading branch information
fantix authored Feb 15, 2024
1 parent a683447 commit 7812f2e
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 2 deletions.
5 changes: 3 additions & 2 deletions edb/server/dbview/dbview.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,7 @@ cdef class DatabaseConnectionView:
self._db._index._global_schema_pickle
self._in_tx_user_config_spec = self._db.user_config_spec
self._in_tx_state_serializer = state_serializer
self._in_tx_dbver = self._db.dbver

cdef _apply_in_tx(self, query_unit):
if query_unit.has_ddl:
Expand All @@ -778,6 +779,7 @@ cdef class DatabaseConnectionView:
if query_unit.has_set:
self._in_tx_with_set = True
if query_unit.user_schema is not None:
self._in_tx_dbver = next_dbver()
self._in_tx_user_schema_pickle = query_unit.user_schema
self._in_tx_user_config_spec = config.FlatSpec(
*query_unit.ext_config_settings
Expand All @@ -804,7 +806,6 @@ cdef class DatabaseConnectionView:
if new_types:
self._db._update_backend_ids(new_types)
if query_unit.user_schema is not None:
self._in_tx_dbver = next_dbver()
self._db._set_and_signal_new_user_schema(
query_unit.user_schema,
query_unit.extensions,
Expand Down Expand Up @@ -977,7 +978,7 @@ cdef class DatabaseConnectionView:
if query_unit_group is None:
# Cache miss; need to compile this query.
cached = False
dbver = self._db.dbver
dbver = self.dbver

try:
query_unit_group = await self._compile(query_req)
Expand Down
48 changes: 48 additions & 0 deletions tests/test_server_proto.py
Original file line number Diff line number Diff line change
Expand Up @@ -2886,6 +2886,54 @@ async def test_server_proto_query_cache_invalidate_10(self):
finally:
await con2.aclose()

async def test_server_proto_query_cache_invalidate_11(self):
typename = 'CacheInv_11'

await self.con.execute(f"""
CREATE TYPE {typename} {{
CREATE PROPERTY prop1 -> std::int64;
}};
INSERT {typename} {{ prop1 := 42 }};
""")

class Rollback(Exception):
pass

with self.assertRaises(Rollback):
async with self.con.transaction():
# make sure the transaction is started
await self.con.query('SELECT 123')

# DDL in another connection
con2 = await self.connect(database=self.con.dbname)
try:
await con2.execute(f"""
ALTER TYPE {typename} {{
DROP PROPERTY prop1;
}};
""")
finally:
await con2.aclose()

# This compiles fine with the schema in the transaction, and
# the compile result is cached with an outdated version.
# However, the execution fails because the column is already
# dropped outside the transaction.
# FIXME: ISE is not an ideal error as for user experience here
with self.assertRaisesRegex(
edgedb.InternalServerError, "column.*does not exist"
):
await self.con.query(f'SELECT {typename}.prop1')

raise Rollback

# Should recompile with latest schema, instead of reusing a wrong cache
with self.assertRaisesRegex(
edgedb.InvalidReferenceError,
"has no link or property 'prop1'"
):
await self.con.query(f'SELECT {typename}.prop1')

async def test_server_proto_backend_tid_propagation_01(self):
async with self._run_and_rollback():
await self.con.execute('''
Expand Down

0 comments on commit 7812f2e

Please sign in to comment.