Skip to content

Commit

Permalink
[apache#1761] fix(mysql)clarify timestamp and datetime type conve…
Browse files Browse the repository at this point in the history
…rt (apache#3477)

### What changes were proposed in this pull request?

 - map `datetime` of MySQL to `timestamp` of Gravitino
 - map `timestamp` of MySQL to `timestamp_tz` type of Gravitino.

### Why are the changes needed?

unify the type semantics

Fix: apache#1761 

### Does this PR introduce _any_ user-facing change?

no

### How was this patch tested?

tests added
  • Loading branch information
mchades authored and diqiu50 committed Jun 13, 2024
1 parent a5dc006 commit 9eb80f8
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,13 @@ public Type toGravitinoType(JdbcTypeBean typeBean) {
return Types.DateType.get();
case TIME:
return Types.TimeType.get();
// MySQL converts TIMESTAMP values from the current time zone to UTC for storage, and back
// from UTC to the current time zone for retrieval. (This does not occur for other types
// such as DATETIME.) see more details:
// https://dev.mysql.com/doc/refman/8.0/en/datetime.html
case TIMESTAMP:
return Types.TimestampType.withTimeZone();
case DATETIME:
return Types.TimestampType.withoutTimeZone();
case DECIMAL:
return Types.DecimalType.of(
Expand Down Expand Up @@ -79,8 +85,11 @@ public String fromGravitinoType(Type type) {
return type.simpleString();
} else if (type instanceof Types.TimeType) {
return type.simpleString();
} else if (type instanceof Types.TimestampType && !((Types.TimestampType) type).hasTimeZone()) {
return type.simpleString();
} else if (type instanceof Types.TimestampType) {
// MySQL converts TIMESTAMP values from the current time zone to UTC for storage, and back
// from UTC to the current time zone for retrieval. (This does not occur for other types
// such as DATETIME.) see more details: https://dev.mysql.com/doc/refman/8.0/en/datetime.html
return ((Types.TimestampType) type).hasTimeZone() ? TIMESTAMP : DATETIME;
} else if (type instanceof Types.DecimalType) {
return type.simpleString();
} else if (type instanceof Types.VarCharType) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import static com.datastrato.gravitino.catalog.mysql.converter.MysqlTypeConverter.BIGINT;
import static com.datastrato.gravitino.catalog.mysql.converter.MysqlTypeConverter.BINARY;
import static com.datastrato.gravitino.catalog.mysql.converter.MysqlTypeConverter.CHAR;
import static com.datastrato.gravitino.catalog.mysql.converter.MysqlTypeConverter.DATETIME;
import static com.datastrato.gravitino.catalog.mysql.converter.MysqlTypeConverter.DECIMAL;
import static com.datastrato.gravitino.catalog.mysql.converter.MysqlTypeConverter.DOUBLE;
import static com.datastrato.gravitino.catalog.mysql.converter.MysqlTypeConverter.FLOAT;
Expand Down Expand Up @@ -39,7 +40,8 @@ public void testToGravitinoType() {
checkJdbcTypeToGravitinoType(Types.DoubleType.get(), DOUBLE, null, null);
checkJdbcTypeToGravitinoType(Types.DateType.get(), DATE, null, null);
checkJdbcTypeToGravitinoType(Types.TimeType.get(), TIME, null, null);
checkJdbcTypeToGravitinoType(Types.TimestampType.withoutTimeZone(), TIMESTAMP, null, null);
checkJdbcTypeToGravitinoType(Types.TimestampType.withoutTimeZone(), DATETIME, null, null);
checkJdbcTypeToGravitinoType(Types.TimestampType.withTimeZone(), TIMESTAMP, null, null);
checkJdbcTypeToGravitinoType(Types.DecimalType.of(10, 2), DECIMAL, "10", "2");
checkJdbcTypeToGravitinoType(Types.VarCharType.of(20), VARCHAR, "20", null);
checkJdbcTypeToGravitinoType(Types.FixedCharType.of(20), CHAR, "20", null);
Expand All @@ -58,7 +60,8 @@ public void testFromGravitinoType() {
checkGravitinoTypeToJdbcType(DOUBLE, Types.DoubleType.get());
checkGravitinoTypeToJdbcType(DATE, Types.DateType.get());
checkGravitinoTypeToJdbcType(TIME, Types.TimeType.get());
checkGravitinoTypeToJdbcType(TIMESTAMP, Types.TimestampType.withoutTimeZone());
checkGravitinoTypeToJdbcType(TIMESTAMP, Types.TimestampType.withTimeZone());
checkGravitinoTypeToJdbcType(DATETIME, Types.TimestampType.withoutTimeZone());
checkGravitinoTypeToJdbcType(DECIMAL + "(10,2)", Types.DecimalType.of(10, 2));
checkGravitinoTypeToJdbcType(VARCHAR + "(20)", Types.VarCharType.of(20));
checkGravitinoTypeToJdbcType(CHAR + "(20)", Types.FixedCharType.of(20));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,87 @@ void testColumnDefaultValueConverter() {
}
}

@Test
void testColumnTypeConverter() {
// test convert from MySQL to Gravitino
String tableName = GravitinoITUtils.genRandomName("test_type_converter");
String fullTableName = schemaName + "." + tableName;
String sql =
"CREATE TABLE "
+ fullTableName
+ " (\n"
+ " tinyint_col tinyint,\n"
+ " smallint_col smallint,\n"
+ " int_col int,\n"
+ " bigint_col bigint,\n"
+ " float_col float,\n"
+ " double_col double,\n"
+ " date_col date,\n"
+ " time_col time,\n"
+ " timestamp_col timestamp,\n"
+ " datetime_col datetime,\n"
+ " decimal_6_2_col decimal(6, 2),\n"
+ " varchar20_col varchar(20),\n"
+ " text_col text,\n"
+ " binary_col binary\n"
+ ");\n";

mysqlService.executeQuery(sql);
Table loadedTable =
catalog
.asTableCatalog()
.loadTable(NameIdentifier.of(metalakeName, catalogName, schemaName, tableName));

for (Column column : loadedTable.columns()) {
switch (column.name()) {
case "tinyint_col":
Assertions.assertEquals(Types.ByteType.get(), column.dataType());
break;
case "smallint_col":
Assertions.assertEquals(Types.ShortType.get(), column.dataType());
break;
case "int_col":
Assertions.assertEquals(Types.IntegerType.get(), column.dataType());
break;
case "bigint_col":
Assertions.assertEquals(Types.LongType.get(), column.dataType());
break;
case "float_col":
Assertions.assertEquals(Types.FloatType.get(), column.dataType());
break;
case "double_col":
Assertions.assertEquals(Types.DoubleType.get(), column.dataType());
break;
case "date_col":
Assertions.assertEquals(Types.DateType.get(), column.dataType());
break;
case "time_col":
Assertions.assertEquals(Types.TimeType.get(), column.dataType());
break;
case "timestamp_col":
Assertions.assertEquals(Types.TimestampType.withTimeZone(), column.dataType());
break;
case "datetime_col":
Assertions.assertEquals(Types.TimestampType.withoutTimeZone(), column.dataType());
break;
case "decimal_6_2_col":
Assertions.assertEquals(Types.DecimalType.of(6, 2), column.dataType());
break;
case "varchar20_col":
Assertions.assertEquals(Types.VarCharType.of(20), column.dataType());
break;
case "text_col":
Assertions.assertEquals(Types.StringType.get(), column.dataType());
break;
case "binary_col":
Assertions.assertEquals(Types.BinaryType.get(), column.dataType());
break;
default:
Assertions.fail("Unexpected column name: " + column.name());
}
}
}

@Test
void testAlterAndDropMysqlTable() {
Column[] columns = createColumns();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,6 @@ public void testCreateNotSupportTypeTable() {
Types.FixedType.of(10),
Types.IntervalDayType.get(),
Types.IntervalYearType.get(),
Types.TimestampType.withTimeZone(),
Types.UUIDType.get(),
Types.ListType.of(Types.DateType.get(), true),
Types.MapType.of(Types.StringType.get(), Types.IntegerType.get(), true),
Expand Down

0 comments on commit 9eb80f8

Please sign in to comment.