From 476759fb24f689869d0550ac0212f555c1982c0a Mon Sep 17 00:00:00 2001 From: Kai Mueller Date: Fri, 9 Aug 2024 12:19:07 +0000 Subject: [PATCH] Add handling for savepoint not found --- CHANGES.rst | 11 ++++++++++ pyproject.toml | 2 +- sqlalchemy_hana/dialect.py | 13 ++++++++++++ sqlalchemy_hana/errors.py | 19 ----------------- test/test_dialect.py | 42 +++++++++++++++++++++++++++++++++++++- test/test_errors.py | 7 ------- 6 files changed, 66 insertions(+), 28 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 6a124ff..b7fd453 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,6 +1,17 @@ Changelog ========= +2.6.0 +----- + +Features +~~~~~~~~ + +- An error during a rollback to a savepoint is ignored, if the transaction was already + rolled back by SAP HANA. + Based on this feature, ``sqlalchemy_hana.errors`` will no longer extract an inner error + if a savepoint was not found. + 2.5.0 ----- diff --git a/pyproject.toml b/pyproject.toml index 348f387..480858d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "sqlalchemy-hana" -version = "2.5.0" +version = "2.6.0" description = "SQLAlchemy dialect for SAP HANA" keywords = ["sqlalchemy", "sap", "hana"] requires-python = "~=3.8" diff --git a/sqlalchemy_hana/dialect.py b/sqlalchemy_hana/dialect.py index 8818dcb..a744700 100644 --- a/sqlalchemy_hana/dialect.py +++ b/sqlalchemy_hana/dialect.py @@ -3,6 +3,7 @@ from __future__ import annotations import contextlib +import sys from contextlib import closing from types import ModuleType from typing import TYPE_CHECKING, Any, Callable, cast @@ -1215,3 +1216,15 @@ def get_table_comment( ) return {"text": result.scalar()} + + def do_rollback_to_savepoint(self, connection: Connection, name: str) -> None: + err = sys.exc_info() + if ( + isinstance(err[1], exc.DBAPIError) + and isinstance(err[1].orig, hdbcli.dbapi.Error) + and err[1].orig.errortext.startswith("transaction rolled back") + ): + # the transaction was already rolled back by SAP HANA, therefore the savepoint + # does no longer exist + return + return super().do_rollback_to_savepoint(connection, name) diff --git a/sqlalchemy_hana/errors.py b/sqlalchemy_hana/errors.py index 7a61da5..9c85f56 100644 --- a/sqlalchemy_hana/errors.py +++ b/sqlalchemy_hana/errors.py @@ -96,30 +96,11 @@ def convert_dbapi_error(dbapi_error: DBAPIError) -> DBAPIError: If it does not contain a hdbcli error, the original exception is returned. Else the error code and error text are further checked. - - In addition, an edge case is handled where SQLAlchemy creates a savepoint and the same - transaction later fails leading to an automatic rollback by HANA. - However, SQLAlchemy still tries to roll back the savepoint, which fails because the savepoint - is no longer valid. - In this case, the cause of the exception is used for further processing """ error = dbapi_error.orig if not isinstance(error, HdbcliError): return dbapi_error - # extract hidden inner exceptions - # TxSavepoint not found should normally only happen if a transaction was rolled back by HANA, - # but SQLAlchemy also tries to perform a savepoint rollback, which fails due to the transaction - # rollback. In this case, we need to check the inner exception (__context__) - if ( - error.__context__ - and isinstance(error.__context__, HdbcliError) - and error.errorcode == 128 - and "TxSavepoint not found" in error.errortext - ): - error = error.__context__ - dbapi_error.orig = error - if error.errorcode in [-10807, -10709]: # sqldbc error codes for connection errors return ClientConnectionError.from_dbapi_error(dbapi_error) if error.errorcode == 613: diff --git a/test/test_dialect.py b/test/test_dialect.py index 61685d6..19e8a71 100644 --- a/test/test_dialect.py +++ b/test/test_dialect.py @@ -2,13 +2,16 @@ from __future__ import annotations +import sys +from unittest import mock from unittest.mock import Mock import pytest from hdbcli.dbapi import Error from sqlalchemy import create_engine +from sqlalchemy.engine.default import DefaultDialect from sqlalchemy.engine.url import make_url -from sqlalchemy.exc import ArgumentError +from sqlalchemy.exc import ArgumentError, DBAPIError from sqlalchemy.testing import assert_raises_message, config, eq_ from sqlalchemy.testing.engines import testing_engine from sqlalchemy.testing.fixtures import TestBase @@ -176,3 +179,40 @@ def test_with_isolation_level_in_create_engine(self) -> None: NON_DEFAULT_ISOLATION_LEVEL, ) conn.close() + + def test_do_rollback_to_savepoint_no_error(self) -> None: + dialect = config.db.dialect + connection = Mock() + + with mock.patch.object( + DefaultDialect, "do_rollback_to_savepoint" + ) as super_rollback: + dialect.do_rollback_to_savepoint(connection, "savepoint") + super_rollback.assert_called_once_with(connection, "savepoint") + + def test_do_rollback_to_savepoint_unrelated_error(self) -> None: + dialect = config.db.dialect + connection = Mock() + + with mock.patch.object( + sys, "exc_info", return_value=(ValueError, ValueError(), Mock()) + ), mock.patch.object( + DefaultDialect, "do_rollback_to_savepoint" + ) as super_rollback: + dialect.do_rollback_to_savepoint(connection, "savepoint") + super_rollback.assert_called_once_with(connection, "savepoint") + + def test_do_rollback_to_savepoint_ignores_error(self) -> None: + dialect = config.db.dialect + connection = Mock() + + error = Error(133, "transaction rolled back: deadlock") + dbapi_error = DBAPIError(None, None, error) + + with mock.patch.object( + sys, "exc_info", return_value=(DBAPIError, dbapi_error, Mock()) + ), mock.patch.object( + DefaultDialect, "do_rollback_to_savepoint" + ) as super_rollback: + dialect.do_rollback_to_savepoint(connection, "savepoint") + super_rollback.assert_not_called() diff --git a/test/test_errors.py b/test/test_errors.py index 1767234..56a91ed 100644 --- a/test/test_errors.py +++ b/test/test_errors.py @@ -27,13 +27,6 @@ class TestConvertDBAPIError(TestBase): - def test_convert_dbapi_error_txsavepoint_not_found(self) -> None: - error = HdbcliError(128, "TxSavepoint not found") - error.__context__ = HdbcliError(133, "some deadlock") - dbapi_error = DBAPIError(None, None, error) - - assert isinstance(convert_dbapi_error(dbapi_error), DeadlockError) - @pytest.mark.parametrize( "errorcode,errortext,expected_exception", [