You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2021/03/14 07:54:57 UTC

[GitHub] [calcite] xy2953396112 opened a new pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

xy2953396112 opened a new pull request #2373:
URL: https://github.com/apache/calcite/pull/2373


   …ng RemoteMeta. (Jin Xing & Xu Zhaohui)
   https://issues.apache.org/jira/browse/CALCITE-3338


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] xy2953396112 commented on a change in pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on a change in pull request #2373:
URL: https://github.com/apache/calcite/pull/2373#discussion_r710726898



##########
File path: core/src/test/java/org/apache/calcite/jdbc/CalciteRemoteDriverTest.java
##########
@@ -880,6 +880,38 @@ public Service create(AvaticaConnection connection) {
     assertThat(updateCount, is(1));
   }
 
+  @Test public void testInsertBatchWithPreparedStatement() throws Exception {
+    final Connection connection = DriverManager.getConnection(
+        "jdbc:avatica:remote:factory="
+            + LocalServiceModifiableFactory.class.getName());
+
+    PreparedStatement pst = connection.prepareStatement(
+        "insert into \"foo\".\"bar\" values (?, ?, ?, ?, ?)");
+    pst.setInt(1, 1);
+    pst.setInt(2, 1);
+    pst.setString(3, "second");
+    pst.setInt(4, 1);
+    pst.setInt(5, 1);
+    pst.addBatch();
+    pst.addBatch();
+
+    int[] updateCounts = pst.executeBatch();
+    assertThat(updateCounts.length, is(2));
+    assertThat(updateCounts[0], is(1));
+    assertThat(updateCounts[1], is(1));
+    ResultSet resultSet = pst.getResultSet();
+    assertThat(resultSet, nullValue());
+
+    // Now empty batch
+    pst.clearBatch();
+    updateCounts = pst.executeBatch();
+    assertThat(updateCounts.length, is(0));
+    resultSet = pst.getResultSet();
+    assertThat(resultSet, nullValue());

Review comment:
       `CalciteRemoteDriverTest` using `MatcherAssert#assertThat()`.
   Should we be consistent with it?
   
   




-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] xy2953396112 commented on pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on pull request #2373:
URL: https://github.com/apache/calcite/pull/2373#issuecomment-941863033


   LGTM~


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] vlsi commented on a change in pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
vlsi commented on a change in pull request #2373:
URL: https://github.com/apache/calcite/pull/2373#discussion_r598661610



##########
File path: core/src/test/java/org/apache/calcite/jdbc/CalciteRemoteDriverTest.java
##########
@@ -880,6 +880,40 @@ public Service create(AvaticaConnection connection) {
     assertThat(updateCount, is(1));
   }
 
+  @Test public void testInsertBatchWithPreparedStatement() throws Exception {
+    final Connection connection = DriverManager.getConnection(
+        "jdbc:avatica:remote:factory="
+            + LocalServiceModifiableFactory.class.getName());
+    assertThat(connection.getMetaData().supportsBatchUpdates(), is(true));
+    assertThat(connection.isClosed(), is(false));

Review comment:
       What is the point of these assertions?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] vlsi commented on a change in pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
vlsi commented on a change in pull request #2373:
URL: https://github.com/apache/calcite/pull/2373#discussion_r598663689



##########
File path: core/src/test/java/org/apache/calcite/jdbc/CalciteRemoteDriverTest.java
##########
@@ -880,6 +880,40 @@ public Service create(AvaticaConnection connection) {
     assertThat(updateCount, is(1));
   }
 
+  @Test public void testInsertBatchWithPreparedStatement() throws Exception {

Review comment:
       Frankly speaking, it is hard to understand what this test verifies. Could you please update test name (or even split the test into multiple tests)




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] xy2953396112 commented on pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on pull request #2373:
URL: https://github.com/apache/calcite/pull/2373#issuecomment-924867830


   @vlsi 
   Any comments?Thanks a lot.


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] vlsi commented on a change in pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
vlsi commented on a change in pull request #2373:
URL: https://github.com/apache/calcite/pull/2373#discussion_r600319174



##########
File path: core/src/test/java/org/apache/calcite/jdbc/CalciteRemoteDriverTest.java
##########
@@ -880,6 +880,40 @@ public Service create(AvaticaConnection connection) {
     assertThat(updateCount, is(1));
   }
 
+  @Test public void testInsertBatchWithPreparedStatement() throws Exception {
+    final Connection connection = DriverManager.getConnection(
+        "jdbc:avatica:remote:factory="
+            + LocalServiceModifiableFactory.class.getName());
+    assertThat(connection.getMetaData().supportsBatchUpdates(), is(true));
+    assertThat(connection.isClosed(), is(false));

Review comment:
       How is it related to CALCITE-3338?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] xy2953396112 commented on a change in pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on a change in pull request #2373:
URL: https://github.com/apache/calcite/pull/2373#discussion_r607702521



##########
File path: core/src/test/java/org/apache/calcite/jdbc/CalciteRemoteDriverTest.java
##########
@@ -880,6 +880,38 @@ public Service create(AvaticaConnection connection) {
     assertThat(updateCount, is(1));
   }
 
+  @Test public void testInsertBatchWithPreparedStatement() throws Exception {
+    final Connection connection = DriverManager.getConnection(
+        "jdbc:avatica:remote:factory="
+            + LocalServiceModifiableFactory.class.getName());
+
+    PreparedStatement pst = connection.prepareStatement(
+        "insert into \"foo\".\"bar\" values (?, ?, ?, ?, ?)");
+    pst.setInt(1, 1);
+    pst.setInt(2, 1);
+    pst.setString(3, "second");
+    pst.setInt(4, 1);
+    pst.setInt(5, 1);
+    pst.addBatch();
+    pst.addBatch();
+
+    int[] updateCounts = pst.executeBatch();
+    assertThat(updateCounts.length, is(2));
+    assertThat(updateCounts[0], is(1));
+    assertThat(updateCounts[1], is(1));
+    ResultSet resultSet = pst.getResultSet();
+    assertThat(resultSet, nullValue());
+
+    // Now empty batch
+    pst.clearBatch();
+    updateCounts = pst.executeBatch();
+    assertThat(updateCounts.length, is(0));
+    resultSet = pst.getResultSet();
+    assertThat(resultSet, nullValue());

Review comment:
       Please review again.
   
   




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] xy2953396112 commented on a change in pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on a change in pull request #2373:
URL: https://github.com/apache/calcite/pull/2373#discussion_r600329602



##########
File path: core/src/test/java/org/apache/calcite/jdbc/CalciteRemoteDriverTest.java
##########
@@ -880,6 +880,40 @@ public Service create(AvaticaConnection connection) {
     assertThat(updateCount, is(1));
   }
 
+  @Test public void testInsertBatchWithPreparedStatement() throws Exception {
+    final Connection connection = DriverManager.getConnection(
+        "jdbc:avatica:remote:factory="
+            + LocalServiceModifiableFactory.class.getName());
+    assertThat(connection.getMetaData().supportsBatchUpdates(), is(true));
+    assertThat(connection.isClosed(), is(false));

Review comment:
       I don't quite understand what you mean. 
   This case comes from [CALCITE-3338](https://issues.apache.org/jira/browse/CALCITE-3338)
   




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] xy2953396112 merged pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
xy2953396112 merged pull request #2373:
URL: https://github.com/apache/calcite/pull/2373


   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] xy2953396112 commented on pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on pull request #2373:
URL: https://github.com/apache/calcite/pull/2373#issuecomment-803496944


   @vlsi
   Please help to review this pr when you have time, thanks.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] xy2953396112 commented on pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on pull request #2373:
URL: https://github.com/apache/calcite/pull/2373#issuecomment-921437585


   @vlsi 
   Please help review this pr again.thanks.


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] vlsi commented on a change in pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
vlsi commented on a change in pull request #2373:
URL: https://github.com/apache/calcite/pull/2373#discussion_r600731964



##########
File path: core/src/test/java/org/apache/calcite/jdbc/CalciteRemoteDriverTest.java
##########
@@ -880,6 +880,38 @@ public Service create(AvaticaConnection connection) {
     assertThat(updateCount, is(1));
   }
 
+  @Test public void testInsertBatchWithPreparedStatement() throws Exception {
+    final Connection connection = DriverManager.getConnection(
+        "jdbc:avatica:remote:factory="
+            + LocalServiceModifiableFactory.class.getName());
+
+    PreparedStatement pst = connection.prepareStatement(
+        "insert into \"foo\".\"bar\" values (?, ?, ?, ?, ?)");
+    pst.setInt(1, 1);
+    pst.setInt(2, 1);
+    pst.setString(3, "second");
+    pst.setInt(4, 1);
+    pst.setInt(5, 1);
+    pst.addBatch();
+    pst.addBatch();
+
+    int[] updateCounts = pst.executeBatch();
+    assertThat(updateCounts.length, is(2));
+    assertThat(updateCounts[0], is(1));
+    assertThat(updateCounts[1], is(1));
+    ResultSet resultSet = pst.getResultSet();
+    assertThat(resultSet, nullValue());
+
+    // Now empty batch
+    pst.clearBatch();
+    updateCounts = pst.executeBatch();
+    assertThat(updateCounts.length, is(0));
+    resultSet = pst.getResultSet();
+    assertThat(resultSet, nullValue());

Review comment:
       Please use assertions in such a way that failures make it clear what is verified and why.
   Currently it is hard to tell which of the assertions are related to 3388, and which of them are not.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] vlsi commented on a change in pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
vlsi commented on a change in pull request #2373:
URL: https://github.com/apache/calcite/pull/2373#discussion_r607779416



##########
File path: core/src/test/java/org/apache/calcite/jdbc/CalciteRemoteDriverTest.java
##########
@@ -880,6 +880,38 @@ public Service create(AvaticaConnection connection) {
     assertThat(updateCount, is(1));
   }
 
+  @Test public void testInsertBatchWithPreparedStatement() throws Exception {
+    final Connection connection = DriverManager.getConnection(
+        "jdbc:avatica:remote:factory="
+            + LocalServiceModifiableFactory.class.getName());
+
+    PreparedStatement pst = connection.prepareStatement(
+        "insert into \"foo\".\"bar\" values (?, ?, ?, ?, ?)");
+    pst.setInt(1, 1);
+    pst.setInt(2, 1);
+    pst.setString(3, "second");
+    pst.setInt(4, 1);
+    pst.setInt(5, 1);
+    pst.addBatch();
+    pst.addBatch();
+
+    int[] updateCounts = pst.executeBatch();
+    assertThat(updateCounts.length, is(2));
+    assertThat(updateCounts[0], is(1));
+    assertThat(updateCounts[1], is(1));
+    ResultSet resultSet = pst.getResultSet();
+    assertThat(resultSet, nullValue());
+
+    // Now empty batch
+    pst.clearBatch();
+    updateCounts = pst.executeBatch();
+    assertThat(updateCounts.length, is(0));
+    resultSet = pst.getResultSet();
+    assertThat(resultSet, nullValue());

Review comment:
       Please use assertions in such a way that failures make it clear what is verified and why.
   




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] xy2953396112 commented on a change in pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on a change in pull request #2373:
URL: https://github.com/apache/calcite/pull/2373#discussion_r600318804



##########
File path: core/src/test/java/org/apache/calcite/jdbc/CalciteRemoteDriverTest.java
##########
@@ -880,6 +880,40 @@ public Service create(AvaticaConnection connection) {
     assertThat(updateCount, is(1));
   }
 
+  @Test public void testInsertBatchWithPreparedStatement() throws Exception {

Review comment:
       It is similar to `org.apache.calcite.jdbc.CalciteRemoteDriverTest#testInsertBatch`.The difference is the `PreparedStatement`.
   
   




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] xy2953396112 commented on a change in pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on a change in pull request #2373:
URL: https://github.com/apache/calcite/pull/2373#discussion_r600580399



##########
File path: core/src/test/java/org/apache/calcite/jdbc/CalciteRemoteDriverTest.java
##########
@@ -880,6 +880,40 @@ public Service create(AvaticaConnection connection) {
     assertThat(updateCount, is(1));
   }
 
+  @Test public void testInsertBatchWithPreparedStatement() throws Exception {
+    final Connection connection = DriverManager.getConnection(
+        "jdbc:avatica:remote:factory="
+            + LocalServiceModifiableFactory.class.getName());
+    assertThat(connection.getMetaData().supportsBatchUpdates(), is(true));
+    assertThat(connection.isClosed(), is(false));

Review comment:
       ok, update the code.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] vlsi commented on a change in pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
vlsi commented on a change in pull request #2373:
URL: https://github.com/apache/calcite/pull/2373#discussion_r608425438



##########
File path: core/src/test/java/org/apache/calcite/jdbc/CalciteRemoteDriverTest.java
##########
@@ -880,6 +880,34 @@ public Service create(AvaticaConnection connection) {
     assertThat(updateCount, is(1));
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-3338">[CALCITE-3338]
+   * Support preparedStatement executeBatch</a>. */
+  @Test public void testInsertBatchWithPreparedStatement() throws Exception {
+    final Connection connection = DriverManager.getConnection(
+        "jdbc:avatica:remote:factory="
+            + LocalServiceModifiableFactory.class.getName());
+
+    PreparedStatement pst = connection.prepareStatement(
+        "insert into \"foo\".\"bar\" values (?, ?, ?, ?, ?)");
+    pst.setInt(1, 1);
+    pst.setInt(2, 1);
+    pst.setString(3, "second");
+    pst.setInt(4, 1);
+    pst.setInt(5, 1);
+    pst.addBatch();
+    pst.addBatch();
+
+    int[] updateCounts = pst.executeBatch();
+    assertThat(updateCounts.length, is(2));
+    assertThat(updateCounts[0], is(1));
+    assertThat(updateCounts[1], is(1));

Review comment:
       Is this array assertion? Why do we verify the array?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] vlsi commented on a change in pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
vlsi commented on a change in pull request #2373:
URL: https://github.com/apache/calcite/pull/2373#discussion_r600340847



##########
File path: core/src/test/java/org/apache/calcite/jdbc/CalciteRemoteDriverTest.java
##########
@@ -880,6 +880,40 @@ public Service create(AvaticaConnection connection) {
     assertThat(updateCount, is(1));
   }
 
+  @Test public void testInsertBatchWithPreparedStatement() throws Exception {
+    final Connection connection = DriverManager.getConnection(
+        "jdbc:avatica:remote:factory="
+            + LocalServiceModifiableFactory.class.getName());
+    assertThat(connection.getMetaData().supportsBatchUpdates(), is(true));
+    assertThat(connection.isClosed(), is(false));

Review comment:
       `connection.isClosed()` behavior is not altered in CALCITE-3338, so I do not see why do you test it here.
   
   Please avoid test duplication. If you want to test `connection.isClosed()`, then create a dedicated test for it.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] xy2953396112 commented on a change in pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on a change in pull request #2373:
URL: https://github.com/apache/calcite/pull/2373#discussion_r710723803



##########
File path: core/src/test/java/org/apache/calcite/jdbc/CalciteRemoteDriverTest.java
##########
@@ -880,6 +880,34 @@ public Service create(AvaticaConnection connection) {
     assertThat(updateCount, is(1));
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-3338">[CALCITE-3338]
+   * Support preparedStatement executeBatch</a>. */
+  @Test public void testInsertBatchWithPreparedStatement() throws Exception {
+    final Connection connection = DriverManager.getConnection(
+        "jdbc:avatica:remote:factory="
+            + LocalServiceModifiableFactory.class.getName());
+
+    PreparedStatement pst = connection.prepareStatement(
+        "insert into \"foo\".\"bar\" values (?, ?, ?, ?, ?)");
+    pst.setInt(1, 1);
+    pst.setInt(2, 1);
+    pst.setString(3, "second");
+    pst.setInt(4, 1);
+    pst.setInt(5, 1);
+    pst.addBatch();
+    pst.addBatch();
+
+    int[] updateCounts = pst.executeBatch();
+    assertThat(updateCounts.length, is(2));
+    assertThat(updateCounts[0], is(1));
+    assertThat(updateCounts[1], is(1));

Review comment:
       check the result of `pst.executeBatch()`




-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] xy2953396112 commented on pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on pull request #2373:
URL: https://github.com/apache/calcite/pull/2373#issuecomment-941863033


   LGTM~


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] xy2953396112 edited a comment on pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
xy2953396112 edited a comment on pull request #2373:
URL: https://github.com/apache/calcite/pull/2373#issuecomment-921437585


   @vlsi @zabetak @julianhyde 
   Please help review this pr again.thanks.


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] xy2953396112 commented on a change in pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on a change in pull request #2373:
URL: https://github.com/apache/calcite/pull/2373#discussion_r601219819



##########
File path: core/src/test/java/org/apache/calcite/jdbc/CalciteRemoteDriverTest.java
##########
@@ -880,6 +880,38 @@ public Service create(AvaticaConnection connection) {
     assertThat(updateCount, is(1));
   }
 
+  @Test public void testInsertBatchWithPreparedStatement() throws Exception {
+    final Connection connection = DriverManager.getConnection(
+        "jdbc:avatica:remote:factory="
+            + LocalServiceModifiableFactory.class.getName());
+
+    PreparedStatement pst = connection.prepareStatement(
+        "insert into \"foo\".\"bar\" values (?, ?, ?, ?, ?)");
+    pst.setInt(1, 1);
+    pst.setInt(2, 1);
+    pst.setString(3, "second");
+    pst.setInt(4, 1);
+    pst.setInt(5, 1);
+    pst.addBatch();
+    pst.addBatch();
+
+    int[] updateCounts = pst.executeBatch();
+    assertThat(updateCounts.length, is(2));
+    assertThat(updateCounts[0], is(1));
+    assertThat(updateCounts[1], is(1));
+    ResultSet resultSet = pst.getResultSet();
+    assertThat(resultSet, nullValue());
+
+    // Now empty batch
+    pst.clearBatch();
+    updateCounts = pst.executeBatch();
+    assertThat(updateCounts.length, is(0));
+    resultSet = pst.getResultSet();
+    assertThat(resultSet, nullValue());

Review comment:
       ok, After the repair, the `PreparedStatement` can execute batch.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] xy2953396112 merged pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
xy2953396112 merged pull request #2373:
URL: https://github.com/apache/calcite/pull/2373


   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] xy2953396112 edited a comment on pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
xy2953396112 edited a comment on pull request #2373:
URL: https://github.com/apache/calcite/pull/2373#issuecomment-921437585


   @vlsi @zabetak @julianhyde @hsyuan 
   Please help review this pr again.thanks.


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [calcite] xy2953396112 commented on a change in pull request #2373: [CALCITE-3338] Error with executeBatch and preparedStatement when usi…

Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on a change in pull request #2373:
URL: https://github.com/apache/calcite/pull/2373#discussion_r600315859



##########
File path: core/src/test/java/org/apache/calcite/jdbc/CalciteRemoteDriverTest.java
##########
@@ -880,6 +880,40 @@ public Service create(AvaticaConnection connection) {
     assertThat(updateCount, is(1));
   }
 
+  @Test public void testInsertBatchWithPreparedStatement() throws Exception {
+    final Connection connection = DriverManager.getConnection(
+        "jdbc:avatica:remote:factory="
+            + LocalServiceModifiableFactory.class.getName());
+    assertThat(connection.getMetaData().supportsBatchUpdates(), is(true));
+    assertThat(connection.isClosed(), is(false));

Review comment:
       To check that the `connection` is correct.
   
   




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org