From e6c12f7474bcbaa18e227780805977c784129997 Mon Sep 17 00:00:00 2001 From: Elvis Pranskevichus Date: Tue, 21 Jan 2025 13:49:41 -0800 Subject: [PATCH] Allow dropping isolation level to REPEATABLE READ in a READ ONLY tx (#8237) This re-enabled REPEATABLE READ isolation in explicit transaction blocks, but only in those that are READ ONLY. Anomalies observed in read statements are normal non-serializability anomalies, so with an explicit opt-in it is a reasonable tradeoff between lessening the transaction serialization load and consistency. --- edb/edgeql-parser/src/keywords.rs | 1 + edb/edgeql/parser/grammar/statements.py | 4 ++++ edb/server/compiler/compiler.py | 13 +++++++++++ tests/test_server_proto.py | 30 +++++++++++++++++++++++++ 4 files changed, 48 insertions(+) diff --git a/edb/edgeql-parser/src/keywords.rs b/edb/edgeql-parser/src/keywords.rs index e79393ea8c3..eb416dbd2fc 100644 --- a/edb/edgeql-parser/src/keywords.rs +++ b/edb/edgeql-parser/src/keywords.rs @@ -79,6 +79,7 @@ pub const UNRESERVED_KEYWORDS: phf::Set<&str> = phf_set!( "reject", "release", "rename", + "repeatable", "required", "reset", "restrict", diff --git a/edb/edgeql/parser/grammar/statements.py b/edb/edgeql/parser/grammar/statements.py index dd3cbef702e..3994896a155 100644 --- a/edb/edgeql/parser/grammar/statements.py +++ b/edb/edgeql/parser/grammar/statements.py @@ -66,6 +66,10 @@ def reduce_ISOLATION_SERIALIZABLE(self, *kids): self.val = (qltypes.TransactionIsolationLevel.SERIALIZABLE, kids[0].span) + def reduce_ISOLATION_REPEATABLE_READ(self, *kids): + self.val = (qltypes.TransactionIsolationLevel.REPEATABLE_READ, + kids[0].span) + def reduce_READ_WRITE(self, *kids): self.val = (qltypes.TransactionAccessMode.READ_WRITE, kids[0].span) diff --git a/edb/server/compiler/compiler.py b/edb/server/compiler/compiler.py index 1206bbcba82..922d7ecb445 100644 --- a/edb/server/compiler/compiler.py +++ b/edb/server/compiler/compiler.py @@ -2153,6 +2153,19 @@ def _compile_ql_transaction( ctx.state.start_tx() sqls = 'START TRANSACTION' + iso = ql.isolation + if iso is not None: + if ( + iso is not qltypes.TransactionIsolationLevel.SERIALIZABLE + and ql.access is not qltypes.TransactionAccessMode.READ_ONLY + ): + raise errors.TransactionError( + f"{iso.value} transaction isolation level is only " + "supported in read-only transactions", + span=ql.span, + hint=f"specify READ ONLY access mode", + ) + sqls += f' ISOLATION LEVEL {iso.value}' if ql.access is not None: sqls += f' {ql.access.value}' if ql.deferrable is not None: diff --git a/tests/test_server_proto.py b/tests/test_server_proto.py index 42f20abab3a..49e5abd1071 100644 --- a/tests/test_server_proto.py +++ b/tests/test_server_proto.py @@ -1577,6 +1577,36 @@ async def test_server_proto_tx_07(self): await self.con.query('SELECT 42'), [42]) + async def test_server_proto_tx_08(self): + try: + await self.con.query(''' + START TRANSACTION ISOLATION REPEATABLE READ, READ ONLY; + ''') + + self.assertEqual( + await self.con.query( + 'select sys::get_transaction_isolation();', + ), + ["RepeatableRead"], + ) + finally: + await self.con.query(f''' + ROLLBACK; + ''') + + async def test_server_proto_tx_09(self): + try: + with self.assertRaisesRegex( + edgedb.TransactionError, + 'only supported in read-only transactions'): + await self.con.query(''' + START TRANSACTION ISOLATION REPEATABLE READ; + ''') + finally: + await self.con.query(f''' + ROLLBACK; + ''') + async def test_server_proto_tx_10(self): # Basic test that ROLLBACK works on SET ALIAS changes.