jarredhj0214 commented on code in PR #13548:
URL: https://github.com/apache/gravitino/pull/13548#discussion_r4228909179
##########
catalogs/catalog-jdbc-mysql/src/main/java/org/apache/gravitino/catalog/mysql/converter/MysqlTypeConverter.java:
##########
@@ -166,4 +168,19 @@ public String fromGravitino(Type type) {
throw new IllegalArgumentException(
String.format("Couldn't convert Gravitino type %s to MySQL type",
type.simpleString()));
}
+
+ private static Type toGravitinoDecimal(JdbcTypeBean typeBean) {
+ int precision = typeBean.getColumnSize();
+ int scale = typeBean.getScale();
+ if (precision <= MAX_DECIMAL_PRECISION) {
+ return Types.DecimalType.of(precision, scale);
+ }
+
+ // MySQL supports precision up to 65, while Gravitino DecimalType is
limited to 38.
+ // Preserve larger decimals as ExternalType so loading the table does not
fail.
+ String unsignedSuffix =
+ DECIMAL_UNSIGNED.equalsIgnoreCase(typeBean.getTypeName()) ? "
unsigned" : "";
Review Comment:
Yes, for precision <= 38, the mapping retains precision and scale but does
not retain the UNSIGNED attribute. This is the existing behavior: both DECIMAL
and DECIMAL UNSIGNED already mapped to DecimalType before this PR, and
DecimalType has no unsigned flag.
UNSIGNED only excludes negative values; it does not increase the maximum
positive value or require extra decimal precision, so DecimalType(p,s) can
represent all source values. However, this mapping does lose the non-negative
constraint and would not preserve UNSIGNED in a type round trip.
This PR keeps that behavior for precision <= 38 and fixes table loading for
precision > 38, preserving the full declaration (including UNSIGNED) via
ExternalType. Preserving UNSIGNED for lower-precision decimals would be a
separate change to the existing mapping. The added boundary tests document this
distinction.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
