diqiu50 commented on code in PR #12906:
URL: https://github.com/apache/gravitino/pull/12906#discussion_r3949851819


##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/jdbc/mysql/MySQLDataTypeTransformer.java:
##########
@@ -49,13 +50,23 @@ public io.trino.spi.type.Type getTrinoType(Type type) {
       return io.trino.spi.type.VarcharType.createUnboundedVarcharType();
     } else if (Name.TIMESTAMP == type.name()) {
       Types.TimestampType timestampType = (Types.TimestampType) type;
-      if (timestampType.hasTimeZone()) {
-        return TimestampWithTimeZoneType.TIMESTAMP_TZ_SECONDS;
-      } else {
-        return TimestampType.TIMESTAMP_SECONDS;
-      }
+      // When the precision is unknown (the MySQL catalog reports it only with 
MySQL Connector/J

Review Comment:
   How should we handle MySQL driver versions earlier than 8.0.16?



##########
catalogs/catalog-jdbc-mysql/src/main/java/org/apache/gravitino/catalog/mysql/operation/MysqlTableOperations.java:
##########
@@ -312,6 +315,29 @@ protected String generateAlterTableSql(
     return result;
   }
 
+  /**
+   * {@inheritDoc}
+   *
+   * <p>MySQL requires the fractional seconds precision of CURRENT_TIMESTAMP 
to match the one of the
+   * column: {@code DATETIME(6) DEFAULT CURRENT_TIMESTAMP} is rejected while 
{@code DATETIME(6)
+   * DEFAULT CURRENT_TIMESTAMP(6)} is accepted. Columns without a precision, 
or with precision 0,
+   * keep the bare CURRENT_TIMESTAMP.
+   *
+   * @see <a 
href="https://dev.mysql.com/doc/refman/8.0/en/timestamp-initialization.html";>Automatic
+   *     Initialization and Updating for TIMESTAMP and DATETIME</a>
+   */
+  @Override
+  protected String renderDefaultValue(Type type, Expression defaultValue) {
+    String rendered = super.renderDefaultValue(type, defaultValue);
+    if (CURRENT_TIMESTAMP.equals(rendered) && type instanceof 
Types.TimestampType) {
+      int precision = ((Types.TimestampType) type).precision();
+      if (precision > 0) {
+        return String.format("%s(%d)", rendered, precision);
+      }
+    }
+    return rendered;
+  }

Review Comment:
   Can we add integration test to cover this logic



-- 
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]

Reply via email to