Add support for customizing transaction start #643
Conversation
Signed-off-by: Billy Yuan <[email protected]>
| @@ -74,4 71,19 @@ public void testDelayedCommit(TestContext ctx) { | |||
| super.testDelayedCommit(ctx); | |||
| } | |||
|
|
|||
| @Override | |||
There was a problem hiding this comment.
@aguibert I failed to make these tests pass, can you help with this? I tried to set transaction characteristic but I got this
>>> BEGIN DB2TransactionTest.testStartReadOnlyTransaction
[2020-05-18 23:53:30] 详细 io.vertx.db2client.impl.codec.DB2Encoder >>> ENCODE SimpleQueryCommandCodec@2e09e4e1 sql=SET TRANSACTION READ ONLY, autoCommit=false, section=null
[2020-05-18 23:53:30] 详细 io.vertx.db2client.impl.codec.DB2Decoder <<< DECODE SimpleQueryCommandCodec@2e09e4e1 sql=SET TRANSACTION READ ONLY, autoCommit=false, section=io.vertx.db2client.impl.drda.Section$ImmediateSection@7b071208{packageName=SYSLH200, sectionNumber=385, cursorName=SQL_CURLH200C} (123 bytes)
io.vertx.db2client.DB2Exception: The SQL syntax provided was invalid
There was a problem hiding this comment.
DB2 doesn't support setting a transaction as read only via SQL. Instead, attributes like read-only and isolation level are typically specified as "query attributes" which can be appended to the SQL. For example:
SELECT DEPTNO, DEPTNAME, MGRNO
FROM DEPT
WHERE ADMRDEPT ='A00'
FOR READ ONLY WITH RS
|
I wonder if we could use interface to replace enum for |
There was a problem hiding this comment.
In DB2 it's more ideal to flow these transaction attributes along with each query that is sent under the tx. For this reason I think we need to rework this PR a bit. Specifically, instead of having the common layer madate that a SQL operation should take place, it should be a more abstract command that the driver can handle however it needs. For other DBs this will look like flowing an SQL statement, but for DB2 it will mean saving off those attributes so they can be appended to all queries that occur under that transaction.
I think we can basically follow the pattern used for the autoCommit() method on SqlClientBase class and subtypes to add a transactionOptions() method that gives the connection awareness of the transaction options, and these transaction options can be passed onto all query commands so that the command codec impls can be aware of the tx options.
| private TransactionIsolationLevel isolationLevel; | ||
| private TransactionAccessMode accessMode; | ||
|
|
||
| public TransactionOptions() { |
There was a problem hiding this comment.
can you add some javadoc to this method? We'll want to indicate that this creates a set of tx options that have an undefined isolation level and access mode
| // read-only transactions | ||
| async.complete(); | ||
| }))); | ||
| } |
There was a problem hiding this comment.
can you also add some tests for isolationLevel as well?
| @Test | ||
| public void testSetReadCommitted() { | ||
| String sql = TransactionSqlBuilder.buildSetTxIsolationLevelSql(TransactionIsolationLevel.READ_COMMITTED, null); | ||
| Assert.assertEquals("SET TRANSACTION ISOLATION LEVEL READ COMMITTED" ,sql); |
There was a problem hiding this comment.
Based on some quick experiments I ran locally this is not supported by DB2. Instead I think we need to use syntax like this:
https://www.ibm.com/support/knowledgecenter/SSEPGG_11.5.0/com.ibm.db2.luw.sql.ref.doc/doc/r0010944.html
Before we add code like this, it would be good to have some more functional tests in place
| @@ -74,4 71,19 @@ public void testDelayedCommit(TestContext ctx) { | |||
| super.testDelayedCommit(ctx); | |||
| } | |||
|
|
|||
| @Override | |||
There was a problem hiding this comment.
DB2 doesn't support setting a transaction as read only via SQL. Instead, attributes like read-only and isolation level are typically specified as "query attributes" which can be appended to the SQL. For example:
SELECT DEPTNO, DEPTNAME, MGRNO
FROM DEPT
WHERE ADMRDEPT ='A00'
FOR READ ONLY WITH RS
|
Thanks for the explanation! @aguibert I will check how to get this works. |
|
Hi, what's the status of this one? |
|
it should be rebased on master, reviewed and finished |
|
I will check how it would work this weekend, last time I stuck on the DB2 transactions since then there have been many changes in the client |
Motivation:
Fixes #432
Conformance:
Your commits should be signed and you should have signed the Eclipse Contributor Agreement as explained in https://github.com/eclipse/vert.x/blob/master/CONTRIBUTING.md
Please also make sure you adhere to the code style guidelines: https://github.com/vert-x3/wiki/wiki/Vert.x-code-style-guidelines