Skip to content

Commit

Permalink
Add handling for savepoint not found
Browse files Browse the repository at this point in the history
  • Loading branch information
kasium committed Aug 9, 2024
1 parent 0fd97bd commit 33830dd
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 28 deletions.
11 changes: 11 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
@@ -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
-----

Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
13 changes: 13 additions & 0 deletions sqlalchemy_hana/dialect.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
super().do_rollback_to_savepoint(connection, name)
19 changes: 0 additions & 19 deletions sqlalchemy_hana/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
42 changes: 41 additions & 1 deletion test/test_dialect.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
7 changes: 0 additions & 7 deletions test/test_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
[
Expand Down

0 comments on commit 33830dd

Please sign in to comment.