use uuid as savepoint name

[AlexErrant]

The primary cause of this PR is this line from the docs:

The ROLLBACK command without a TO clause rolls backs all transactions

I think the current behavior of busting out of all transactions seems too much. Note that even though the rollback causes "The SAVEPOINT with the matching name [to] remains on the transaction stack", the subsequent RELEASE on L109 will (more or less) close out the transaction and commit nothing. I think. I've tested nothing 😅

I also made the names UUIDs because even though "The transaction names need not be unique", it just feels safer. Feel free to assuage me of my paranoia.

[tantaman]

I don't think we want finally here.

finally will run even after catch which means that we'll rollback (if there was an exception) and then release. We just want to do one or the other.

What problems were you experiencing?

[AlexErrant]

Hm, let me elaborate on the OP. From the docs:

The ROLLBACK command with a TO clause rolls back transactions going backwards in time back to the most recent SAVEPOINT with a matching name. The SAVEPOINT with the matching name remains on the transaction stack.

Emphasis mine. Since that SAVEPOINT remains on the stack, we must RELEASE to close out that savepoint.

The RELEASE command starts with the most recent addition to the transaction stack and releases savepoints backwards in time until it releases a savepoint with a matching savepoint-name.

---

However, I'm now thinking about what happens if the ROLLBACK fails - with finally we RELEASE without a ROLLBACK. That's undesirable. If the ROLLBACK fails, we should just throw. The new commit reflects this realization.

What problems were you experiencing?

No problems on the application side - I started to investigate nested transactions, so just read the docs and other people's code (including this).

[tantaman]

Ah. Thanks so much for pointing this out. I had no idea that it remained on the stack (illustrated below).

[tantaman]

oh interesting.. I had the completely wrong mental model of savepoints and didn't realize a rollback needs to be followed by a release to actually pop it off the stack o_O

Thanks for catching this.

sqlite> savepoint b;
sqlite> create table boo (a);
sqlite> rollback to b;
sqlite> create table bar (a);
sqlite> release b;
sqlite> .tables
bar  foo

[tantaman]

better illustration:

sqlite> savepoint b;
sqlite> savepoint b;
sqlite> rollback to b;
sqlite> release b;
sqlite> release b;
sqlite> release b;
Runtime error: no such savepoint: b

Even with rollback we must release

[tantaman]

I'll merge this today. I also need to update core extension code in cr-sqlite base on these learnings 😬

[AlexErrant]

Gonna assume here that

Error: unrecognized token: "496204c772af430b9c19a3a0df61094d"

in the failed run is due to starting with a numeric

sqlite> savepoint 2good4u;
Parse error: unrecognized token: "2good4u"
  savepoint 2good4u;
            ^--- error here
sqlite> savepoint x2good4u;
sqlite>