Xeinn commented on issue #4104: NIFI-7159 URL: https://github.com/apache/nifi/pull/4104#issuecomment-597106942 Thanks for the feedback. Good idea will be making some changes as per Mike’s feedback later in the week so will incorporate the below then. Regards Chris Sent from Mail<https://go.microsoft.com/fwlink/?LinkId=550986> for Windows 10 ________________________________ From: tpalfy <[email protected]> Sent: Friday, March 6, 2020 7:31:43 PM To: apache/nifi <[email protected]> Cc: Chris Manniex <[email protected]>; Author <[email protected]> Subject: Re: [apache/nifi] NIFI-7159 (#4104) @tpalfy requested changes on this pull request. ________________________________ In nifi-commons/nifi-record/src/main/java/org/apache/nifi/serialization/record/type/DecimalDataType.java<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fnifi%2Fpull%2F4104%23discussion_r389051423&data=02%7C01%7C%7Cb6ae8c600b2441190d0408d7c20500ed%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637191199047595864&sdata=yejHbBKmWv0PfQHtTJFaDrgRy0NCyByfe%2FCSbLI1cJU%3D&reserved=0>: > + return true; + } + if (obj == null) { + return false; + } + if (!(obj instanceof DecimalDataType)) { + return false; + } + + final DecimalDataType other = (DecimalDataType) obj; + return precision == other.getPrecision() && scale == other.getScale(); + } + + @Override + public String toString() { + return "DECIMAL" + Integer.toString(precision) + Integer.toString(scale); Might be a good idea to add underscores (for example) so it looks like this: DECIMAL_11_11 instead of this: DECIMAL1111 Btw Integer.toString is redundant with the string literal plus '+' operator (makes it a bit less readable). ________________________________ In nifi-commons/nifi-record/src/main/java/org/apache/nifi/serialization/record/util/DataTypeUtils.java<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fnifi%2Fpull%2F4104%23discussion_r389052612&data=02%7C01%7C%7Cb6ae8c600b2441190d0408d7c20500ed%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637191199047605858&sdata=oAr4Ue8tgmCg0LLB5mi1yghxl9kYgY1OblG9YLjyfTs%3D&reserved=0>: > @@ -1237,10 +1246,38 @@ public static Double toDouble(final Object value, final String fieldName) { throw new IllegalTypeConversionException("Cannot convert value [" + value + "] of type " + value.getClass() + " to Double for field " + fieldName); } + public static BigDecimal toBigDecimal(final Object value, final String fieldName) { + if (value == null) { Shouldn't we cover all the Number cases? I think new BigDecimal(value.toString()); might be enough. (Can't think of an example where the string representation of a Number would not be good.) ________________________________ In nifi-commons/nifi-record/src/main/java/org/apache/nifi/serialization/record/RecordFieldType.java<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fnifi%2Fpull%2F4104%23discussion_r389092840&data=02%7C01%7C%7Cb6ae8c600b2441190d0408d7c20500ed%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637191199047605858&sdata=oOrZRC7UnSHPpsuDAe51e6gy2O%2BgHY%2BIe%2Bk1hff0piM%3D&reserved=0>: > @@ -280,6 +286,25 @@ public DataType getRecordDataType(final RecordSchema childSchema) { return new RecordDataType(childSchema); } + /** + * Returns a data type that represents a DECIMAL value with given precision and scale + * + * @param precision indicates the expected number of digits before the decimal point Precision is the number of all digits. ________________________________ In nifi-commons/nifi-record/src/main/java/org/apache/nifi/serialization/record/util/DataTypeUtils.java<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fnifi%2Fpull%2F4104%23discussion_r389097315&data=02%7C01%7C%7Cb6ae8c600b2441190d0408d7c20500ed%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637191199047615853&sdata=ztQpc0iqHV7rQndHeBKvWxQhosmnOOxINZfGdv910os%3D&reserved=0>: > @@ -34,6 +34,7 @@ import java.io.InputStream; import java.io.Reader; import java.lang.reflect.Array; +import java.math.BigDecimal; Currently DataTypeUtils.findMostSuitableType( new BigDecimal("11.110"), Arrays.asList(RecordFieldType.DECIMAL.getDecimalDataType(5, 3)), Function.identity() ); correctly returns Optional.of(RecordFieldType.DECIMAL.getDecimalDataType(5, 3));, but DataTypeUtils.findMostSuitableType( new BigDecimal("1.110"), Arrays.asList(RecordFieldType.DECIMAL.getDecimalDataType(5, 3)), Function.identity() ); ("1.110" instead of "11.111") returns Optional.empty(); This is because the inferred type DECIMAL(4,2) is not equal to the provided DECIMAL(5,2) so the getWiderType method is called which doesn't handle cases where the two types are the same in general, only differ in their details. Probably the public static Optional<DataType> getWiderType(final DataType thisDataType, final DataType otherDataType) should handle two different DECIMAL types by returning another DECIMAL that is the superset of the two in terms of scale and precision. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fnifi%2Fpull%2F4104%3Femail_source%3Dnotifications%26email_token%3DAFHXE7DKUBOFT4EWJBAGEADRGFFR7A5CNFSM4K72OIGKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCYKZB2Y%23pullrequestreview-370512107&data=02%7C01%7C%7Cb6ae8c600b2441190d0408d7c20500ed%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637191199047615853&sdata=LU88VM5%2Fw%2FMrEzyJUdVu5yOxlkVlwOg2sRdz5JmyvGo%3D&reserved=0>, or unsubscribe<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAFHXE7EJCF5ODMWQD2DE64LRGFFR7ANCNFSM4K72OIGA&data=02%7C01%7C%7Cb6ae8c600b2441190d0408d7c20500ed%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637191199047615853&sdata=f2DOBtkZihGP3GAS%2FKlmBymYTT%2B6lVgI7RWjo74SipY%3D&reserved=0>.
---------------------------------------------------------------- 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
