This is an automated email from the ASF dual-hosted git repository.
diqiu50 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new ddf9fc7bd0 [#13032] fix(catalog-jdbc-mysql): Preserve full type
declaration for lossy MySQL column types (#13036)
ddf9fc7bd0 is described below
commit ddf9fc7bd083ffd31b4d8c1773d901b9df93fadd
Author: Yuhui <[email protected]>
AuthorDate: Wed Sep 9 21:10:19 2026 +0800
[#13032] fix(catalog-jdbc-mysql): Preserve full type declaration for lossy
MySQL column types (#13036)
### What changes were proposed in this pull request?
Recover the full type declaration from information_schema.columns when
MySQL's JDBC metadata alone would lose it, and represent it as an
external type. Also fixes the Trino connector's exact-string type
matching, which would otherwise misclassify the new parameterized forms
as VARCHAR.
### Why are the changes needed?
MySQL columns like VARBINARY(100), ENUM(...), SET(...), BIT(8), and
BINARY(16) lost their length/values when loaded, and BIT/BINARY of any
width were indistinguishable from each other.
Fix: #13032
### Does this PR introduce any user-facing change?
Yes: these column types now report a more precise external type with the
full declaration instead of a bare or widthless one.
### How was this patch tested?
Added/extended unit, Docker, and Trino connector tests covering the
fixed types and boundary cases; full suites pass.
---
.../apache/gravitino/catalog/jdbc/JdbcTable.java | 8 ++
.../mysql/operation/MysqlTableOperations.java | 100 +++++++++++++++++++++
.../mysql/integration/test/CatalogMysqlIT.java | 18 +++-
.../mysql/operation/TestMysqlTableOperations.java | 53 +++++++++++
.../catalog/jdbc/mysql/MySQLExternalDataType.java | 12 ++-
.../jdbc/mysql/TestMySQLDataTypeTransformer.java | 22 +++++
6 files changed, 211 insertions(+), 2 deletions(-)
diff --git
a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcTable.java
b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcTable.java
index f1008f2d03..477cc17296 100644
---
a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcTable.java
+++
b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcTable.java
@@ -25,6 +25,7 @@ import org.apache.commons.lang3.ArrayUtils;
import org.apache.gravitino.catalog.jdbc.operation.TableOperation;
import org.apache.gravitino.connector.BaseTable;
import org.apache.gravitino.connector.TableOperations;
+import org.apache.gravitino.rel.Column;
import org.apache.gravitino.rel.SupportsPartitions;
/** Represents a Jdbc Table entity in the jdbc table. */
@@ -113,6 +114,13 @@ public class JdbcTable extends BaseTable {
public Map<String, String> properties() {
return properties;
}
+
+ /**
+ * @return The columns currently set on this builder.
+ */
+ public Column[] columns() {
+ return columns;
+ }
}
/**
diff --git
a/catalogs/catalog-jdbc-mysql/src/main/java/org/apache/gravitino/catalog/mysql/operation/MysqlTableOperations.java
b/catalogs/catalog-jdbc-mysql/src/main/java/org/apache/gravitino/catalog/mysql/operation/MysqlTableOperations.java
index f30ef31f4d..813f44b6de 100644
---
a/catalogs/catalog-jdbc-mysql/src/main/java/org/apache/gravitino/catalog/mysql/operation/MysqlTableOperations.java
+++
b/catalogs/catalog-jdbc-mysql/src/main/java/org/apache/gravitino/catalog/mysql/operation/MysqlTableOperations.java
@@ -36,6 +36,8 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import java.util.stream.Collectors;
import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.collections4.MapUtils;
@@ -55,6 +57,7 @@ import
org.apache.gravitino.rel.expressions.distributions.Distributions;
import org.apache.gravitino.rel.expressions.transforms.Transform;
import org.apache.gravitino.rel.indexes.Index;
import org.apache.gravitino.rel.indexes.Indexes;
+import org.apache.gravitino.rel.types.Type;
import org.apache.gravitino.rel.types.Types;
/** Table operations for MySQL. */
@@ -65,6 +68,14 @@ public class MysqlTableOperations extends
JdbcTableOperations {
private static final String MYSQL_NOT_SUPPORT_NESTED_COLUMN_MSG =
"Mysql does not support nested column names.";
+ // MySQL's JDBC driver reduces these types to a bare name and/or an unusable
COLUMN_SIZE via
+ // DatabaseMetaData.getColumns, so the JdbcTypeConverter maps them to
Types.BinaryType or a
+ // bare Types.ExternalType, losing the length/enumerated values. See
correctLossyTypeColumns.
+ private static final String VARBINARY = "varbinary";
+ private static final String ENUM = "enum";
+ private static final String SET = "set";
+ private static final Pattern LENGTH_PATTERN =
Pattern.compile("\\((\\d+)\\)");
+
@Override
protected String generateCreateTableSql(
String tableName,
@@ -203,6 +214,95 @@ public class MysqlTableOperations extends
JdbcTableOperations {
tableBuilder.withComment(
tableBuilder.properties().getOrDefault(COMMENT,
tableBuilder.comment()));
}
+ correctLossyTypeColumns(connection, databaseName, tableName, tableBuilder);
+ }
+
+ /**
+ * VARBINARY/ENUM/SET fall back to a bare Types.ExternalType, and BIT/BINARY
of any width both
+ * collapse to plain Types.BinaryType, because JdbcTypeConverter only sees
TYPE_NAME/COLUMN_SIZE.
+ * If any column needs it, this queries information_schema.columns once for
the whole table to
+ * recover the full declaration (e.g. "enum('a','b','c')", "binary(16)") and
fixes those columns.
+ */
+ private static void correctLossyTypeColumns(
+ Connection connection, String databaseName, String tableName,
JdbcTable.Builder tableBuilder)
+ throws SQLException {
+ Column[] columns = tableBuilder.columns();
+ if (ArrayUtils.isEmpty(columns)
+ ||
Arrays.stream(columns).noneMatch(MysqlTableOperations::isLossyTypeCandidate)) {
+ return;
+ }
+
+ Map<String, String> fullTypesByColumn =
+ fetchColumnFullTypes(connection, databaseName, tableName);
+ JdbcColumn[] correctedColumns = new JdbcColumn[columns.length];
+ for (int i = 0; i < columns.length; i++) {
+ JdbcColumn column = (JdbcColumn) columns[i];
+ correctedColumns[i] =
+ isLossyTypeCandidate(column)
+ ? correctColumnType(column, fullTypesByColumn.get(column.name()))
+ : column;
+ }
+ tableBuilder.withColumns(correctedColumns);
+ }
+
+ private static boolean isLossyTypeCandidate(Column column) {
+ Type type = column.dataType();
+ if (type instanceof Types.BinaryType) {
+ return true;
+ }
+ if (type instanceof Types.ExternalType) {
+ String catalogString = ((Types.ExternalType) type).catalogString();
+ return VARBINARY.equalsIgnoreCase(catalogString)
+ || ENUM.equalsIgnoreCase(catalogString)
+ || SET.equalsIgnoreCase(catalogString);
+ }
+ return false;
+ }
+
+ private static JdbcColumn correctColumnType(JdbcColumn column, String
fullType) {
+ if (StringUtils.isEmpty(fullType)) {
+ return column;
+ }
+ // A plain BINARY(1) (or unspecified length) already round-trips to
Types.BinaryType as-is.
+ if (column.dataType() instanceof Types.BinaryType && parseLength(fullType)
<= 1) {
+ return column;
+ }
+ return JdbcColumn.builder()
+ .withName(column.name())
+ .withType(Types.ExternalType.of(fullType))
+ .withComment(column.comment())
+ .withNullable(column.nullable())
+ .withAutoIncrement(column.autoIncrement())
+ .withDefaultValue(column.defaultValue())
+ .build();
+ }
+
+ private static int parseLength(String fullType) {
+ Matcher matcher = LENGTH_PATTERN.matcher(fullType);
+ return matcher.find() ? Integer.parseInt(matcher.group(1)) : 1;
+ }
+
+ private static Map<String, String> fetchColumnFullTypes(
+ Connection connection, String databaseName, String tableName) throws
SQLException {
+ Map<String, String> columnTypes = new HashMap<>();
+ // TABLE_SCHEMA/TABLE_NAME comparisons in information_schema use a
case-insensitive collation,
+ // so with lower_case_table_names=0 a schema holding both "a_b" and "A_B"
would return rows for
+ // both; re-check TABLE_NAME exactly, matching the pattern used in
getColumnBuilder.
+ String query =
+ "SELECT TABLE_NAME, COLUMN_NAME, COLUMN_TYPE FROM
information_schema.columns "
+ + "WHERE TABLE_SCHEMA = ? AND TABLE_NAME = ?";
+ try (PreparedStatement statement = connection.prepareStatement(query)) {
+ statement.setString(1, databaseName);
+ statement.setString(2, tableName);
+ try (ResultSet resultSet = statement.executeQuery()) {
+ while (resultSet.next()) {
+ if (Objects.equals(resultSet.getString("TABLE_NAME"), tableName)) {
+ columnTypes.put(resultSet.getString("COLUMN_NAME"),
resultSet.getString("COLUMN_TYPE"));
+ }
+ }
+ }
+ }
+ return columnTypes;
}
@Override
diff --git
a/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/integration/test/CatalogMysqlIT.java
b/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/integration/test/CatalogMysqlIT.java
index 0aba67f646..d496c5f2dd 100644
---
a/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/integration/test/CatalogMysqlIT.java
+++
b/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/integration/test/CatalogMysqlIT.java
@@ -714,6 +714,10 @@ public class CatalogMysqlIT extends BaseIT {
+ " varchar20_col varchar(20),\n"
+ " text_col text,\n"
+ " binary_col binary,\n"
+ + " binary_col_16 binary(16),\n"
+ + " varbinary_col varbinary(100),\n"
+ + " enum_col enum('a','b','c'),\n"
+ + " set_col set('x','y','z'),\n"
+ " blob_col blob,\n"
+ " bit_col_8 bit(8),\n"
+ " bit_col bit\n"
@@ -797,8 +801,20 @@ public class CatalogMysqlIT extends BaseIT {
case "binary_col":
Assertions.assertEquals(Types.BinaryType.get(), column.dataType());
break;
+ case "binary_col_16":
+ Assertions.assertEquals(Types.ExternalType.of("binary(16)"),
column.dataType());
+ break;
+ case "varbinary_col":
+ Assertions.assertEquals(Types.ExternalType.of("varbinary(100)"),
column.dataType());
+ break;
+ case "enum_col":
+ Assertions.assertEquals(Types.ExternalType.of("enum('a','b','c')"),
column.dataType());
+ break;
+ case "set_col":
+ Assertions.assertEquals(Types.ExternalType.of("set('x','y','z')"),
column.dataType());
+ break;
case "bit_col_8":
- Assertions.assertEquals(Types.BinaryType.get(), column.dataType());
+ Assertions.assertEquals(Types.ExternalType.of("bit(8)"),
column.dataType());
break;
case "bit_col":
Assertions.assertEquals(Types.BooleanType.get(), column.dataType());
diff --git
a/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperations.java
b/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperations.java
index eb47047219..f4cd86e9e6 100644
---
a/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperations.java
+++
b/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperations.java
@@ -21,6 +21,8 @@ package org.apache.gravitino.catalog.mysql.operation;
import static
org.apache.gravitino.catalog.mysql.MysqlTablePropertiesMetadata.MYSQL_AUTO_INCREMENT_OFFSET_KEY;
import static
org.apache.gravitino.catalog.mysql.MysqlTablePropertiesMetadata.MYSQL_ENGINE_KEY;
+import java.sql.Connection;
+import java.sql.Statement;
import java.time.LocalDateTime;
import java.util.ArrayList;
import java.util.Arrays;
@@ -29,10 +31,12 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
+import javax.sql.DataSource;
import org.apache.commons.lang3.RandomStringUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.catalog.jdbc.JdbcColumn;
import org.apache.gravitino.catalog.jdbc.JdbcTable;
+import org.apache.gravitino.catalog.jdbc.utils.DataSourceUtils;
import org.apache.gravitino.exceptions.GravitinoRuntimeException;
import org.apache.gravitino.rel.Column;
import org.apache.gravitino.rel.TableChange;
@@ -1139,6 +1143,55 @@ public class TestMysqlTableOperations extends TestMysql {
"Non-datetime type should return 0 precision");
}
+ @Test
+ public void testLoadColumnsWithLossyTypeDeclarations() throws Exception {
+ // These types either drop their parameters (VARBINARY/ENUM/SET) or
collapse distinct widths
+ // into the same Gravitino type (BIT/BINARY) when only JDBC's
TYPE_NAME/COLUMN_SIZE metadata
+ // is used.
+ String tableName = RandomStringUtils.randomAlphabetic(16) +
"_lossy_type_table";
+ DataSource dataSource =
DataSourceUtils.createDataSource(getMySQLCatalogProperties());
+ try {
+ try (Connection connection = dataSource.getConnection()) {
+ connection.setCatalog(TEST_DB_NAME.toString());
+ try (Statement statement = connection.createStatement()) {
+ statement.execute(
+ String.format(
+ "CREATE TABLE `%s` ("
+ + "c_int INT, "
+ + "c_varbinary VARBINARY(100), "
+ + "c_enum ENUM('a','b','c'), "
+ + "c_set SET('x','y','z'), "
+ + "c_bit BIT(8), "
+ + "c_bit1 BIT(1), "
+ + "c_binary1 BINARY(1), "
+ + "c_binary BINARY(16))",
+ tableName));
+ }
+ }
+ } finally {
+ DataSourceUtils.closeDataSource(dataSource);
+ }
+
+ JdbcTable table = TABLE_OPERATIONS.load(TEST_DB_NAME.toString(),
tableName);
+ Map<String, Type> typeByColumn =
+ Arrays.stream(table.columns()).collect(Collectors.toMap(Column::name,
Column::dataType));
+
+ // A plain, non-candidate column must be left untouched by the correction.
+ Assertions.assertEquals(Types.IntegerType.get(),
typeByColumn.get("c_int"));
+ Assertions.assertEquals(
+ Types.ExternalType.of("varbinary(100)"),
typeByColumn.get("c_varbinary"));
+ Assertions.assertEquals(Types.ExternalType.of("enum('a','b','c')"),
typeByColumn.get("c_enum"));
+ Assertions.assertEquals(Types.ExternalType.of("set('x','y','z')"),
typeByColumn.get("c_set"));
+ Assertions.assertEquals(Types.ExternalType.of("bit(8)"),
typeByColumn.get("c_bit"));
+ Assertions.assertEquals(Types.ExternalType.of("binary(16)"),
typeByColumn.get("c_binary"));
+ // BIT(1) still maps to boolean, and a default-width BINARY(1) already
round-trips correctly
+ // as Types.BinaryType, so neither must be affected by this fix.
+ Assertions.assertEquals(Types.BooleanType.get(),
typeByColumn.get("c_bit1"));
+ Assertions.assertEquals(Types.BinaryType.get(),
typeByColumn.get("c_binary1"));
+
+ TABLE_OPERATIONS.drop(TEST_DB_NAME.toString(), tableName);
+ }
+
@Test
public void testCalculateDatetimePrecisionWithUnsupportedDriverVersion() {
MysqlTableOperations operationsWithOldDriver =
diff --git
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/jdbc/mysql/MySQLExternalDataType.java
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/jdbc/mysql/MySQLExternalDataType.java
index 03f4661689..916f50ca7f 100644
---
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/jdbc/mysql/MySQLExternalDataType.java
+++
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/jdbc/mysql/MySQLExternalDataType.java
@@ -49,6 +49,8 @@ public enum MySQLExternalDataType {
SET("set", VarcharType.VARCHAR),
JSON("json", JSON_TYPE),
VARBINARY("varbinary", VarbinaryType.VARBINARY),
+ BINARY("binary", VarbinaryType.VARBINARY),
+ BIT("bit", VarbinaryType.VARBINARY),
TINYBLOB("tinyblob", VarbinaryType.VARBINARY),
BLOB("blob", VarbinaryType.VARBINARY),
MEDIUMBLOB("mediumblob", VarbinaryType.VARBINARY),
@@ -84,11 +86,19 @@ public enum MySQLExternalDataType {
}
public static MySQLExternalDataType safeValueOf(String mysqlTypeName) {
+ // catalogString may carry type parameters, e.g. "varbinary(100)" or
"enum('a','b','c')", so
+ // match on the bare type name.
+ String baseTypeName = stripTypeParameters(mysqlTypeName);
for (MySQLExternalDataType mySQLExternalDataType :
MySQLExternalDataType.values()) {
- if (mySQLExternalDataType.mysqlTypeName.equalsIgnoreCase(mysqlTypeName))
{
+ if (mySQLExternalDataType.mysqlTypeName.equalsIgnoreCase(baseTypeName)) {
return mySQLExternalDataType;
}
}
return UNKNOWN;
}
+
+ private static String stripTypeParameters(String mysqlTypeName) {
+ int parenIndex = mysqlTypeName.indexOf('(');
+ return parenIndex == -1 ? mysqlTypeName.trim() :
mysqlTypeName.substring(0, parenIndex).trim();
+ }
}
diff --git
a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/jdbc/mysql/TestMySQLDataTypeTransformer.java
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/jdbc/mysql/TestMySQLDataTypeTransformer.java
index 3629edd883..8aee5f8e02 100644
---
a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/jdbc/mysql/TestMySQLDataTypeTransformer.java
+++
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/jdbc/mysql/TestMySQLDataTypeTransformer.java
@@ -157,6 +157,28 @@ public class TestMySQLDataTypeTransformer {
Assertions.assertEquals(
generalDataTypeTransformer.getTrinoType(varbinaryType),
VarbinaryType.VARBINARY);
+ // catalogString may carry type parameters, e.g. "varbinary(100)",
"enum('a','b','c')",
+ // "binary(16)" or "bit(8)" -- these must resolve the same as their bare
form.
+ Type varbinaryWithLengthType = Types.ExternalType.of("varbinary(100)");
+ Assertions.assertEquals(
+ generalDataTypeTransformer.getTrinoType(varbinaryWithLengthType),
VarbinaryType.VARBINARY);
+
+ Type enumWithValuesType = Types.ExternalType.of("enum('a','b','c')");
+ Assertions.assertEquals(
+ generalDataTypeTransformer.getTrinoType(enumWithValuesType),
VarcharType.VARCHAR);
+
+ Type setWithValuesType = Types.ExternalType.of("set('x','y','z')");
+ Assertions.assertEquals(
+ generalDataTypeTransformer.getTrinoType(setWithValuesType),
VarcharType.VARCHAR);
+
+ Type binaryWithLengthType = Types.ExternalType.of("binary(16)");
+ Assertions.assertEquals(
+ generalDataTypeTransformer.getTrinoType(binaryWithLengthType),
VarbinaryType.VARBINARY);
+
+ Type bitWithLengthType = Types.ExternalType.of("bit(8)");
+ Assertions.assertEquals(
+ generalDataTypeTransformer.getTrinoType(bitWithLengthType),
VarbinaryType.VARBINARY);
+
Type tinyblobType = Types.ExternalType.of("tinyblob");
Assertions.assertEquals(
generalDataTypeTransformer.getTrinoType(tinyblobType),
VarbinaryType.VARBINARY);
