You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by zs...@apache.org on 2023/10/27 04:48:33 UTC
[ignite-3] branch main updated: IGNITE-20712 Throws correct error for DML within RO transaction (#2740)
This is an automated email from the ASF dual-hosted git repository.
zstan pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new 6b6f0cb610 IGNITE-20712 Throws correct error for DML within RO transaction (#2740)
6b6f0cb610 is described below
commit 6b6f0cb6109d7906a980a31522a1b9c36f88e43a
Author: ygerzhedovich <41...@users.noreply.github.com>
AuthorDate: Fri Oct 27 07:48:27 2023 +0300
IGNITE-20712 Throws correct error for DML within RO transaction (#2740)
---
.../apache/ignite/jdbc/ItJdbcErrorsSelfTest.java | 35 ++++++++--------------
.../ignite/internal/sql/api/ItSqlApiBaseTest.java | 16 +++++++---
.../internal/sql/engine/SqlQueryProcessor.java | 19 ++++++++----
.../sql/engine/exec/ExchangeServiceImpl.java | 6 ++--
.../engine/QueryTransactionWrapperSelfTest.java | 11 -------
5 files changed, 41 insertions(+), 46 deletions(-)
diff --git a/modules/jdbc/src/integrationTest/java/org/apache/ignite/jdbc/ItJdbcErrorsSelfTest.java b/modules/jdbc/src/integrationTest/java/org/apache/ignite/jdbc/ItJdbcErrorsSelfTest.java
index 3cda98d6ff..6a19c5d517 100644
--- a/modules/jdbc/src/integrationTest/java/org/apache/ignite/jdbc/ItJdbcErrorsSelfTest.java
+++ b/modules/jdbc/src/integrationTest/java/org/apache/ignite/jdbc/ItJdbcErrorsSelfTest.java
@@ -22,8 +22,6 @@ import static org.apache.ignite.internal.jdbc.proto.SqlStateCode.INVALID_TRANSAC
import static org.apache.ignite.internal.jdbc.proto.SqlStateCode.UNSUPPORTED_OPERATION;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.junit.jupiter.api.Assertions.fail;
import java.sql.BatchUpdateException;
import java.sql.Connection;
@@ -117,25 +115,21 @@ public class ItJdbcErrorsSelfTest extends ItJdbcErrorsAbstractSelfTest {
*/
@Test
public void testBatchUpdateException() throws SQLException {
- try {
- stmt.executeUpdate("CREATE TABLE test2 (id int primary key, val varchar)");
- stmt.addBatch("insert into test2 (id, val) values (1, 'val1')");
- stmt.addBatch("insert into test2 (id, val) values (2, 'val2')");
- stmt.addBatch("insert into test2 (id1, val1) values (3, 'val3')");
+ stmt.executeUpdate("CREATE TABLE test2 (id int primary key, val varchar)");
- stmt.executeBatch();
+ stmt.addBatch("insert into test2 (id, val) values (1, 'val1')");
+ stmt.addBatch("insert into test2 (id, val) values (2, 'val2')");
+ stmt.addBatch("insert into test2 (id1, val1) values (3, 'val3')");
- fail("BatchUpdateException is expected");
- } catch (BatchUpdateException e) {
- assertEquals(2, e.getUpdateCounts().length);
+ BatchUpdateException batchException = JdbcTestUtils.assertThrowsSqlException(
+ BatchUpdateException.class,
+ "Unknown target column 'ID1'",
+ () -> stmt.executeBatch());
- assertArrayEquals(new int[] {1, 1}, e.getUpdateCounts());
+ assertEquals(2, batchException.getUpdateCounts().length);
- assertTrue(e.getMessage() != null
- && e.getMessage().contains("Unknown target column 'ID1'"),
- "Unexpected error message: " + e.getMessage());
- }
+ assertArrayEquals(new int[]{1, 1}, batchException.getUpdateCounts());
}
/**
@@ -148,13 +142,8 @@ public class ItJdbcErrorsSelfTest extends ItJdbcErrorsAbstractSelfTest {
conn.setAutoCommit(false);
try (Statement stmt = conn.createStatement()) {
- stmt.executeUpdate("CREATE TABLE test2 (id int primary key, val varchar)");
-
- fail("SQLException is expected");
- } catch (SQLException e) {
- assertTrue(e.getMessage() != null
- && e.getMessage().contains("DDL doesn't support transactions."),
- "Unexpected error message: " + e.getMessage());
+ JdbcTestUtils.assertThrowsSqlException("DDL doesn't support transactions.",
+ () -> stmt.executeUpdate("CREATE TABLE test2 (id int primary key, val varchar)"));
}
}
diff --git a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlApiBaseTest.java b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlApiBaseTest.java
index 65dd25b2da..b6cfb185b7 100644
--- a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlApiBaseTest.java
+++ b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlApiBaseTest.java
@@ -360,18 +360,26 @@ public abstract class ItSqlApiBaseTest extends BaseSqlIntegrationTest {
}
private void checkTx(Session ses, boolean readOnly, boolean commit, boolean explicit, Matcher<String> planMatcher) {
- Transaction outerTx = explicit ? (readOnly ? igniteTx().begin(new TransactionOptions().readOnly(true)) : igniteTx().begin()) : null;
+ Transaction outerTx = explicit ? igniteTx().begin(new TransactionOptions().readOnly(readOnly)) : null;
- String query = "SELECT VAL0 FROM TEST ORDER BY VAL0";
+ String queryRo = "SELECT VAL0 FROM TEST ORDER BY VAL0";
- assertQuery(outerTx, query).matches(planMatcher).check();
+ assertQuery(outerTx, queryRo).matches(planMatcher).check();
- ResultSet<SqlRow> rs = executeForRead(ses, outerTx, query);
+ ResultSet<SqlRow> rs = executeForRead(ses, outerTx, queryRo);
assertEquals(ROW_COUNT, asStream(rs).count());
rs.close();
+ String queryRw = "UPDATE TEST SET VAL0=VAL0+1";
+ if (explicit && readOnly) {
+ assertThrowsSqlException(Sql.STMT_VALIDATION_ERR, "DML query cannot be started by using read only transactions.",
+ () -> execute(outerTx, ses, queryRw));
+ } else {
+ checkDml(ROW_COUNT, outerTx, ses, queryRw);
+ }
+
if (outerTx != null) {
if (commit) {
outerTx.commit();
diff --git a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/SqlQueryProcessor.java b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/SqlQueryProcessor.java
index 8ff3359a0b..425d791d8e 100644
--- a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/SqlQueryProcessor.java
+++ b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/SqlQueryProcessor.java
@@ -443,7 +443,7 @@ public class SqlQueryProcessor implements QueryProcessor {
CompletableFuture<AsyncSqlCursor<List<Object>>> stage = start.thenCompose(ignored -> {
ParsedResult result = parserService.parse(sql);
- validateParsedStatement(context, result, params);
+ validateParsedStatement(context, result, params, outerTx);
QueryTransactionWrapper txWrapper = wrapTxOrStartImplicit(result.queryType(), transactions, outerTx);
@@ -556,10 +556,6 @@ public class SqlQueryProcessor implements QueryProcessor {
return new QueryTransactionWrapper(tx, true);
}
- if (SqlQueryType.DDL == queryType) {
- throw new SqlException(STMT_VALIDATION_ERR, "DDL doesn't support transactions.");
- }
-
return new QueryTransactionWrapper(outerTx, false);
}
@@ -572,11 +568,22 @@ public class SqlQueryProcessor implements QueryProcessor {
private static void validateParsedStatement(
QueryContext context,
ParsedResult parsedResult,
- Object[] params
+ Object[] params,
+ InternalTransaction outerTx
) {
Set<SqlQueryType> allowedTypes = context.allowedQueryTypes();
SqlQueryType queryType = parsedResult.queryType();
+ if (outerTx != null) {
+ if (SqlQueryType.DDL == queryType) {
+ throw new SqlException(STMT_VALIDATION_ERR, "DDL doesn't support transactions.");
+ }
+
+ if (SqlQueryType.DML == queryType && outerTx.isReadOnly()) {
+ throw new SqlException(STMT_VALIDATION_ERR, "DML query cannot be started by using read only transactions.");
+ }
+ }
+
if (parsedResult.queryType() == SqlQueryType.TX_CONTROL) {
String message = "Transaction control statement can not be executed as an independent statement";
diff --git a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExchangeServiceImpl.java b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExchangeServiceImpl.java
index 859670c7b9..b9d13cad3e 100644
--- a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExchangeServiceImpl.java
+++ b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExchangeServiceImpl.java
@@ -117,9 +117,11 @@ public class ExchangeServiceImpl implements ExchangeService {
}
if (!(traceableErr instanceof QueryCancelledException)) {
- LOG.info(format("Failed to execute query fragment: queryId={}, fragmentId={}", queryId, fragmentId), error);
+ LOG.info(format("Failed to execute query fragment: traceId={}, queryId={}, fragmentId={}",
+ ((TraceableException) traceableErr).traceId(), queryId, fragmentId), error);
} else if (LOG.isDebugEnabled()) {
- LOG.debug(format("Failed to execute query fragment: queryId={}, fragmentId={}", queryId, fragmentId), error);
+ LOG.debug(format("Failed to execute query fragment: traceId={}, queryId={}, fragmentId={}",
+ ((TraceableException) traceableErr).traceId(), queryId, fragmentId), error);
}
return messageService.send(
diff --git a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/QueryTransactionWrapperSelfTest.java b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/QueryTransactionWrapperSelfTest.java
index 2e48558339..6e63296207 100644
--- a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/QueryTransactionWrapperSelfTest.java
+++ b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/QueryTransactionWrapperSelfTest.java
@@ -18,7 +18,6 @@
package org.apache.ignite.internal.sql.engine;
import static org.apache.ignite.internal.sql.engine.SqlQueryProcessor.wrapTxOrStartImplicit;
-import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.assertThrowsSqlException;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
@@ -33,7 +32,6 @@ import static org.mockito.Mockito.when;
import java.util.EnumSet;
import org.apache.ignite.internal.sql.engine.framework.NoOpTransaction;
import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest;
-import org.apache.ignite.lang.ErrorGroups;
import org.apache.ignite.tx.IgniteTransactions;
import org.apache.ignite.tx.TransactionOptions;
import org.junit.jupiter.api.Test;
@@ -49,15 +47,6 @@ public class QueryTransactionWrapperSelfTest extends BaseIgniteAbstractTest {
@Mock
private IgniteTransactions transactions;
- @Test
- public void throwsExceptionForDdlWithExternalTransaction() {
- //noinspection ThrowableNotThrown
- assertThrowsSqlException(ErrorGroups.Sql.STMT_VALIDATION_ERR,
- "DDL doesn't support transactions",
- () -> wrapTxOrStartImplicit(SqlQueryType.DDL, transactions, new NoOpTransaction("test")));
- verifyNoInteractions(transactions);
- }
-
@Test
public void testImplicitTransactionAttributes() {
when(transactions.begin(any())).thenAnswer(