diff --git a/edb/ir/statypes.py b/edb/ir/statypes.py index 8c834d7622b..09f122161e9 100644 --- a/edb/ir/statypes.py +++ b/edb/ir/statypes.py @@ -801,3 +801,13 @@ def get_translation_map(cls) -> Mapping[TransactionIsolationEnum, str]: TransactionIsolationEnum.Serializable: "serializable", TransactionIsolationEnum.RepeatableRead: "repeatable read", } + + def to_qltypes(self) -> qltypes.TransactionIsolationLevel: + from edb.edgeql import qltypes + match self._val: + case TransactionIsolationEnum.Serializable: + return qltypes.TransactionIsolationLevel.SERIALIZABLE + case TransactionIsolationEnum.RepeatableRead: + return qltypes.TransactionIsolationLevel.REPEATABLE_READ + case _: + raise AssertionError(f"unexpected value: {self._val!r}") diff --git a/edb/lib/cfg.edgeql b/edb/lib/cfg.edgeql index a60eac426bb..bf8cb298c1f 100644 --- a/edb/lib/cfg.edgeql +++ b/edb/lib/cfg.edgeql @@ -191,6 +191,7 @@ ALTER TYPE cfg::AbstractConfig { CREATE REQUIRED PROPERTY default_transaction_isolation -> sys::TransactionIsolation { + CREATE ANNOTATION cfg::affects_compilation := 'true'; CREATE ANNOTATION cfg::backend_setting := '"default_transaction_isolation"'; CREATE ANNOTATION std::description := diff --git a/edb/server/compiler/compiler.py b/edb/server/compiler/compiler.py index 9b51c86f87d..6f2eb95ceb4 100644 --- a/edb/server/compiler/compiler.py +++ b/edb/server/compiler/compiler.py @@ -2153,27 +2153,39 @@ def _compile_ql_transaction( ctx.state.start_tx() - sqls = 'START TRANSACTION' + # Compute the effective isolation level + iso_config: statypes.TransactionIsolation = _get_config_val( + ctx, "default_transaction_isolation" + ) + default_iso = iso_config.to_qltypes() 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", + if iso is None: + iso = default_iso + + # Compute the effective access mode + access = ql.access + if access is None: + if default_iso is qltypes.TransactionIsolationLevel.SERIALIZABLE: + access_mode: statypes.TransactionAccessMode = _get_config_val( + ctx, "default_transaction_access_mode" ) - sqls += f' ISOLATION LEVEL {iso.value}' - if ql.access is None: - access_mode: statypes.TransactionAccessMode = _get_config_val( - ctx, "default_transaction_access_mode" + access = access_mode.to_qltypes() + else: + access = qltypes.TransactionAccessMode.READ_ONLY + + # Guard against unsupported isolation + access combinations + if ( + iso is not qltypes.TransactionIsolationLevel.SERIALIZABLE + and 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' {access_mode.to_qltypes().value}' - else: - sqls += f' {ql.access.value}' + + sqls = f'START TRANSACTION ISOLATION LEVEL {iso.value} {access.value}' if ql.deferrable is not None: sqls += f' {ql.deferrable.value}' sqls += ';' diff --git a/tests/test_server_proto.py b/tests/test_server_proto.py index 9d5f0f4a53a..38dc61580b0 100644 --- a/tests/test_server_proto.py +++ b/tests/test_server_proto.py @@ -2069,6 +2069,304 @@ async def test_server_proto_tx_22(self): self.assertEqual(await self.con.query_single('SELECT 42'), 42) + async def test_server_proto_tx_23(self): + # Test that default_transaction_isolation is respected + + await self.con.query(''' + CONFIGURE SESSION + SET default_transaction_isolation := 'RepeatableRead'; + ''') + + try: + self.assertEqual( + await self.con.query( + 'select sys::get_transaction_isolation();', + ), + ["RepeatableRead"], + ) + finally: + await self.con.query(''' + CONFIGURE SESSION + RESET default_transaction_isolation; + ''') + + async def test_server_proto_tx_24(self): + # default_transaction_isolation < Serializable enforces read-only + + await self.con.query(''' + CONFIGURE SESSION + SET default_transaction_isolation := 'RepeatableRead'; + ''') + + try: + with self.assertRaisesRegex( + edgedb.TransactionError, + 'cannot execute.*RepeatableRead', + ): + await self.con.query(''' + INSERT Tmp { + tmp := 'aaa' + }; + ''') + finally: + await self.con.query(''' + CONFIGURE SESSION + RESET default_transaction_isolation; + ''') + + self.assertEqual( + await self.con.query('SELECT 42'), + [42]) + + async def test_server_proto_tx_25(self): + # default_transaction_isolation < Serializable overrides read-write + + try: + await self.con.query(''' + CONFIGURE SESSION + SET default_transaction_isolation := 'RepeatableRead'; + ''') + await self.con.query(''' + CONFIGURE SESSION + SET default_transaction_access_mode := 'ReadWrite'; + ''') + + with self.assertRaisesRegex( + edgedb.TransactionError, + 'cannot execute.*RepeatableRead', + ): + await self.con.query(''' + INSERT Tmp { + tmp := 'aaa' + }; + ''') + finally: + await self.con.query(''' + CONFIGURE SESSION + RESET default_transaction_access_mode; + ''') + await self.con.query(''' + CONFIGURE SESSION + RESET default_transaction_isolation; + ''') + + self.assertEqual( + await self.con.query('SELECT 42'), + [42]) + + async def test_server_proto_tx_26(self): + # Test that default_transaction_access_mode is respected + + await self.con.query(''' + CONFIGURE SESSION + SET default_transaction_access_mode := 'ReadOnly'; + ''') + + try: + self.assertEqual( + await self.con.query( + 'select sys::get_transaction_isolation();', + ), + ["Serializable"], + ) + + with self.assertRaisesRegex( + edgedb.TransactionError, + 'cannot execute.*ReadOnly', + ): + await self.con.query(''' + INSERT Tmp { + tmp := 'aaa' + }; + ''') + finally: + await self.con.query(''' + CONFIGURE SESSION + RESET default_transaction_access_mode; + ''') + + self.assertEqual( + await self.con.query('SELECT 42'), + [42]) + + async def test_server_proto_tx_27(self): + # Test that START TRANSACTION respects the default isolation + + try: + await self.con.query(''' + CONFIGURE SESSION + SET default_transaction_isolation := 'RepeatableRead'; + ''') + await self.con.query(''' + START TRANSACTION; + ''') + + self.assertEqual( + await self.con.query( + 'select sys::get_transaction_isolation();', + ), + ["RepeatableRead"], + ) + + finally: + await self.con.query(f''' + ROLLBACK; + ''') + await self.con.query(''' + CONFIGURE SESSION + RESET default_transaction_isolation; + ''') + + self.assertEqual( + await self.con.query('SELECT 42'), + [42]) + + async def test_server_proto_tx_28(self): + # Test that non-serializable START TRANSACTION enforces read-only + + try: + await self.con.query(''' + CONFIGURE SESSION + SET default_transaction_isolation := 'RepeatableRead'; + ''') + await self.con.query(''' + START TRANSACTION; + ''') + + with self.assertRaisesRegex( + edgedb.TransactionError, + 'read-only transaction'): + + await self.con.query(''' + INSERT Tmp { + tmp := 'aaa' + }; + ''') + finally: + await self.con.query(f''' + ROLLBACK; + ''') + await self.con.query(''' + CONFIGURE SESSION + RESET default_transaction_isolation; + ''') + + self.assertEqual( + await self.con.query('SELECT 42'), + [42]) + + async def test_server_proto_tx_29(self): + # Test that START TRANSACTION respects default read-only + + try: + await self.con.query(''' + CONFIGURE SESSION + SET default_transaction_access_mode:= 'ReadOnly'; + ''') + await self.con.query(''' + START TRANSACTION; + ''') + + self.assertEqual( + await self.con.query( + 'select sys::get_transaction_isolation();', + ), + ["Serializable"], + ) + + with self.assertRaisesRegex( + edgedb.TransactionError, + 'read-only transaction'): + + await self.con.query(''' + INSERT Tmp { + tmp := 'aaa' + }; + ''') + finally: + await self.con.query(f''' + ROLLBACK; + ''') + await self.con.query(''' + CONFIGURE SESSION + RESET default_transaction_access_mode; + ''') + + self.assertEqual( + await self.con.query('SELECT 42'), + [42]) + + async def test_server_proto_tx_30(self): + # Test that non-serializable START TRANSACTION conflicts read-write + + try: + await self.con.query(''' + CONFIGURE SESSION + SET default_transaction_isolation := 'RepeatableRead'; + ''') + with self.assertRaisesRegex( + edgedb.TransactionError, + 'only supported in read-only transactions', + ): + await self.con.query(''' + START TRANSACTION READ WRITE; + ''') + + finally: + await self.con.query(f''' + ROLLBACK; + ''') + await self.con.query(''' + CONFIGURE SESSION + RESET default_transaction_isolation; + ''') + + self.assertEqual( + await self.con.query('SELECT 42'), + [42]) + + async def test_server_proto_tx_31(self): + # Test that non-serializable START TRANSACTION works fine with the + # default read-only + + try: + await self.con.query(''' + CONFIGURE SESSION + SET default_transaction_access_mode:= 'ReadOnly'; + ''') + await self.con.query(''' + START TRANSACTION ISOLATION REPEATABLE READ; + ''') + + self.assertEqual( + await self.con.query( + 'select sys::get_transaction_isolation();', + ), + ["RepeatableRead"], + ) + + with self.assertRaisesRegex( + edgedb.TransactionError, + 'read-only transaction'): + + await self.con.query(''' + INSERT Tmp { + tmp := 'aaa' + }; + ''') + finally: + await self.con.query(f''' + ROLLBACK; + ''') + await self.con.query(''' + CONFIGURE SESSION + RESET default_transaction_access_mode; + ''') + + self.assertEqual( + await self.con.query('SELECT 42'), + [42]) + class TestServerProtoMigration(tb.QueryTestCase):