markap14 commented on a change in pull request #3724: NIFI-6640 - UNION/CHOICE
types not handled correctly
URL: https://github.com/apache/nifi/pull/3724#discussion_r324304574
##########
File path:
nifi-commons/nifi-record/src/main/java/org/apache/nifi/serialization/record/util/DataTypeUtils.java
##########
@@ -225,17 +232,109 @@ public static boolean isCompatibleDataType(final Object
value, final DataType da
}
public static DataType chooseDataType(final Object value, final
ChoiceDataType choiceType) {
- for (final DataType subType : choiceType.getPossibleSubTypes()) {
- if (isCompatibleDataType(value, subType)) {
- if (subType.getFieldType() == RecordFieldType.CHOICE) {
- return chooseDataType(value, (ChoiceDataType) subType);
- }
+ Queue<DataType> possibleSubTypes = new
LinkedList<>(choiceType.getPossibleSubTypes());
+ Set<DataType> possibleSimpleSubTypes = new HashSet<>();
- return subType;
+ while (possibleSubTypes.peek() != null) {
+ DataType subType = possibleSubTypes.poll();
+ if (subType instanceof ChoiceDataType) {
+ possibleSubTypes.addAll(((ChoiceDataType)
subType).getPossibleSubTypes());
+ } else {
+ possibleSimpleSubTypes.add(subType);
}
}
- return null;
+ List<DataType> compatibleSimpleSubTypes =
possibleSimpleSubTypes.stream()
+ .filter(subType -> isCompatibleDataType(value, subType))
+ .collect(Collectors.toList());
+
+ int nrOfCompatibleSimpleSubTypes = compatibleSimpleSubTypes.size();
+
+ DataType chosenSimpleType;
+ if (nrOfCompatibleSimpleSubTypes == 0) {
+ chosenSimpleType = null;
+ } else if (nrOfCompatibleSimpleSubTypes == 1) {
+ chosenSimpleType = compatibleSimpleSubTypes.get(0);
+ } else {
+ chosenSimpleType = findMostSuitableType(value,
compatibleSimpleSubTypes, Function.identity())
+ .orElse(compatibleSimpleSubTypes.get(0));
+ }
+
+ return chosenSimpleType;
+ }
+
+ public static <T> Optional<T> findMostSuitableType(Object value, List<T>
types, Function<T, DataType> dataTypeMapper) {
+ final Optional<T> mostSuitableType;
+
+ Optional<DataType> inferredDataTypeOptional =
Optional.ofNullable(inferDataType(value, null))
+ .filter(dataType ->
!dataType.getFieldType().equals(RecordFieldType.STRING));
+
+ if (value instanceof String) {
+ mostSuitableType = findMostSuitableTypeByStringValue((String)
value, types, dataTypeMapper);
+ } else if (inferredDataTypeOptional.isPresent()) {
+ DataType inferredDataType = inferredDataTypeOptional.get();
+
+ Optional<T> inferredTypeOptional = types.stream()
+ .filter(type ->
dataTypeMapper.apply(type).equals(inferredDataType))
+ .findFirst();
+
+ if (inferredTypeOptional.isPresent()) {
+ mostSuitableType = inferredTypeOptional;
+ } else {
+ Optional<T> widerAvailableTypeOptional = types.stream()
+ .map(type -> getWiderType(dataTypeMapper.apply(type),
inferredDataType).isPresent() ? type : null)
+ .filter(Objects::nonNull)
+ .findFirst();
+
+ if (widerAvailableTypeOptional.isPresent()) {
+ mostSuitableType = widerAvailableTypeOptional;
+ } else {
+ mostSuitableType = Optional.empty();
+ }
+ }
+ } else {
+ mostSuitableType = Optional.empty();
+ }
+
+ return mostSuitableType;
+ }
+
+ public static <T> Optional<T> findMostSuitableTypeByStringValue(String
valueAsString, List<T> types, Function<T, DataType> dataTypeMapper) {
+ Optional<T> mostSuitableType = types.stream()
+ // Sorting based on the RecordFieldType enum ordering looks
appropriate here as we want simpler types
+ // first and the enum's ordering seems to reflect that
+ .sorted((type1, type2) -> {
+ int comparison;
+
+ RecordFieldType dataType1 =
dataTypeMapper.apply(type1).getFieldType();
+ RecordFieldType dataType2 =
dataTypeMapper.apply(type2).getFieldType();
+
+ // Moving TIMESTAMP at the front (at least it
should precede DATE)
+ if (dataType1 == RecordFieldType.TIMESTAMP) {
+ comparison = -1;
+ } else if (dataType2 == RecordFieldType.TIMESTAMP)
{
+ comparison = 1;
+ } else {
+ comparison = dataType1.compareTo(dataType2);
+ }
+
+ return comparison;
+ }
+ )
+ .filter(type -> {
+ boolean compatible;
+
+ try {
+ compatible = isCompatibleDataType(valueAsString,
dataTypeMapper.apply(type));
Review comment:
This can be made much simpler by just simply returning the value. I.e.,
`return isCompatibleDataType(...);` and then `return false` if an Exception is
caught.
----------------------------------------------------------------
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]
With regards,
Apache Git Services