xtern commented on code in PR #4814:
URL: https://github.com/apache/ignite-3/pull/4814#discussion_r1870590505


##########
modules/client-handler/src/main/java/org/apache/ignite/client/handler/JdbcQueryEventHandlerImpl.java:
##########
@@ -415,5 +463,59 @@ void close() {
                 finishTransactionAsync(false);
             }
         }
+
+        CancellationToken registerExecution(long token) {
+            CancelHandle handle = CancelHandle.create();
+
+            CancelHandle previousHandle = cancelHandles.putIfAbsent(token, 
handle);
+
+            assert previousHandle == null;
+
+            return handle.token();
+        }
+
+        void deregisterExecution(long token) {
+            cancelHandles.remove(token);
+        }
+
+        CompletableFuture<Void> cancelExecution(long token) {
+            CancelHandle handle = cancelHandles.remove(token);
+
+            if (handle == null) {
+                return nullCompletedFuture();
+            }
+
+            return handle.cancelAsync();
+        }
+    }
+
+    private void doWhenAllCursorsComplete(
+            CompletableFuture<AsyncSqlCursor<InternalSqlRow>> cursorFuture, 
Runnable action
+    ) {
+        List<CompletableFuture<?>> dependency = new ArrayList<>();
+        var cursorChainTraverser = new Function<AsyncSqlCursor<?>, 
CompletableFuture<AsyncSqlCursor<?>>>() {
+            @Override
+            public CompletableFuture<AsyncSqlCursor<?>> 
apply(AsyncSqlCursor<?> cursor) {
+                dependency.add(cursor.onClose());
+
+                if (cursor.hasNextResult()) {
+                    return cursor.nextResult().thenCompose(this);
+                }
+
+                return allOf(dependency)
+                        .thenRun(action)
+                        .thenApply(ignored -> cursor);
+            }
+        };
+
+        cursorFuture
+                .thenCompose(cursorChainTraverser)
+                .handle((ignored, ex) -> {
+                    if (ex != null) {
+                        action.run();
+                    }
+
+                    return null;
+                });

Review Comment:
   ```suggestion
                   .exceptionally(e -> {
                       action.run();
   
                       return null;
                   });
   ```



##########
modules/jdbc/src/integrationTest/java/org/apache/ignite/jdbc/ItJdbcStatementCancelSelfTest.java:
##########
@@ -17,140 +17,221 @@
 
 package org.apache.ignite.jdbc;
 
+import static org.apache.ignite.internal.testframework.IgniteTestUtils.await;
+import static 
org.apache.ignite.internal.testframework.IgniteTestUtils.runAsync;
+import static 
org.apache.ignite.internal.testframework.IgniteTestUtils.waitForCondition;
+import static 
org.apache.ignite.jdbc.util.JdbcTestUtils.assertThrowsSqlException;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.junit.jupiter.api.Assertions.fail;
 
+import java.sql.PreparedStatement;
 import java.sql.ResultSet;
+import java.sql.SQLException;
 import java.sql.Statement;
-import org.apache.ignite.jdbc.util.JdbcTestUtils;
-import org.junit.jupiter.api.Disabled;
+import java.util.concurrent.CompletableFuture;
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Test;
 
 /**
  * Statement cancel test.
  */
-@Disabled("https://issues.apache.org/jira/browse/IGNITE-16205";)
-public class ItJdbcStatementCancelSelfTest extends 
ItJdbcAbstractStatementSelfTest {
-    /**
-     * Trying to cancel stament without query. In given case cancel is noop, 
so no exception expected.
-     */
-    @Test
-    public void testCancelingStmtWithoutQuery() {
-        try {
-            stmt.cancel();
-        } catch (Exception e) {
-            log.error("Unexpected exception.", e);
-
-            fail("Unexpected exception");
+@SuppressWarnings({"ThrowableNotThrown", 
"JDBCResourceOpenedButNotSafelyClosed"})
+public class ItJdbcStatementCancelSelfTest extends AbstractJdbcSelfTest {
+    @AfterEach
+    void reset() throws SQLException {
+        if (!stmt.isClosed()) {
+            stmt.setFetchSize(1024);
         }
+
+        dropAllTables();
     }
 
-    /**
-     * Trying to retrieve result set of a canceled query.
-     * SQLException with message "The query was cancelled while executing." 
expected.
-     *
-     * @throws Exception If failed.
-     */
     @Test
-    public void testResultSetRetrievalInCanceledStatement() throws Exception {
-        stmt.execute("SELECT 1; SELECT 2; SELECT 3;");
+    void cancelIsNoopWhenThereIsNoRunningQuery() throws SQLException {
+        stmt.cancel();
+    }
 
-        assertNotNull(stmt.getResultSet());
+    @Test
+    void cancellationOfLongRunningQuery() throws Exception {
+        CompletableFuture<?> result = runAsync(() ->
+                stmt.executeQuery("SELECT count(*) FROM system_range(0, 
10000000000)")
+        );
+
+        assertTrue(
+                waitForCondition(() -> sql("SELECT * FROM 
system.sql_queries").size() == 2, 5_000),
+                "Query didn't appear in running queries view or disappeared 
too fast"
+        );
 
         stmt.cancel();
 
-        JdbcTestUtils.assertThrowsSqlException("The query was cancelled while 
executing.", stmt::getResultSet);
+        // second cancellation should not throw any error
+        stmt.cancel();
+
+        assertThrowsSqlException(
+                "The query was cancelled while executing.",
+                () -> await(result)
+        );
     }
 
-    /**
-     * Trying to cancel already cancelled query.
-     * No exceptions exceped.
-     *
-     * @throws Exception If failed.
-     */
     @Test
-    public void testCancelCanceledQuery() throws Exception {
-        stmt.execute("SELECT 1;");
+    void cancellationOfMultiStatementQuery() throws Exception {
+        stmt.executeUpdate("CREATE TABLE dummy (id INT PRIMARY KEY, val INT)");
+        stmt.setFetchSize(1);
 
-        assertNotNull(stmt.getResultSet());
+        stmt.execute("START TRANSACTION;"
+                + "SELECT x FROM system_range(0, 100000) ORDER BY x;" // 
result should be big enough, so it doesn't fit into a single page
+                + "COMMIT;" // script processing is expected to hung on COMMIT 
until all cursors have been closed
+                + "INSERT INTO dummy VALUES (1, 1);");
 
-        stmt.cancel();
+        stmt.getMoreResults(); // move to SELECT
+
+        ResultSet rs = stmt.getResultSet();
+
+        assertNotNull(rs);
+
+        assertTrue(rs.next());
+        assertEquals(0, rs.getInt(1));
+        assertTrue(rs.next());
+        assertEquals(1, rs.getInt(1));
 
         stmt.cancel();
 
-        JdbcTestUtils.assertThrowsSqlException("The query was cancelled while 
executing.", stmt::getResultSet);
+        assertThrowsSqlException(
+                "The query was cancelled while executing.",

Review Comment:
   may be it's better to use const QueryCancelledException.CANCEL_MSG in all 
similar places



##########
modules/jdbc/src/integrationTest/java/org/apache/ignite/jdbc/ItJdbcStatementCancelSelfTest.java:
##########
@@ -17,140 +17,221 @@
 
 package org.apache.ignite.jdbc;
 
+import static org.apache.ignite.internal.testframework.IgniteTestUtils.await;
+import static 
org.apache.ignite.internal.testframework.IgniteTestUtils.runAsync;
+import static 
org.apache.ignite.internal.testframework.IgniteTestUtils.waitForCondition;
+import static 
org.apache.ignite.jdbc.util.JdbcTestUtils.assertThrowsSqlException;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.junit.jupiter.api.Assertions.fail;
 
+import java.sql.PreparedStatement;
 import java.sql.ResultSet;
+import java.sql.SQLException;
 import java.sql.Statement;
-import org.apache.ignite.jdbc.util.JdbcTestUtils;
-import org.junit.jupiter.api.Disabled;
+import java.util.concurrent.CompletableFuture;
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Test;
 
 /**
  * Statement cancel test.
  */
-@Disabled("https://issues.apache.org/jira/browse/IGNITE-16205";)
-public class ItJdbcStatementCancelSelfTest extends 
ItJdbcAbstractStatementSelfTest {
-    /**
-     * Trying to cancel stament without query. In given case cancel is noop, 
so no exception expected.
-     */
-    @Test
-    public void testCancelingStmtWithoutQuery() {
-        try {
-            stmt.cancel();
-        } catch (Exception e) {
-            log.error("Unexpected exception.", e);
-
-            fail("Unexpected exception");
+@SuppressWarnings({"ThrowableNotThrown", 
"JDBCResourceOpenedButNotSafelyClosed"})
+public class ItJdbcStatementCancelSelfTest extends AbstractJdbcSelfTest {
+    @AfterEach
+    void reset() throws SQLException {
+        if (!stmt.isClosed()) {
+            stmt.setFetchSize(1024);
         }
+
+        dropAllTables();
     }
 
-    /**
-     * Trying to retrieve result set of a canceled query.
-     * SQLException with message "The query was cancelled while executing." 
expected.
-     *
-     * @throws Exception If failed.
-     */
     @Test
-    public void testResultSetRetrievalInCanceledStatement() throws Exception {
-        stmt.execute("SELECT 1; SELECT 2; SELECT 3;");
+    void cancelIsNoopWhenThereIsNoRunningQuery() throws SQLException {
+        stmt.cancel();
+    }
 
-        assertNotNull(stmt.getResultSet());
+    @Test
+    void cancellationOfLongRunningQuery() throws Exception {
+        CompletableFuture<?> result = runAsync(() ->
+                stmt.executeQuery("SELECT count(*) FROM system_range(0, 
10000000000)")
+        );
+
+        assertTrue(
+                waitForCondition(() -> sql("SELECT * FROM 
system.sql_queries").size() == 2, 5_000),
+                "Query didn't appear in running queries view or disappeared 
too fast"
+        );
 
         stmt.cancel();
 
-        JdbcTestUtils.assertThrowsSqlException("The query was cancelled while 
executing.", stmt::getResultSet);
+        // second cancellation should not throw any error
+        stmt.cancel();
+
+        assertThrowsSqlException(
+                "The query was cancelled while executing.",

Review Comment:
   ```suggestion
                   QueryCancelledException.CANCEL_MSG,
   ```
   :thinking: 



-- 
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: notifications-unsubscr...@ignite.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to