mark-bathori commented on code in PR #7239:
URL: https://github.com/apache/nifi/pull/7239#discussion_r1203802213
##########
nifi-nar-bundles/nifi-iceberg-bundle/nifi-iceberg-processors/src/main/java/org/apache/nifi/processors/iceberg/converter/GenericDataConverters.java:
##########
@@ -123,10 +175,15 @@ static class BigDecimalConverter extends
DataConverter<BigDecimal, BigDecimal> {
}
@Override
- public BigDecimal convert(BigDecimal data) {
- Validate.isTrue(data.scale() == scale, "Cannot write value as
decimal(%s,%s), wrong scale: %s", precision, scale, data);
- Validate.isTrue(data.precision() <= precision, "Cannot write value
as decimal(%s,%s), invalid precision: %s", precision, scale, data);
- return data;
+ public BigDecimal convert(Object data) {
+ if (data instanceof BigDecimal) {
+ BigDecimal bigDecimal = (BigDecimal) data;
+ Validate.isTrue(bigDecimal.scale() == scale, "Cannot write
value as decimal(%s,%s), wrong scale %s for value: %s", precision, scale,
bigDecimal.scale(), data);
+ Validate.isTrue(bigDecimal.precision() <= precision, "Cannot
write value as decimal(%s,%s), invalid precision %s for value: %s",
+ precision, scale, bigDecimal.precision(), data);
+ return bigDecimal;
+ }
+ return (BigDecimal) DataTypeUtils.convertType(data,
RecordFieldType.DECIMAL.getDecimalDataType(precision, scale), null);
Review Comment:
You can use `DataTypeUtils.toBigDecimal` here, the `precision` and `scale`
parameters are not used in the `DataTypeUtils.convertType` either.
##########
nifi-nar-bundles/nifi-iceberg-bundle/nifi-iceberg-processors/src/main/java/org/apache/nifi/processors/iceberg/converter/IcebergRecordConverter.java:
##########
@@ -46,6 +46,7 @@
public class IcebergRecordConverter {
private final DataConverter<Record, GenericRecord> converter;
+ final FileFormat fileFormat;
Review Comment:
You can remove this since it is unused.
##########
nifi-nar-bundles/nifi-iceberg-bundle/nifi-iceberg-processors/src/main/java/org/apache/nifi/processors/iceberg/converter/IcebergRecordConverter.java:
##########
@@ -54,6 +55,7 @@ public GenericRecord convert(Record record) {
@SuppressWarnings("unchecked")
public IcebergRecordConverter(Schema schema, RecordSchema recordSchema,
FileFormat fileFormat) {
this.converter = (DataConverter<Record, GenericRecord>)
IcebergSchemaVisitor.visit(schema, new RecordDataType(recordSchema),
fileFormat);
+ this.fileFormat = fileFormat;
Review Comment:
You can remove this since it is unused.
##########
nifi-nar-bundles/nifi-iceberg-bundle/nifi-iceberg-processors/src/main/java/org/apache/nifi/processors/iceberg/converter/GenericDataConverters.java:
##########
@@ -47,45 +50,94 @@
*/
public class GenericDataConverters {
- static class SameTypeConverter extends DataConverter<Object, Object> {
+ static class PrimitiveTypeConverter extends DataConverter<Object, Object> {
+ final Type.PrimitiveType targetType;
+ final DataType sourceType;
+
+ public PrimitiveTypeConverter(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.toBoolean(data, null);
+ case INTEGER:
+ return DataTypeUtils.toInteger(data, null);
+ case LONG:
+ return DataTypeUtils.toLong(data, null);
+ case FLOAT:
+ return DataTypeUtils.toFloat(data, null);
+ case DOUBLE:
+ return DataTypeUtils.toDouble(data, null);
+ case DATE:
+ return DataTypeUtils.toLocalDate(data, () ->
DataTypeUtils.getDateTimeFormatter(sourceType.getFormat(),
ZoneId.systemDefault()), null);
+ case UUID:
+ return DataTypeUtils.toUUID(data);
+ case STRING:
+ default:
+ return DataTypeUtils.toString(data, () -> null);
+ }
}
}
- static class TimeConverter extends DataConverter<Time, LocalTime> {
+ static class TimeConverter extends DataConverter<Object, LocalTime> {
+
+ private final String timeFormat;
+
+ public TimeConverter(final String format) {
+ this.timeFormat = format;
+ }
@Override
- public LocalTime convert(Time data) {
- return data.toLocalTime();
+ public LocalTime convert(Object data) {
+ if (data instanceof Time) {
Review Comment:
This is still not needed. The `DataTypeUtils.toTime ` contains this type
check.
--
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]