You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "ygerzhedovich (via GitHub)" <gi...@apache.org> on 2023/09/21 13:07:58 UTC
[GitHub] [ignite-3] ygerzhedovich opened a new pull request, #2613: IGNITE-20387: check on internal Exceptions.
ygerzhedovich opened a new pull request, #2613:
URL: https://github.com/apache/ignite-3/pull/2613
Thank you for submitting the pull request.
To streamline the review process of the patch and ensure better code quality
we ask both an author and a reviewer to verify the following:
### The Review Checklist
- [ ] **Formal criteria:** TC status, codestyle, mandatory documentation. Also make sure to complete the following:
\- There is a single JIRA ticket related to the pull request.
\- The web-link to the pull request is attached to the JIRA ticket.
\- The JIRA ticket has the Patch Available state.
\- The description of the JIRA ticket explains WHAT was made, WHY and HOW.
\- The pull request title is treated as the final commit message. The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
- [ ] **Design:** new code conforms with the design principles of the components it is added to.
- [ ] **Patch quality:** patch cannot be split into smaller pieces, its size must be reasonable.
- [ ] **Code quality:** code is clean and readable, necessary developer documentation is added if needed.
- [ ] **Tests code quality:** test set covers positive/negative scenarios, happy/edge cases. Tests are effective in terms of execution time and resources.
### Notes
- [Apache Ignite Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Java+Code+Style+Guide)
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1350188938
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItTablesApiTest.java:
##########
@@ -140,8 +142,10 @@ public void testTableAlreadyCreated() {
Table tbl = createTable(ignite0, TABLE_NAME);
- // TODO: IGNITE-20388 Fix it
- assertThrowsWithCause(() -> createTable(ignite0, TABLE_NAME), TableExistsValidationException.class);
+ assertThrowsSqlException(
+ Catalog.VALIDATION_ERR,
Review Comment:
Thanks for your attention :)
This is a really big problem that this PR highlighted and that will need to be resolved in a separate ticket that I will open
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1350394279
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlApiBaseTest.java:
##########
@@ -0,0 +1,860 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.sql.api;
+
+import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsIndexScan;
+import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsTableScan;
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.asStream;
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.assertThrowsSqlException;
+import static org.apache.ignite.internal.testframework.IgniteTestUtils.assertThrowsWithCause;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.catalog.commands.CatalogUtils;
+import org.apache.ignite.internal.client.sql.ClientSql;
+import org.apache.ignite.internal.sql.api.ColumnMetadataImpl.ColumnOriginImpl;
+import org.apache.ignite.internal.sql.engine.ClusterPerClassIntegrationTest;
+import org.apache.ignite.internal.sql.engine.QueryCancelledException;
+import org.apache.ignite.internal.testframework.IgniteTestUtils;
+import org.apache.ignite.internal.tx.TxManager;
+import org.apache.ignite.lang.ColumnAlreadyExistsException;
+import org.apache.ignite.lang.ColumnNotFoundException;
+import org.apache.ignite.lang.ErrorGroups;
+import org.apache.ignite.lang.ErrorGroups.Catalog;
+import org.apache.ignite.lang.ErrorGroups.Index;
+import org.apache.ignite.lang.ErrorGroups.Sql;
+import org.apache.ignite.lang.ErrorGroups.Transactions;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.IndexAlreadyExistsException;
+import org.apache.ignite.lang.IndexNotFoundException;
+import org.apache.ignite.lang.TableAlreadyExistsException;
+import org.apache.ignite.lang.TableNotFoundException;
+import org.apache.ignite.sql.BatchedArguments;
+import org.apache.ignite.sql.ColumnMetadata;
+import org.apache.ignite.sql.ColumnType;
+import org.apache.ignite.sql.CursorClosedException;
+import org.apache.ignite.sql.IgniteSql;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+import org.apache.ignite.sql.ResultSet;
+import org.apache.ignite.sql.ResultSetMetadata;
+import org.apache.ignite.sql.Session;
+import org.apache.ignite.sql.SqlBatchException;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.SqlRow;
+import org.apache.ignite.table.Table;
+import org.apache.ignite.tx.Transaction;
+import org.apache.ignite.tx.TransactionOptions;
+import org.hamcrest.Matcher;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests for SQL API.
+ * Tests will be run through synchronous, asynchronous API and client entry points.
+ * By default, any SQL API test should be added to the base class and use special provided methods to interact
+ * with the API in a API-type-independent manner. For any API-specific test, should be used the appropriate subclass.
+ */
+public abstract class ItSqlApiBaseTest extends ClusterPerClassIntegrationTest {
+ protected static final int ROW_COUNT = 16;
+
+ @AfterEach
+ public void dropTables() {
+ for (Table t : CLUSTER_NODES.get(0).tables().tables()) {
+ sql("DROP TABLE " + t.name());
+ }
+ }
+
+ @Test
+ @Disabled("https://issues.apache.org/jira/browse/IGNITE-20096")
+ public void ddl() throws Exception {
+ IgniteSql sql = igniteSql();
+ Session ses = sql.createSession();
+
+ // CREATE TABLE
+ checkDdl(true, ses, "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+ checkError(
+ TableAlreadyExistsException.class,
+ ErrorGroups.Table.TABLE_ALREADY_EXISTS_ERR,
+ "Table already exists [name=\"PUBLIC\".\"TEST\"]",
+ ses,
+ "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)"
+ );
+ checkSqlError(
+ ErrorGroups.Table.TABLE_DEFINITION_ERR,
+ "Can't create table with duplicate columns: ID, VAL, VAL",
+ ses,
+ "CREATE TABLE TEST1(ID INT PRIMARY KEY, VAL INT, VAL INT)"
+ );
+ checkDdl(false, ses, "CREATE TABLE IF NOT EXISTS TEST(ID INT PRIMARY KEY, VAL VARCHAR)");
+
+ // ADD COLUMN
+ checkDdl(true, ses, "ALTER TABLE TEST ADD COLUMN VAL1 VARCHAR");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"NOT_EXISTS_TABLE\"]",
+ ses,
+ "ALTER TABLE NOT_EXISTS_TABLE ADD COLUMN VAL1 VARCHAR"
+ );
+ checkDdl(false, ses, "ALTER TABLE IF EXISTS NOT_EXISTS_TABLE ADD COLUMN VAL1 VARCHAR");
+ checkError(
+ ColumnAlreadyExistsException.class,
+ ErrorGroups.Table.COLUMN_ALREADY_EXISTS_ERR,
+ "Column already exists [name=\"VAL1\"]",
+ ses,
+ "ALTER TABLE TEST ADD COLUMN VAL1 INT"
+ );
+
+ // CREATE INDEX
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX ON TEST(VAL0)");
+ checkError(
+ IndexAlreadyExistsException.class,
+ Index.INDEX_ALREADY_EXISTS_ERR,
+ "Index already exists [name=\"PUBLIC\".\"TEST_IDX\"]",
+ ses,
+ "CREATE INDEX TEST_IDX ON TEST(VAL1)"
+ );
+ checkDdl(false, ses, "CREATE INDEX IF NOT EXISTS TEST_IDX ON TEST(VAL1)");
+
+ // TODO: IGNITE-19150 We are waiting for schema synchronization to avoid races to create and destroy indexes
+ waitForIndexBuild("TEST", "TEST_IDX");
+
+ checkDdl(true, ses, "DROP INDEX TESt_iDX");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX1 ON TEST(VAL0)");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX2 ON TEST(VAL0)");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX3 ON TEST(ID, VAL0, VAL1)");
+ checkSqlError(
+ Index.INVALID_INDEX_DEFINITION_ERR,
+ "Can't create index on duplicate columns: VAL0, VAL0",
+ ses,
+ "CREATE INDEX TEST_IDX4 ON TEST(VAL0, VAL0)"
+ );
+
+ checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column(s). Column VAL1 is used by indexes [TEST_IDX3].",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN val1"
+ );
+
+ SqlException ex = checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column(s).",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN (val0, val1)"
+ );
+
+ String msg = ex.getMessage();
+ String explainMsg = "Unexpected error message: " + msg;
+
+ assertTrue(msg.contains("Column VAL0 is used by indexes ["), explainMsg);
+ assertTrue(msg.contains("TEST_IDX1") && msg.contains("TEST_IDX2") && msg.contains("TEST_IDX3"), explainMsg);
+ assertTrue(msg.contains("Column VAL1 is used by indexes [TEST_IDX3]"), explainMsg);
+
+ checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column, belongs to primary key: [name=ID]",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN id"
+ );
+
+ // TODO: IGNITE-19150 We are waiting for schema synchronization to avoid races to create and destroy indexes
+ waitForIndexBuild("TEST", "TEST_IDX3");
+ checkDdl(true, ses, "DROP INDEX TESt_iDX3");
+
+ // DROP COLUMNS
+ checkDdl(true, ses, "ALTER TABLE TEST DROP COLUMN VAL1");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"NOT_EXISTS_TABLE\"]",
+ ses,
+ "ALTER TABLE NOT_EXISTS_TABLE DROP COLUMN VAL1"
+ );
+ checkDdl(false, ses, "ALTER TABLE IF EXISTS NOT_EXISTS_TABLE DROP COLUMN VAL1");
+ checkError(
+ ColumnNotFoundException.class,
+ ErrorGroups.Table.COLUMN_NOT_FOUND_ERR,
+ "Column does not exist [tableName=\"PUBLIC\".\"TEST\", columnName=\"VAL1\"]",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN VAL1"
+ );
+
+ // DROP TABLE
+ checkDdl(false, ses, "DROP TABLE IF EXISTS NOT_EXISTS_TABLE");
+
+ checkDdl(true, ses, "DROP TABLE TEST");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"TEST\"]",
+ ses,
+ "DROP TABLE TEST"
+ );
+
+ checkDdl(false, ses, "DROP INDEX IF EXISTS TEST_IDX");
+
+ checkError(
+ IndexNotFoundException.class,
+ Index.INDEX_NOT_FOUND_ERR,
+ "Index does not exist [name=\"PUBLIC\".\"TEST_IDX\"]", ses,
+ "DROP INDEX TEST_IDX"
+ );
+ }
+
+ /** Check all transactions are processed correctly even with case of sql Exception raised. */
+ @Test
+ public void implicitTransactionsStates() {
+ IgniteSql sql = igniteSql();
+
+ if (sql instanceof ClientSql) {
+ return;
+ }
+
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+ Session ses = sql.createSession();
+
+ TxManager txManager = txManager();
+
+ for (int i = 0; i < ROW_COUNT; ++i) {
+ assertThrowsSqlException(
+ Catalog.VALIDATION_ERR,
+ "Table with name 'PUBLIC.TEST' already exists",
+ () -> execute(ses, "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)")
+ );
+ }
+
+ // No new transactions through ddl.
+ assertEquals(0, txManager.pending());
+ }
+
+ /** Check correctness of implicit and explicit transactions. */
+ @Test
+ public void checkTransactionsWithDml() {
+ IgniteSql sql = igniteSql();
+
+ if (sql instanceof ClientSql) {
+ return;
+ }
+
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+ Session ses = sql.createSession();
+
+ TxManager txManagerInternal = txManager();
+
+ int txPrevCnt = txManagerInternal.finished();
+
+ for (int i = 0; i < ROW_COUNT; ++i) {
+ checkDml(1, ses, "INSERT INTO TEST VALUES (?, ?)", i, i);
+ }
+
+ // Outer tx with further commit.
+ Transaction outerTx = igniteTx().begin();
+
+ for (int i = ROW_COUNT; i < 2 * ROW_COUNT; ++i) {
+ checkDml(1, outerTx, ses, "INSERT INTO TEST VALUES (?, ?)", i, i);
+ }
+
+ commit(outerTx);
+
+ // Outdated tx.
+ Transaction outerTx0 = outerTx;
+ //ToDo: IGNITE-20387 , here should be used assertThrowsSqlException method with code and message `"Transaction is already finished"
Review Comment:
no, it has been removed )
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1348484039
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlApiBaseTest.java:
##########
@@ -0,0 +1,860 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.sql.api;
+
+import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsIndexScan;
+import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsTableScan;
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.asStream;
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.assertThrowsSqlException;
+import static org.apache.ignite.internal.testframework.IgniteTestUtils.assertThrowsWithCause;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.catalog.commands.CatalogUtils;
+import org.apache.ignite.internal.client.sql.ClientSql;
+import org.apache.ignite.internal.sql.api.ColumnMetadataImpl.ColumnOriginImpl;
+import org.apache.ignite.internal.sql.engine.ClusterPerClassIntegrationTest;
+import org.apache.ignite.internal.sql.engine.QueryCancelledException;
+import org.apache.ignite.internal.testframework.IgniteTestUtils;
+import org.apache.ignite.internal.tx.TxManager;
+import org.apache.ignite.lang.ColumnAlreadyExistsException;
+import org.apache.ignite.lang.ColumnNotFoundException;
+import org.apache.ignite.lang.ErrorGroups;
+import org.apache.ignite.lang.ErrorGroups.Catalog;
+import org.apache.ignite.lang.ErrorGroups.Index;
+import org.apache.ignite.lang.ErrorGroups.Sql;
+import org.apache.ignite.lang.ErrorGroups.Transactions;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.IndexAlreadyExistsException;
+import org.apache.ignite.lang.IndexNotFoundException;
+import org.apache.ignite.lang.TableAlreadyExistsException;
+import org.apache.ignite.lang.TableNotFoundException;
+import org.apache.ignite.sql.BatchedArguments;
+import org.apache.ignite.sql.ColumnMetadata;
+import org.apache.ignite.sql.ColumnType;
+import org.apache.ignite.sql.CursorClosedException;
+import org.apache.ignite.sql.IgniteSql;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+import org.apache.ignite.sql.ResultSet;
+import org.apache.ignite.sql.ResultSetMetadata;
+import org.apache.ignite.sql.Session;
+import org.apache.ignite.sql.SqlBatchException;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.SqlRow;
+import org.apache.ignite.table.Table;
+import org.apache.ignite.tx.Transaction;
+import org.apache.ignite.tx.TransactionOptions;
+import org.hamcrest.Matcher;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests for SQL API.
+ * Tests will be run through synchronous, asynchronous API and client entry points.
+ * By default, any SQL API test should be added to the base class and use special provided methods to interact
+ * with the API in a API-type-independent manner. For any API-specific test, should be used the appropriate subclass.
+ */
+public abstract class ItSqlApiBaseTest extends ClusterPerClassIntegrationTest {
+ protected static final int ROW_COUNT = 16;
+
+ @AfterEach
+ public void dropTables() {
+ for (Table t : CLUSTER_NODES.get(0).tables().tables()) {
+ sql("DROP TABLE " + t.name());
+ }
+ }
+
+ @Test
+ @Disabled("https://issues.apache.org/jira/browse/IGNITE-20096")
+ public void ddl() throws Exception {
+ IgniteSql sql = igniteSql();
+ Session ses = sql.createSession();
+
+ // CREATE TABLE
+ checkDdl(true, ses, "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+ checkError(
+ TableAlreadyExistsException.class,
+ ErrorGroups.Table.TABLE_ALREADY_EXISTS_ERR,
+ "Table already exists [name=\"PUBLIC\".\"TEST\"]",
+ ses,
+ "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)"
+ );
+ checkSqlError(
+ ErrorGroups.Table.TABLE_DEFINITION_ERR,
+ "Can't create table with duplicate columns: ID, VAL, VAL",
+ ses,
+ "CREATE TABLE TEST1(ID INT PRIMARY KEY, VAL INT, VAL INT)"
+ );
+ checkDdl(false, ses, "CREATE TABLE IF NOT EXISTS TEST(ID INT PRIMARY KEY, VAL VARCHAR)");
+
+ // ADD COLUMN
+ checkDdl(true, ses, "ALTER TABLE TEST ADD COLUMN VAL1 VARCHAR");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"NOT_EXISTS_TABLE\"]",
+ ses,
+ "ALTER TABLE NOT_EXISTS_TABLE ADD COLUMN VAL1 VARCHAR"
+ );
+ checkDdl(false, ses, "ALTER TABLE IF EXISTS NOT_EXISTS_TABLE ADD COLUMN VAL1 VARCHAR");
+ checkError(
+ ColumnAlreadyExistsException.class,
+ ErrorGroups.Table.COLUMN_ALREADY_EXISTS_ERR,
+ "Column already exists [name=\"VAL1\"]",
+ ses,
+ "ALTER TABLE TEST ADD COLUMN VAL1 INT"
+ );
+
+ // CREATE INDEX
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX ON TEST(VAL0)");
+ checkError(
+ IndexAlreadyExistsException.class,
+ Index.INDEX_ALREADY_EXISTS_ERR,
+ "Index already exists [name=\"PUBLIC\".\"TEST_IDX\"]",
+ ses,
+ "CREATE INDEX TEST_IDX ON TEST(VAL1)"
+ );
+ checkDdl(false, ses, "CREATE INDEX IF NOT EXISTS TEST_IDX ON TEST(VAL1)");
+
+ // TODO: IGNITE-19150 We are waiting for schema synchronization to avoid races to create and destroy indexes
+ waitForIndexBuild("TEST", "TEST_IDX");
+
+ checkDdl(true, ses, "DROP INDEX TESt_iDX");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX1 ON TEST(VAL0)");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX2 ON TEST(VAL0)");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX3 ON TEST(ID, VAL0, VAL1)");
+ checkSqlError(
+ Index.INVALID_INDEX_DEFINITION_ERR,
+ "Can't create index on duplicate columns: VAL0, VAL0",
+ ses,
+ "CREATE INDEX TEST_IDX4 ON TEST(VAL0, VAL0)"
+ );
+
+ checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column(s). Column VAL1 is used by indexes [TEST_IDX3].",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN val1"
+ );
+
+ SqlException ex = checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column(s).",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN (val0, val1)"
+ );
+
+ String msg = ex.getMessage();
+ String explainMsg = "Unexpected error message: " + msg;
+
+ assertTrue(msg.contains("Column VAL0 is used by indexes ["), explainMsg);
+ assertTrue(msg.contains("TEST_IDX1") && msg.contains("TEST_IDX2") && msg.contains("TEST_IDX3"), explainMsg);
+ assertTrue(msg.contains("Column VAL1 is used by indexes [TEST_IDX3]"), explainMsg);
+
+ checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column, belongs to primary key: [name=ID]",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN id"
+ );
+
+ // TODO: IGNITE-19150 We are waiting for schema synchronization to avoid races to create and destroy indexes
+ waitForIndexBuild("TEST", "TEST_IDX3");
+ checkDdl(true, ses, "DROP INDEX TESt_iDX3");
+
+ // DROP COLUMNS
+ checkDdl(true, ses, "ALTER TABLE TEST DROP COLUMN VAL1");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"NOT_EXISTS_TABLE\"]",
+ ses,
+ "ALTER TABLE NOT_EXISTS_TABLE DROP COLUMN VAL1"
+ );
+ checkDdl(false, ses, "ALTER TABLE IF EXISTS NOT_EXISTS_TABLE DROP COLUMN VAL1");
+ checkError(
+ ColumnNotFoundException.class,
+ ErrorGroups.Table.COLUMN_NOT_FOUND_ERR,
+ "Column does not exist [tableName=\"PUBLIC\".\"TEST\", columnName=\"VAL1\"]",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN VAL1"
+ );
+
+ // DROP TABLE
+ checkDdl(false, ses, "DROP TABLE IF EXISTS NOT_EXISTS_TABLE");
+
+ checkDdl(true, ses, "DROP TABLE TEST");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"TEST\"]",
+ ses,
+ "DROP TABLE TEST"
+ );
+
+ checkDdl(false, ses, "DROP INDEX IF EXISTS TEST_IDX");
+
+ checkError(
+ IndexNotFoundException.class,
+ Index.INDEX_NOT_FOUND_ERR,
+ "Index does not exist [name=\"PUBLIC\".\"TEST_IDX\"]", ses,
+ "DROP INDEX TEST_IDX"
+ );
+ }
+
+ /** Check all transactions are processed correctly even with case of sql Exception raised. */
+ @Test
+ public void implicitTransactionsStates() {
+ IgniteSql sql = igniteSql();
+
+ if (sql instanceof ClientSql) {
+ return;
+ }
+
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+ Session ses = sql.createSession();
+
+ TxManager txManager = txManager();
+
+ for (int i = 0; i < ROW_COUNT; ++i) {
+ assertThrowsSqlException(
+ Catalog.VALIDATION_ERR,
+ "Table with name 'PUBLIC.TEST' already exists",
+ () -> execute(ses, "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)")
+ );
+ }
+
+ // No new transactions through ddl.
+ assertEquals(0, txManager.pending());
+ }
+
+ /** Check correctness of implicit and explicit transactions. */
+ @Test
+ public void checkTransactionsWithDml() {
+ IgniteSql sql = igniteSql();
+
+ if (sql instanceof ClientSql) {
+ return;
+ }
+
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+ Session ses = sql.createSession();
+
+ TxManager txManagerInternal = txManager();
+
+ int txPrevCnt = txManagerInternal.finished();
+
+ for (int i = 0; i < ROW_COUNT; ++i) {
+ checkDml(1, ses, "INSERT INTO TEST VALUES (?, ?)", i, i);
+ }
+
+ // Outer tx with further commit.
+ Transaction outerTx = igniteTx().begin();
+
+ for (int i = ROW_COUNT; i < 2 * ROW_COUNT; ++i) {
+ checkDml(1, outerTx, ses, "INSERT INTO TEST VALUES (?, ?)", i, i);
+ }
+
+ commit(outerTx);
+
+ // Outdated tx.
+ Transaction outerTx0 = outerTx;
+ //ToDo: IGNITE-20387 , here should be used assertThrowsSqlException method with code and message `"Transaction is already finished"
+ IgniteException e = assertThrows(IgniteException.class,
+ () -> checkDml(1, outerTx0, ses, "INSERT INTO TEST VALUES (?, ?)", ROW_COUNT, Integer.MAX_VALUE));
+ assertEquals(Transactions.TX_FAILED_READ_WRITE_OPERATION_ERR, e.code());
+
+ assertThrowsSqlException(
+ Sql.CONSTRAINT_VIOLATION_ERR,
+ "PK unique constraint is violated",
+ () -> checkDml(1, ses, "INSERT INTO TEST VALUES (?, ?)", ROW_COUNT, Integer.MAX_VALUE));
+
+ ResultSet<SqlRow> rs = executeForRead(ses, "SELECT VAL0 FROM TEST ORDER BY VAL0");
+
+ assertEquals(2 * ROW_COUNT, asStream(rs).count());
+
+ rs.close();
+
+ outerTx = igniteTx().begin();
+
+ rs = executeForRead(ses, outerTx, "SELECT VAL0 FROM TEST ORDER BY VAL0");
+
+ assertEquals(2 * ROW_COUNT, asStream(rs).count());
+
+ rs.close();
+
+ outerTx.commit();
+
+ checkDml(2 * ROW_COUNT, ses, "UPDATE TEST SET VAL0 = VAL0 + ?", 1);
+
+ checkDml(2 * ROW_COUNT, ses, "DELETE FROM TEST WHERE VAL0 >= 0");
+
+ assertEquals(ROW_COUNT + 1 + 1 + 1 + 1 + 1 + 1, txManagerInternal.finished() - txPrevCnt);
+
+ assertEquals(0, txManagerInternal.pending());
+ }
+
+ /** Check correctness of explicit transaction rollback. */
+ @Test
+ public void checkExplicitTxRollback() {
+ IgniteSql sql = igniteSql();
+
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+ Session ses = sql.createSession();
+
+ // Outer tx with further commit.
+ Transaction outerTx = igniteTx().begin();
+
+ for (int i = 0; i < ROW_COUNT; ++i) {
+ checkDml(1, outerTx, ses, "INSERT INTO TEST VALUES (?, ?)", i, i);
+ }
+
+ rollback(outerTx);
+
+ ResultSet<SqlRow> rs = executeForRead(ses, "SELECT VAL0 FROM TEST ORDER BY VAL0");
+
+ asStream(rs);
+ assertEquals(0, asStream(rs).count());
+
+ rs.close();
+ }
+
+ /** Check correctness of rw and ro transactions for table scan. */
+ @Test
+ public void checkMixedTransactionsForTable() {
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+ Matcher<String> planMatcher = containsTableScan("PUBLIC", "TEST");
+
+ checkMixedTransactions(planMatcher);
+ }
+
+
+ /** Check correctness of rw and ro transactions for index scan. */
+ @Test
+ public void checkMixedTransactionsForIndex() throws Exception {
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+ sql("CREATE INDEX TEST_IDX ON TEST(VAL0)");
+
+ Matcher<String> planMatcher = containsIndexScan("PUBLIC", "TEST", "TEST_IDX");
+
+ checkMixedTransactions(planMatcher);
+ }
+
+ private void checkMixedTransactions(Matcher<String> planMatcher) {
+ IgniteSql sql = igniteSql();
+
+ if (sql instanceof ClientSql) {
Review Comment:
I think it is better to move such guards into a test case body directly, so we know that the test is not expected to run for sql clients.
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlApiBaseTest.java:
##########
@@ -0,0 +1,860 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.sql.api;
+
+import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsIndexScan;
+import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsTableScan;
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.asStream;
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.assertThrowsSqlException;
+import static org.apache.ignite.internal.testframework.IgniteTestUtils.assertThrowsWithCause;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.catalog.commands.CatalogUtils;
+import org.apache.ignite.internal.client.sql.ClientSql;
+import org.apache.ignite.internal.sql.api.ColumnMetadataImpl.ColumnOriginImpl;
+import org.apache.ignite.internal.sql.engine.ClusterPerClassIntegrationTest;
+import org.apache.ignite.internal.sql.engine.QueryCancelledException;
+import org.apache.ignite.internal.testframework.IgniteTestUtils;
+import org.apache.ignite.internal.tx.TxManager;
+import org.apache.ignite.lang.ColumnAlreadyExistsException;
+import org.apache.ignite.lang.ColumnNotFoundException;
+import org.apache.ignite.lang.ErrorGroups;
+import org.apache.ignite.lang.ErrorGroups.Catalog;
+import org.apache.ignite.lang.ErrorGroups.Index;
+import org.apache.ignite.lang.ErrorGroups.Sql;
+import org.apache.ignite.lang.ErrorGroups.Transactions;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.IndexAlreadyExistsException;
+import org.apache.ignite.lang.IndexNotFoundException;
+import org.apache.ignite.lang.TableAlreadyExistsException;
+import org.apache.ignite.lang.TableNotFoundException;
+import org.apache.ignite.sql.BatchedArguments;
+import org.apache.ignite.sql.ColumnMetadata;
+import org.apache.ignite.sql.ColumnType;
+import org.apache.ignite.sql.CursorClosedException;
+import org.apache.ignite.sql.IgniteSql;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+import org.apache.ignite.sql.ResultSet;
+import org.apache.ignite.sql.ResultSetMetadata;
+import org.apache.ignite.sql.Session;
+import org.apache.ignite.sql.SqlBatchException;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.SqlRow;
+import org.apache.ignite.table.Table;
+import org.apache.ignite.tx.Transaction;
+import org.apache.ignite.tx.TransactionOptions;
+import org.hamcrest.Matcher;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests for SQL API.
+ * Tests will be run through synchronous, asynchronous API and client entry points.
+ * By default, any SQL API test should be added to the base class and use special provided methods to interact
+ * with the API in a API-type-independent manner. For any API-specific test, should be used the appropriate subclass.
+ */
+public abstract class ItSqlApiBaseTest extends ClusterPerClassIntegrationTest {
+ protected static final int ROW_COUNT = 16;
+
+ @AfterEach
+ public void dropTables() {
+ for (Table t : CLUSTER_NODES.get(0).tables().tables()) {
+ sql("DROP TABLE " + t.name());
+ }
+ }
+
+ @Test
+ @Disabled("https://issues.apache.org/jira/browse/IGNITE-20096")
+ public void ddl() throws Exception {
+ IgniteSql sql = igniteSql();
+ Session ses = sql.createSession();
+
+ // CREATE TABLE
+ checkDdl(true, ses, "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+ checkError(
+ TableAlreadyExistsException.class,
+ ErrorGroups.Table.TABLE_ALREADY_EXISTS_ERR,
+ "Table already exists [name=\"PUBLIC\".\"TEST\"]",
+ ses,
+ "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)"
+ );
+ checkSqlError(
+ ErrorGroups.Table.TABLE_DEFINITION_ERR,
+ "Can't create table with duplicate columns: ID, VAL, VAL",
+ ses,
+ "CREATE TABLE TEST1(ID INT PRIMARY KEY, VAL INT, VAL INT)"
+ );
+ checkDdl(false, ses, "CREATE TABLE IF NOT EXISTS TEST(ID INT PRIMARY KEY, VAL VARCHAR)");
+
+ // ADD COLUMN
+ checkDdl(true, ses, "ALTER TABLE TEST ADD COLUMN VAL1 VARCHAR");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"NOT_EXISTS_TABLE\"]",
+ ses,
+ "ALTER TABLE NOT_EXISTS_TABLE ADD COLUMN VAL1 VARCHAR"
+ );
+ checkDdl(false, ses, "ALTER TABLE IF EXISTS NOT_EXISTS_TABLE ADD COLUMN VAL1 VARCHAR");
+ checkError(
+ ColumnAlreadyExistsException.class,
+ ErrorGroups.Table.COLUMN_ALREADY_EXISTS_ERR,
+ "Column already exists [name=\"VAL1\"]",
+ ses,
+ "ALTER TABLE TEST ADD COLUMN VAL1 INT"
+ );
+
+ // CREATE INDEX
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX ON TEST(VAL0)");
+ checkError(
+ IndexAlreadyExistsException.class,
+ Index.INDEX_ALREADY_EXISTS_ERR,
+ "Index already exists [name=\"PUBLIC\".\"TEST_IDX\"]",
+ ses,
+ "CREATE INDEX TEST_IDX ON TEST(VAL1)"
+ );
+ checkDdl(false, ses, "CREATE INDEX IF NOT EXISTS TEST_IDX ON TEST(VAL1)");
+
+ // TODO: IGNITE-19150 We are waiting for schema synchronization to avoid races to create and destroy indexes
+ waitForIndexBuild("TEST", "TEST_IDX");
+
+ checkDdl(true, ses, "DROP INDEX TESt_iDX");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX1 ON TEST(VAL0)");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX2 ON TEST(VAL0)");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX3 ON TEST(ID, VAL0, VAL1)");
+ checkSqlError(
+ Index.INVALID_INDEX_DEFINITION_ERR,
+ "Can't create index on duplicate columns: VAL0, VAL0",
+ ses,
+ "CREATE INDEX TEST_IDX4 ON TEST(VAL0, VAL0)"
+ );
+
+ checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column(s). Column VAL1 is used by indexes [TEST_IDX3].",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN val1"
+ );
+
+ SqlException ex = checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column(s).",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN (val0, val1)"
+ );
+
+ String msg = ex.getMessage();
+ String explainMsg = "Unexpected error message: " + msg;
+
+ assertTrue(msg.contains("Column VAL0 is used by indexes ["), explainMsg);
+ assertTrue(msg.contains("TEST_IDX1") && msg.contains("TEST_IDX2") && msg.contains("TEST_IDX3"), explainMsg);
+ assertTrue(msg.contains("Column VAL1 is used by indexes [TEST_IDX3]"), explainMsg);
+
+ checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column, belongs to primary key: [name=ID]",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN id"
+ );
+
+ // TODO: IGNITE-19150 We are waiting for schema synchronization to avoid races to create and destroy indexes
+ waitForIndexBuild("TEST", "TEST_IDX3");
+ checkDdl(true, ses, "DROP INDEX TESt_iDX3");
+
+ // DROP COLUMNS
+ checkDdl(true, ses, "ALTER TABLE TEST DROP COLUMN VAL1");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"NOT_EXISTS_TABLE\"]",
+ ses,
+ "ALTER TABLE NOT_EXISTS_TABLE DROP COLUMN VAL1"
+ );
+ checkDdl(false, ses, "ALTER TABLE IF EXISTS NOT_EXISTS_TABLE DROP COLUMN VAL1");
+ checkError(
+ ColumnNotFoundException.class,
+ ErrorGroups.Table.COLUMN_NOT_FOUND_ERR,
+ "Column does not exist [tableName=\"PUBLIC\".\"TEST\", columnName=\"VAL1\"]",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN VAL1"
+ );
+
+ // DROP TABLE
+ checkDdl(false, ses, "DROP TABLE IF EXISTS NOT_EXISTS_TABLE");
+
+ checkDdl(true, ses, "DROP TABLE TEST");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"TEST\"]",
+ ses,
+ "DROP TABLE TEST"
+ );
+
+ checkDdl(false, ses, "DROP INDEX IF EXISTS TEST_IDX");
+
+ checkError(
+ IndexNotFoundException.class,
+ Index.INDEX_NOT_FOUND_ERR,
+ "Index does not exist [name=\"PUBLIC\".\"TEST_IDX\"]", ses,
+ "DROP INDEX TEST_IDX"
+ );
+ }
+
+ /** Check all transactions are processed correctly even with case of sql Exception raised. */
+ @Test
+ public void implicitTransactionsStates() {
+ IgniteSql sql = igniteSql();
+
+ if (sql instanceof ClientSql) {
+ return;
+ }
+
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+ Session ses = sql.createSession();
+
+ TxManager txManager = txManager();
+
+ for (int i = 0; i < ROW_COUNT; ++i) {
+ assertThrowsSqlException(
+ Catalog.VALIDATION_ERR,
+ "Table with name 'PUBLIC.TEST' already exists",
+ () -> execute(ses, "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)")
+ );
+ }
+
+ // No new transactions through ddl.
+ assertEquals(0, txManager.pending());
+ }
+
+ /** Check correctness of implicit and explicit transactions. */
+ @Test
+ public void checkTransactionsWithDml() {
+ IgniteSql sql = igniteSql();
+
+ if (sql instanceof ClientSql) {
+ return;
+ }
+
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+ Session ses = sql.createSession();
+
+ TxManager txManagerInternal = txManager();
+
+ int txPrevCnt = txManagerInternal.finished();
+
+ for (int i = 0; i < ROW_COUNT; ++i) {
+ checkDml(1, ses, "INSERT INTO TEST VALUES (?, ?)", i, i);
+ }
+
+ // Outer tx with further commit.
+ Transaction outerTx = igniteTx().begin();
+
+ for (int i = ROW_COUNT; i < 2 * ROW_COUNT; ++i) {
+ checkDml(1, outerTx, ses, "INSERT INTO TEST VALUES (?, ?)", i, i);
+ }
+
+ commit(outerTx);
+
+ // Outdated tx.
+ Transaction outerTx0 = outerTx;
+ //ToDo: IGNITE-20387 , here should be used assertThrowsSqlException method with code and message `"Transaction is already finished"
+ IgniteException e = assertThrows(IgniteException.class,
+ () -> checkDml(1, outerTx0, ses, "INSERT INTO TEST VALUES (?, ?)", ROW_COUNT, Integer.MAX_VALUE));
+ assertEquals(Transactions.TX_FAILED_READ_WRITE_OPERATION_ERR, e.code());
+
+ assertThrowsSqlException(
+ Sql.CONSTRAINT_VIOLATION_ERR,
+ "PK unique constraint is violated",
+ () -> checkDml(1, ses, "INSERT INTO TEST VALUES (?, ?)", ROW_COUNT, Integer.MAX_VALUE));
+
+ ResultSet<SqlRow> rs = executeForRead(ses, "SELECT VAL0 FROM TEST ORDER BY VAL0");
+
+ assertEquals(2 * ROW_COUNT, asStream(rs).count());
+
+ rs.close();
+
+ outerTx = igniteTx().begin();
+
+ rs = executeForRead(ses, outerTx, "SELECT VAL0 FROM TEST ORDER BY VAL0");
+
+ assertEquals(2 * ROW_COUNT, asStream(rs).count());
+
+ rs.close();
+
+ outerTx.commit();
+
+ checkDml(2 * ROW_COUNT, ses, "UPDATE TEST SET VAL0 = VAL0 + ?", 1);
+
+ checkDml(2 * ROW_COUNT, ses, "DELETE FROM TEST WHERE VAL0 >= 0");
+
+ assertEquals(ROW_COUNT + 1 + 1 + 1 + 1 + 1 + 1, txManagerInternal.finished() - txPrevCnt);
+
+ assertEquals(0, txManagerInternal.pending());
+ }
+
+ /** Check correctness of explicit transaction rollback. */
+ @Test
+ public void checkExplicitTxRollback() {
+ IgniteSql sql = igniteSql();
+
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+ Session ses = sql.createSession();
+
+ // Outer tx with further commit.
+ Transaction outerTx = igniteTx().begin();
+
+ for (int i = 0; i < ROW_COUNT; ++i) {
+ checkDml(1, outerTx, ses, "INSERT INTO TEST VALUES (?, ?)", i, i);
+ }
+
+ rollback(outerTx);
+
+ ResultSet<SqlRow> rs = executeForRead(ses, "SELECT VAL0 FROM TEST ORDER BY VAL0");
+
+ asStream(rs);
+ assertEquals(0, asStream(rs).count());
+
+ rs.close();
+ }
+
+ /** Check correctness of rw and ro transactions for table scan. */
+ @Test
+ public void checkMixedTransactionsForTable() {
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+ Matcher<String> planMatcher = containsTableScan("PUBLIC", "TEST");
+
+ checkMixedTransactions(planMatcher);
+ }
+
+
+ /** Check correctness of rw and ro transactions for index scan. */
+ @Test
+ public void checkMixedTransactionsForIndex() throws Exception {
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+ sql("CREATE INDEX TEST_IDX ON TEST(VAL0)");
+
+ Matcher<String> planMatcher = containsIndexScan("PUBLIC", "TEST", "TEST_IDX");
+
+ checkMixedTransactions(planMatcher);
+ }
+
+ private void checkMixedTransactions(Matcher<String> planMatcher) {
+ IgniteSql sql = igniteSql();
+
+ if (sql instanceof ClientSql) {
Review Comment:
I think it is better to move such guards into a test case body directly, so we know that a test case is not expected to run for sql clients.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1350393746
##########
modules/platforms/dotnet/Apache.Ignite.Tests/Sql/SqlTests.cs:
##########
@@ -344,8 +344,7 @@ public void TestInvalidSqlThrowsException()
[Test]
public void TestCreateTableExistsThrowsException()
{
- // TODO: IGNITE-20388 Fix it
Review Comment:
yes, I'l handle it separatelly
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1350334992
##########
modules/platforms/dotnet/Apache.Ignite.Tests/Sql/SqlTests.cs:
##########
@@ -344,8 +344,7 @@ public void TestInvalidSqlThrowsException()
[Test]
public void TestCreateTableExistsThrowsException()
{
- // TODO: IGNITE-20388 Fix it
Review Comment:
I see that some mentions of IGNITE-20388 exist in cpp files, are we going to do something with them?
##########
modules/core/src/test/java/org/apache/ignite/internal/lang/IgniteExceptionMapperUtilTest.java:
##########
@@ -204,4 +239,19 @@ public CustomNoMappingException() {
super(INTERNAL_ERR, "Test internal exception [err=no mapping]");
}
}
+
+ /**
+ * Test runtime exception which does not have a mapper.
+ */
+ public static class CustomInternalNotMappedException extends IgniteInternalException {
Review Comment:
We have `CustomNoMappingException` and `CustomInternalNotMappedException` it's confusing a bit.
I suggest use existing `CustomNoMappingException` (just put `code` to the constructor).
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlApiBaseTest.java:
##########
@@ -0,0 +1,860 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.sql.api;
+
+import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsIndexScan;
+import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsTableScan;
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.asStream;
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.assertThrowsSqlException;
+import static org.apache.ignite.internal.testframework.IgniteTestUtils.assertThrowsWithCause;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.catalog.commands.CatalogUtils;
+import org.apache.ignite.internal.client.sql.ClientSql;
+import org.apache.ignite.internal.sql.api.ColumnMetadataImpl.ColumnOriginImpl;
+import org.apache.ignite.internal.sql.engine.ClusterPerClassIntegrationTest;
+import org.apache.ignite.internal.sql.engine.QueryCancelledException;
+import org.apache.ignite.internal.testframework.IgniteTestUtils;
+import org.apache.ignite.internal.tx.TxManager;
+import org.apache.ignite.lang.ColumnAlreadyExistsException;
+import org.apache.ignite.lang.ColumnNotFoundException;
+import org.apache.ignite.lang.ErrorGroups;
+import org.apache.ignite.lang.ErrorGroups.Catalog;
+import org.apache.ignite.lang.ErrorGroups.Index;
+import org.apache.ignite.lang.ErrorGroups.Sql;
+import org.apache.ignite.lang.ErrorGroups.Transactions;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.IndexAlreadyExistsException;
+import org.apache.ignite.lang.IndexNotFoundException;
+import org.apache.ignite.lang.TableAlreadyExistsException;
+import org.apache.ignite.lang.TableNotFoundException;
+import org.apache.ignite.sql.BatchedArguments;
+import org.apache.ignite.sql.ColumnMetadata;
+import org.apache.ignite.sql.ColumnType;
+import org.apache.ignite.sql.CursorClosedException;
+import org.apache.ignite.sql.IgniteSql;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+import org.apache.ignite.sql.ResultSet;
+import org.apache.ignite.sql.ResultSetMetadata;
+import org.apache.ignite.sql.Session;
+import org.apache.ignite.sql.SqlBatchException;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.SqlRow;
+import org.apache.ignite.table.Table;
+import org.apache.ignite.tx.Transaction;
+import org.apache.ignite.tx.TransactionOptions;
+import org.hamcrest.Matcher;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests for SQL API.
+ * Tests will be run through synchronous, asynchronous API and client entry points.
+ * By default, any SQL API test should be added to the base class and use special provided methods to interact
+ * with the API in a API-type-independent manner. For any API-specific test, should be used the appropriate subclass.
+ */
+public abstract class ItSqlApiBaseTest extends ClusterPerClassIntegrationTest {
+ protected static final int ROW_COUNT = 16;
+
+ @AfterEach
+ public void dropTables() {
+ for (Table t : CLUSTER_NODES.get(0).tables().tables()) {
+ sql("DROP TABLE " + t.name());
+ }
+ }
+
+ @Test
+ @Disabled("https://issues.apache.org/jira/browse/IGNITE-20096")
+ public void ddl() throws Exception {
+ IgniteSql sql = igniteSql();
+ Session ses = sql.createSession();
+
+ // CREATE TABLE
+ checkDdl(true, ses, "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+ checkError(
+ TableAlreadyExistsException.class,
+ ErrorGroups.Table.TABLE_ALREADY_EXISTS_ERR,
+ "Table already exists [name=\"PUBLIC\".\"TEST\"]",
+ ses,
+ "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)"
+ );
+ checkSqlError(
+ ErrorGroups.Table.TABLE_DEFINITION_ERR,
+ "Can't create table with duplicate columns: ID, VAL, VAL",
+ ses,
+ "CREATE TABLE TEST1(ID INT PRIMARY KEY, VAL INT, VAL INT)"
+ );
+ checkDdl(false, ses, "CREATE TABLE IF NOT EXISTS TEST(ID INT PRIMARY KEY, VAL VARCHAR)");
+
+ // ADD COLUMN
+ checkDdl(true, ses, "ALTER TABLE TEST ADD COLUMN VAL1 VARCHAR");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"NOT_EXISTS_TABLE\"]",
+ ses,
+ "ALTER TABLE NOT_EXISTS_TABLE ADD COLUMN VAL1 VARCHAR"
+ );
+ checkDdl(false, ses, "ALTER TABLE IF EXISTS NOT_EXISTS_TABLE ADD COLUMN VAL1 VARCHAR");
+ checkError(
+ ColumnAlreadyExistsException.class,
+ ErrorGroups.Table.COLUMN_ALREADY_EXISTS_ERR,
+ "Column already exists [name=\"VAL1\"]",
+ ses,
+ "ALTER TABLE TEST ADD COLUMN VAL1 INT"
+ );
+
+ // CREATE INDEX
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX ON TEST(VAL0)");
+ checkError(
+ IndexAlreadyExistsException.class,
+ Index.INDEX_ALREADY_EXISTS_ERR,
+ "Index already exists [name=\"PUBLIC\".\"TEST_IDX\"]",
+ ses,
+ "CREATE INDEX TEST_IDX ON TEST(VAL1)"
+ );
+ checkDdl(false, ses, "CREATE INDEX IF NOT EXISTS TEST_IDX ON TEST(VAL1)");
+
+ // TODO: IGNITE-19150 We are waiting for schema synchronization to avoid races to create and destroy indexes
+ waitForIndexBuild("TEST", "TEST_IDX");
+
+ checkDdl(true, ses, "DROP INDEX TESt_iDX");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX1 ON TEST(VAL0)");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX2 ON TEST(VAL0)");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX3 ON TEST(ID, VAL0, VAL1)");
+ checkSqlError(
+ Index.INVALID_INDEX_DEFINITION_ERR,
+ "Can't create index on duplicate columns: VAL0, VAL0",
+ ses,
+ "CREATE INDEX TEST_IDX4 ON TEST(VAL0, VAL0)"
+ );
+
+ checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column(s). Column VAL1 is used by indexes [TEST_IDX3].",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN val1"
+ );
+
+ SqlException ex = checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column(s).",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN (val0, val1)"
+ );
+
+ String msg = ex.getMessage();
+ String explainMsg = "Unexpected error message: " + msg;
+
+ assertTrue(msg.contains("Column VAL0 is used by indexes ["), explainMsg);
+ assertTrue(msg.contains("TEST_IDX1") && msg.contains("TEST_IDX2") && msg.contains("TEST_IDX3"), explainMsg);
+ assertTrue(msg.contains("Column VAL1 is used by indexes [TEST_IDX3]"), explainMsg);
+
+ checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column, belongs to primary key: [name=ID]",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN id"
+ );
+
+ // TODO: IGNITE-19150 We are waiting for schema synchronization to avoid races to create and destroy indexes
+ waitForIndexBuild("TEST", "TEST_IDX3");
+ checkDdl(true, ses, "DROP INDEX TESt_iDX3");
+
+ // DROP COLUMNS
+ checkDdl(true, ses, "ALTER TABLE TEST DROP COLUMN VAL1");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"NOT_EXISTS_TABLE\"]",
+ ses,
+ "ALTER TABLE NOT_EXISTS_TABLE DROP COLUMN VAL1"
+ );
+ checkDdl(false, ses, "ALTER TABLE IF EXISTS NOT_EXISTS_TABLE DROP COLUMN VAL1");
+ checkError(
+ ColumnNotFoundException.class,
+ ErrorGroups.Table.COLUMN_NOT_FOUND_ERR,
+ "Column does not exist [tableName=\"PUBLIC\".\"TEST\", columnName=\"VAL1\"]",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN VAL1"
+ );
+
+ // DROP TABLE
+ checkDdl(false, ses, "DROP TABLE IF EXISTS NOT_EXISTS_TABLE");
+
+ checkDdl(true, ses, "DROP TABLE TEST");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"TEST\"]",
+ ses,
+ "DROP TABLE TEST"
+ );
+
+ checkDdl(false, ses, "DROP INDEX IF EXISTS TEST_IDX");
+
+ checkError(
+ IndexNotFoundException.class,
+ Index.INDEX_NOT_FOUND_ERR,
+ "Index does not exist [name=\"PUBLIC\".\"TEST_IDX\"]", ses,
+ "DROP INDEX TEST_IDX"
+ );
+ }
+
+ /** Check all transactions are processed correctly even with case of sql Exception raised. */
+ @Test
+ public void implicitTransactionsStates() {
+ IgniteSql sql = igniteSql();
+
+ if (sql instanceof ClientSql) {
+ return;
+ }
+
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+ Session ses = sql.createSession();
+
+ TxManager txManager = txManager();
+
+ for (int i = 0; i < ROW_COUNT; ++i) {
+ assertThrowsSqlException(
+ Catalog.VALIDATION_ERR,
+ "Table with name 'PUBLIC.TEST' already exists",
+ () -> execute(ses, "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)")
+ );
+ }
+
+ // No new transactions through ddl.
+ assertEquals(0, txManager.pending());
+ }
+
+ /** Check correctness of implicit and explicit transactions. */
+ @Test
+ public void checkTransactionsWithDml() {
+ IgniteSql sql = igniteSql();
+
+ if (sql instanceof ClientSql) {
+ return;
+ }
+
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+ Session ses = sql.createSession();
+
+ TxManager txManagerInternal = txManager();
+
+ int txPrevCnt = txManagerInternal.finished();
+
+ for (int i = 0; i < ROW_COUNT; ++i) {
+ checkDml(1, ses, "INSERT INTO TEST VALUES (?, ?)", i, i);
+ }
+
+ // Outer tx with further commit.
+ Transaction outerTx = igniteTx().begin();
+
+ for (int i = ROW_COUNT; i < 2 * ROW_COUNT; ++i) {
+ checkDml(1, outerTx, ses, "INSERT INTO TEST VALUES (?, ?)", i, i);
+ }
+
+ commit(outerTx);
+
+ // Outdated tx.
+ Transaction outerTx0 = outerTx;
+ //ToDo: IGNITE-20387 , here should be used assertThrowsSqlException method with code and message `"Transaction is already finished"
Review Comment:
Apparently there should be a link to another ticket? :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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "korlov42 (via GitHub)" <gi...@apache.org>.
korlov42 commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1358274498
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/lang/SqlExceptionMapperUtil.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.lang;
+
+import static org.apache.ignite.internal.lang.IgniteExceptionMapperUtil.mapToPublicException;
+import static org.apache.ignite.lang.ErrorGroups.Common.INTERNAL_ERR;
+
+import org.apache.ignite.lang.ErrorGroups.Common;
+import org.apache.ignite.lang.IgniteCheckedException;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.TraceableException;
+import org.apache.ignite.sql.SqlException;
+
+/**
+ * This utility class provides an ability to map Ignite internal exceptions to public SqlException.
+ */
+public class SqlExceptionMapperUtil {
+
+ /**
+ * This method provides a mapping from internal exception to SQL public ones.
+ *
+ * <p>The rules of mapping are the following:</p>
+ * <ul>
+ * <li>any instance of {@link Error} is returned as is, except {@link AssertionError}
+ * that will always be mapped to {@link IgniteException} with the {@link Common#INTERNAL_ERR} error code.</li>
+ * <li>any instance of {@link IgniteException} or {@link IgniteCheckedException} is returned as is.</li>
Review Comment:
the same, any instance of IgniteException and IgniteCheckedException will be mapped to SqlException
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1352069822
##########
modules/core/src/main/java/org/apache/ignite/internal/lang/IgniteExceptionMapperUtil.java:
##########
@@ -83,36 +86,59 @@ static void registerMapping(
* @return Public exception.
*/
public static Throwable mapToPublicException(Throwable origin) {
- if (origin instanceof Error) {
- if (origin instanceof AssertionError) {
- return new IgniteException(INTERNAL_ERR, origin);
+ return mapToPublicExceptionInternal(origin, (e) -> {
+ if (e instanceof Error) {
+ return e;
}
- return origin;
- }
-
- IgniteExceptionMapper<? extends Exception, ? extends Exception> m = EXCEPTION_CONVERTERS.get(origin.getClass());
- if (m != null) {
- Exception mapped = map(m, origin);
+ if (e instanceof IgniteException || e instanceof IgniteCheckedException) {
+ return e;
+ }
- assert mapped instanceof IgniteException || mapped instanceof IgniteCheckedException :
- "Unexpected mapping of internal exception to a public one [origin=" + origin + ", mapped=" + mapped + ']';
+ // There are no exception mappings for the given exception. This case should be considered as internal error.
+ return new IgniteException(INTERNAL_ERR, origin);
+ });
+ }
- return mapped;
- }
+ /**
+ * This method provides a mapping from internal exception to SQL public ones.
+ *
+ * <p>The rules of mapping are the following:</p>
+ * <ul>
+ * <li>any instance of {@link Error} is returned as is, except {@link AssertionError}
+ * that will always be mapped to {@link IgniteException} with the {@link Common#INTERNAL_ERR} error code.</li>
+ * <li>any instance of {@link IgniteException} or {@link IgniteCheckedException} is returned as is.</li>
+ * <li>any other instance of {@link TraceableException} is wrapped into {@link SqlException}
+ * with the original {@link TraceableException#traceId() traceUd} and {@link TraceableException#code() code}.</li>
+ * <li>if there are no any mappers that can do a mapping from the given error to a public exception,
+ * then {@link SqlException} with the {@link Common#INTERNAL_ERR} error code is returned.</li>
+ * </ul>
+ *
+ * @param origin Exception to be mapped.
+ * @return Public exception.
+ */
+ public static Throwable mapToPublicSqlException(Throwable origin) {
Review Comment:
moved the utility method to sql module
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1356112694
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/lang/SqlExceptionMapperUtil.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.lang;
+
+import static org.apache.ignite.internal.lang.IgniteExceptionMapperUtil.mapToPublicException;
+import static org.apache.ignite.lang.ErrorGroups.Common.INTERNAL_ERR;
+
+import org.apache.ignite.lang.ErrorGroups.Common;
+import org.apache.ignite.lang.IgniteCheckedException;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.TraceableException;
+import org.apache.ignite.sql.SqlException;
+
+/**
+ * This utility class provides an ability to map Ignite internal exceptions to public SqlException.
+ */
+public class SqlExceptionMapperUtil {
+
+ /**
+ * This method provides a mapping from internal exception to SQL public ones.
+ *
+ * <p>The rules of mapping are the following:</p>
+ * <ul>
+ * <li>any instance of {@link Error} is returned as is, except {@link AssertionError}
+ * that will always be mapped to {@link IgniteException} with the {@link Common#INTERNAL_ERR} error code.</li>
+ * <li>any instance of {@link IgniteException} or {@link IgniteCheckedException} is returned as is.</li>
+ * <li>any other instance of {@link TraceableException} is wrapped into {@link SqlException}
+ * with the original {@link TraceableException#traceId() traceUd} and {@link TraceableException#code() code}.</li>
+ * <li>if there are no any mappers that can do a mapping from the given error to a public exception,
+ * then {@link SqlException} with the {@link Common#INTERNAL_ERR} error code is returned.</li>
+ * </ul>
+ *
+ * @param origin Exception to be mapped.
+ * @return Public exception.
+ */
+ public static Throwable mapToPublicSqlException(Throwable origin) {
+ Throwable e = mapToPublicException(origin);
+ if (e instanceof Error) {
+ return e;
+ }
+ if (e instanceof SqlException) {
+ return e;
+ }
+ if (e instanceof IgniteException && ((IgniteException) e).code() == INTERNAL_ERR) {
+ return new SqlException(((IgniteException) e).traceId(), INTERNAL_ERR, e.getCause());
Review Comment:
We don't put message for non SQL API public exceptions, so it shouldn't be here too.
Original stacktrace is not lost - it's contains in cause, no anybody internally should create INTERNAL ERROR I supposed
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1358393440
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/lang/SqlExceptionMapperUtil.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.lang;
+
+import static org.apache.ignite.internal.lang.IgniteExceptionMapperUtil.mapToPublicException;
+import static org.apache.ignite.lang.ErrorGroups.Common.INTERNAL_ERR;
+
+import org.apache.ignite.lang.ErrorGroups.Common;
+import org.apache.ignite.lang.IgniteCheckedException;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.TraceableException;
+import org.apache.ignite.sql.SqlException;
+
+/**
+ * This utility class provides an ability to map Ignite internal exceptions to public SqlException.
+ */
+public class SqlExceptionMapperUtil {
+
+ /**
+ * This method provides a mapping from internal exception to SQL public ones.
+ *
+ * <p>The rules of mapping are the following:</p>
+ * <ul>
+ * <li>any instance of {@link Error} is returned as is, except {@link AssertionError}
+ * that will always be mapped to {@link IgniteException} with the {@link Common#INTERNAL_ERR} error code.</li>
+ * <li>any instance of {@link IgniteException} or {@link IgniteCheckedException} is returned as is.</li>
+ * <li>any other instance of {@link TraceableException} is wrapped into {@link SqlException}
+ * with the original {@link TraceableException#traceId() traceUd} and {@link TraceableException#code() code}.</li>
+ * <li>if there are no any mappers that can do a mapping from the given error to a public exception,
+ * then {@link SqlException} with the {@link Common#INTERNAL_ERR} error code is returned.</li>
+ * </ul>
+ *
+ * @param origin Exception to be mapped.
+ * @return Public exception.
+ */
+ public static Throwable mapToPublicSqlException(Throwable origin) {
+ Throwable e = mapToPublicException(origin);
+ if (e instanceof Error) {
+ return e;
+ }
+ if (e instanceof SqlException) {
+ return e;
+ }
+ if (e instanceof IgniteException && ((IgniteException) e).code() == INTERNAL_ERR) {
+ return new SqlException(((IgniteException) e).traceId(), INTERNAL_ERR, e.getCause());
Review Comment:
accepted the arguments after a personal conversation
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "korlov42 (via GitHub)" <gi...@apache.org>.
korlov42 merged PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1356112694
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/lang/SqlExceptionMapperUtil.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.lang;
+
+import static org.apache.ignite.internal.lang.IgniteExceptionMapperUtil.mapToPublicException;
+import static org.apache.ignite.lang.ErrorGroups.Common.INTERNAL_ERR;
+
+import org.apache.ignite.lang.ErrorGroups.Common;
+import org.apache.ignite.lang.IgniteCheckedException;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.TraceableException;
+import org.apache.ignite.sql.SqlException;
+
+/**
+ * This utility class provides an ability to map Ignite internal exceptions to public SqlException.
+ */
+public class SqlExceptionMapperUtil {
+
+ /**
+ * This method provides a mapping from internal exception to SQL public ones.
+ *
+ * <p>The rules of mapping are the following:</p>
+ * <ul>
+ * <li>any instance of {@link Error} is returned as is, except {@link AssertionError}
+ * that will always be mapped to {@link IgniteException} with the {@link Common#INTERNAL_ERR} error code.</li>
+ * <li>any instance of {@link IgniteException} or {@link IgniteCheckedException} is returned as is.</li>
+ * <li>any other instance of {@link TraceableException} is wrapped into {@link SqlException}
+ * with the original {@link TraceableException#traceId() traceUd} and {@link TraceableException#code() code}.</li>
+ * <li>if there are no any mappers that can do a mapping from the given error to a public exception,
+ * then {@link SqlException} with the {@link Common#INTERNAL_ERR} error code is returned.</li>
+ * </ul>
+ *
+ * @param origin Exception to be mapped.
+ * @return Public exception.
+ */
+ public static Throwable mapToPublicSqlException(Throwable origin) {
+ Throwable e = mapToPublicException(origin);
+ if (e instanceof Error) {
+ return e;
+ }
+ if (e instanceof SqlException) {
+ return e;
+ }
+ if (e instanceof IgniteException && ((IgniteException) e).code() == INTERNAL_ERR) {
+ return new SqlException(((IgniteException) e).traceId(), INTERNAL_ERR, e.getCause());
Review Comment:
We don't put message for non SQL API public exceptions, so it shouldn't be here too.
Original stacktrace is not lost - it's contains in cause, no anybody internally should create public exception with code INTERNAL_ERR I supposed
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1350187955
##########
modules/core/src/main/java/org/apache/ignite/internal/lang/IgniteExceptionMapperUtil.java:
##########
@@ -83,36 +86,59 @@ static void registerMapping(
* @return Public exception.
*/
public static Throwable mapToPublicException(Throwable origin) {
- if (origin instanceof Error) {
- if (origin instanceof AssertionError) {
- return new IgniteException(INTERNAL_ERR, origin);
+ return mapToPublicExceptionInternal(origin, (e) -> {
+ if (e instanceof Error) {
+ return e;
}
- return origin;
- }
-
- IgniteExceptionMapper<? extends Exception, ? extends Exception> m = EXCEPTION_CONVERTERS.get(origin.getClass());
- if (m != null) {
- Exception mapped = map(m, origin);
+ if (e instanceof IgniteException || e instanceof IgniteCheckedException) {
+ return e;
+ }
- assert mapped instanceof IgniteException || mapped instanceof IgniteCheckedException :
- "Unexpected mapping of internal exception to a public one [origin=" + origin + ", mapped=" + mapped + ']';
+ // There are no exception mappings for the given exception. This case should be considered as internal error.
+ return new IgniteException(INTERNAL_ERR, origin);
+ });
+ }
- return mapped;
- }
+ /**
+ * This method provides a mapping from internal exception to SQL public ones.
+ *
+ * <p>The rules of mapping are the following:</p>
+ * <ul>
+ * <li>any instance of {@link Error} is returned as is, except {@link AssertionError}
+ * that will always be mapped to {@link IgniteException} with the {@link Common#INTERNAL_ERR} error code.</li>
+ * <li>any instance of {@link IgniteException} or {@link IgniteCheckedException} is returned as is.</li>
+ * <li>any other instance of {@link TraceableException} is wrapped into {@link SqlException}
+ * with the original {@link TraceableException#traceId() traceUd} and {@link TraceableException#code() code}.</li>
+ * <li>if there are no any mappers that can do a mapping from the given error to a public exception,
+ * then {@link SqlException} with the {@link Common#INTERNAL_ERR} error code is returned.</li>
+ * </ul>
+ *
+ * @param origin Exception to be mapped.
+ * @return Public exception.
+ */
+ public static Throwable mapToPublicSqlException(Throwable origin) {
Review Comment:
In general agree with the statement, but it really convenient to have it in single to reuse common code for mapping. Otherwise we should do internal private method public, which is not to good.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1350168070
##########
modules/core/src/main/java/org/apache/ignite/internal/sql/SyncResultSetAdapter.java:
##########
@@ -32,7 +32,7 @@
/**
* Synchronous wrapper over {@link org.apache.ignite.sql.async.AsyncResultSet}.
*/
-class SyncResultSetAdapter<T> implements ResultSet<T> {
+public class SyncResultSetAdapter<T> implements ResultSet<T> {
Review Comment:
Let's leave it public then.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "korlov42 (via GitHub)" <gi...@apache.org>.
korlov42 commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1350164871
##########
modules/client-handler/src/main/java/org/apache/ignite/client/handler/JdbcQueryEventHandlerImpl.java:
##########
@@ -498,7 +498,7 @@ <T> CompletableFuture<T> doInSession(SessionAwareAction<T> action) {
return action.perform(newSessionId);
}
- return CompletableFuture.failedFuture(IgniteExceptionMapperUtil.mapToPublicException(error));
+ return CompletableFuture.failedFuture(IgniteExceptionMapperUtil.mapToPublicSqlException(error));
Review Comment:
as far as I see, none of the tests are affected by this change. So, why have you changed this?
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItTablesApiTest.java:
##########
@@ -140,8 +142,10 @@ public void testTableAlreadyCreated() {
Table tbl = createTable(ignite0, TABLE_NAME);
- // TODO: IGNITE-20388 Fix it
- assertThrowsWithCause(() -> createTable(ignite0, TABLE_NAME), TableExistsValidationException.class);
+ assertThrowsSqlException(
+ Catalog.VALIDATION_ERR,
Review Comment:
I'm wondering why does it throw `Catalog.VALIDATION_ERR` and not `Sql.STMT_VALIDATION_ERR`?
The same question for rest of the changes in this class
##########
modules/core/src/main/java/org/apache/ignite/internal/lang/IgniteExceptionMapperUtil.java:
##########
@@ -83,36 +86,59 @@ static void registerMapping(
* @return Public exception.
*/
public static Throwable mapToPublicException(Throwable origin) {
- if (origin instanceof Error) {
- if (origin instanceof AssertionError) {
- return new IgniteException(INTERNAL_ERR, origin);
+ return mapToPublicExceptionInternal(origin, (e) -> {
+ if (e instanceof Error) {
+ return e;
}
- return origin;
- }
-
- IgniteExceptionMapper<? extends Exception, ? extends Exception> m = EXCEPTION_CONVERTERS.get(origin.getClass());
- if (m != null) {
- Exception mapped = map(m, origin);
+ if (e instanceof IgniteException || e instanceof IgniteCheckedException) {
+ return e;
+ }
- assert mapped instanceof IgniteException || mapped instanceof IgniteCheckedException :
- "Unexpected mapping of internal exception to a public one [origin=" + origin + ", mapped=" + mapped + ']';
+ // There are no exception mappings for the given exception. This case should be considered as internal error.
+ return new IgniteException(INTERNAL_ERR, origin);
+ });
+ }
- return mapped;
- }
+ /**
+ * This method provides a mapping from internal exception to SQL public ones.
+ *
+ * <p>The rules of mapping are the following:</p>
+ * <ul>
+ * <li>any instance of {@link Error} is returned as is, except {@link AssertionError}
+ * that will always be mapped to {@link IgniteException} with the {@link Common#INTERNAL_ERR} error code.</li>
+ * <li>any instance of {@link IgniteException} or {@link IgniteCheckedException} is returned as is.</li>
+ * <li>any other instance of {@link TraceableException} is wrapped into {@link SqlException}
+ * with the original {@link TraceableException#traceId() traceUd} and {@link TraceableException#code() code}.</li>
+ * <li>if there are no any mappers that can do a mapping from the given error to a public exception,
+ * then {@link SqlException} with the {@link Common#INTERNAL_ERR} error code is returned.</li>
+ * </ul>
+ *
+ * @param origin Exception to be mapped.
+ * @return Public exception.
+ */
+ public static Throwable mapToPublicSqlException(Throwable origin) {
Review Comment:
I would say such method should not be a part of the core library. Only sql should return SqlException, thus this mapping should be placed to sql-related module
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1350166346
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlApiBaseTest.java:
##########
@@ -0,0 +1,860 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.sql.api;
+
+import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsIndexScan;
+import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsTableScan;
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.asStream;
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.assertThrowsSqlException;
+import static org.apache.ignite.internal.testframework.IgniteTestUtils.assertThrowsWithCause;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.catalog.commands.CatalogUtils;
+import org.apache.ignite.internal.client.sql.ClientSql;
+import org.apache.ignite.internal.sql.api.ColumnMetadataImpl.ColumnOriginImpl;
+import org.apache.ignite.internal.sql.engine.ClusterPerClassIntegrationTest;
+import org.apache.ignite.internal.sql.engine.QueryCancelledException;
+import org.apache.ignite.internal.testframework.IgniteTestUtils;
+import org.apache.ignite.internal.tx.TxManager;
+import org.apache.ignite.lang.ColumnAlreadyExistsException;
+import org.apache.ignite.lang.ColumnNotFoundException;
+import org.apache.ignite.lang.ErrorGroups;
+import org.apache.ignite.lang.ErrorGroups.Catalog;
+import org.apache.ignite.lang.ErrorGroups.Index;
+import org.apache.ignite.lang.ErrorGroups.Sql;
+import org.apache.ignite.lang.ErrorGroups.Transactions;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.IndexAlreadyExistsException;
+import org.apache.ignite.lang.IndexNotFoundException;
+import org.apache.ignite.lang.TableAlreadyExistsException;
+import org.apache.ignite.lang.TableNotFoundException;
+import org.apache.ignite.sql.BatchedArguments;
+import org.apache.ignite.sql.ColumnMetadata;
+import org.apache.ignite.sql.ColumnType;
+import org.apache.ignite.sql.CursorClosedException;
+import org.apache.ignite.sql.IgniteSql;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+import org.apache.ignite.sql.ResultSet;
+import org.apache.ignite.sql.ResultSetMetadata;
+import org.apache.ignite.sql.Session;
+import org.apache.ignite.sql.SqlBatchException;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.SqlRow;
+import org.apache.ignite.table.Table;
+import org.apache.ignite.tx.Transaction;
+import org.apache.ignite.tx.TransactionOptions;
+import org.hamcrest.Matcher;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests for SQL API.
+ * Tests will be run through synchronous, asynchronous API and client entry points.
+ * By default, any SQL API test should be added to the base class and use special provided methods to interact
+ * with the API in a API-type-independent manner. For any API-specific test, should be used the appropriate subclass.
+ */
+public abstract class ItSqlApiBaseTest extends ClusterPerClassIntegrationTest {
+ protected static final int ROW_COUNT = 16;
+
+ @AfterEach
+ public void dropTables() {
+ for (Table t : CLUSTER_NODES.get(0).tables().tables()) {
+ sql("DROP TABLE " + t.name());
+ }
+ }
+
+ @Test
+ @Disabled("https://issues.apache.org/jira/browse/IGNITE-20096")
+ public void ddl() throws Exception {
+ IgniteSql sql = igniteSql();
+ Session ses = sql.createSession();
+
+ // CREATE TABLE
+ checkDdl(true, ses, "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+ checkError(
+ TableAlreadyExistsException.class,
+ ErrorGroups.Table.TABLE_ALREADY_EXISTS_ERR,
+ "Table already exists [name=\"PUBLIC\".\"TEST\"]",
+ ses,
+ "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)"
+ );
+ checkSqlError(
+ ErrorGroups.Table.TABLE_DEFINITION_ERR,
+ "Can't create table with duplicate columns: ID, VAL, VAL",
+ ses,
+ "CREATE TABLE TEST1(ID INT PRIMARY KEY, VAL INT, VAL INT)"
+ );
+ checkDdl(false, ses, "CREATE TABLE IF NOT EXISTS TEST(ID INT PRIMARY KEY, VAL VARCHAR)");
+
+ // ADD COLUMN
+ checkDdl(true, ses, "ALTER TABLE TEST ADD COLUMN VAL1 VARCHAR");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"NOT_EXISTS_TABLE\"]",
+ ses,
+ "ALTER TABLE NOT_EXISTS_TABLE ADD COLUMN VAL1 VARCHAR"
+ );
+ checkDdl(false, ses, "ALTER TABLE IF EXISTS NOT_EXISTS_TABLE ADD COLUMN VAL1 VARCHAR");
+ checkError(
+ ColumnAlreadyExistsException.class,
+ ErrorGroups.Table.COLUMN_ALREADY_EXISTS_ERR,
+ "Column already exists [name=\"VAL1\"]",
+ ses,
+ "ALTER TABLE TEST ADD COLUMN VAL1 INT"
+ );
+
+ // CREATE INDEX
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX ON TEST(VAL0)");
+ checkError(
+ IndexAlreadyExistsException.class,
+ Index.INDEX_ALREADY_EXISTS_ERR,
+ "Index already exists [name=\"PUBLIC\".\"TEST_IDX\"]",
+ ses,
+ "CREATE INDEX TEST_IDX ON TEST(VAL1)"
+ );
+ checkDdl(false, ses, "CREATE INDEX IF NOT EXISTS TEST_IDX ON TEST(VAL1)");
+
+ // TODO: IGNITE-19150 We are waiting for schema synchronization to avoid races to create and destroy indexes
+ waitForIndexBuild("TEST", "TEST_IDX");
+
+ checkDdl(true, ses, "DROP INDEX TESt_iDX");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX1 ON TEST(VAL0)");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX2 ON TEST(VAL0)");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX3 ON TEST(ID, VAL0, VAL1)");
+ checkSqlError(
+ Index.INVALID_INDEX_DEFINITION_ERR,
+ "Can't create index on duplicate columns: VAL0, VAL0",
+ ses,
+ "CREATE INDEX TEST_IDX4 ON TEST(VAL0, VAL0)"
+ );
+
+ checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column(s). Column VAL1 is used by indexes [TEST_IDX3].",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN val1"
+ );
+
+ SqlException ex = checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column(s).",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN (val0, val1)"
+ );
+
+ String msg = ex.getMessage();
+ String explainMsg = "Unexpected error message: " + msg;
+
+ assertTrue(msg.contains("Column VAL0 is used by indexes ["), explainMsg);
+ assertTrue(msg.contains("TEST_IDX1") && msg.contains("TEST_IDX2") && msg.contains("TEST_IDX3"), explainMsg);
+ assertTrue(msg.contains("Column VAL1 is used by indexes [TEST_IDX3]"), explainMsg);
+
+ checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column, belongs to primary key: [name=ID]",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN id"
+ );
+
+ // TODO: IGNITE-19150 We are waiting for schema synchronization to avoid races to create and destroy indexes
+ waitForIndexBuild("TEST", "TEST_IDX3");
+ checkDdl(true, ses, "DROP INDEX TESt_iDX3");
+
+ // DROP COLUMNS
+ checkDdl(true, ses, "ALTER TABLE TEST DROP COLUMN VAL1");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"NOT_EXISTS_TABLE\"]",
+ ses,
+ "ALTER TABLE NOT_EXISTS_TABLE DROP COLUMN VAL1"
+ );
+ checkDdl(false, ses, "ALTER TABLE IF EXISTS NOT_EXISTS_TABLE DROP COLUMN VAL1");
+ checkError(
+ ColumnNotFoundException.class,
+ ErrorGroups.Table.COLUMN_NOT_FOUND_ERR,
+ "Column does not exist [tableName=\"PUBLIC\".\"TEST\", columnName=\"VAL1\"]",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN VAL1"
+ );
+
+ // DROP TABLE
+ checkDdl(false, ses, "DROP TABLE IF EXISTS NOT_EXISTS_TABLE");
+
+ checkDdl(true, ses, "DROP TABLE TEST");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"TEST\"]",
+ ses,
+ "DROP TABLE TEST"
+ );
+
+ checkDdl(false, ses, "DROP INDEX IF EXISTS TEST_IDX");
+
+ checkError(
+ IndexNotFoundException.class,
+ Index.INDEX_NOT_FOUND_ERR,
+ "Index does not exist [name=\"PUBLIC\".\"TEST_IDX\"]", ses,
+ "DROP INDEX TEST_IDX"
+ );
+ }
+
+ /** Check all transactions are processed correctly even with case of sql Exception raised. */
+ @Test
+ public void implicitTransactionsStates() {
+ IgniteSql sql = igniteSql();
+
+ if (sql instanceof ClientSql) {
+ return;
+ }
+
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+ Session ses = sql.createSession();
+
+ TxManager txManager = txManager();
+
+ for (int i = 0; i < ROW_COUNT; ++i) {
+ assertThrowsSqlException(
+ Catalog.VALIDATION_ERR,
+ "Table with name 'PUBLIC.TEST' already exists",
+ () -> execute(ses, "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)")
+ );
+ }
+
+ // No new transactions through ddl.
+ assertEquals(0, txManager.pending());
+ }
+
+ /** Check correctness of implicit and explicit transactions. */
+ @Test
+ public void checkTransactionsWithDml() {
+ IgniteSql sql = igniteSql();
+
+ if (sql instanceof ClientSql) {
Review Comment:
agree
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1356112694
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/lang/SqlExceptionMapperUtil.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.lang;
+
+import static org.apache.ignite.internal.lang.IgniteExceptionMapperUtil.mapToPublicException;
+import static org.apache.ignite.lang.ErrorGroups.Common.INTERNAL_ERR;
+
+import org.apache.ignite.lang.ErrorGroups.Common;
+import org.apache.ignite.lang.IgniteCheckedException;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.TraceableException;
+import org.apache.ignite.sql.SqlException;
+
+/**
+ * This utility class provides an ability to map Ignite internal exceptions to public SqlException.
+ */
+public class SqlExceptionMapperUtil {
+
+ /**
+ * This method provides a mapping from internal exception to SQL public ones.
+ *
+ * <p>The rules of mapping are the following:</p>
+ * <ul>
+ * <li>any instance of {@link Error} is returned as is, except {@link AssertionError}
+ * that will always be mapped to {@link IgniteException} with the {@link Common#INTERNAL_ERR} error code.</li>
+ * <li>any instance of {@link IgniteException} or {@link IgniteCheckedException} is returned as is.</li>
+ * <li>any other instance of {@link TraceableException} is wrapped into {@link SqlException}
+ * with the original {@link TraceableException#traceId() traceUd} and {@link TraceableException#code() code}.</li>
+ * <li>if there are no any mappers that can do a mapping from the given error to a public exception,
+ * then {@link SqlException} with the {@link Common#INTERNAL_ERR} error code is returned.</li>
+ * </ul>
+ *
+ * @param origin Exception to be mapped.
+ * @return Public exception.
+ */
+ public static Throwable mapToPublicSqlException(Throwable origin) {
+ Throwable e = mapToPublicException(origin);
+ if (e instanceof Error) {
+ return e;
+ }
+ if (e instanceof SqlException) {
+ return e;
+ }
+ if (e instanceof IgniteException && ((IgniteException) e).code() == INTERNAL_ERR) {
+ return new SqlException(((IgniteException) e).traceId(), INTERNAL_ERR, e.getCause());
Review Comment:
We don't put message for non SQL API public exceptions, so it shouldn't be here too.
Original stacktrace is not lost - it's contains in cause
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "korlov42 (via GitHub)" <gi...@apache.org>.
korlov42 commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1358272642
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/lang/SqlExceptionMapperUtil.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.lang;
+
+import static org.apache.ignite.internal.lang.IgniteExceptionMapperUtil.mapToPublicException;
+import static org.apache.ignite.lang.ErrorGroups.Common.INTERNAL_ERR;
+
+import org.apache.ignite.lang.ErrorGroups.Common;
+import org.apache.ignite.lang.IgniteCheckedException;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.TraceableException;
+import org.apache.ignite.sql.SqlException;
+
+/**
+ * This utility class provides an ability to map Ignite internal exceptions to public SqlException.
+ */
+public class SqlExceptionMapperUtil {
+
+ /**
+ * This method provides a mapping from internal exception to SQL public ones.
+ *
+ * <p>The rules of mapping are the following:</p>
+ * <ul>
+ * <li>any instance of {@link Error} is returned as is, except {@link AssertionError}
+ * that will always be mapped to {@link IgniteException} with the {@link Common#INTERNAL_ERR} error code.</li>
Review Comment:
as far as I can see, AssertionError will be mapped to SqlException
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1348484039
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlApiBaseTest.java:
##########
@@ -0,0 +1,860 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.sql.api;
+
+import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsIndexScan;
+import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsTableScan;
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.asStream;
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.assertThrowsSqlException;
+import static org.apache.ignite.internal.testframework.IgniteTestUtils.assertThrowsWithCause;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.catalog.commands.CatalogUtils;
+import org.apache.ignite.internal.client.sql.ClientSql;
+import org.apache.ignite.internal.sql.api.ColumnMetadataImpl.ColumnOriginImpl;
+import org.apache.ignite.internal.sql.engine.ClusterPerClassIntegrationTest;
+import org.apache.ignite.internal.sql.engine.QueryCancelledException;
+import org.apache.ignite.internal.testframework.IgniteTestUtils;
+import org.apache.ignite.internal.tx.TxManager;
+import org.apache.ignite.lang.ColumnAlreadyExistsException;
+import org.apache.ignite.lang.ColumnNotFoundException;
+import org.apache.ignite.lang.ErrorGroups;
+import org.apache.ignite.lang.ErrorGroups.Catalog;
+import org.apache.ignite.lang.ErrorGroups.Index;
+import org.apache.ignite.lang.ErrorGroups.Sql;
+import org.apache.ignite.lang.ErrorGroups.Transactions;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.IndexAlreadyExistsException;
+import org.apache.ignite.lang.IndexNotFoundException;
+import org.apache.ignite.lang.TableAlreadyExistsException;
+import org.apache.ignite.lang.TableNotFoundException;
+import org.apache.ignite.sql.BatchedArguments;
+import org.apache.ignite.sql.ColumnMetadata;
+import org.apache.ignite.sql.ColumnType;
+import org.apache.ignite.sql.CursorClosedException;
+import org.apache.ignite.sql.IgniteSql;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+import org.apache.ignite.sql.ResultSet;
+import org.apache.ignite.sql.ResultSetMetadata;
+import org.apache.ignite.sql.Session;
+import org.apache.ignite.sql.SqlBatchException;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.SqlRow;
+import org.apache.ignite.table.Table;
+import org.apache.ignite.tx.Transaction;
+import org.apache.ignite.tx.TransactionOptions;
+import org.hamcrest.Matcher;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests for SQL API.
+ * Tests will be run through synchronous, asynchronous API and client entry points.
+ * By default, any SQL API test should be added to the base class and use special provided methods to interact
+ * with the API in a API-type-independent manner. For any API-specific test, should be used the appropriate subclass.
+ */
+public abstract class ItSqlApiBaseTest extends ClusterPerClassIntegrationTest {
+ protected static final int ROW_COUNT = 16;
+
+ @AfterEach
+ public void dropTables() {
+ for (Table t : CLUSTER_NODES.get(0).tables().tables()) {
+ sql("DROP TABLE " + t.name());
+ }
+ }
+
+ @Test
+ @Disabled("https://issues.apache.org/jira/browse/IGNITE-20096")
+ public void ddl() throws Exception {
+ IgniteSql sql = igniteSql();
+ Session ses = sql.createSession();
+
+ // CREATE TABLE
+ checkDdl(true, ses, "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+ checkError(
+ TableAlreadyExistsException.class,
+ ErrorGroups.Table.TABLE_ALREADY_EXISTS_ERR,
+ "Table already exists [name=\"PUBLIC\".\"TEST\"]",
+ ses,
+ "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)"
+ );
+ checkSqlError(
+ ErrorGroups.Table.TABLE_DEFINITION_ERR,
+ "Can't create table with duplicate columns: ID, VAL, VAL",
+ ses,
+ "CREATE TABLE TEST1(ID INT PRIMARY KEY, VAL INT, VAL INT)"
+ );
+ checkDdl(false, ses, "CREATE TABLE IF NOT EXISTS TEST(ID INT PRIMARY KEY, VAL VARCHAR)");
+
+ // ADD COLUMN
+ checkDdl(true, ses, "ALTER TABLE TEST ADD COLUMN VAL1 VARCHAR");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"NOT_EXISTS_TABLE\"]",
+ ses,
+ "ALTER TABLE NOT_EXISTS_TABLE ADD COLUMN VAL1 VARCHAR"
+ );
+ checkDdl(false, ses, "ALTER TABLE IF EXISTS NOT_EXISTS_TABLE ADD COLUMN VAL1 VARCHAR");
+ checkError(
+ ColumnAlreadyExistsException.class,
+ ErrorGroups.Table.COLUMN_ALREADY_EXISTS_ERR,
+ "Column already exists [name=\"VAL1\"]",
+ ses,
+ "ALTER TABLE TEST ADD COLUMN VAL1 INT"
+ );
+
+ // CREATE INDEX
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX ON TEST(VAL0)");
+ checkError(
+ IndexAlreadyExistsException.class,
+ Index.INDEX_ALREADY_EXISTS_ERR,
+ "Index already exists [name=\"PUBLIC\".\"TEST_IDX\"]",
+ ses,
+ "CREATE INDEX TEST_IDX ON TEST(VAL1)"
+ );
+ checkDdl(false, ses, "CREATE INDEX IF NOT EXISTS TEST_IDX ON TEST(VAL1)");
+
+ // TODO: IGNITE-19150 We are waiting for schema synchronization to avoid races to create and destroy indexes
+ waitForIndexBuild("TEST", "TEST_IDX");
+
+ checkDdl(true, ses, "DROP INDEX TESt_iDX");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX1 ON TEST(VAL0)");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX2 ON TEST(VAL0)");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX3 ON TEST(ID, VAL0, VAL1)");
+ checkSqlError(
+ Index.INVALID_INDEX_DEFINITION_ERR,
+ "Can't create index on duplicate columns: VAL0, VAL0",
+ ses,
+ "CREATE INDEX TEST_IDX4 ON TEST(VAL0, VAL0)"
+ );
+
+ checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column(s). Column VAL1 is used by indexes [TEST_IDX3].",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN val1"
+ );
+
+ SqlException ex = checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column(s).",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN (val0, val1)"
+ );
+
+ String msg = ex.getMessage();
+ String explainMsg = "Unexpected error message: " + msg;
+
+ assertTrue(msg.contains("Column VAL0 is used by indexes ["), explainMsg);
+ assertTrue(msg.contains("TEST_IDX1") && msg.contains("TEST_IDX2") && msg.contains("TEST_IDX3"), explainMsg);
+ assertTrue(msg.contains("Column VAL1 is used by indexes [TEST_IDX3]"), explainMsg);
+
+ checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column, belongs to primary key: [name=ID]",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN id"
+ );
+
+ // TODO: IGNITE-19150 We are waiting for schema synchronization to avoid races to create and destroy indexes
+ waitForIndexBuild("TEST", "TEST_IDX3");
+ checkDdl(true, ses, "DROP INDEX TESt_iDX3");
+
+ // DROP COLUMNS
+ checkDdl(true, ses, "ALTER TABLE TEST DROP COLUMN VAL1");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"NOT_EXISTS_TABLE\"]",
+ ses,
+ "ALTER TABLE NOT_EXISTS_TABLE DROP COLUMN VAL1"
+ );
+ checkDdl(false, ses, "ALTER TABLE IF EXISTS NOT_EXISTS_TABLE DROP COLUMN VAL1");
+ checkError(
+ ColumnNotFoundException.class,
+ ErrorGroups.Table.COLUMN_NOT_FOUND_ERR,
+ "Column does not exist [tableName=\"PUBLIC\".\"TEST\", columnName=\"VAL1\"]",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN VAL1"
+ );
+
+ // DROP TABLE
+ checkDdl(false, ses, "DROP TABLE IF EXISTS NOT_EXISTS_TABLE");
+
+ checkDdl(true, ses, "DROP TABLE TEST");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"TEST\"]",
+ ses,
+ "DROP TABLE TEST"
+ );
+
+ checkDdl(false, ses, "DROP INDEX IF EXISTS TEST_IDX");
+
+ checkError(
+ IndexNotFoundException.class,
+ Index.INDEX_NOT_FOUND_ERR,
+ "Index does not exist [name=\"PUBLIC\".\"TEST_IDX\"]", ses,
+ "DROP INDEX TEST_IDX"
+ );
+ }
+
+ /** Check all transactions are processed correctly even with case of sql Exception raised. */
+ @Test
+ public void implicitTransactionsStates() {
+ IgniteSql sql = igniteSql();
+
+ if (sql instanceof ClientSql) {
+ return;
+ }
+
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+ Session ses = sql.createSession();
+
+ TxManager txManager = txManager();
+
+ for (int i = 0; i < ROW_COUNT; ++i) {
+ assertThrowsSqlException(
+ Catalog.VALIDATION_ERR,
+ "Table with name 'PUBLIC.TEST' already exists",
+ () -> execute(ses, "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)")
+ );
+ }
+
+ // No new transactions through ddl.
+ assertEquals(0, txManager.pending());
+ }
+
+ /** Check correctness of implicit and explicit transactions. */
+ @Test
+ public void checkTransactionsWithDml() {
+ IgniteSql sql = igniteSql();
+
+ if (sql instanceof ClientSql) {
+ return;
+ }
+
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+ Session ses = sql.createSession();
+
+ TxManager txManagerInternal = txManager();
+
+ int txPrevCnt = txManagerInternal.finished();
+
+ for (int i = 0; i < ROW_COUNT; ++i) {
+ checkDml(1, ses, "INSERT INTO TEST VALUES (?, ?)", i, i);
+ }
+
+ // Outer tx with further commit.
+ Transaction outerTx = igniteTx().begin();
+
+ for (int i = ROW_COUNT; i < 2 * ROW_COUNT; ++i) {
+ checkDml(1, outerTx, ses, "INSERT INTO TEST VALUES (?, ?)", i, i);
+ }
+
+ commit(outerTx);
+
+ // Outdated tx.
+ Transaction outerTx0 = outerTx;
+ //ToDo: IGNITE-20387 , here should be used assertThrowsSqlException method with code and message `"Transaction is already finished"
+ IgniteException e = assertThrows(IgniteException.class,
+ () -> checkDml(1, outerTx0, ses, "INSERT INTO TEST VALUES (?, ?)", ROW_COUNT, Integer.MAX_VALUE));
+ assertEquals(Transactions.TX_FAILED_READ_WRITE_OPERATION_ERR, e.code());
+
+ assertThrowsSqlException(
+ Sql.CONSTRAINT_VIOLATION_ERR,
+ "PK unique constraint is violated",
+ () -> checkDml(1, ses, "INSERT INTO TEST VALUES (?, ?)", ROW_COUNT, Integer.MAX_VALUE));
+
+ ResultSet<SqlRow> rs = executeForRead(ses, "SELECT VAL0 FROM TEST ORDER BY VAL0");
+
+ assertEquals(2 * ROW_COUNT, asStream(rs).count());
+
+ rs.close();
+
+ outerTx = igniteTx().begin();
+
+ rs = executeForRead(ses, outerTx, "SELECT VAL0 FROM TEST ORDER BY VAL0");
+
+ assertEquals(2 * ROW_COUNT, asStream(rs).count());
+
+ rs.close();
+
+ outerTx.commit();
+
+ checkDml(2 * ROW_COUNT, ses, "UPDATE TEST SET VAL0 = VAL0 + ?", 1);
+
+ checkDml(2 * ROW_COUNT, ses, "DELETE FROM TEST WHERE VAL0 >= 0");
+
+ assertEquals(ROW_COUNT + 1 + 1 + 1 + 1 + 1 + 1, txManagerInternal.finished() - txPrevCnt);
+
+ assertEquals(0, txManagerInternal.pending());
+ }
+
+ /** Check correctness of explicit transaction rollback. */
+ @Test
+ public void checkExplicitTxRollback() {
+ IgniteSql sql = igniteSql();
+
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+ Session ses = sql.createSession();
+
+ // Outer tx with further commit.
+ Transaction outerTx = igniteTx().begin();
+
+ for (int i = 0; i < ROW_COUNT; ++i) {
+ checkDml(1, outerTx, ses, "INSERT INTO TEST VALUES (?, ?)", i, i);
+ }
+
+ rollback(outerTx);
+
+ ResultSet<SqlRow> rs = executeForRead(ses, "SELECT VAL0 FROM TEST ORDER BY VAL0");
+
+ asStream(rs);
+ assertEquals(0, asStream(rs).count());
+
+ rs.close();
+ }
+
+ /** Check correctness of rw and ro transactions for table scan. */
+ @Test
+ public void checkMixedTransactionsForTable() {
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+ Matcher<String> planMatcher = containsTableScan("PUBLIC", "TEST");
+
+ checkMixedTransactions(planMatcher);
+ }
+
+
+ /** Check correctness of rw and ro transactions for index scan. */
+ @Test
+ public void checkMixedTransactionsForIndex() throws Exception {
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+ sql("CREATE INDEX TEST_IDX ON TEST(VAL0)");
+
+ Matcher<String> planMatcher = containsIndexScan("PUBLIC", "TEST", "TEST_IDX");
+
+ checkMixedTransactions(planMatcher);
+ }
+
+ private void checkMixedTransactions(Matcher<String> planMatcher) {
+ IgniteSql sql = igniteSql();
+
+ if (sql instanceof ClientSql) {
Review Comment:
I think it is better to move such guards into a test case body directly, so we know that a test case is not expected to run for sql clients.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1355846290
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/lang/SqlExceptionMapperUtil.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.lang;
+
+import static org.apache.ignite.internal.lang.IgniteExceptionMapperUtil.mapToPublicException;
+import static org.apache.ignite.lang.ErrorGroups.Common.INTERNAL_ERR;
+
+import org.apache.ignite.lang.ErrorGroups.Common;
+import org.apache.ignite.lang.IgniteCheckedException;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.TraceableException;
+import org.apache.ignite.sql.SqlException;
+
+/**
+ * This utility class provides an ability to map Ignite internal exceptions to public SqlException.
+ */
+public class SqlExceptionMapperUtil {
+
+ /**
+ * This method provides a mapping from internal exception to SQL public ones.
+ *
+ * <p>The rules of mapping are the following:</p>
+ * <ul>
+ * <li>any instance of {@link Error} is returned as is, except {@link AssertionError}
+ * that will always be mapped to {@link IgniteException} with the {@link Common#INTERNAL_ERR} error code.</li>
+ * <li>any instance of {@link IgniteException} or {@link IgniteCheckedException} is returned as is.</li>
+ * <li>any other instance of {@link TraceableException} is wrapped into {@link SqlException}
+ * with the original {@link TraceableException#traceId() traceUd} and {@link TraceableException#code() code}.</li>
+ * <li>if there are no any mappers that can do a mapping from the given error to a public exception,
+ * then {@link SqlException} with the {@link Common#INTERNAL_ERR} error code is returned.</li>
+ * </ul>
+ *
+ * @param origin Exception to be mapped.
+ * @return Public exception.
+ */
+ public static Throwable mapToPublicSqlException(Throwable origin) {
+ Throwable e = mapToPublicException(origin);
+ if (e instanceof Error) {
+ return e;
+ }
+ if (e instanceof SqlException) {
+ return e;
+ }
+ if (e instanceof IgniteException && ((IgniteException) e).code() == INTERNAL_ERR) {
+ return new SqlException(((IgniteException) e).traceId(), INTERNAL_ERR, e.getCause());
Review Comment:
:thinking: in this case, the original stacktrace is lost and I'm not sure that this original exception was created by `mapToPublicException` call
p.s. as I understand the original error message is also lost
##########
modules/core/src/main/java/org/apache/ignite/internal/lang/IgniteExceptionMapperUtil.java:
##########
@@ -87,32 +87,40 @@ public static Throwable mapToPublicException(Throwable origin) {
if (origin instanceof AssertionError) {
return new IgniteException(INTERNAL_ERR, origin);
}
-
return origin;
}
- IgniteExceptionMapper<? extends Exception, ? extends Exception> m = EXCEPTION_CONVERTERS.get(origin.getClass());
+ Throwable res;
+
+ // Try to find appropriate mapper, moving from original class to supper-classes step by step.
+ Class exceptionClass = origin.getClass();
+ IgniteExceptionMapper<? extends Exception, ? extends Exception> m;
+ while ((m = EXCEPTION_CONVERTERS.get(exceptionClass)) == null && exceptionClass != Throwable.class) {
+ exceptionClass = exceptionClass.getSuperclass();
+ }
+
if (m != null) {
- Exception mapped = map(m, origin);
+ res = map(m, origin);
- assert mapped instanceof IgniteException || mapped instanceof IgniteCheckedException :
- "Unexpected mapping of internal exception to a public one [origin=" + origin + ", mapped=" + mapped + ']';
+ assert res instanceof IgniteException || res instanceof IgniteCheckedException :
+ "Unexpected mapping of internal exception to a public one [origin=" + origin + ", mapped=" + res + ']';
- return mapped;
+ } else {
+ res = origin;
}
- if (origin instanceof IgniteException || origin instanceof IgniteCheckedException) {
-
- return origin;
+ if (res instanceof IgniteException || res instanceof IgniteCheckedException) {
+ return res;
}
// There are no exception mappings for the given exception. This case should be considered as internal error.
return new IgniteException(INTERNAL_ERR, origin);
+
Review Comment:
```suggestion
```
##########
modules/core/src/test/java/org/apache/ignite/internal/lang/IgniteExceptionMapperUtilTest.java:
##########
@@ -203,5 +203,12 @@ public static class CustomNoMappingException extends IgniteInternalException {
public CustomNoMappingException() {
super(INTERNAL_ERR, "Test internal exception [err=no mapping]");
}
+
+ /**
+ * Creates a new instance of CustomNoMappingException with given code.
+ */
+ public CustomNoMappingException(int code) {
Review Comment:
it seems this constructor is no longer used
##########
modules/system-view/src/test/java/org/apache/ignite/internal/lang/SqlExceptionMapperUtilTest.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.lang;
+
+import static org.apache.ignite.internal.lang.SqlExceptionMapperUtil.mapToPublicSqlException;
+import static org.apache.ignite.lang.ErrorGroups.Common.INTERNAL_ERR;
+import static org.apache.ignite.lang.ErrorGroups.Sql.EXECUTION_CANCELLED_ERR;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertSame;
+
+import org.apache.ignite.sql.NoRowSetExpectedException;
+import org.apache.ignite.sql.SqlException;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests mapping internal exceptions to public SqlException.
+ */
+class SqlExceptionMapperUtilTest {
+ /**
+ * Tests a default mapping of internal exceptions passed from the sql engine.
+ */
+ @Test
+ public void testSqlInternalExceptionDefaultMapping() {
+ CustomNoMappingException internalSqlErr = new CustomNoMappingException(EXECUTION_CANCELLED_ERR);
+ Throwable mappedErr = mapToPublicSqlException(internalSqlErr);
+
+ assertThat(mappedErr, instanceOf(SqlException.class));
+
+ SqlException mappedSqlErr = (SqlException) mappedErr;
+
+ assertThat("Mapped exception should have the same trace identifier.", mappedSqlErr.traceId(), is(internalSqlErr.traceId()));
+ assertThat("Mapped exception shouldn't have the same error code.", mappedSqlErr.code(), is(INTERNAL_ERR));
+ }
+
+ /**
+ * Tests a default mapping of internal exceptions passed from the sql engine.
+ */
+ @Test
+ public void testSqlInternalExceptionDefaultMappingForSqlException() {
+ NoRowSetExpectedException sqlErr = new NoRowSetExpectedException();
+
+ Throwable mappedErr = mapToPublicSqlException(sqlErr);
+
+ assertSame(sqlErr, mappedErr);
+ }
+
+ /**
+ * Test exception.
+ */
+ public static class CustomNoMappingException extends IgniteInternalException {
+ /** Serial version UID. */
+ private static final long serialVersionUID = 0L;
+
+ /**
+ * Creates a new instance of CustomNoMappingException.
+ */
+ public CustomNoMappingException() {
Review Comment:
this constructor is not used
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1350395935
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItTablesApiTest.java:
##########
@@ -140,8 +142,10 @@ public void testTableAlreadyCreated() {
Table tbl = createTable(ignite0, TABLE_NAME);
- // TODO: IGNITE-20388 Fix it
- assertThrowsWithCause(() -> createTable(ignite0, TABLE_NAME), TableExistsValidationException.class);
+ assertThrowsSqlException(
+ Catalog.VALIDATION_ERR,
Review Comment:
Catalog.VALIDATION_ERR code has been mapped to Sql.STMT_VALIDATION_ERR. For other cases I think we should as is
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1350165947
##########
modules/core/src/main/java/org/apache/ignite/internal/sql/SyncResultSetAdapter.java:
##########
@@ -32,7 +32,7 @@
/**
* Synchronous wrapper over {@link org.apache.ignite.sql.async.AsyncResultSet}.
*/
-class SyncResultSetAdapter<T> implements ResultSet<T> {
+public class SyncResultSetAdapter<T> implements ResultSet<T> {
Review Comment:
The class requires also for test.I see two options here:
1. It can be public here
2 Move to utils package, but will be public in any case
Do you have another options?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1356119316
##########
modules/system-view/src/test/java/org/apache/ignite/internal/lang/SqlExceptionMapperUtilTest.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.lang;
+
+import static org.apache.ignite.internal.lang.SqlExceptionMapperUtil.mapToPublicSqlException;
+import static org.apache.ignite.lang.ErrorGroups.Common.INTERNAL_ERR;
+import static org.apache.ignite.lang.ErrorGroups.Sql.EXECUTION_CANCELLED_ERR;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertSame;
+
+import org.apache.ignite.sql.NoRowSetExpectedException;
+import org.apache.ignite.sql.SqlException;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests mapping internal exceptions to public SqlException.
+ */
+class SqlExceptionMapperUtilTest {
+ /**
+ * Tests a default mapping of internal exceptions passed from the sql engine.
+ */
+ @Test
+ public void testSqlInternalExceptionDefaultMapping() {
+ CustomNoMappingException internalSqlErr = new CustomNoMappingException(EXECUTION_CANCELLED_ERR);
+ Throwable mappedErr = mapToPublicSqlException(internalSqlErr);
+
+ assertThat(mappedErr, instanceOf(SqlException.class));
+
+ SqlException mappedSqlErr = (SqlException) mappedErr;
+
+ assertThat("Mapped exception should have the same trace identifier.", mappedSqlErr.traceId(), is(internalSqlErr.traceId()));
+ assertThat("Mapped exception shouldn't have the same error code.", mappedSqlErr.code(), is(INTERNAL_ERR));
+ }
+
+ /**
+ * Tests a default mapping of internal exceptions passed from the sql engine.
+ */
+ @Test
+ public void testSqlInternalExceptionDefaultMappingForSqlException() {
+ NoRowSetExpectedException sqlErr = new NoRowSetExpectedException();
+
+ Throwable mappedErr = mapToPublicSqlException(sqlErr);
+
+ assertSame(sqlErr, mappedErr);
+ }
+
+ /**
+ * Test exception.
+ */
+ public static class CustomNoMappingException extends IgniteInternalException {
+ /** Serial version UID. */
+ private static final long serialVersionUID = 0L;
+
+ /**
+ * Creates a new instance of CustomNoMappingException.
+ */
+ public CustomNoMappingException() {
Review Comment:
yep
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1358393821
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/lang/SqlExceptionMapperUtil.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.lang;
+
+import static org.apache.ignite.internal.lang.IgniteExceptionMapperUtil.mapToPublicException;
+import static org.apache.ignite.lang.ErrorGroups.Common.INTERNAL_ERR;
+
+import org.apache.ignite.lang.ErrorGroups.Common;
+import org.apache.ignite.lang.IgniteCheckedException;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.TraceableException;
+import org.apache.ignite.sql.SqlException;
+
+/**
+ * This utility class provides an ability to map Ignite internal exceptions to public SqlException.
+ */
+public class SqlExceptionMapperUtil {
+
+ /**
+ * This method provides a mapping from internal exception to SQL public ones.
+ *
+ * <p>The rules of mapping are the following:</p>
+ * <ul>
+ * <li>any instance of {@link Error} is returned as is, except {@link AssertionError}
+ * that will always be mapped to {@link IgniteException} with the {@link Common#INTERNAL_ERR} error code.</li>
Review Comment:
fixed
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/lang/SqlExceptionMapperUtil.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.lang;
+
+import static org.apache.ignite.internal.lang.IgniteExceptionMapperUtil.mapToPublicException;
+import static org.apache.ignite.lang.ErrorGroups.Common.INTERNAL_ERR;
+
+import org.apache.ignite.lang.ErrorGroups.Common;
+import org.apache.ignite.lang.IgniteCheckedException;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.TraceableException;
+import org.apache.ignite.sql.SqlException;
+
+/**
+ * This utility class provides an ability to map Ignite internal exceptions to public SqlException.
+ */
+public class SqlExceptionMapperUtil {
+
+ /**
+ * This method provides a mapping from internal exception to SQL public ones.
+ *
+ * <p>The rules of mapping are the following:</p>
+ * <ul>
+ * <li>any instance of {@link Error} is returned as is, except {@link AssertionError}
+ * that will always be mapped to {@link IgniteException} with the {@link Common#INTERNAL_ERR} error code.</li>
+ * <li>any instance of {@link IgniteException} or {@link IgniteCheckedException} is returned as is.</li>
Review Comment:
fixed
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1350396420
##########
modules/client-handler/src/main/java/org/apache/ignite/client/handler/JdbcQueryEventHandlerImpl.java:
##########
@@ -498,7 +498,7 @@ <T> CompletableFuture<T> doInSession(SessionAwareAction<T> action) {
return action.perform(newSessionId);
}
- return CompletableFuture.failedFuture(IgniteExceptionMapperUtil.mapToPublicException(error));
+ return CompletableFuture.failedFuture(IgniteExceptionMapperUtil.mapToPublicSqlException(error));
Review Comment:
agree
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1348531510
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlApiBaseTest.java:
##########
@@ -0,0 +1,860 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.sql.api;
+
+import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsIndexScan;
+import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsTableScan;
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.asStream;
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.assertThrowsSqlException;
+import static org.apache.ignite.internal.testframework.IgniteTestUtils.assertThrowsWithCause;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.catalog.commands.CatalogUtils;
+import org.apache.ignite.internal.client.sql.ClientSql;
+import org.apache.ignite.internal.sql.api.ColumnMetadataImpl.ColumnOriginImpl;
+import org.apache.ignite.internal.sql.engine.ClusterPerClassIntegrationTest;
+import org.apache.ignite.internal.sql.engine.QueryCancelledException;
+import org.apache.ignite.internal.testframework.IgniteTestUtils;
+import org.apache.ignite.internal.tx.TxManager;
+import org.apache.ignite.lang.ColumnAlreadyExistsException;
+import org.apache.ignite.lang.ColumnNotFoundException;
+import org.apache.ignite.lang.ErrorGroups;
+import org.apache.ignite.lang.ErrorGroups.Catalog;
+import org.apache.ignite.lang.ErrorGroups.Index;
+import org.apache.ignite.lang.ErrorGroups.Sql;
+import org.apache.ignite.lang.ErrorGroups.Transactions;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.IndexAlreadyExistsException;
+import org.apache.ignite.lang.IndexNotFoundException;
+import org.apache.ignite.lang.TableAlreadyExistsException;
+import org.apache.ignite.lang.TableNotFoundException;
+import org.apache.ignite.sql.BatchedArguments;
+import org.apache.ignite.sql.ColumnMetadata;
+import org.apache.ignite.sql.ColumnType;
+import org.apache.ignite.sql.CursorClosedException;
+import org.apache.ignite.sql.IgniteSql;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+import org.apache.ignite.sql.ResultSet;
+import org.apache.ignite.sql.ResultSetMetadata;
+import org.apache.ignite.sql.Session;
+import org.apache.ignite.sql.SqlBatchException;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.SqlRow;
+import org.apache.ignite.table.Table;
+import org.apache.ignite.tx.Transaction;
+import org.apache.ignite.tx.TransactionOptions;
+import org.hamcrest.Matcher;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests for SQL API.
+ * Tests will be run through synchronous, asynchronous API and client entry points.
+ * By default, any SQL API test should be added to the base class and use special provided methods to interact
+ * with the API in a API-type-independent manner. For any API-specific test, should be used the appropriate subclass.
+ */
+public abstract class ItSqlApiBaseTest extends ClusterPerClassIntegrationTest {
+ protected static final int ROW_COUNT = 16;
+
+ @AfterEach
+ public void dropTables() {
+ for (Table t : CLUSTER_NODES.get(0).tables().tables()) {
+ sql("DROP TABLE " + t.name());
+ }
+ }
+
+ @Test
+ @Disabled("https://issues.apache.org/jira/browse/IGNITE-20096")
+ public void ddl() throws Exception {
+ IgniteSql sql = igniteSql();
+ Session ses = sql.createSession();
+
+ // CREATE TABLE
+ checkDdl(true, ses, "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+ checkError(
+ TableAlreadyExistsException.class,
+ ErrorGroups.Table.TABLE_ALREADY_EXISTS_ERR,
+ "Table already exists [name=\"PUBLIC\".\"TEST\"]",
+ ses,
+ "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)"
+ );
+ checkSqlError(
+ ErrorGroups.Table.TABLE_DEFINITION_ERR,
+ "Can't create table with duplicate columns: ID, VAL, VAL",
+ ses,
+ "CREATE TABLE TEST1(ID INT PRIMARY KEY, VAL INT, VAL INT)"
+ );
+ checkDdl(false, ses, "CREATE TABLE IF NOT EXISTS TEST(ID INT PRIMARY KEY, VAL VARCHAR)");
+
+ // ADD COLUMN
+ checkDdl(true, ses, "ALTER TABLE TEST ADD COLUMN VAL1 VARCHAR");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"NOT_EXISTS_TABLE\"]",
+ ses,
+ "ALTER TABLE NOT_EXISTS_TABLE ADD COLUMN VAL1 VARCHAR"
+ );
+ checkDdl(false, ses, "ALTER TABLE IF EXISTS NOT_EXISTS_TABLE ADD COLUMN VAL1 VARCHAR");
+ checkError(
+ ColumnAlreadyExistsException.class,
+ ErrorGroups.Table.COLUMN_ALREADY_EXISTS_ERR,
+ "Column already exists [name=\"VAL1\"]",
+ ses,
+ "ALTER TABLE TEST ADD COLUMN VAL1 INT"
+ );
+
+ // CREATE INDEX
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX ON TEST(VAL0)");
+ checkError(
+ IndexAlreadyExistsException.class,
+ Index.INDEX_ALREADY_EXISTS_ERR,
+ "Index already exists [name=\"PUBLIC\".\"TEST_IDX\"]",
+ ses,
+ "CREATE INDEX TEST_IDX ON TEST(VAL1)"
+ );
+ checkDdl(false, ses, "CREATE INDEX IF NOT EXISTS TEST_IDX ON TEST(VAL1)");
+
+ // TODO: IGNITE-19150 We are waiting for schema synchronization to avoid races to create and destroy indexes
+ waitForIndexBuild("TEST", "TEST_IDX");
+
+ checkDdl(true, ses, "DROP INDEX TESt_iDX");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX1 ON TEST(VAL0)");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX2 ON TEST(VAL0)");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX3 ON TEST(ID, VAL0, VAL1)");
+ checkSqlError(
+ Index.INVALID_INDEX_DEFINITION_ERR,
+ "Can't create index on duplicate columns: VAL0, VAL0",
+ ses,
+ "CREATE INDEX TEST_IDX4 ON TEST(VAL0, VAL0)"
+ );
+
+ checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column(s). Column VAL1 is used by indexes [TEST_IDX3].",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN val1"
+ );
+
+ SqlException ex = checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column(s).",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN (val0, val1)"
+ );
+
+ String msg = ex.getMessage();
+ String explainMsg = "Unexpected error message: " + msg;
+
+ assertTrue(msg.contains("Column VAL0 is used by indexes ["), explainMsg);
+ assertTrue(msg.contains("TEST_IDX1") && msg.contains("TEST_IDX2") && msg.contains("TEST_IDX3"), explainMsg);
+ assertTrue(msg.contains("Column VAL1 is used by indexes [TEST_IDX3]"), explainMsg);
+
+ checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column, belongs to primary key: [name=ID]",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN id"
+ );
+
+ // TODO: IGNITE-19150 We are waiting for schema synchronization to avoid races to create and destroy indexes
+ waitForIndexBuild("TEST", "TEST_IDX3");
+ checkDdl(true, ses, "DROP INDEX TESt_iDX3");
+
+ // DROP COLUMNS
+ checkDdl(true, ses, "ALTER TABLE TEST DROP COLUMN VAL1");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"NOT_EXISTS_TABLE\"]",
+ ses,
+ "ALTER TABLE NOT_EXISTS_TABLE DROP COLUMN VAL1"
+ );
+ checkDdl(false, ses, "ALTER TABLE IF EXISTS NOT_EXISTS_TABLE DROP COLUMN VAL1");
+ checkError(
+ ColumnNotFoundException.class,
+ ErrorGroups.Table.COLUMN_NOT_FOUND_ERR,
+ "Column does not exist [tableName=\"PUBLIC\".\"TEST\", columnName=\"VAL1\"]",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN VAL1"
+ );
+
+ // DROP TABLE
+ checkDdl(false, ses, "DROP TABLE IF EXISTS NOT_EXISTS_TABLE");
+
+ checkDdl(true, ses, "DROP TABLE TEST");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"TEST\"]",
+ ses,
+ "DROP TABLE TEST"
+ );
+
+ checkDdl(false, ses, "DROP INDEX IF EXISTS TEST_IDX");
+
+ checkError(
+ IndexNotFoundException.class,
+ Index.INDEX_NOT_FOUND_ERR,
+ "Index does not exist [name=\"PUBLIC\".\"TEST_IDX\"]", ses,
+ "DROP INDEX TEST_IDX"
+ );
+ }
+
+ /** Check all transactions are processed correctly even with case of sql Exception raised. */
+ @Test
+ public void implicitTransactionsStates() {
+ IgniteSql sql = igniteSql();
+
+ if (sql instanceof ClientSql) {
+ return;
+ }
+
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+ Session ses = sql.createSession();
+
+ TxManager txManager = txManager();
+
+ for (int i = 0; i < ROW_COUNT; ++i) {
+ assertThrowsSqlException(
+ Catalog.VALIDATION_ERR,
+ "Table with name 'PUBLIC.TEST' already exists",
+ () -> execute(ses, "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)")
+ );
+ }
+
+ // No new transactions through ddl.
+ assertEquals(0, txManager.pending());
+ }
+
+ /** Check correctness of implicit and explicit transactions. */
+ @Test
+ public void checkTransactionsWithDml() {
+ IgniteSql sql = igniteSql();
+
+ if (sql instanceof ClientSql) {
Review Comment:
Looks like these test fail with the same error (` Error occurred when notifying safe time advanced callback`), so I think it is better to disable these test cases and provide a link to a ticket.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1348842659
##########
modules/core/src/main/java/org/apache/ignite/internal/sql/SyncResultSetAdapter.java:
##########
@@ -32,7 +32,7 @@
/**
* Synchronous wrapper over {@link org.apache.ignite.sql.async.AsyncResultSet}.
*/
-class SyncResultSetAdapter<T> implements ResultSet<T> {
+public class SyncResultSetAdapter<T> implements ResultSet<T> {
Review Comment:
Do we really need to change visibility to public?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1348480445
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlApiBaseTest.java:
##########
@@ -0,0 +1,860 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.sql.api;
+
+import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsIndexScan;
+import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsTableScan;
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.asStream;
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.assertThrowsSqlException;
+import static org.apache.ignite.internal.testframework.IgniteTestUtils.assertThrowsWithCause;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.catalog.commands.CatalogUtils;
+import org.apache.ignite.internal.client.sql.ClientSql;
+import org.apache.ignite.internal.sql.api.ColumnMetadataImpl.ColumnOriginImpl;
+import org.apache.ignite.internal.sql.engine.ClusterPerClassIntegrationTest;
+import org.apache.ignite.internal.sql.engine.QueryCancelledException;
+import org.apache.ignite.internal.testframework.IgniteTestUtils;
+import org.apache.ignite.internal.tx.TxManager;
+import org.apache.ignite.lang.ColumnAlreadyExistsException;
+import org.apache.ignite.lang.ColumnNotFoundException;
+import org.apache.ignite.lang.ErrorGroups;
+import org.apache.ignite.lang.ErrorGroups.Catalog;
+import org.apache.ignite.lang.ErrorGroups.Index;
+import org.apache.ignite.lang.ErrorGroups.Sql;
+import org.apache.ignite.lang.ErrorGroups.Transactions;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.IndexAlreadyExistsException;
+import org.apache.ignite.lang.IndexNotFoundException;
+import org.apache.ignite.lang.TableAlreadyExistsException;
+import org.apache.ignite.lang.TableNotFoundException;
+import org.apache.ignite.sql.BatchedArguments;
+import org.apache.ignite.sql.ColumnMetadata;
+import org.apache.ignite.sql.ColumnType;
+import org.apache.ignite.sql.CursorClosedException;
+import org.apache.ignite.sql.IgniteSql;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+import org.apache.ignite.sql.ResultSet;
+import org.apache.ignite.sql.ResultSetMetadata;
+import org.apache.ignite.sql.Session;
+import org.apache.ignite.sql.SqlBatchException;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.SqlRow;
+import org.apache.ignite.table.Table;
+import org.apache.ignite.tx.Transaction;
+import org.apache.ignite.tx.TransactionOptions;
+import org.hamcrest.Matcher;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests for SQL API.
+ * Tests will be run through synchronous, asynchronous API and client entry points.
+ * By default, any SQL API test should be added to the base class and use special provided methods to interact
+ * with the API in a API-type-independent manner. For any API-specific test, should be used the appropriate subclass.
+ */
+public abstract class ItSqlApiBaseTest extends ClusterPerClassIntegrationTest {
+ protected static final int ROW_COUNT = 16;
+
+ @AfterEach
+ public void dropTables() {
+ for (Table t : CLUSTER_NODES.get(0).tables().tables()) {
+ sql("DROP TABLE " + t.name());
+ }
+ }
+
+ @Test
+ @Disabled("https://issues.apache.org/jira/browse/IGNITE-20096")
+ public void ddl() throws Exception {
+ IgniteSql sql = igniteSql();
+ Session ses = sql.createSession();
+
+ // CREATE TABLE
+ checkDdl(true, ses, "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+ checkError(
+ TableAlreadyExistsException.class,
+ ErrorGroups.Table.TABLE_ALREADY_EXISTS_ERR,
+ "Table already exists [name=\"PUBLIC\".\"TEST\"]",
+ ses,
+ "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)"
+ );
+ checkSqlError(
+ ErrorGroups.Table.TABLE_DEFINITION_ERR,
+ "Can't create table with duplicate columns: ID, VAL, VAL",
+ ses,
+ "CREATE TABLE TEST1(ID INT PRIMARY KEY, VAL INT, VAL INT)"
+ );
+ checkDdl(false, ses, "CREATE TABLE IF NOT EXISTS TEST(ID INT PRIMARY KEY, VAL VARCHAR)");
+
+ // ADD COLUMN
+ checkDdl(true, ses, "ALTER TABLE TEST ADD COLUMN VAL1 VARCHAR");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"NOT_EXISTS_TABLE\"]",
+ ses,
+ "ALTER TABLE NOT_EXISTS_TABLE ADD COLUMN VAL1 VARCHAR"
+ );
+ checkDdl(false, ses, "ALTER TABLE IF EXISTS NOT_EXISTS_TABLE ADD COLUMN VAL1 VARCHAR");
+ checkError(
+ ColumnAlreadyExistsException.class,
+ ErrorGroups.Table.COLUMN_ALREADY_EXISTS_ERR,
+ "Column already exists [name=\"VAL1\"]",
+ ses,
+ "ALTER TABLE TEST ADD COLUMN VAL1 INT"
+ );
+
+ // CREATE INDEX
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX ON TEST(VAL0)");
+ checkError(
+ IndexAlreadyExistsException.class,
+ Index.INDEX_ALREADY_EXISTS_ERR,
+ "Index already exists [name=\"PUBLIC\".\"TEST_IDX\"]",
+ ses,
+ "CREATE INDEX TEST_IDX ON TEST(VAL1)"
+ );
+ checkDdl(false, ses, "CREATE INDEX IF NOT EXISTS TEST_IDX ON TEST(VAL1)");
+
+ // TODO: IGNITE-19150 We are waiting for schema synchronization to avoid races to create and destroy indexes
+ waitForIndexBuild("TEST", "TEST_IDX");
+
+ checkDdl(true, ses, "DROP INDEX TESt_iDX");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX1 ON TEST(VAL0)");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX2 ON TEST(VAL0)");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX3 ON TEST(ID, VAL0, VAL1)");
+ checkSqlError(
+ Index.INVALID_INDEX_DEFINITION_ERR,
+ "Can't create index on duplicate columns: VAL0, VAL0",
+ ses,
+ "CREATE INDEX TEST_IDX4 ON TEST(VAL0, VAL0)"
+ );
+
+ checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column(s). Column VAL1 is used by indexes [TEST_IDX3].",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN val1"
+ );
+
+ SqlException ex = checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column(s).",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN (val0, val1)"
+ );
+
+ String msg = ex.getMessage();
+ String explainMsg = "Unexpected error message: " + msg;
+
+ assertTrue(msg.contains("Column VAL0 is used by indexes ["), explainMsg);
+ assertTrue(msg.contains("TEST_IDX1") && msg.contains("TEST_IDX2") && msg.contains("TEST_IDX3"), explainMsg);
+ assertTrue(msg.contains("Column VAL1 is used by indexes [TEST_IDX3]"), explainMsg);
+
+ checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column, belongs to primary key: [name=ID]",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN id"
+ );
+
+ // TODO: IGNITE-19150 We are waiting for schema synchronization to avoid races to create and destroy indexes
+ waitForIndexBuild("TEST", "TEST_IDX3");
+ checkDdl(true, ses, "DROP INDEX TESt_iDX3");
+
+ // DROP COLUMNS
+ checkDdl(true, ses, "ALTER TABLE TEST DROP COLUMN VAL1");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"NOT_EXISTS_TABLE\"]",
+ ses,
+ "ALTER TABLE NOT_EXISTS_TABLE DROP COLUMN VAL1"
+ );
+ checkDdl(false, ses, "ALTER TABLE IF EXISTS NOT_EXISTS_TABLE DROP COLUMN VAL1");
+ checkError(
+ ColumnNotFoundException.class,
+ ErrorGroups.Table.COLUMN_NOT_FOUND_ERR,
+ "Column does not exist [tableName=\"PUBLIC\".\"TEST\", columnName=\"VAL1\"]",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN VAL1"
+ );
+
+ // DROP TABLE
+ checkDdl(false, ses, "DROP TABLE IF EXISTS NOT_EXISTS_TABLE");
+
+ checkDdl(true, ses, "DROP TABLE TEST");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"TEST\"]",
+ ses,
+ "DROP TABLE TEST"
+ );
+
+ checkDdl(false, ses, "DROP INDEX IF EXISTS TEST_IDX");
+
+ checkError(
+ IndexNotFoundException.class,
+ Index.INDEX_NOT_FOUND_ERR,
+ "Index does not exist [name=\"PUBLIC\".\"TEST_IDX\"]", ses,
+ "DROP INDEX TEST_IDX"
+ );
+ }
+
+ /** Check all transactions are processed correctly even with case of sql Exception raised. */
+ @Test
+ public void implicitTransactionsStates() {
+ IgniteSql sql = igniteSql();
+
+ if (sql instanceof ClientSql) {
+ return;
+ }
+
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+ Session ses = sql.createSession();
+
+ TxManager txManager = txManager();
+
+ for (int i = 0; i < ROW_COUNT; ++i) {
+ assertThrowsSqlException(
+ Catalog.VALIDATION_ERR,
+ "Table with name 'PUBLIC.TEST' already exists",
+ () -> execute(ses, "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)")
+ );
+ }
+
+ // No new transactions through ddl.
+ assertEquals(0, txManager.pending());
+ }
+
+ /** Check correctness of implicit and explicit transactions. */
+ @Test
+ public void checkTransactionsWithDml() {
+ IgniteSql sql = igniteSql();
+
+ if (sql instanceof ClientSql) {
Review Comment:
I think if some test cases are not expected to run for sql clients, then it is better to use `Assumptions.assumeFalse(sql instanceof ClientSql)' instead of simple returns.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1348531510
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlApiBaseTest.java:
##########
@@ -0,0 +1,860 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.sql.api;
+
+import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsIndexScan;
+import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsTableScan;
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.asStream;
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.assertThrowsSqlException;
+import static org.apache.ignite.internal.testframework.IgniteTestUtils.assertThrowsWithCause;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.catalog.commands.CatalogUtils;
+import org.apache.ignite.internal.client.sql.ClientSql;
+import org.apache.ignite.internal.sql.api.ColumnMetadataImpl.ColumnOriginImpl;
+import org.apache.ignite.internal.sql.engine.ClusterPerClassIntegrationTest;
+import org.apache.ignite.internal.sql.engine.QueryCancelledException;
+import org.apache.ignite.internal.testframework.IgniteTestUtils;
+import org.apache.ignite.internal.tx.TxManager;
+import org.apache.ignite.lang.ColumnAlreadyExistsException;
+import org.apache.ignite.lang.ColumnNotFoundException;
+import org.apache.ignite.lang.ErrorGroups;
+import org.apache.ignite.lang.ErrorGroups.Catalog;
+import org.apache.ignite.lang.ErrorGroups.Index;
+import org.apache.ignite.lang.ErrorGroups.Sql;
+import org.apache.ignite.lang.ErrorGroups.Transactions;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.IndexAlreadyExistsException;
+import org.apache.ignite.lang.IndexNotFoundException;
+import org.apache.ignite.lang.TableAlreadyExistsException;
+import org.apache.ignite.lang.TableNotFoundException;
+import org.apache.ignite.sql.BatchedArguments;
+import org.apache.ignite.sql.ColumnMetadata;
+import org.apache.ignite.sql.ColumnType;
+import org.apache.ignite.sql.CursorClosedException;
+import org.apache.ignite.sql.IgniteSql;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+import org.apache.ignite.sql.ResultSet;
+import org.apache.ignite.sql.ResultSetMetadata;
+import org.apache.ignite.sql.Session;
+import org.apache.ignite.sql.SqlBatchException;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.SqlRow;
+import org.apache.ignite.table.Table;
+import org.apache.ignite.tx.Transaction;
+import org.apache.ignite.tx.TransactionOptions;
+import org.hamcrest.Matcher;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests for SQL API.
+ * Tests will be run through synchronous, asynchronous API and client entry points.
+ * By default, any SQL API test should be added to the base class and use special provided methods to interact
+ * with the API in a API-type-independent manner. For any API-specific test, should be used the appropriate subclass.
+ */
+public abstract class ItSqlApiBaseTest extends ClusterPerClassIntegrationTest {
+ protected static final int ROW_COUNT = 16;
+
+ @AfterEach
+ public void dropTables() {
+ for (Table t : CLUSTER_NODES.get(0).tables().tables()) {
+ sql("DROP TABLE " + t.name());
+ }
+ }
+
+ @Test
+ @Disabled("https://issues.apache.org/jira/browse/IGNITE-20096")
+ public void ddl() throws Exception {
+ IgniteSql sql = igniteSql();
+ Session ses = sql.createSession();
+
+ // CREATE TABLE
+ checkDdl(true, ses, "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+ checkError(
+ TableAlreadyExistsException.class,
+ ErrorGroups.Table.TABLE_ALREADY_EXISTS_ERR,
+ "Table already exists [name=\"PUBLIC\".\"TEST\"]",
+ ses,
+ "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)"
+ );
+ checkSqlError(
+ ErrorGroups.Table.TABLE_DEFINITION_ERR,
+ "Can't create table with duplicate columns: ID, VAL, VAL",
+ ses,
+ "CREATE TABLE TEST1(ID INT PRIMARY KEY, VAL INT, VAL INT)"
+ );
+ checkDdl(false, ses, "CREATE TABLE IF NOT EXISTS TEST(ID INT PRIMARY KEY, VAL VARCHAR)");
+
+ // ADD COLUMN
+ checkDdl(true, ses, "ALTER TABLE TEST ADD COLUMN VAL1 VARCHAR");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"NOT_EXISTS_TABLE\"]",
+ ses,
+ "ALTER TABLE NOT_EXISTS_TABLE ADD COLUMN VAL1 VARCHAR"
+ );
+ checkDdl(false, ses, "ALTER TABLE IF EXISTS NOT_EXISTS_TABLE ADD COLUMN VAL1 VARCHAR");
+ checkError(
+ ColumnAlreadyExistsException.class,
+ ErrorGroups.Table.COLUMN_ALREADY_EXISTS_ERR,
+ "Column already exists [name=\"VAL1\"]",
+ ses,
+ "ALTER TABLE TEST ADD COLUMN VAL1 INT"
+ );
+
+ // CREATE INDEX
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX ON TEST(VAL0)");
+ checkError(
+ IndexAlreadyExistsException.class,
+ Index.INDEX_ALREADY_EXISTS_ERR,
+ "Index already exists [name=\"PUBLIC\".\"TEST_IDX\"]",
+ ses,
+ "CREATE INDEX TEST_IDX ON TEST(VAL1)"
+ );
+ checkDdl(false, ses, "CREATE INDEX IF NOT EXISTS TEST_IDX ON TEST(VAL1)");
+
+ // TODO: IGNITE-19150 We are waiting for schema synchronization to avoid races to create and destroy indexes
+ waitForIndexBuild("TEST", "TEST_IDX");
+
+ checkDdl(true, ses, "DROP INDEX TESt_iDX");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX1 ON TEST(VAL0)");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX2 ON TEST(VAL0)");
+ checkDdl(true, ses, "CREATE INDEX TEST_IDX3 ON TEST(ID, VAL0, VAL1)");
+ checkSqlError(
+ Index.INVALID_INDEX_DEFINITION_ERR,
+ "Can't create index on duplicate columns: VAL0, VAL0",
+ ses,
+ "CREATE INDEX TEST_IDX4 ON TEST(VAL0, VAL0)"
+ );
+
+ checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column(s). Column VAL1 is used by indexes [TEST_IDX3].",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN val1"
+ );
+
+ SqlException ex = checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column(s).",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN (val0, val1)"
+ );
+
+ String msg = ex.getMessage();
+ String explainMsg = "Unexpected error message: " + msg;
+
+ assertTrue(msg.contains("Column VAL0 is used by indexes ["), explainMsg);
+ assertTrue(msg.contains("TEST_IDX1") && msg.contains("TEST_IDX2") && msg.contains("TEST_IDX3"), explainMsg);
+ assertTrue(msg.contains("Column VAL1 is used by indexes [TEST_IDX3]"), explainMsg);
+
+ checkSqlError(
+ Sql.STMT_VALIDATION_ERR,
+ "Can`t delete column, belongs to primary key: [name=ID]",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN id"
+ );
+
+ // TODO: IGNITE-19150 We are waiting for schema synchronization to avoid races to create and destroy indexes
+ waitForIndexBuild("TEST", "TEST_IDX3");
+ checkDdl(true, ses, "DROP INDEX TESt_iDX3");
+
+ // DROP COLUMNS
+ checkDdl(true, ses, "ALTER TABLE TEST DROP COLUMN VAL1");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"NOT_EXISTS_TABLE\"]",
+ ses,
+ "ALTER TABLE NOT_EXISTS_TABLE DROP COLUMN VAL1"
+ );
+ checkDdl(false, ses, "ALTER TABLE IF EXISTS NOT_EXISTS_TABLE DROP COLUMN VAL1");
+ checkError(
+ ColumnNotFoundException.class,
+ ErrorGroups.Table.COLUMN_NOT_FOUND_ERR,
+ "Column does not exist [tableName=\"PUBLIC\".\"TEST\", columnName=\"VAL1\"]",
+ ses,
+ "ALTER TABLE TEST DROP COLUMN VAL1"
+ );
+
+ // DROP TABLE
+ checkDdl(false, ses, "DROP TABLE IF EXISTS NOT_EXISTS_TABLE");
+
+ checkDdl(true, ses, "DROP TABLE TEST");
+ checkError(
+ TableNotFoundException.class,
+ ErrorGroups.Table.TABLE_NOT_FOUND_ERR,
+ "The table does not exist [name=\"PUBLIC\".\"TEST\"]",
+ ses,
+ "DROP TABLE TEST"
+ );
+
+ checkDdl(false, ses, "DROP INDEX IF EXISTS TEST_IDX");
+
+ checkError(
+ IndexNotFoundException.class,
+ Index.INDEX_NOT_FOUND_ERR,
+ "Index does not exist [name=\"PUBLIC\".\"TEST_IDX\"]", ses,
+ "DROP INDEX TEST_IDX"
+ );
+ }
+
+ /** Check all transactions are processed correctly even with case of sql Exception raised. */
+ @Test
+ public void implicitTransactionsStates() {
+ IgniteSql sql = igniteSql();
+
+ if (sql instanceof ClientSql) {
+ return;
+ }
+
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+ Session ses = sql.createSession();
+
+ TxManager txManager = txManager();
+
+ for (int i = 0; i < ROW_COUNT; ++i) {
+ assertThrowsSqlException(
+ Catalog.VALIDATION_ERR,
+ "Table with name 'PUBLIC.TEST' already exists",
+ () -> execute(ses, "CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)")
+ );
+ }
+
+ // No new transactions through ddl.
+ assertEquals(0, txManager.pending());
+ }
+
+ /** Check correctness of implicit and explicit transactions. */
+ @Test
+ public void checkTransactionsWithDml() {
+ IgniteSql sql = igniteSql();
+
+ if (sql instanceof ClientSql) {
Review Comment:
Looks like these test fail with the same error (` Error occurred when notifying safe time advanced callback`), so I think it is better to disable these test cases and provide a like to a ticket.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1350394502
##########
modules/core/src/test/java/org/apache/ignite/internal/lang/IgniteExceptionMapperUtilTest.java:
##########
@@ -204,4 +239,19 @@ public CustomNoMappingException() {
super(INTERNAL_ERR, "Test internal exception [err=no mapping]");
}
}
+
+ /**
+ * Test runtime exception which does not have a mapper.
+ */
+ public static class CustomInternalNotMappedException extends IgniteInternalException {
Review Comment:
agree
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1356119126
##########
modules/core/src/main/java/org/apache/ignite/internal/lang/IgniteExceptionMapperUtil.java:
##########
@@ -87,32 +87,40 @@ public static Throwable mapToPublicException(Throwable origin) {
if (origin instanceof AssertionError) {
return new IgniteException(INTERNAL_ERR, origin);
}
-
return origin;
}
- IgniteExceptionMapper<? extends Exception, ? extends Exception> m = EXCEPTION_CONVERTERS.get(origin.getClass());
+ Throwable res;
+
+ // Try to find appropriate mapper, moving from original class to supper-classes step by step.
+ Class exceptionClass = origin.getClass();
+ IgniteExceptionMapper<? extends Exception, ? extends Exception> m;
+ while ((m = EXCEPTION_CONVERTERS.get(exceptionClass)) == null && exceptionClass != Throwable.class) {
+ exceptionClass = exceptionClass.getSuperclass();
+ }
+
if (m != null) {
- Exception mapped = map(m, origin);
+ res = map(m, origin);
- assert mapped instanceof IgniteException || mapped instanceof IgniteCheckedException :
- "Unexpected mapping of internal exception to a public one [origin=" + origin + ", mapped=" + mapped + ']';
+ assert res instanceof IgniteException || res instanceof IgniteCheckedException :
+ "Unexpected mapping of internal exception to a public one [origin=" + origin + ", mapped=" + res + ']';
- return mapped;
+ } else {
+ res = origin;
}
- if (origin instanceof IgniteException || origin instanceof IgniteCheckedException) {
-
- return origin;
+ if (res instanceof IgniteException || res instanceof IgniteCheckedException) {
+ return res;
}
// There are no exception mappings for the given exception. This case should be considered as internal error.
return new IgniteException(INTERNAL_ERR, origin);
+
Review Comment:
yep
##########
modules/core/src/test/java/org/apache/ignite/internal/lang/IgniteExceptionMapperUtilTest.java:
##########
@@ -203,5 +203,12 @@ public static class CustomNoMappingException extends IgniteInternalException {
public CustomNoMappingException() {
super(INTERNAL_ERR, "Test internal exception [err=no mapping]");
}
+
+ /**
+ * Creates a new instance of CustomNoMappingException with given code.
+ */
+ public CustomNoMappingException(int code) {
Review Comment:
yep
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API [ignite-3]
Posted by "korlov42 (via GitHub)" <gi...@apache.org>.
korlov42 commented on code in PR #2613:
URL: https://github.com/apache/ignite-3/pull/2613#discussion_r1358268905
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/lang/SqlExceptionMapperUtil.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.lang;
+
+import static org.apache.ignite.internal.lang.IgniteExceptionMapperUtil.mapToPublicException;
+import static org.apache.ignite.lang.ErrorGroups.Common.INTERNAL_ERR;
+
+import org.apache.ignite.lang.ErrorGroups.Common;
+import org.apache.ignite.lang.IgniteCheckedException;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.TraceableException;
+import org.apache.ignite.sql.SqlException;
+
+/**
+ * This utility class provides an ability to map Ignite internal exceptions to public SqlException.
+ */
+public class SqlExceptionMapperUtil {
+
+ /**
+ * This method provides a mapping from internal exception to SQL public ones.
+ *
+ * <p>The rules of mapping are the following:</p>
+ * <ul>
+ * <li>any instance of {@link Error} is returned as is, except {@link AssertionError}
+ * that will always be mapped to {@link IgniteException} with the {@link Common#INTERNAL_ERR} error code.</li>
+ * <li>any instance of {@link IgniteException} or {@link IgniteCheckedException} is returned as is.</li>
+ * <li>any other instance of {@link TraceableException} is wrapped into {@link SqlException}
+ * with the original {@link TraceableException#traceId() traceUd} and {@link TraceableException#code() code}.</li>
+ * <li>if there are no any mappers that can do a mapping from the given error to a public exception,
+ * then {@link SqlException} with the {@link Common#INTERNAL_ERR} error code is returned.</li>
+ * </ul>
+ *
+ * @param origin Exception to be mapped.
+ * @return Public exception.
+ */
+ public static Throwable mapToPublicSqlException(Throwable origin) {
+ Throwable e = mapToPublicException(origin);
+ if (e instanceof Error) {
+ return e;
+ }
+ if (e instanceof SqlException) {
+ return e;
+ }
+ if (e instanceof IgniteException && ((IgniteException) e).code() == INTERNAL_ERR) {
+ return new SqlException(((IgniteException) e).traceId(), INTERNAL_ERR, e.getCause());
Review Comment:
> We don't put message for non SQL API public exceptions
why?
I agree with @xtern on this: we just lost here an entire frame with message. What if it was the only frame? Why should we omit message? IgniteException is public exception after all
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org