exceptionfactory commented on code in PR #9857:
URL: https://github.com/apache/nifi/pull/9857#discussion_r2134118631
##########
nifi-extension-bundles/nifi-cdc/nifi-cdc-api/src/main/java/org/apache/nifi/cdc/event/ColumnDefinition.java:
##########
@@ -24,14 +24,16 @@
public class ColumnDefinition {
private int type;
+ private Boolean isSigned;
Review Comment:
Using the `Boolean` box type is usually unnecessary, and prefixing the
property with `is` also brings unnecessary wording. Naming the property
`signed` is one option, with a primitive `boolean` type. If the signed status
can be undefined, then an `enum` should be used with `SIGNED`, `UNSIGNED`, and
`UNDEFINED`.
##########
nifi-extension-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java:
##########
@@ -1253,7 +1253,9 @@ protected TableInfo loadTableInfo(TableInfoCacheKey key)
throws SQLException {
for (int i = 1; i <= numCols; i++) {
// Use the column label if it exists, otherwise use the
column name. We're not doing aliasing here, but it's better practice.
String columnLabel = rsmd.getColumnLabel(i);
- columnDefinitions.add(new
ColumnDefinition(rsmd.getColumnType(i), columnLabel != null ? columnLabel :
rsmd.getColumnName(i)));
+ columnDefinitions.add(new ColumnDefinition(
+ rsmd.isSigned(i), rsmd.getColumnType(i),
columnLabel != null ? columnLabel : rsmd.getColumnName(i)
Review Comment:
It would be helpful to assign these arguments to variables for readability,
instead of the inline ternary.
##########
nifi-extension-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/test/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQLTest.java:
##########
@@ -500,6 +501,165 @@ public void testExtendedTransaction() {
testRunner.getProvenanceEvents().forEach(event ->
assertEquals(ProvenanceEventType.RECEIVE, event.getEventType()));
}
+ @Test
+ public void testInsertUpdateDeleteSignedAndUnsigned(@Mock Connection
connection) throws JsonProcessingException {
Review Comment:
It would be helpful to break this method into some smaller private methods
for better maintainability.
##########
nifi-extension-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/event/io/AbstractBinlogTableEventWriter.java:
##########
@@ -16,36 +16,44 @@
*/
package org.apache.nifi.cdc.mysql.event.io;
+import com.fasterxml.jackson.core.JsonGenerator;
+import org.apache.nifi.cdc.event.ColumnDefinition;
import org.apache.nifi.cdc.mysql.event.BinlogTableEventInfo;
import java.io.IOException;
import java.io.Serializable;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Function;
/**
* An abstract base class for writing MYSQL table-related binlog events into
flow file(s), e.g.
*/
public abstract class AbstractBinlogTableEventWriter<T extends
BinlogTableEventInfo> extends AbstractBinlogEventWriter<T> {
- protected Object getWritableObject(Integer type, Serializable value) {
+ private final static Map<Integer, Function<Number, String>>
UNSIGNED_SQLTYPE_MAP = new HashMap<>(Map.of(
+ java.sql.Types.BIGINT, (z) ->
Long.toUnsignedString(z.longValue()),
+ java.sql.Types.INTEGER, (z) ->
String.valueOf(Integer.toUnsignedLong(z.intValue())),
+ java.sql.Types.SMALLINT, (z) ->
String.valueOf(Short.toUnsignedInt(z.shortValue())),
+ java.sql.Types.TINYINT, (z) ->
String.valueOf(Byte.toUnsignedInt(z.byteValue()))
+ ));
+
+ protected void writeObjectAsValueField(JsonGenerator jg, String fieldName,
ColumnDefinition columnDefinition,
Review Comment:
This approach changes the basic strategy of this class, but writing out the
value instead of returning a value. Unless there is a strong reason, it seems
like the existing approach should be retained.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
