korlov42 commented on code in PR #2906:
URL: https://github.com/apache/ignite-3/pull/2906#discussion_r1424071952
##########
modules/jdbc/src/main/java/org/apache/ignite/internal/jdbc/JdbcResultSet.java:
##########
@@ -215,6 +218,37 @@ public JdbcResultSet(List<List<Object>> rows,
List<JdbcColumnMeta> meta) throws
initColumnOrder();
}
+ @Nullable JdbcResultSet getNextResultSet() throws SQLException {
+ try {
+ JdbcQueryFetchRequest req = new JdbcQueryFetchRequest(cursorId,
fetchSize);
+ JdbcQuerySingleResult res =
cursorHandler.getMoreResultsAsync(req).get();
+
+ close0(true);
+
+ if (!res.hasResults()) {
Review Comment:
what if we got an error from handler? Let's add a test to cover this case
##########
modules/jdbc/src/integrationTest/java/org/apache/ignite/jdbc/ItJdbcMultiStatementSelfTest.java:
##########
@@ -52,48 +54,231 @@ 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);
+ assertFalse(res);
Review Comment:
```suggestion
assertFalse(res);
```
##########
modules/client-common/src/main/java/org/apache/ignite/internal/jdbc/proto/JdbcQueryCursorHandler.java:
##########
@@ -37,6 +39,14 @@ public interface JdbcQueryCursorHandler {
*/
CompletableFuture<JdbcQueryFetchResult> fetchAsync(JdbcQueryFetchRequest
req);
+ /**
+ * {@link Statement#getMoreResults()} command implementor.
+ *
+ * @param req Results request.
+ * @return Result future.
+ */
+ CompletableFuture<JdbcQuerySingleResult>
getMoreResultsAsync(JdbcQueryFetchRequest req);
Review Comment:
usage of `JdbcQueryFetchRequest` as an input argument to `getMoreResults`
handler kinda misleading. Let's introduce separate class
##########
modules/jdbc/src/integrationTest/java/org/apache/ignite/jdbc/ItJdbcMultiStatementSelfTest.java:
##########
@@ -52,48 +54,231 @@ 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);
+ assertFalse(res);
+ assertNull(stmt.getResultSet());
+ assertFalse(stmt.getMoreResults());
+ assertEquals(-1, getResultSetSize());
+
+ stmt.execute("INSERT INTO TEST_TX VALUES (6, 5, '5');");
+ assertEquals(-1, getResultSetSize());
Review Comment:
same here
##########
modules/jdbc/src/integrationTest/java/org/apache/ignite/jdbc/ItJdbcStatementSelfTest.java:
##########
@@ -783,4 +779,27 @@ public void testStatementTypeMismatchUpdate() throws
Exception {
+ "Because update statement is executed via
'executeQuery' method."
+ " Data [val=" + rs.getString(1) + ']');
}
+
+ @Test
+ public void testOpenCursorsPureQuery() throws Exception {
+ 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));
+ }
+ }
+
+ @Test
+ public void testOpenCursorsWithDdl() throws Exception {
Review Comment:
why doesn't this test have a single assertion?
##########
modules/jdbc/src/integrationTest/java/org/apache/ignite/jdbc/ItJdbcMultiStatementSelfTest.java:
##########
@@ -52,48 +54,231 @@ 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);
+ assertFalse(res);
+ assertNull(stmt.getResultSet());
+ assertFalse(stmt.getMoreResults());
+ assertEquals(-1, getResultSetSize());
Review Comment:
if you want to check updateCount, please do it explicitly
##########
modules/jdbc/src/integrationTest/java/org/apache/ignite/jdbc/ItJdbcStatementSelfTest.java:
##########
@@ -783,4 +779,27 @@ public void testStatementTypeMismatchUpdate() throws
Exception {
+ "Because update statement is executed via
'executeQuery' method."
+ " Data [val=" + rs.getString(1) + ']');
}
+
+ @Test
+ public void testOpenCursorsPureQuery() throws Exception {
+ 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));
+ }
Review Comment:
what is the purpose of this loop?
##########
modules/jdbc/src/main/java/org/apache/ignite/internal/jdbc/JdbcStatement.java:
##########
@@ -396,7 +398,7 @@ public ResultSet getResultSet() throws SQLException {
JdbcResultSet rs = resSets.get(curRes);
- if (!rs.isQuery()) {
+ if (rs == null || !rs.isQuery()) {
Review Comment:
this one was not addressed, as far as I can see
##########
modules/jdbc/src/main/java/org/apache/ignite/internal/jdbc/JdbcResultSet.java:
##########
@@ -215,6 +218,37 @@ public JdbcResultSet(List<List<Object>> rows,
List<JdbcColumnMeta> meta) throws
initColumnOrder();
}
+ @Nullable JdbcResultSet getNextResultSet() throws SQLException {
+ try {
+ JdbcQueryFetchRequest req = new JdbcQueryFetchRequest(cursorId,
fetchSize);
+ JdbcQuerySingleResult res =
cursorHandler.getMoreResultsAsync(req).get();
+
+ close0(true);
Review Comment:
you're using internal `close0` that lacks support 'closeOnCompletion'. We
need to add this test
--
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]
