You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "mark-bathori (via GitHub)" <gi...@apache.org> on 2023/05/15 12:14:56 UTC
[GitHub] [nifi] mark-bathori commented on a diff in pull request #7239: NIFI-11538: Fix primitive type conversion for PutIceberg
mark-bathori commented on code in PR #7239:
URL: https://github.com/apache/nifi/pull/7239#discussion_r1193745161
##########
nifi-nar-bundles/nifi-iceberg-bundle/nifi-iceberg-processors/src/main/java/org/apache/nifi/processors/iceberg/converter/GenericDataConverters.java:
##########
@@ -48,10 +51,28 @@
public class GenericDataConverters {
static class SameTypeConverter extends DataConverter<Object, Object> {
+ final Type.PrimitiveType targetType;
+ final DataType sourceType;
+
+ public SameTypeConverter(final Type.PrimitiveType type, final DataType dataType) {
+ targetType = type;
+ sourceType = dataType;
Review Comment:
This sourceType variable is never used so it can be removed from the class and the constructor.
##########
nifi-nar-bundles/nifi-iceberg-bundle/nifi-iceberg-processors/src/main/java/org/apache/nifi/processors/iceberg/converter/GenericDataConverters.java:
##########
@@ -48,10 +51,28 @@
public class GenericDataConverters {
static class SameTypeConverter extends DataConverter<Object, Object> {
Review Comment:
Since the functionality is changed, the method should be renamed to reflect its current function.
##########
nifi-nar-bundles/nifi-iceberg-bundle/nifi-iceberg-processors/src/main/java/org/apache/nifi/processors/iceberg/converter/GenericDataConverters.java:
##########
@@ -48,10 +51,28 @@
public class GenericDataConverters {
static class SameTypeConverter extends DataConverter<Object, Object> {
+ final Type.PrimitiveType targetType;
+ final DataType sourceType;
+
+ public SameTypeConverter(final Type.PrimitiveType type, final DataType dataType) {
+ targetType = type;
+ sourceType = dataType;
+ }
@Override
public Object convert(Object data) {
- return data;
+ switch (targetType.typeId()) {
+ case BOOLEAN: return DataTypeUtils.convertType(data, RecordFieldType.BOOLEAN.getDataType(), null);
+ case INTEGER: return DataTypeUtils.convertType(data, RecordFieldType.INT.getDataType(), null);
+ case LONG: return DataTypeUtils.convertType(data, RecordFieldType.LONG.getDataType(), null);
+ case FLOAT: return DataTypeUtils.convertType(data, RecordFieldType.FLOAT.getDataType(), null);
+ case DOUBLE: return DataTypeUtils.convertType(data, RecordFieldType.DOUBLE.getDataType(), null);
+ // DATE objects are already LocalDates, so return it untouched
+ case DATE: return data;
+ case STRING:
+ default:
+ return DataTypeUtils.convertRecordFieldtoObject(data, RecordFieldType.STRING.getDataType());
Review Comment:
The UUID data type is also using the `SameTypeConverter` and since it is not handled in the switch case it is going to be converted to String instead of UUID.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org