korlov42 commented on code in PR #2906:
URL: https://github.com/apache/ignite-3/pull/2906#discussion_r1415747367


##########
modules/jdbc/src/main/java/org/apache/ignite/internal/jdbc/JdbcStatement.java:
##########
@@ -432,10 +438,14 @@ public boolean getMoreResults() throws SQLException {
     public boolean getMoreResults(int curr) throws SQLException {

Review Comment:
   looks like this has not been addressed



##########
modules/jdbc/src/integrationTest/java/org/apache/ignite/jdbc/AbstractJdbcSelfTest.java:
##########
@@ -168,4 +173,20 @@ protected void checkConnectionClosed(Executable ex) {
     protected void checkNotSupported(Executable ex) {
         assertThrows(SQLFeatureNotSupportedException.class, ex);
     }
+
+    /** Return a size of stored resources. Reflection based implementation, 
need to be refactored. */
+    int openCursorsRegistered() throws Exception {
+        // a bit hack, instead of calling stmt.close(), gives a chance to 
catch potential forgiven cursor.
+        stmt.execute("SELECT 1");
+        ResultSet rs = stmt.getResultSet();
+        rs.close();

Review Comment:
   to be honest, I don't quite get the purpose of these lines



##########
modules/jdbc/src/integrationTest/java/org/apache/ignite/jdbc/ItJdbcMultiStatementSelfTest.java:
##########
@@ -53,47 +55,211 @@ public void setupTables() throws Exception {
     }
 
     /**
-     * Execute sql script using thin driver.
+     * Check correctness of {@link Statement#execute(String)} processing.
      */
-    private void execute(String sql) throws Exception {
-        stmt.executeUpdate(sql);
+    @Test
+    public void testResultsFromExecute() throws Exception {

Review Comment:
   let's derive every test case to separate method with meaningful name 



##########
modules/client-handler/src/main/java/org/apache/ignite/client/handler/ClientHandlerModule.java:
##########
@@ -294,6 +301,7 @@ protected void initChannel(Channel ch) {
 
                             ClientInboundMessageHandler messageHandler = 
createInboundMessageHandler(
                                     configuration, clusterId, connectionId);
+                            handlers.put("messageHandler", messageHandler);

Review Comment:
   why do you need map if you only using a constant key?



##########
modules/jdbc/src/integrationTest/java/org/apache/ignite/jdbc/ItJdbcMultiStatementSelfTest.java:
##########
@@ -53,47 +55,211 @@ public void setupTables() throws Exception {
     }
 
     /**
-     * Execute sql script using thin driver.
+     * Check correctness of {@link Statement#execute(String)} processing.
      */
-    private void execute(String sql) throws Exception {
-        stmt.executeUpdate(sql);
+    @Test
+    public void testResultsFromExecute() throws Exception {
+        int initial = openCursorsRegistered();
+        boolean res;
+
+        // TODO: Wait for fix from IGNITE-20453
+        /*res = stmt.execute("START TRANSACTION; COMMIT");
+        assertEquals(false, res);
+        assertNull(stmt.getResultSet());
+        assertEquals(0, stmt.getUpdateCount());
+        assertFalse(stmt.getMoreResults());
+
+        res = stmt.execute("COMMIT");
+        assertEquals(false, res);
+        assertNull(stmt.getResultSet());
+        assertEquals(0, stmt.getUpdateCount());
+        assertFalse(stmt.getMoreResults());*/
+
+        res = stmt.execute("INSERT INTO TEST_TX VALUES (5, 5, '5');");
+        assertEquals(false, res);
+        stmt.getResultSet();
+        stmt.getMoreResults();
+        assertEquals(-1, getResultSetSize());
+
+        // empty resultset 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());
+
+        // TODO: Wait for fix from IGNITE-20453
+        /*stmt.execute("START TRANSACTION; SELECT 1; COMMIT");
+        ResultSet resultSet = stmt.getResultSet();
+        assertNull(resultSet);
+        assertEquals(0, stmt.getUpdateCount());
+
+        assertTrue(stmt.getMoreResults());
+        resultSet = stmt.getResultSet();
+        assertNotNull(resultSet);
+
+        assertFalse(stmt.getMoreResults());
+        resultSet = stmt.getResultSet();
+        assertNull(resultSet);
+        assertEquals(0, stmt.getUpdateCount());*/
+
+        res = stmt.execute("DROP TABLE IF EXISTS TEST_TX; DROP TABLE IF EXISTS 
PUBLIC.TRANSACTIONS;");
+        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());
+
+        // TODO: Wait for fix from IGNITE-20453
+        //res = stmt.execute(";;;;");
+        //assertEquals(false, res);
+
+        res = stmt.execute("START TRANSACTION; INSERT INTO TEST_TX VALUES (5, 
19, 'Nick'); COMMIT");
+        assertEquals(false, res);
+        assertEquals(-1, getResultSetSize());
+
+        res = stmt.execute("SELECT ID FROM TEST_TX; SELECT 1;");
+        assertEquals(true, res);
+        stmt.getResultSet();
+        ResultSet rs = stmt.getResultSet();
+
+        res = stmt.getMoreResults();
+        assertEquals(true, res);
+
+        assertTrue(rs.isClosed());
+        assertEquals(1, getResultSetSize());
+
+        res = stmt.execute("SELECT ID FROM TEST_TX; SELECT 1;");
+        assertEquals(true, res);
+        assertEquals(5, getResultSetSize());
+
+        res = stmt.execute("SELECT 1; SELECT 1;");
+        assertEquals(true, res);
+        assertEquals(2, getResultSetSize());
+
+        stmt.execute("SELECT 1; SELECT 2; SELECT 3;");
+        ResultSet rs1 = stmt.getResultSet();
+        stmt.getMoreResults();
+        assertTrue(rs1.isClosed());
+        ResultSet rs2 = stmt.getResultSet();
+        stmt.getMoreResults();
+        assertTrue(rs2.isClosed());
+        rs = stmt.getResultSet();
+        assertTrue(rs.next());
+        assertEquals(3, rs.getObject(1));
+        rs.close();
+
+        assertEquals(0, openCursorsRegistered() - initial);
     }
 
     /**
-     * Assert that script containing both h2 and non h2 (native) sql 
statements is handled correctly.
+     * Check correctness of {@link Statement#getUpdateCount()} processing.
      */
     @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 updateCount() throws Exception {

Review Comment:
   the same



-- 
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]

Reply via email to