Skip to content
Snippets Groups Projects
  • Vladimir Davydov's avatar
    5ea035ba
    txn: fix rollback of sub-statements · 5ea035ba
    Vladimir Davydov authored
    If space.on_replace callback fails (throws), we must rollback all
    statements inserted by the callback before failing and the statement
    that triggered the callback while currently we only rollback the last
    statement on txn->stmts list. To fix this, let's remember the position
    to rollback to in case of failure for each sub statement, similarly to
    how we do with savepoints.
    
    Note, there's a comment to txn_rollback_stmt() that says that it doesn't
    remove the last statement from the txn->stmts list, just clears it:
    
            /**
             * Void all effects of the statement, but
             * keep it in the list - to maintain
             * limit on the number of statements in a
             * transaction.
             */
            void
            txn_rollback_stmt()
    
    It isn't going to hold after this patch, because this patch reuses the
    savepoint code to rollback statements on failure. Anyway, I haven't
    managed to figure out why we would ever need to keep statements on the
    list after rollback. The comment is clearly misleading as we don't have
    any limits on the number of statements, and even if we had, a statement
    counter would suffice. I guess the real reason why we don't delete
    statements from the list is that it is simply impossible to do in case
    of a singly linked list like stailq, but now it isn't going to be a
    problem. So remove the comment and the test case of engine/savepoint
    which relies on it.
    
    Needed for #2993
    Closes #3020
    5ea035ba
    History
    txn: fix rollback of sub-statements
    Vladimir Davydov authored
    If space.on_replace callback fails (throws), we must rollback all
    statements inserted by the callback before failing and the statement
    that triggered the callback while currently we only rollback the last
    statement on txn->stmts list. To fix this, let's remember the position
    to rollback to in case of failure for each sub statement, similarly to
    how we do with savepoints.
    
    Note, there's a comment to txn_rollback_stmt() that says that it doesn't
    remove the last statement from the txn->stmts list, just clears it:
    
            /**
             * Void all effects of the statement, but
             * keep it in the list - to maintain
             * limit on the number of statements in a
             * transaction.
             */
            void
            txn_rollback_stmt()
    
    It isn't going to hold after this patch, because this patch reuses the
    savepoint code to rollback statements on failure. Anyway, I haven't
    managed to figure out why we would ever need to keep statements on the
    list after rollback. The comment is clearly misleading as we don't have
    any limits on the number of statements, and even if we had, a statement
    counter would suffice. I guess the real reason why we don't delete
    statements from the list is that it is simply impossible to do in case
    of a singly linked list like stailq, but now it isn't going to be a
    problem. So remove the comment and the test case of engine/savepoint
    which relies on it.
    
    Needed for #2993
    Closes #3020
txn.c 11.22 KiB