From 125c9fe9e7c4912088e19cde587baa0f1040a324 Mon Sep 17 00:00:00 2001 From: Leo Xuzhang Lin Date: Tue, 14 Aug 2018 21:55:00 -0400 Subject: [PATCH 1/4] Exclude insertId 0 from batchUpsert generatedKeys Signed-off-by: Leo Xuzhang Lin --- .../main/java/io/vitess/jdbc/VitessStatement.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/java/jdbc/src/main/java/io/vitess/jdbc/VitessStatement.java b/java/jdbc/src/main/java/io/vitess/jdbc/VitessStatement.java index 278bea6d809..8770e6be14f 100644 --- a/java/jdbc/src/main/java/io/vitess/jdbc/VitessStatement.java +++ b/java/jdbc/src/main/java/io/vitess/jdbc/VitessStatement.java @@ -591,7 +591,7 @@ protected void checkSQLNullOrEmpty(String sql) throws SQLException { protected int[] generateBatchUpdateResult(List cursorWithErrorList, List batchedArgs) throws BatchUpdateException { int[] updateCounts = new int[cursorWithErrorList.size()]; - long[][] generatedKeys = new long[cursorWithErrorList.size()][2]; + ArrayList generatedKeys = new ArrayList(); Vtrpc.RPCError rpcError = null; String batchCommand = null; @@ -603,6 +603,7 @@ protected int[] generateBatchUpdateResult(List cursorWithErrorL try { long rowsAffected = cursorWithError.getCursor().getRowsAffected(); int truncatedUpdateCount; + boolean queryBatchUpsert = false; if (rowsAffected > Integer.MAX_VALUE) { truncatedUpdateCount = Integer.MAX_VALUE; } else { @@ -610,13 +611,15 @@ protected int[] generateBatchUpdateResult(List cursorWithErrorL // mimicking mysql-connector-j here. // but it would fail for: insert into t1 values ('a'), ('b') on duplicate key update ts = now(); truncatedUpdateCount = 1; + queryBatchUpsert = true; } else { truncatedUpdateCount = (int) rowsAffected; } } updateCounts[i] = truncatedUpdateCount; - if (this.retrieveGeneratedKeys) { - generatedKeys[i] = new long[]{cursorWithError.getCursor().getInsertId(), truncatedUpdateCount}; + long insertId = cursorWithError.getCursor().getInsertId(); + if (this.retrieveGeneratedKeys && !queryBatchUpsert || insertId > 0) { + generatedKeys.add(new long[]{cursorWithError.getCursor().getInsertId(), truncatedUpdateCount}); } } catch (SQLException ex) { /* This case should not happen as API has returned cursor and not error. @@ -624,14 +627,14 @@ protected int[] generateBatchUpdateResult(List cursorWithErrorL */ updateCounts[i] = Statement.SUCCESS_NO_INFO; if (this.retrieveGeneratedKeys) { - generatedKeys[i] = new long[]{Statement.SUCCESS_NO_INFO, Statement.SUCCESS_NO_INFO}; + generatedKeys.add(new long[]{Statement.SUCCESS_NO_INFO, Statement.SUCCESS_NO_INFO}); } } } else { rpcError = cursorWithError.getError(); updateCounts[i] = Statement.EXECUTE_FAILED; if (this.retrieveGeneratedKeys) { - generatedKeys[i] = new long[]{Statement.EXECUTE_FAILED, Statement.EXECUTE_FAILED}; + generatedKeys.add(new long[]{Statement.EXECUTE_FAILED, Statement.EXECUTE_FAILED}); } } } @@ -642,7 +645,7 @@ protected int[] generateBatchUpdateResult(List cursorWithErrorL throw new BatchUpdateException(rpcError.toString(), sqlState, errno, updateCounts); } if (this.retrieveGeneratedKeys) { - this.batchGeneratedKeys = generatedKeys; + this.batchGeneratedKeys = generatedKeys.toArray(new long[generatedKeys.size()][2]); } return updateCounts; } From b195f3076b125b0eb50bc4e1274ec7be24e5628a Mon Sep 17 00:00:00 2001 From: Leo Xuzhang Lin Date: Tue, 14 Aug 2018 22:03:44 -0400 Subject: [PATCH 2/4] Add test case for upsert insertId 0 fix Signed-off-by: Leo Xuzhang Lin --- .../test/java/io/vitess/jdbc/VitessStatementTest.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/java/jdbc/src/test/java/io/vitess/jdbc/VitessStatementTest.java b/java/jdbc/src/test/java/io/vitess/jdbc/VitessStatementTest.java index 6e0e4a77799..39482e9f14c 100644 --- a/java/jdbc/src/test/java/io/vitess/jdbc/VitessStatementTest.java +++ b/java/jdbc/src/test/java/io/vitess/jdbc/VitessStatementTest.java @@ -742,5 +742,15 @@ private void testExecute(int fetchSize, boolean simpleExecute, boolean shouldRun Assert.assertEquals(i, 0); // we should only have one i++; } + + VitessStatement noUpdate = new VitessStatement(mockConn); + PowerMockito.when(mockCursor.getInsertId()).thenReturn(0L); + PowerMockito.when(mockCursor.getRowsAffected()).thenReturn(1L); + + noUpdate.addBatch(sqlUpsert); + noUpdate.executeBatch(); + + ResultSet empty = noUpdate.getGeneratedKeys(); + Assert.assertFalse(empty.next()); } } From 76f9c603c349965216cbab2eece3b845ad367c2f Mon Sep 17 00:00:00 2001 From: Leo Xuzhang Lin Date: Wed, 15 Aug 2018 13:07:25 -0400 Subject: [PATCH 3/4] Reuse local insertId Signed-off-by: Leo Xuzhang Lin --- java/jdbc/src/main/java/io/vitess/jdbc/VitessStatement.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/jdbc/src/main/java/io/vitess/jdbc/VitessStatement.java b/java/jdbc/src/main/java/io/vitess/jdbc/VitessStatement.java index 8770e6be14f..a30755820d0 100644 --- a/java/jdbc/src/main/java/io/vitess/jdbc/VitessStatement.java +++ b/java/jdbc/src/main/java/io/vitess/jdbc/VitessStatement.java @@ -619,7 +619,7 @@ protected int[] generateBatchUpdateResult(List cursorWithErrorL updateCounts[i] = truncatedUpdateCount; long insertId = cursorWithError.getCursor().getInsertId(); if (this.retrieveGeneratedKeys && !queryBatchUpsert || insertId > 0) { - generatedKeys.add(new long[]{cursorWithError.getCursor().getInsertId(), truncatedUpdateCount}); + generatedKeys.add(new long[]{insertId, truncatedUpdateCount}); } } catch (SQLException ex) { /* This case should not happen as API has returned cursor and not error. From 8ff0d4b8fc5c03d63612f4c47c7abf4883e0ad7f Mon Sep 17 00:00:00 2001 From: Leo Xuzhang Lin Date: Mon, 27 Aug 2018 11:44:54 -0400 Subject: [PATCH 4/4] Paren around boolean statement for clarity Signed-off-by: Leo Xuzhang Lin --- java/jdbc/src/main/java/io/vitess/jdbc/VitessStatement.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/jdbc/src/main/java/io/vitess/jdbc/VitessStatement.java b/java/jdbc/src/main/java/io/vitess/jdbc/VitessStatement.java index a30755820d0..f4a0904824f 100644 --- a/java/jdbc/src/main/java/io/vitess/jdbc/VitessStatement.java +++ b/java/jdbc/src/main/java/io/vitess/jdbc/VitessStatement.java @@ -618,7 +618,7 @@ protected int[] generateBatchUpdateResult(List cursorWithErrorL } updateCounts[i] = truncatedUpdateCount; long insertId = cursorWithError.getCursor().getInsertId(); - if (this.retrieveGeneratedKeys && !queryBatchUpsert || insertId > 0) { + if (this.retrieveGeneratedKeys && (!queryBatchUpsert || insertId > 0)) { generatedKeys.add(new long[]{insertId, truncatedUpdateCount}); } } catch (SQLException ex) {