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