You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2020/05/15 03:22:24 UTC

[GitHub] [shardingsphere] terrymanu commented on a change in pull request #5590: Issue5423 and issue5465

terrymanu commented on a change in pull request #5590:
URL: https://github.com/apache/shardingsphere/pull/5590#discussion_r425543959



##########
File path: sharding-jdbc/sharding-jdbc-core/src/test/java/org/apache/shardingsphere/shardingjdbc/jdbc/core/statement/ShardingSpherePreparedStatementTest.java
##########
@@ -83,7 +87,7 @@ public void assertAddBatch() throws SQLException {
             }
         }
     }
-
+    

Review comment:
       Please keep original indent

##########
File path: sharding-jdbc/sharding-jdbc-core/src/test/java/org/apache/shardingsphere/shardingjdbc/jdbc/core/statement/ShardingSpherePreparedStatementTest.java
##########
@@ -126,7 +130,7 @@ public void assertMultiValuesWithGenerateShardingKeyColumn() throws SQLException
             }
         }
     }
-
+    

Review comment:
       Please keep original indent

##########
File path: sharding-jdbc/sharding-jdbc-core/src/test/java/org/apache/shardingsphere/shardingjdbc/jdbc/core/statement/ShardingSpherePreparedStatementTest.java
##########
@@ -316,6 +320,77 @@ public void assertAddBatchWithGenerateKeyColumn() throws SQLException {
         }
     }
     
+    @Test
+    public void assertAddOnDuplicateKey() throws SQLException {
+        int itemId = 1;
+        int userId1 = 101;
+        int userId2 = 102;
+        int orderId = 200;
+        String status = "init";
+        String updatedStatus = "updated on duplicate key";
+        try (Connection connection = getShardingSphereDataSource().getConnection();
+             PreparedStatement preparedStatement = connection.prepareStatement(INSERT_ON_DUPLICATE_KEY_SQL);
+             PreparedStatement queryStatement = connection.prepareStatement(SELECT_SQL_WITH_PARAMETER_MARKER_RETURN_STATUS)) {
+            preparedStatement.setInt(1, itemId);
+            preparedStatement.setInt(2, orderId);
+            preparedStatement.setInt(3, userId1);
+            preparedStatement.setString(4, status);
+            preparedStatement.setInt(5, itemId);
+            preparedStatement.setInt(6, orderId);
+            preparedStatement.setInt(7, userId2);
+            preparedStatement.setString(8, status);
+            preparedStatement.setString(9, updatedStatus);
+            int result = preparedStatement.executeUpdate();
+            assertThat(result, is(2));
+            

Review comment:
       Please remove useless blank lines.
   If we think use blank line to separate diff logic block, it is better extract them as a new private method. 

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-binder/src/main/java/org/apache/shardingsphere/sql/parser/binder/statement/dml/InsertStatementContext.java
##########
@@ -93,9 +110,20 @@ public InsertStatementContext(final SchemaMetaData schemaMetaData, final List<Ob
         return result;
     }
     
+    /**
+     * Get OnDuplicateKeyUpdateParameters.

Review comment:
       Please fix javadoc, use words or {@code } to label class name




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