-
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
Vladimir Davydov authoredIf 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