DonalEvans commented on a change in pull request #5442:
URL: https://github.com/apache/geode/pull/5442#discussion_r474948338



##########
File path: 
geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/SqlHandler.java
##########
@@ -255,6 +265,10 @@ private PreparedStatement getPreparedStatement(Connection 
connection,
       TableMetaDataView tableMetaData, EntryColumnData entryColumnData, 
Operation operation)
       throws SQLException {
     String sqlStr = getSqlString(tableMetaData, entryColumnData, operation);
+    if (logger.isDebugEnabled()) {
+      logger.debug("SQL:{} key:{} value:{}", sqlStr, 
entryColumnData.getEntryKeyColumnData(),

Review comment:
       This log output could be a little more descriptive. Possibly something 
like "Got SQL string:{} with key:{} value:{}"?

##########
File path: 
geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcAsyncWriterIntegrationTest.java
##########
@@ -310,6 +372,55 @@ public void canInsertBecomeUpdate() throws Exception {
     assertThat(resultSet.next()).isFalse();
   }
 
+  @Test
+  public void updateBecomeInsertFieldLengthOversTableColumnLengthFails() 
throws Exception {

Review comment:
       This might be better named 
"updateBecomeInsertFieldLengthGreaterThanTableColumnLengthFails"

##########
File path: 
geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcAsyncWriterIntegrationTest.java
##########
@@ -158,8 +181,21 @@ public void putNonPdxInstanceThatIsPdxSerializable()
     assertThat(resultSet.next()).isFalse();
   }
 
+  @Test
+  public void putInstanceFieldLengthOversTableColumnLengthFails()

Review comment:
       This might be better named 
"putInstanceFieldLengthGreaterThanTableColumnLengthFails"

##########
File path: 
geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcAsyncWriterIntegrationTest.java
##########
@@ -274,8 +315,28 @@ public void canUpdateTableWithCompositeKey() throws 
Exception {
     assertThat(resultSet.next()).isFalse();
   }
 
+  @Test
+  public void updateInstanceFieldLengthOversTableColumnLengthFails()

Review comment:
       This might be better named 
"updateInstanceFieldLengthGreaterThanTableColumnLengthFails"

##########
File path: 
geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcAsyncWriterIntegrationTest.java
##########
@@ -310,6 +372,55 @@ public void canInsertBecomeUpdate() throws Exception {
     assertThat(resultSet.next()).isFalse();
   }
 
+  @Test
+  public void updateBecomeInsertFieldLengthOversTableColumnLengthFails() 
throws Exception {
+    createTable();
+    setupRegion("id");
+    employees.put("1", pdxEmployee1);
+    awaitUntil(() -> 
assertThat(jdbcWriter.getSuccessfulEvents()).isEqualTo(1));
+    statement.execute("delete from " + REGION_TABLE_NAME + " where id = '1'");
+    validateTableRowCount(0);
+
+    employees.put("1", illegalPdxEmployee);
+
+    awaitUntil(() -> assertThat(jdbcWriter.getFailedEvents()).isEqualTo(1));
+    awaitUntil(() -> assertThat(jdbcWriter.getTotalEvents()).isEqualTo(2));
+  }
+
+  @Test
+  public void insertBecomeUpdateFieldLengthOversTableColumnLengthFails() 
throws Exception {

Review comment:
       This might be better named 
"insertBecomeUpdateFieldLengthGreaterThanTableColumnLengthFails"

##########
File path: 
geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcWriterIntegrationTest.java
##########
@@ -268,6 +283,18 @@ public void putNonPdxInstanceThatIsPdxSerializable()
     assertThat(resultSet.next()).isFalse();
   }
 
+  @Test
+  public void putInstanceFieldLengthOversTableColumnLengthFails()

Review comment:
       This might be better named 
"putInstanceFieldLengthGreaterThanTableColumnLengthFails"

##########
File path: 
geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcWriterIntegrationTest.java
##########
@@ -370,6 +397,24 @@ public void canUpdateTableWithCompositeKey() throws 
Exception {
     assertThat(resultSet.next()).isFalse();
   }
 
+  @Test
+  public void updateInstanceFieldLengthOversTableColumnLengthFails()

Review comment:
       This might be better named 
"updateInstanceFieldLengthGreaterThanTableColumnLengthFails"

##########
File path: 
geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcWriterIntegrationTest.java
##########
@@ -402,6 +447,53 @@ public void canInsertBecomeUpdate() throws Exception {
     assertThat(resultSet.next()).isFalse();
   }
 
+  @Test
+  public void updateBecomeInsertFieldLengthOversTableColumnLengthFails() 
throws Exception {

Review comment:
       This might be better named 
"updateBecomeInsertFieldLengthGreaterThanTableColumnLengthFails"

##########
File path: 
geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcWriterIntegrationTest.java
##########
@@ -402,6 +447,53 @@ public void canInsertBecomeUpdate() throws Exception {
     assertThat(resultSet.next()).isFalse();
   }
 
+  @Test
+  public void updateBecomeInsertFieldLengthOversTableColumnLengthFails() 
throws Exception {
+    createTable();
+    setupRegion("id");
+    employees.put("1", pdx1);
+
+    statement.execute("delete from " + REGION_TABLE_NAME + " where id = '1'");
+    validateTableRowCount(0);
+
+    Throwable thrown = catchThrowable(() -> employees.put("1", illegalPdx));
+
+    assertThat(thrown).isInstanceOf(JdbcConnectorException.class);
+    
assertThat(thrown.getMessage()).startsWith(getDataTooLongSqlErrorMessage());
+  }
+
+  @Test
+  public void insertBecomeUpdateFieldLengthOversTableColumnLengthFails() 
throws Exception {

Review comment:
       This might be better named 
"insertBecomeUpdateFieldLengthGreaterThanTableColumnLengthFails"




----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to