Re: [PR] IGNITE-28649 Support JDBC API for savepoint [ignite]
vldpyatkov merged PR #13112: URL: https://github.com/apache/ignite/pull/13112 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] IGNITE-28649 Support JDBC API for savepoint [ignite]
sonarqubecloud[bot] commented on PR #13112: URL: https://github.com/apache/ignite/pull/13112#issuecomment-4431670156 ## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=13112) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_ignite&pullRequest=13112&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_ignite&pullRequest=13112&issueStatuses=ACCEPTED) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_ignite&pullRequest=13112&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=13112&metric=new_coverage&view=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=13112&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=13112) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] IGNITE-28649 Support JDBC API for savepoint [ignite]
zstan commented on code in PR #13112:
URL: https://github.com/apache/ignite/pull/13112#discussion_r3226641736
##
docs/_docs/SQL/JDBC/jdbc-driver.adoc:
##
@@ -555,6 +555,46 @@ In addition to generic DataSource properties,
`IgniteJdbcThinDataSource` support
Refer to the
link:{javadoc_base_url}/org/apache/ignite/IgniteJdbcThinDataSource.html[JavaDocs]
for more details.
+== Transaction Savepoints
+
+JDBC Thin Driver supports the standard JDBC savepoint API for explicit
transactions:
+
+* `Connection.setSavepoint()`
+* `Connection.setSavepoint(String name)`
+* `Connection.rollback(Savepoint savepoint)`
+* `Connection.releaseSavepoint(Savepoint savepoint)`
+
+Savepoints are available for JDBC connections that use the Calcite-based SQL
engine and explicit `PESSIMISTIC` transactions.
+Disable auto-commit before creating a savepoint.
+
+[source,java]
+
+try (Connection conn = DriverManager.getConnection(
+"jdbc:ignite:thin://127.0.0.1?transactionConcurrency=PESSIMISTIC")) {
+conn.setAutoCommit(false);
+
+try (Statement stmt = conn.createStatement()) {
+stmt.executeUpdate("INSERT INTO Person(id, name) VALUES (1, 'John')");
+
+Savepoint savepoint = conn.setSavepoint("before_update");
+
+stmt.executeUpdate("UPDATE Person SET name = 'Jane' WHERE id = 1");
+
+conn.rollback(savepoint);
+conn.releaseSavepoint(savepoint);
Review Comment:
I think current exception message is not enough ?
**Transaction cannot be rolled back explicitly in auto-commit mode.**
It need to be extended like :
Transaction cannot be rolled back explicitly in auto-commit mode, Set {@code
Connection#setAutoCommit} to {@code false} if you want ... smth like this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] IGNITE-28649 Support JDBC API for savepoint [ignite]
zstan commented on code in PR #13112:
URL: https://github.com/apache/ignite/pull/13112#discussion_r3226641736
##
docs/_docs/SQL/JDBC/jdbc-driver.adoc:
##
@@ -555,6 +555,46 @@ In addition to generic DataSource properties,
`IgniteJdbcThinDataSource` support
Refer to the
link:{javadoc_base_url}/org/apache/ignite/IgniteJdbcThinDataSource.html[JavaDocs]
for more details.
+== Transaction Savepoints
+
+JDBC Thin Driver supports the standard JDBC savepoint API for explicit
transactions:
+
+* `Connection.setSavepoint()`
+* `Connection.setSavepoint(String name)`
+* `Connection.rollback(Savepoint savepoint)`
+* `Connection.releaseSavepoint(Savepoint savepoint)`
+
+Savepoints are available for JDBC connections that use the Calcite-based SQL
engine and explicit `PESSIMISTIC` transactions.
+Disable auto-commit before creating a savepoint.
+
+[source,java]
+
+try (Connection conn = DriverManager.getConnection(
+"jdbc:ignite:thin://127.0.0.1?transactionConcurrency=PESSIMISTIC")) {
+conn.setAutoCommit(false);
+
+try (Statement stmt = conn.createStatement()) {
+stmt.executeUpdate("INSERT INTO Person(id, name) VALUES (1, 'John')");
+
+Savepoint savepoint = conn.setSavepoint("before_update");
+
+stmt.executeUpdate("UPDATE Person SET name = 'Jane' WHERE id = 1");
+
+conn.rollback(savepoint);
+conn.releaseSavepoint(savepoint);
Review Comment:
I think current exception message is not enough ?
**Transaction cannot be rolled back explicitly in auto-commit mode.**
It need to be extended like :
Transaction cannot be rolled back explicitly in auto-commit mode, Set {@code
Connection#setAutoCommit} to {@code false} if you want ... smth like this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] IGNITE-28649 Support JDBC API for savepoint [ignite]
sonarqubecloud[bot] commented on PR #13112: URL: https://github.com/apache/ignite/pull/13112#issuecomment-4431162174 ## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=13112) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_ignite&pullRequest=13112&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_ignite&pullRequest=13112&issueStatuses=ACCEPTED) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_ignite&pullRequest=13112&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=13112&metric=new_coverage&view=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=13112&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=13112) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] IGNITE-28649 Support JDBC API for savepoint [ignite]
zstan commented on code in PR #13112:
URL: https://github.com/apache/ignite/pull/13112#discussion_r3226668169
##
docs/_docs/SQL/JDBC/jdbc-driver.adoc:
##
@@ -555,6 +555,46 @@ In addition to generic DataSource properties,
`IgniteJdbcThinDataSource` support
Refer to the
link:{javadoc_base_url}/org/apache/ignite/IgniteJdbcThinDataSource.html[JavaDocs]
for more details.
+== Transaction Savepoints
+
+JDBC Thin Driver supports the standard JDBC savepoint API for explicit
transactions:
+
+* `Connection.setSavepoint()`
+* `Connection.setSavepoint(String name)`
+* `Connection.rollback(Savepoint savepoint)`
+* `Connection.releaseSavepoint(Savepoint savepoint)`
+
+Savepoints are available for JDBC connections that use the Calcite-based SQL
engine and explicit `PESSIMISTIC` transactions.
+Disable auto-commit before creating a savepoint.
Review Comment:
It need to be smth like :
If auto-commit is left enabled, JDBC savepoint API calls requiring an
explicit transaction will fail with an SQLException.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] IGNITE-28649 Support JDBC API for savepoint [ignite]
zstan commented on code in PR #13112:
URL: https://github.com/apache/ignite/pull/13112#discussion_r3226641736
##
docs/_docs/SQL/JDBC/jdbc-driver.adoc:
##
@@ -555,6 +555,46 @@ In addition to generic DataSource properties,
`IgniteJdbcThinDataSource` support
Refer to the
link:{javadoc_base_url}/org/apache/ignite/IgniteJdbcThinDataSource.html[JavaDocs]
for more details.
+== Transaction Savepoints
+
+JDBC Thin Driver supports the standard JDBC savepoint API for explicit
transactions:
+
+* `Connection.setSavepoint()`
+* `Connection.setSavepoint(String name)`
+* `Connection.rollback(Savepoint savepoint)`
+* `Connection.releaseSavepoint(Savepoint savepoint)`
+
+Savepoints are available for JDBC connections that use the Calcite-based SQL
engine and explicit `PESSIMISTIC` transactions.
+Disable auto-commit before creating a savepoint.
+
+[source,java]
+
+try (Connection conn = DriverManager.getConnection(
+"jdbc:ignite:thin://127.0.0.1?transactionConcurrency=PESSIMISTIC")) {
+conn.setAutoCommit(false);
+
+try (Statement stmt = conn.createStatement()) {
+stmt.executeUpdate("INSERT INTO Person(id, name) VALUES (1, 'John')");
+
+Savepoint savepoint = conn.setSavepoint("before_update");
+
+stmt.executeUpdate("UPDATE Person SET name = 'Jane' WHERE id = 1");
+
+conn.rollback(savepoint);
+conn.releaseSavepoint(savepoint);
Review Comment:
I think thid exception message is not enough ?
Transaction cannot be rolled back explicitly in auto-commit mode.
And need to be extended like :
Transaction cannot be rolled back explicitly in auto-commit mode, Set {@code
Connection#setAutoCommit} to {@code false} if you want ... smth like this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] IGNITE-28649 Support JDBC API for savepoint [ignite]
sonarqubecloud[bot] commented on PR #13112: URL: https://github.com/apache/ignite/pull/13112#issuecomment-4429084014 ## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=13112) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_ignite&pullRequest=13112&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_ignite&pullRequest=13112&issueStatuses=ACCEPTED) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_ignite&pullRequest=13112&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=13112&metric=new_coverage&view=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=13112&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=13112) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] IGNITE-28649 Support JDBC API for savepoint [ignite]
vldpyatkov commented on code in PR #13112:
URL: https://github.com/apache/ignite/pull/13112#discussion_r3224872714
##
docs/_docs/SQL/JDBC/jdbc-driver.adoc:
##
@@ -555,6 +555,46 @@ In addition to generic DataSource properties,
`IgniteJdbcThinDataSource` support
Refer to the
link:{javadoc_base_url}/org/apache/ignite/IgniteJdbcThinDataSource.html[JavaDocs]
for more details.
+== Transaction Savepoints
+
+JDBC Thin Driver supports the standard JDBC savepoint API for explicit
transactions:
+
+* `Connection.setSavepoint()`
+* `Connection.setSavepoint(String name)`
+* `Connection.rollback(Savepoint savepoint)`
+* `Connection.releaseSavepoint(Savepoint savepoint)`
+
+Savepoints are available for JDBC connections that use the Calcite-based SQL
engine and explicit `PESSIMISTIC` transactions.
+Disable auto-commit before creating a savepoint.
Review Comment:
Added note:
`If auto-commit remains enabled, JDBC savepoint API calls that require an
explicit transaction fail with `SQLException`.`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] IGNITE-28649 Support JDBC API for savepoint [ignite]
vldpyatkov commented on code in PR #13112:
URL: https://github.com/apache/ignite/pull/13112#discussion_r3224833430
##
docs/_docs/SQL/JDBC/jdbc-driver.adoc:
##
@@ -555,6 +555,46 @@ In addition to generic DataSource properties,
`IgniteJdbcThinDataSource` support
Refer to the
link:{javadoc_base_url}/org/apache/ignite/IgniteJdbcThinDataSource.html[JavaDocs]
for more details.
+== Transaction Savepoints
+
+JDBC Thin Driver supports the standard JDBC savepoint API for explicit
transactions:
+
+* `Connection.setSavepoint()`
+* `Connection.setSavepoint(String name)`
+* `Connection.rollback(Savepoint savepoint)`
+* `Connection.releaseSavepoint(Savepoint savepoint)`
+
+Savepoints are available for JDBC connections that use the Calcite-based SQL
engine and explicit `PESSIMISTIC` transactions.
+Disable auto-commit before creating a savepoint.
+
+[source,java]
+
+try (Connection conn = DriverManager.getConnection(
+"jdbc:ignite:thin://127.0.0.1?transactionConcurrency=PESSIMISTIC")) {
+conn.setAutoCommit(false);
+
+try (Statement stmt = conn.createStatement()) {
+stmt.executeUpdate("INSERT INTO Person(id, name) VALUES (1, 'John')");
+
+Savepoint savepoint = conn.setSavepoint("before_update");
+
+stmt.executeUpdate("UPDATE Person SET name = 'Jane' WHERE id = 1");
+
+conn.rollback(savepoint);
+conn.releaseSavepoint(savepoint);
Review Comment:
Added a check that it is not possible to roll back on an already rolled back
savepoint.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] IGNITE-28649 Support JDBC API for savepoint [ignite]
zstan commented on code in PR #13112:
URL: https://github.com/apache/ignite/pull/13112#discussion_r3208122389
##
docs/_docs/SQL/JDBC/jdbc-driver.adoc:
##
@@ -555,6 +555,46 @@ In addition to generic DataSource properties,
`IgniteJdbcThinDataSource` support
Refer to the
link:{javadoc_base_url}/org/apache/ignite/IgniteJdbcThinDataSource.html[JavaDocs]
for more details.
+== Transaction Savepoints
+
+JDBC Thin Driver supports the standard JDBC savepoint API for explicit
transactions:
+
+* `Connection.setSavepoint()`
+* `Connection.setSavepoint(String name)`
+* `Connection.rollback(Savepoint savepoint)`
+* `Connection.releaseSavepoint(Savepoint savepoint)`
+
+Savepoints are available for JDBC connections that use the Calcite-based SQL
engine and explicit `PESSIMISTIC` transactions.
+Disable auto-commit before creating a savepoint.
Review Comment:
Otherwize what ?
##
modules/clients/src/test/java/org/apache/ignite/jdbc/thin/JdbcThinConnectionSelfTest.java:
##
@@ -1695,11 +1695,17 @@ public void testReleaseSavepoint() throws Exception {
final Savepoint savepoint = getFakeSavepoint();
-checkNotSupported(new RunnableX() {
-@Override public void runx() throws Exception {
-conn.releaseSavepoint(savepoint);
-}
-});
+assertThrows(log,
+new Callable() {
Review Comment:
```suggestion
new Callable<>() {
```
##
docs/_docs/SQL/JDBC/jdbc-driver.adoc:
##
@@ -555,6 +555,46 @@ In addition to generic DataSource properties,
`IgniteJdbcThinDataSource` support
Refer to the
link:{javadoc_base_url}/org/apache/ignite/IgniteJdbcThinDataSource.html[JavaDocs]
for more details.
+== Transaction Savepoints
+
+JDBC Thin Driver supports the standard JDBC savepoint API for explicit
transactions:
+
+* `Connection.setSavepoint()`
+* `Connection.setSavepoint(String name)`
+* `Connection.rollback(Savepoint savepoint)`
+* `Connection.releaseSavepoint(Savepoint savepoint)`
+
+Savepoints are available for JDBC connections that use the Calcite-based SQL
engine and explicit `PESSIMISTIC` transactions.
+Disable auto-commit before creating a savepoint.
+
+[source,java]
+
+try (Connection conn = DriverManager.getConnection(
+"jdbc:ignite:thin://127.0.0.1?transactionConcurrency=PESSIMISTIC")) {
+conn.setAutoCommit(false);
+
+try (Statement stmt = conn.createStatement()) {
+stmt.executeUpdate("INSERT INTO Person(id, name) VALUES (1, 'John')");
+
+Savepoint savepoint = conn.setSavepoint("before_update");
+
+stmt.executeUpdate("UPDATE Person SET name = 'Jane' WHERE id = 1");
+
+conn.rollback(savepoint);
+conn.releaseSavepoint(savepoint);
Review Comment:
from this example is not clear - what was the final state of row **name**
where **id** = 1, can you extend this test plz ? I suppose pure java:
**assert** expectation would be enough here
##
modules/clients/src/test/java/org/apache/ignite/jdbc/thin/JdbcThinConnectionSavepointTest.java:
##
@@ -0,0 +1,256 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.jdbc.thin;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.sql.Savepoint;
+import java.sql.Statement;
+import java.util.Arrays;
+import java.util.List;
+import org.apache.ignite.calcite.CalciteQueryEngineConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.configuration.SqlConfiguration;
+import org.apache.ignite.configuration.TransactionConfiguration;
+import org.junit.Test;
+
+/** Savepoint tests for thin JDBC connection. */
+public class JdbcThinConnectionSavepointTest extends JdbcThinAbstractSelfTest {
+/** */
+private static final String TBL = "SAVEPOINT_TEST_TABLE";
+
+/** URL. */
+private String url = partitionAwareness ?
+"jdbc:ignite:thin://127.0.0.1:10800..10802" :
+"jdbc:ignite:thin://127.0.0.1";
+
+/** Nodes count. */
+private int nodesCnt = partitionAwareness ? 4 : 2;
+
+/** {@inheritDoc} */
+@Override
Re: [PR] IGNITE-28649 Support JDBC API for savepoint [ignite]
zstan commented on PR #13112: URL: https://github.com/apache/ignite/pull/13112#issuecomment-4405916701 what about JdbcThinFeature and correct before\after savepoint jdbc vs old\new savepoint cluster correct handling tests ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] IGNITE-28649 Support JDBC API for savepoint [ignite]
sonarqubecloud[bot] commented on PR #13112: URL: https://github.com/apache/ignite/pull/13112#issuecomment-4397056541 ## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=13112) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_ignite&pullRequest=13112&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_ignite&pullRequest=13112&issueStatuses=ACCEPTED) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_ignite&pullRequest=13112&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=13112&metric=new_coverage&view=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=13112&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=13112) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
