xtern commented on code in PR #4706:
URL: https://github.com/apache/ignite-3/pull/4706#discussion_r1850481790
##########
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:
Thanks. Reverted this change.
From my point of view, it is enough to pass null for the child query.
##########
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:
Fixed.
##########
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:
`ItCancelScriptTest` also requires this method,
##########
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:
Fixed, thanks.
--
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
