bchapuis commented on code in PR #3668: URL: https://github.com/apache/calcite/pull/3668#discussion_r1487852604
########## core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactoryImpl.java: ########## @@ -39,9 +39,11 @@ import com.google.common.collect.Interners; import org.checkerframework.checker.nullness.qual.Nullable; +import org.locationtech.jts.geom.Geometry; Review Comment: I'm trying to wrap my head around this. From what I understand, calling a method of RelDataTypeFactoryImpl without JTS would trigger a class not found exception. As the use of JTS is currently circumscribed to a few spatial type classes, calcite works just fine without adding a dependency to JTS. Is that correct? Given the popularity of JTS in the geospatial world, I wouldn't try to abstract it completely in JTS. An option could be to use reflection to dynamically load the Geometry class and provide some sort of fallback if the class is not in the class path. ```java public static class GeometryFallback { } static Class<?> loadGeometryClass() { try { return Class.forName("org.locationtech.jts.geom.Geometry"); } catch (ClassNotFoundException e) { return GeometryFallback.class; } } private static final Map<Class, RelDataTypeFamily> CLASS_FAMILIES = ImmutableMap.<Class, RelDataTypeFamily>builder() .put(String.class, SqlTypeFamily.CHARACTER) // ... .put(loadGeometryClass(), SqlTypeFamily.GEO) .build(); ``` What do you think? -- 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: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org