Copilot commented on code in PR #12244:
URL: https://github.com/apache/gravitino/pull/12244#discussion_r3670489722
##########
flink-connector/flink-common/src/main/java/org/apache/gravitino/flink/connector/catalog/BaseCatalog.java:
##########
@@ -127,6 +129,18 @@ protected BaseCatalog(
protected abstract AbstractCatalog realCatalog();
+ /**
+ * Converts a Flink logical type to a Gravitino type. Subclasses may
override this to special-case
+ * types whose default mapping does not fit a particular catalog (e.g.
Oracle has no pure date
+ * type, so its catalog maps Flink's {@code DATE} to a Gravitino timestamp
instead).
+ *
+ * @param logicalType the Flink logical type
+ * @return the corresponding Gravitino type
+ */
+ protected Type toGravitinoType(LogicalType logicalType) {
+ return TypeUtils.toGravitinoType(logicalType);
+ }
Review Comment:
The new overridable `toGravitinoType(LogicalType)` hook is introduced, but
there is no unit test asserting that an override is actually honored (e.g.,
during `createTable` column conversion, table-change conversion, or view
replace conversion). Please add a targeted test that subclasses `BaseCatalog`,
overrides `toGravitinoType`, and verifies the overridden mapping is used
end-to-end.
--
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]
