zstan commented on code in PR #4706:
URL: https://github.com/apache/ignite-3/pull/4706#discussion_r1850016261
##########
modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlSynchronousApiTest.java:
##########
@@ -74,8 +75,23 @@ public void cancelQueryString() throws InterruptedException {
.build();
Transaction transaction = igniteTx().begin();
+
+ transactions.add(transaction);
+
return sql.execute(transaction, token, statement);
});
+
+ // Checks the exception that is thrown if a query is canceled before a
cursor is obtained.
+ CancelHandle cancelHandle = CancelHandle.create();
+ CancellationToken token = cancelHandle.token();
+ cancelHandle.cancel();
+
+ //noinspection resource
+ assertThrowsSqlException(
+ Sql.EXECUTION_CANCELLED_ERR,
+ "The query was cancelled while executing.",
Review Comment:
```suggestion
CANCEL_MSG,
```
##########
modules/sql-engine/src/testFixtures/java/org/apache/ignite/internal/sql/engine/util/SqlTestUtils.java:
##########
@@ -558,4 +560,23 @@ public static void executeUpdate(String query, IgniteSql
sql, @Nullable Transact
public static void executeUpdate(String query, IgniteSql sql) {
executeUpdate(query, sql, null);
}
+
+ /** The action is expected to throw a {@link SqlException} with code
{@link Sql#EXECUTION_CANCELLED_ERR}. */
+ public static void expectQueryCancelled(Executable action) {
+ assertThrowsSqlException(
+ Sql.EXECUTION_CANCELLED_ERR,
+ "The query was cancelled while executing.",
+ action
+ );
+ }
+
+ /** The action is expected to throw a {@link QueryCancelledException}. */
+ public static void expectQueryCancelledInternalException(Executable
action) {
+ //noinspection ThrowableNotThrown
+ IgniteTestUtils.assertThrows(
+ QueryCancelledException.class,
+ action,
+ "The query was cancelled while executing."
Review Comment:
```suggestion
CANCEL_MSG
```
##########
modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCancelQueryTest.java:
##########
@@ -18,30 +18,29 @@
package org.apache.ignite.internal.sql.engine;
import static
org.apache.ignite.internal.sql.engine.QueryProperty.ALLOWED_QUERY_TYPES;
+import static
org.apache.ignite.internal.sql.engine.util.SqlTestUtils.expectQueryCancelled;
+import static
org.apache.ignite.internal.sql.engine.util.SqlTestUtils.expectQueryCancelledInternalException;
+import static org.apache.ignite.internal.testframework.IgniteTestUtils.await;
import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertInstanceOf;
-import static org.junit.jupiter.api.Assertions.assertThrows;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.CompletionException;
import org.apache.ignite.internal.TestWrappers;
import org.apache.ignite.internal.app.IgniteImpl;
import org.apache.ignite.internal.sql.BaseSqlIntegrationTest;
import org.apache.ignite.internal.sql.engine.property.SqlProperties;
import org.apache.ignite.internal.sql.engine.property.SqlPropertiesHelper;
import org.apache.ignite.internal.tx.HybridTimestampTracker;
-import org.apache.ignite.internal.util.AsyncCursor.BatchedResult;
import org.apache.ignite.lang.CancelHandle;
import org.apache.ignite.lang.CancellationToken;
-import org.apache.ignite.lang.ErrorGroups.Sql;
-import org.apache.ignite.sql.SqlException;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.function.Executable;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
/** Set of test cases for query cancellation. */
-public class ItQueryCancelTest extends BaseSqlIntegrationTest {
+public class ItCancelQueryTest extends BaseSqlIntegrationTest {
+ private static final String QUERY_CANCELED_ERR = "The query was cancelled
while executing";
Review Comment:
not used
##########
modules/sql-engine/src/testFixtures/java/org/apache/ignite/internal/sql/engine/util/SqlTestUtils.java:
##########
@@ -558,4 +560,23 @@ public static void executeUpdate(String query, IgniteSql
sql, @Nullable Transact
public static void executeUpdate(String query, IgniteSql sql) {
executeUpdate(query, sql, null);
}
+
+ /** The action is expected to throw a {@link SqlException} with code
{@link Sql#EXECUTION_CANCELLED_ERR}. */
+ public static void expectQueryCancelled(Executable action) {
+ assertThrowsSqlException(
+ Sql.EXECUTION_CANCELLED_ERR,
+ "The query was cancelled while executing.",
Review Comment:
```suggestion
CANCEL_MSG
```
##########
modules/sql-engine/src/testFixtures/java/org/apache/ignite/internal/sql/engine/util/SqlTestUtils.java:
##########
@@ -558,4 +560,23 @@ public static void executeUpdate(String query, IgniteSql
sql, @Nullable Transact
public static void executeUpdate(String query, IgniteSql sql) {
executeUpdate(query, sql, null);
}
+
+ /** The action is expected to throw a {@link SqlException} with code
{@link Sql#EXECUTION_CANCELLED_ERR}. */
+ public static void expectQueryCancelled(Executable action) {
+ assertThrowsSqlException(
+ Sql.EXECUTION_CANCELLED_ERR,
+ "The query was cancelled while executing.",
+ action
+ );
+ }
+
+ /** The action is expected to throw a {@link QueryCancelledException}. */
+ public static void expectQueryCancelledInternalException(Executable
action) {
Review Comment:
probably this utility methods need to belong to ItCancelQueryTest ? no one
is used it more.
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/tx/ScriptTransactionContext.java:
##########
@@ -106,7 +116,17 @@ public CompletableFuture<Void>
handleControlStatement(SqlNode node) {
boolean readOnly = ((IgniteSqlStartTransaction) node).getMode() ==
IgniteSqlStartTransactionMode.READ_ONLY;
InternalTransaction tx =
txContext.getOrStartImplicit(readOnly).unwrap();
- this.wrapper = new ScriptTransactionWrapperImpl(tx, txTracker);
+ ScriptTransactionWrapperImpl wrapper0 = new
ScriptTransactionWrapperImpl(tx, txTracker);
+ this.wrapper = wrapper0;
+
+ if (cancellationToken != null) {
+ // If the user cancels the script, we need to rollback the
script transaction.
+ CancelHandleHelper.addCancelAction(
+ cancellationToken,
+ () -> wrapper0.rollback(new QueryCancelledException()),
+ nullCompletedFuture()
Review Comment:
can we change this nullCompletedFuture?
##########
modules/api/src/main/java/org/apache/ignite/sql/IgniteSql.java:
##########
@@ -379,6 +379,16 @@ <T> CompletableFuture<AsyncResultSet<T>> executeAsync(
*/
void executeScript(String query, @Nullable Object... arguments);
+ /**
+ * Executes a multi-statement SQL query.
+ *
+ * @param cancellationToken Cancellation token or {@code null}.
+ * @param query SQL query template.
+ * @param arguments Arguments for the template (optional).
+ * @throws SqlException If failed.
+ */
+ void executeScript(@Nullable CancellationToken cancellationToken, String
query, @Nullable Object... arguments);
Review Comment:
We have no concrete rules for parameters sequence, but i prefer that query
will be first param, wdyt ?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/fsm/QueryExecutor.java:
##########
@@ -202,14 +202,16 @@ CompletableFuture<AsyncSqlCursor<InternalSqlRow>>
executeChildQuery(
}
try {
- trackQuery(query, null);
+ trackQuery(query, parent.cancellationToken);
Review Comment:
seems we need not register additional cancel actions for childs ? i.e. we
need different trackQuery implementation without "addCancelAction" actions
--
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
