xtern commented on code in PR #2906:
URL: https://github.com/apache/ignite-3/pull/2906#discussion_r1420125914
##########
modules/jdbc/src/integrationTest/java/org/apache/ignite/jdbc/ItJdbcMultiStatementSelfTest.java:
##########
@@ -17,21 +17,23 @@
package org.apache.ignite.jdbc;
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
+import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
/**
- * Tests for ddl queries that contain multiply sql statements, separated by
";".
+ * Tests for queries that contain multiply sql statements, separated by ";".
Review Comment:
```suggestion
* Tests for queries containing multiple sql statements, separated by ";".
```
##########
modules/jdbc/src/integrationTest/java/org/apache/ignite/jdbc/ItJdbcMultiStatementSelfTest.java:
##########
@@ -17,21 +17,23 @@
package org.apache.ignite.jdbc;
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
+import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
/**
- * Tests for ddl queries that contain multiply sql statements, separated by
";".
+ * Tests for queries that contain multiply sql statements, separated by ";".
Review Comment:
please add some test(s) for autocommit=false (without TX_CONTROL statements)
seems this should work fine,
##########
modules/jdbc/src/integrationTest/java/org/apache/ignite/jdbc/ItJdbcMultiStatementSelfTest.java:
##########
@@ -17,21 +17,23 @@
package org.apache.ignite.jdbc;
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
+import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
/**
- * Tests for ddl queries that contain multiply sql statements, separated by
";".
+ * Tests for queries that contain multiply sql statements, separated by ";".
Review Comment:
also, this is out of scope, since IGNITE-20463 delayed 😠, but it would be
great if you investigate how to handle
TxControlInsideExternalTxNotSupportedException (that will be introduced in
IGNITE-20463). may be you can add muted test that will show the desired
behavior 😞
```
conn.setAutoCommit(false)
execute("START TRANSACTION; COMMIT")
// query processor will throw
TxControlInsideExternalTxNotSupportedException, but user must see something
like "transaction control statements are not supported in non autocommit mode" 🤔
```
(see https://issues.apache.org/jira/browse/IGNITE-21020 )
##########
modules/jdbc/src/integrationTest/java/org/apache/ignite/jdbc/ItJdbcMultiStatementSelfTest.java:
##########
@@ -52,48 +54,229 @@ public void setupTables() throws Exception {
+ "(4, 19, 'Nick');");
}
- /**
- * Execute sql script using thin driver.
- */
- private void execute(String sql) throws Exception {
- stmt.executeUpdate(sql);
+ @AfterEach
+ void tearDown() throws Exception {
+ // only connection context.
+ assertEquals(1, openCursorsRegistered());
}
- /**
- * Assert that script containing both h2 and non h2 (native) sql
statements is handled correctly.
- */
@Test
- public void testMixedCommands() throws Exception {
- execute("CREATE TABLE public.transactions (pk INT, id INT, k VARCHAR,
v VARCHAR, PRIMARY KEY (pk, id)); "
- + "CREATE INDEX transactions_id_k_v ON public.transactions
(id, k, v) INLINE_SIZE 150; "
- + "INSERT INTO public.transactions VALUES (1,2,'some', 'word')
; "
- + "CREATE INDEX transactions_k_v_id ON public.transactions (k,
v, id) INLINE_SIZE 150; "
- + "CREATE INDEX transactions_pk_id ON public.transactions (pk,
id) INLINE_SIZE 20;");
+ public void testSimpleQueryExecute() throws Exception {
+ int initial = openCursorsRegistered();
+
+ boolean res = stmt.execute("INSERT INTO TEST_TX VALUES (5, 5, '5');");
+ assertEquals(false, res);
+ stmt.getResultSet();
+ stmt.getMoreResults();
Review Comment:
```suggestion
assertFalse(res);
assertNull(stmt.getResultSet());
assertFalse(stmt.getMoreResults());
```
:thinking:
##########
modules/jdbc/src/integrationTest/java/org/apache/ignite/jdbc/ItJdbcMultiStatementSelfTest.java:
##########
@@ -52,48 +54,229 @@ public void setupTables() throws Exception {
+ "(4, 19, 'Nick');");
}
- /**
- * Execute sql script using thin driver.
- */
- private void execute(String sql) throws Exception {
- stmt.executeUpdate(sql);
+ @AfterEach
+ void tearDown() throws Exception {
+ // only connection context.
+ assertEquals(1, openCursorsRegistered());
}
- /**
- * Assert that script containing both h2 and non h2 (native) sql
statements is handled correctly.
- */
@Test
- public void testMixedCommands() throws Exception {
- execute("CREATE TABLE public.transactions (pk INT, id INT, k VARCHAR,
v VARCHAR, PRIMARY KEY (pk, id)); "
- + "CREATE INDEX transactions_id_k_v ON public.transactions
(id, k, v) INLINE_SIZE 150; "
- + "INSERT INTO public.transactions VALUES (1,2,'some', 'word')
; "
- + "CREATE INDEX transactions_k_v_id ON public.transactions (k,
v, id) INLINE_SIZE 150; "
- + "CREATE INDEX transactions_pk_id ON public.transactions (pk,
id) INLINE_SIZE 20;");
+ public void testSimpleQueryExecute() throws Exception {
+ int initial = openCursorsRegistered();
+
+ boolean res = stmt.execute("INSERT INTO TEST_TX VALUES (5, 5, '5');");
+ assertEquals(false, res);
+ stmt.getResultSet();
+ stmt.getMoreResults();
+ assertEquals(-1, getResultSetSize());
+
+ stmt.execute("INSERT INTO TEST_TX VALUES (6, 5, '5');");
+ assertEquals(-1, getResultSetSize());
+
+ assertEquals(0, openCursorsRegistered() - initial);
+ }
+
+ @Test
+ public void testMixedDmlQueryExecute() throws Exception {
+ boolean res = stmt.execute("INSERT INTO TEST_TX VALUES (6, 5, '5');
DELETE FROM TEST_TX WHERE ID=6; SELECT 1;");
+ assertEquals(false, res);
+ assertEquals(1, getResultSetSize());
+
+ res = stmt.execute("SELECT 1; INSERT INTO TEST_TX VALUES (7, 5, '5');
DELETE FROM TEST_TX WHERE ID=6;");
+ assertEquals(true, res);
+ assertEquals(1, getResultSetSize());
+
+ // empty results set in the middle
+ res = stmt.execute("SELECT * FROM TEST_TX; INSERT INTO TEST_TX VALUES
(6, 6, '6'); SELECT * FROM TEST_TX;");
+ assertEquals(true, res);
+ assertEquals(11, getResultSetSize());
+ }
+
+ @Test
+ public void testMiscDmlExecute() throws Exception {
+ boolean res = stmt.execute("DROP TABLE IF EXISTS TEST_TX; DROP TABLE
IF EXISTS SOME_UNEXISTING_TBL;");
+ assertEquals(false, res);
+ assertEquals(-1, getResultSetSize());
+
+ res = stmt.execute("CREATE TABLE TEST_TX (ID INT PRIMARY KEY, AGE INT,
NAME VARCHAR) ");
+ assertEquals(false, res);
+ assertEquals(-1, getResultSetSize());
+
+ res = stmt.execute("INSERT INTO TEST_TX VALUES (1, 17, 'James'), (2,
43, 'Valery');");
+ assertEquals(false, res);
+ assertEquals(-1, getResultSetSize());
+
+ res = stmt.execute("DROP TABLE IF EXISTS PUBLIC.TRANSACTIONS; INSERT
INTO TEST_TX VALUES (3, 25, 'Michel');");
+ assertEquals(false, res);
+ assertEquals(-1, getResultSetSize());
+ }
+
+ @Test
+ public void testTransactionsRelatedExecute() throws Exception {
+ boolean res = stmt.execute("START TRANSACTION; INSERT INTO TEST_TX
VALUES (5, 19, 'Nick'); COMMIT");
+ assertEquals(false, res);
+ assertEquals(-1, getResultSetSize());
+
+ // TODO: Wait for fix from IGNITE-20453
Review Comment:
```suggestion
// TODO: Wait for fix from IGNITE-20463
```
##########
modules/jdbc/src/integrationTest/java/org/apache/ignite/jdbc/ItJdbcMultiStatementSelfTest.java:
##########
@@ -17,21 +17,23 @@
package org.apache.ignite.jdbc;
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
+import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
/**
- * Tests for ddl queries that contain multiply sql statements, separated by
";".
+ * Tests for queries that contain multiply sql statements, separated by ";".
Review Comment:
also, this is out of scope, since IGNITE-20463 delayed 😠, but it would be
great if you investigate how to handle
TxControlInsideExternalTxNotSupportedException (that will be introduced in
IGNITE-20463). may be you can add muted test that will show the desired
behavior 😞
```
conn.setAutoCommit(false)
execute("START TRANSACTION; COMMIT")
// query processor will throw
TxControlInsideExternalTxNotSupportedException, but user must see something
like "transaction control statements are not supported in non autocommit mode" 🤔
```
(see https://issues.apache.org/jira/browse/IGNITE-21020 )
##########
modules/jdbc/src/integrationTest/java/org/apache/ignite/jdbc/ItJdbcStatementSelfTest.java:
##########
@@ -783,4 +779,33 @@ public void testStatementTypeMismatchUpdate() throws
Exception {
+ "Because update statement is executed via
'executeQuery' method."
+ " Data [val=" + rs.getString(1) + ']');
}
+
+ @Test
+ public void testOpenCursorsPureQuery() throws Exception {
+ int initial = openCursorsRegistered();
+ stmt.execute("SELECT 1; SELECT 2;");
+ ResultSet rs = stmt.getResultSet();
+ stmt.execute("SELECT 3;");
+ assertTrue(rs.isClosed());
+
+ assertTrue(populateStmtCnt < 100);
+ //more than one fetch request
+ for (int i = populateStmtCnt; i < stmt.getMaxRows() + 100; ++i) {
+ stmt.execute(String.format("INSERT INTO TEST VALUES (%d, '1')",
i));
+ }
+ //stmt.close();
Review Comment:
```suggestion
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
