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