tpalfy commented on a change in pull request #4223:
URL: https://github.com/apache/nifi/pull/4223#discussion_r430518456
##########
File path:
nifi-commons/nifi-record/src/main/java/org/apache/nifi/serialization/record/util/DataTypeUtils.java
##########
@@ -1232,6 +1242,10 @@ public static boolean isBigIntTypeCompatible(final
Object value) {
return isNumberTypeCompatible(value, DataTypeUtils::isIntegral);
}
+ public static boolean isBigDecimalTypeCompatible(final Object value) {
+ return isNumberTypeCompatible(value, DataTypeUtils::isFloatingPoint);
Review comment:
This could be an unarmed landmine.
The `DataTypeUtils::isFloatingPoint` has a `Float.parse()` with the comment
above it: `// Just to ensure that the exponents are in range, etc.`
It ensures _no_ such thing in fact (if the number is out of range the parse
still succeeds and returns infinitiy), but this might get fixed later ("arming"
the landmine), after which for `BigDecimal` it might start to cause issues.
##########
File path:
nifi-commons/nifi-record/src/main/java/org/apache/nifi/serialization/record/util/DataTypeUtils.java
##########
@@ -34,6 +34,7 @@
import java.io.InputStream;
import java.io.Reader;
import java.lang.reflect.Array;
+import java.math.BigDecimal;
Review comment:
I think `public static Optional<DataType> getWiderType(final DataType
thisDataType, final DataType otherDataType)` should be updated as well.
Could add along a new test in `TestFieldTypeInference`:
```java
@Test
public void test() {
// GIVEN
List<DataType> dataTypes = Arrays.asList(
RecordFieldType.DECIMAL.getDecimalDataType(10, 1),
RecordFieldType.DECIMAL.getDecimalDataType(10, 3),
RecordFieldType.DECIMAL.getDecimalDataType(7, 3),
RecordFieldType.DECIMAL.getDecimalDataType(7, 5),
RecordFieldType.DECIMAL.getDecimalDataType(7, 7),
RecordFieldType.FLOAT.getDataType(),
RecordFieldType.DOUBLE.getDataType()
);
DataType expected = RecordFieldType.DECIMAL.getDecimalDataType(10,
7);
// WHEN
// THEN
runWithAllPermutations(this::testToDataTypeShouldReturnSingleType,
dataTypes, expected);
}
```
----------------------------------------------------------------
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]
