Skip to content

Savepoints - #1666

Open
doxlik wants to merge 13 commits into
eclipse-vertx:masterfrom
doxlik:savepoints
Open

Savepoints#1666
doxlik wants to merge 13 commits into
eclipse-vertx:masterfrom
doxlik:savepoints

Conversation

@doxlik

@doxlik doxlik commented May 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

This PR adds API for Savepoints and implementation of it for Postgres client.

Closes #1369

@tsegismont

tsegismont commented Jun 2, 2026

Copy link
Copy Markdown
Member

Can you please rebase the PR? The unrelated CI failures should be fixed now

@doxlik

doxlik commented Jun 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@tsegismont rebasing done!

CI still have some failure but seems like it couldn't build in action with Postgres14, not any test problem.
Not sure what is reason for that, seems like it is some maven issue in that particular action.

@tsegismont

tsegismont commented Jun 15, 2026

Copy link
Copy Markdown
Member

The CI has been stabilized hopefully, would you mind rebasing again? Thank you

@doxlik

doxlik commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

@tsegismont Hi, there were no new commits since the last rebase, so I amended my latest commit to trigger a push. It seems CI is passing now. Looking forward to your review!

@doxlik

doxlik commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

@vietj @tsegismont Hi. I added support for other DBs. Please have a look once you have time.

@doxlik @tsegismont
savepoints
06835be 
Signed-off-by: doxlik <doxlikx@gmail.com>
@doxlik @tsegismont
savepoints
d8f639d 
Signed-off-by: doxlik <doxlikx@gmail.com>
@doxlik @tsegismont
savepoints
9a4fa74 
Signed-off-by: doxlik <doxlikx@gmail.com>
@doxlik @tsegismont
Support savepoints on MySQL, DB2, SQL Server and Oracle
c31adc2 
Savepoints were implemented for PostgreSQL only. The command itself is
plain SQL, so the remaining drivers mostly need to route SavepointCommand
to a simple query, but they do not all agree on the syntax and two of them
cannot release a savepoint at all.

Driver gains supportsSavepointRelease(). Microsoft SQL Server and Oracle
create savepoints but have no statement that discards one, so releasing on
those drivers fails with an UnsupportedOperationException rather than
pretending to succeed, and the savepoint stays usable for a rollback.

MySQL uses the standard syntax. DB2 requires the mandatory ON ROLLBACK
RETAIN CURSORS clause when a savepoint is created. Transact-SQL names the
statements SAVE TRANSACTION and ROLLBACK TRANSACTION. Oracle runs the
statement on its JDBC connection through a new OracleSavepointCommand.

Generated savepoint names are now VX_SP_<n>: an unquoted Oracle identifier
cannot start with an underscore, so the previous __vx_sp_<n> was invalid
there.

The behaviour every database agrees on moved to TransactionTestBase, gated
on the two capabilities, so each driver inherits it. What the databases do
not agree on stays in the driver tests: PostgreSQL fails the whole
transaction when a statement fails, whereas MySQL leaves the transaction
usable, and MySQLTransactionTest now covers that difference.

Verified against PostgreSQL, MySQL and Oracle. SQL Server and DB2 could not
be started locally, both for reasons unrelated to this change.

Signed-off-by: doxlik <doxlikx@gmail.com>
@doxlik @tsegismont
Fix the savepoint build on SQL Server and restore the formatting
94dc0dd 
Removing the duplicated helpers from PgTransactionTest left three imports
behind, and MySQLTransactionTest imported Future without using it, both of
which fail spotless:check.

SQL Server drops a savepoint once the transaction has been rolled back to
it and answers a second rollback with "No transaction or savepoint of that
name was found", so rolling back twice to the same savepoint is now a
capability the drivers report, and SQL Server opts out.

Signed-off-by: doxlik <doxlikx@gmail.com>
@doxlik @tsegismont
Cover the savepoint behaviour that differs between databases
93761f6 
Whether a failed statement also fails the surrounding transaction was only
covered for PostgreSQL and MySQL, and only MySQL stated it. It is now a
capability the drivers report, statementErrorFailsTransaction(), with a
test for each answer and a third test showing that rolling back to a
savepoint recovers the transaction either way.

The drivers that had no savepoint test of their own get one for the part
that is specific to them: SQL Server that a savepoint does not nest the
transaction, so @@TRANCOUNT stays at one across SAVE TRANSACTION and the
rollback; DB2 that a cursor opened before the savepoint survives the
rollback, which is what ON ROLLBACK RETAIN CURSORS is for; Oracle that a
savepoint leaves the autocommit handling around commit and rollback alone.

Signed-off-by: doxlik <doxlikx@gmail.com>
@doxlik @tsegismont
Do not close the pooled connection in the Oracle savepoint test
28500fd 
The connector hands out a pooled connection that is released when the
transaction ends, so closing it again failed the test with "Connection
released twice". Acquire the connection from the pool instead, which is
what proves it went back, and assert on the rows the transaction committed.

Signed-off-by: doxlik <doxlikx@gmail.com>
tsegismont
tsegismont requested changes Sep 17, 2026
View reviewed changes

@tsegismont tsegismont left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @doxlik

Overall, this looks good to me, but we're missing documentation for this feature.

Comment thread vertx-sql-client/src/main/java/io/vertx/sqlclient/impl/SavepointImpl.java Outdated
Comment thread vertx-sql-client/src/main/java/io/vertx/sqlclient/Transaction.java
@doxlik
Document savepoints and simplify SavepointImpl
39e4758 
The transactions documentation now has a savepoint section covering
rollback, release and the databases that cannot release a savepoint, with
an example for each of the two operations in every client module.

SavepointImpl carried a private functional interface only to defer the two
transaction calls. A BiFunction does the same, so rollback and release are
method references now.

Signed-off-by: doxlik <doxlikx@gmail.com>
@doxlik
Keep the database specific savepoint notes out of the shared page
29eb201 
The transactions documentation is included by every client, so the section
describing what PostgreSQL, SQL Server and Oracle do would have appeared on
all of the pages. The shared section only states what the API does now, and
each client contributes its own savepoint_note.adoc, the way cursor.adoc
already lets MySQL add its ProxySQL warning.

Signed-off-by: doxlik <doxlikx@gmail.com>
@doxlik
Let the application name a savepoint
d7331a2 
createSavepoint(String) creates a savepoint under a name the application
chooses, which is what shows up when the database mentions the savepoint in
an error message. The no-argument method keeps generating VX_SP_<n>.

The name is written to the statement as an unquoted identifier. Quoting it
would have to be done per database and would make the name case sensitive,
so a name is instead restricted to what all the supported databases accept
unquoted: a letter followed by letters, digits or underscores. Anything
else is rejected with an IllegalArgumentException before the statement is
built, leaving the transaction untouched.

Signed-off-by: doxlik <doxlikx@gmail.com>
@doxlik

doxlik commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

@tsegismont thank you for review. I added docs, refactor to BiFunction and added API for user supplied savepoint name, can you please have a look!

tsegismont
tsegismont reviewed Sep 18, 2026
View reviewed changes

@tsegismont tsegismont left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we have access to the driver, perhaps could we have a new method in the driver that tells how to quote a name, and then lift restrictions on the savepoint name

@doxlik
Quote savepoint names through the driver instead of restricting them
f2377f3 
A savepoint name was restricted to a letter followed by letters, digits or
underscores so it could be written to the statement unquoted. The driver
already knows its dialect, it decides how to write a parameter placeholder,
so let it write a delimited identifier too: appendQuotedIdentifier uses the
standard double quote by default, MySQL overrides it with backticks and
Transact-SQL with square brackets, each escaping its own delimiter.

The name is quoted once when the savepoint is created and the rollback and
release statements reuse it. What a name may hold is no longer restricted,
only its length: SQL Server keeps the first 32 characters of a savepoint
name, so longer names would silently collide there.

Signed-off-by: doxlik <doxlikx@gmail.com>
@doxlik
Refuse a savepoint name holding a double quote
fdfb2d9 
Oracle rejects a savepoint identifier that holds a double quote even when it
is escaped, ORA-25716, while the other databases accept it. A name that
works on one database should work on all of them, so the double quote is
refused everywhere rather than on Oracle alone.

Signed-off-by: doxlik <doxlikx@gmail.com>
@doxlik
Correct a stale comment on savepoint name validation
a74273d 
The comment claimed length was the only restriction, which stopped being
true when the double quote was refused.

Signed-off-by: doxlik <doxlikx@gmail.com>
@doxlik

doxlik commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

@tsegismont done, can you please check

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tsegismont tsegismont tsegismont requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support rollback to savepoint after failure

2 participants

@doxlik @tsegismont