[
https://issues.apache.org/jira/browse/GEODE-8420?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17182143#comment-17182143
]
ASF GitHub Bot commented on GEODE-8420:
---------------------------------------
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:
[email protected]
> Throw AssertionError when SQLException occurs in JdbcWriter
> -----------------------------------------------------------
>
> Key: GEODE-8420
> URL: https://issues.apache.org/jira/browse/GEODE-8420
> Project: Geode
> Issue Type: Improvement
> Components: jdbc
> Reporter: Masaki Yamakawa
> Priority: Minor
> Labels: pull-request-available
>
> When using JdbcWriter or JdbcAsyncWriter of Simple JDBC Connector,
> SQLException occurs if you set the character string exceeding 10 digits in
> the column defined as varchar(10) in RDB column.
> However, JdbcWriter and JdbcAsyncWriter throws AssertionError.
> This error is difficult to notice the problem, so I would like to change it
> to throw SQLException.
> The cause is that the upsert of
> `org.apache.geode.connectors.jdbc.internal.SqlHandler#write` is considered.
> There are cases in which SQLException is ignored during this consideration.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
