jiangxt2 commented on code in PR #12105:
URL: https://github.com/apache/gravitino/pull/12105#discussion_r3662413051
##########
spark-connector/spark-common/src/main/java/org/apache/gravitino/spark/connector/SparkTypeConverter.java:
##########
@@ -192,6 +192,11 @@ public DataType toSparkType(Type gravitinoType) {
return DataTypes.createStructType(fields);
} else if (gravitinoType instanceof Types.NullType) {
return DataTypes.NullType;
+ } else if (gravitinoType instanceof Types.ExternalType) {
+ // ExternalType represents types with no Gravitino-native mapping (e.g.
ClickHouse
+ // IPv4/IPv6, Doris LARGEINT/BITMAP, Glue unknown types). Map to
StringType so that
+ // tables containing these columns remain loadable in Spark. Users can
CAST as needed.
+ return DataTypes.StringType;
Review Comment:
Thanks for the feedback! I agree that blindly converting everything to
STRING has drawbacks — precision loss and efficiency concerns are both valid.
However, I'd like to clarify the scope and rationale of this PR:
**1. This is a last-resort fallback, not a general-purpose conversion
strategy.**
ExternalType only appears when a type has *no* Gravitino-native mapping
whatsoever (IPv4, BITMAP, ENUM, etc.). The alternative today is
`UnsupportedOperationException` during `loadTable()` — which makes the *entire
table* inaccessible in Spark, even for columns that have perfectly valid types.
**2. This follows established precedents in both Trino connector and Spark
itself:**
- Gravitino's own Trino connector: `MySQLExternalDataType` maps unknown
types to `VARCHAR` with an explicit comment: *"For unsupported data types,
standardize mapping to varchar for enhanced usability. Equivalent to default
configuration of the unsupported_type_handling is CONVERT_TO_VARCHAR"*
- Spark's built-in PostgreSQL JDBC dialect: `case Types.OTHER =>
Some(StringType)` — same pattern, same rationale.
- The existing `UUID → StringType` mapping in this same `SparkTypeConverter`
follows the same philosophy.
**3. For types where a better mapping exists, we can improve incrementally:**
Some types like `MEDIUMINT` could map to `IntegerType` (as the Trino
connector already does). But for types like `LARGEINT` (128-bit signed, max 39
digits — exceeding Spark's max decimal precision of 38), there's genuinely no
lossless numeric type in Spark — STRING is the only option that preserves the
full value. Similarly, `BITMAP`/`HLL` are opaque aggregate types with no
meaningful numeric or binary representation in Spark's type system.
**4. The data path is consistent with how Spark reads any VARCHAR column:**
When Spark reads a StringType column, it calls `ResultSet.getString()`,
which the JDBC driver natively supports for all types. The data path is
identical to how Spark reads any VARCHAR column — no custom serialization layer
is introduced.
**Summary:** This PR addresses an availability issue (table completely
inaccessible → table loadable with some columns degraded to STRING). A
configurable approach like `gravitino.spark.unsupported-type-handling =
CONVERT_TO_VARCHAR | IGNORE | FAIL` (similar to Trino's
`unsupported_type_handling`) could be a follow-up improvement to give users
more control.
--
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]
