exceptionfactory commented on a change in pull request #4938:
URL: https://github.com/apache/nifi/pull/4938#discussion_r603300543
##########
File path:
nifi-commons/nifi-record/src/main/java/org/apache/nifi/serialization/record/ResultSetRecordSet.java
##########
@@ -239,6 +239,11 @@ private DataType getDataType(final int sqlType, final
ResultSet rs, final int co
// Default scale is used to preserve decimals in such case.
decimalScale = resultSetScale > 0 ? resultSetScale :
defaultScale;
}
+ // Scale can be bigger than precision in some cases (Oracle,
e.g.) If this is the case, assume precision refers to the number of
+ // decimal digits and thus precision = scale
+ if (decimalScale > decimalPrecision) {
+ decimalPrecision = decimalScale;
Review comment:
It would be useful to add a unit test method in `ResultSetRecordSetTest`
to verify the expected behavior.
##########
File path:
nifi-nar-bundles/nifi-extension-utils/nifi-database-utils/src/test/java/org/apache/nifi/util/db/TestJdbcCommon.java
##########
@@ -465,7 +474,12 @@ private void
testConvertToAvroStreamForBigDecimal(BigDecimal bigDecimal, int dbP
assertEquals("decimal", logicalType.getName());
LogicalTypes.Decimal decimalType = (LogicalTypes.Decimal)
logicalType;
assertEquals(expectedPrecision, decimalType.getPrecision());
- assertEquals(expectedScale, decimalType.getScale());
+ // If scale > precision then precision will be set to the scale
value
+ if (expectedScale > expectedPrecision) {
Review comment:
This conditional always evaluates to false when running the unit test in
debug mode. Does something need to be changed in the new unit test method?
##########
File path:
nifi-nar-bundles/nifi-extension-utils/nifi-database-utils/src/test/java/org/apache/nifi/util/db/TestJdbcCommon.java
##########
@@ -465,7 +474,12 @@ private void
testConvertToAvroStreamForBigDecimal(BigDecimal bigDecimal, int dbP
assertEquals("decimal", logicalType.getName());
LogicalTypes.Decimal decimalType = (LogicalTypes.Decimal)
logicalType;
assertEquals(expectedPrecision, decimalType.getPrecision());
- assertEquals(expectedScale, decimalType.getScale());
+ // If scale > precision then precision will be set to the scale
value
+ if (expectedScale > expectedPrecision) {
+ assertEquals(expectedScale, decimalType.getScale());
+ } else {
+ assertEquals(expectedScale, decimalType.getScale());
Review comment:
This assertion is the same as the one used when `expectedScale` is
greater than `expectedPrecision`, should this be different?
--
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]
