diqiu50 commented on code in PR #12527:
URL: https://github.com/apache/gravitino/pull/12527#discussion_r3821969865
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoDynamicFilter.java:
##########
@@ -55,6 +56,14 @@ public boolean isAwaitable() {
return delegate.isAwaitable();
}
+ // Note: this method is not annotated with @Override because it does not
exist in the
+ // DynamicFilter interface of the baseline open-source Trino SPI version
this connector compiles
+ // against. It is present in newer Trino/Starburst SPI versions, where it is
dispatched at
+ // runtime by signature, providing cross-version compatibility.
+ public OptionalLong getPreferredDynamicFilterTimeout() {
Review Comment:
Fixed: `getPreferredDynamicFilterTimeout()` now reflectively forwards to
`delegate.getPreferredDynamicFilterTimeout()` when present, falling back to
`empty()` only if the delegate doesn't implement it.
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/util/json/JsonCodec.java:
##########
@@ -103,14 +110,107 @@ static TypeManager createTypeManager(ClassLoader
classLoader) {
static BlockEncodingSerde createBlockEncodingSerde(TypeManager typeManager)
throws Exception {
ClassLoader classLoader = typeManager.getClass().getClassLoader();
- Class blockEncodingManagerClass =
+ Class<?> blockEncodingManagerClass =
classLoader.loadClass("io.trino.metadata.BlockEncodingManager");
- Class internalBlockEncodingSerdeClass =
+ Class<?> internalBlockEncodingSerdeClass =
classLoader.loadClass("io.trino.metadata.InternalBlockEncodingSerde");
+ Object blockEncodingManager =
+ instantiateBlockEncodingManager(blockEncodingManagerClass,
classLoader);
return (BlockEncodingSerde)
internalBlockEncodingSerdeClass
.getConstructor(blockEncodingManagerClass, TypeManager.class)
-
.newInstance(blockEncodingManagerClass.getConstructor().newInstance(),
typeManager);
+ .newInstance(blockEncodingManager, typeManager);
+ }
+
+ /**
+ * Instantiate BlockEncodingManager across Trino/Starburst variants. OSS
Trino exposes a public
+ * no-arg constructor; Starburst replaces it with
BlockEncodingManager(FeaturesConfig) (used to
+ * gate type-specific encodings via feature flags). Newer Trino branches
additionally publish a
+ * {@code Set<BlockEncoding>} variant for Guice multibindings. We probe each
known signature in
+ * order, then fall back to a generic constructor scan that fills unknown
reference parameters
+ * with default values as a last-resort compatibility mechanism.
+ */
+ private static Object instantiateBlockEncodingManager(
+ Class<?> blockEncodingManagerClass, ClassLoader classLoader) throws
Exception {
+ try {
+ Object instance =
blockEncodingManagerClass.getConstructor().newInstance();
+ LOG.debug("Instantiated BlockEncodingManager with its public no-argument
constructor");
+ return instance;
+ } catch (NoSuchMethodException ignored) {
+ // fall through to parameterized variants
+ }
Review Comment:
Added TestJsonCodec covering the no-arg / FeaturesConfig / Set /
reflective-fallback constructor selection order, plus the no-usable-constructor
error case.
--
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]
