Aaaaaaron commented on a change in pull request #2161:
URL: https://github.com/apache/calcite/pull/2161#discussion_r493164685
##########
File path: core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java
##########
@@ -1668,12 +1668,19 @@ public static boolean isAtomic(RelDataType type) {
|| SqlTypeUtil.isBoolean(type);
}
- /** Returns a DECIMAL type with the maximum precision/scale for the current
+ /** Returns a DECIMAL type with the maximum precision for the current
* type system. */
public static RelDataType getMaxPrecisionScaleDecimal(RelDataTypeFactory
factory) {
int maxPrecision = factory.getTypeSystem().getMaxNumericPrecision();
int maxScale = factory.getTypeSystem().getMaxNumericScale();
+ // maxScale should not greater than maxPrecision.
+ // If they are equal, e.g. Decimal(19,19)
+ // means we can only have decimal places.
+ while (maxScale >= maxPrecision) {
+ maxScale = maxScale / 2;
+ }
Review comment:
> I'm afraid not all the sql engines have the rule that `maxScale =
maxScale / 2`, we should add a check for the validity instead of modifying the
value silently. And i'm inclined to fix the default max precision/scale for
Calcite type system.
This is also a proposal, I'll take a look at Hive/Spark decimal's max
scale/repcision, but I'm afraid they may be the same, or we don't get max but
get default?
----------------------------------------------------------------
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]
