Feature Request / Improvement
Problem
DynConstructors.Builder accepts a baseClass via DynConstructors.builder(Class<?> baseClass) and is used throughout Iceberg to load pluggable implementations at runtime (e.g. Catalog, FileIO, AuthManager, LockManager).
When an implementation class does not implement or extend the expected base class, the error is deferred until Ctor.newInstance() is called, where it surfaces as a ClassCastException. This makes misconfiguration (wrong class name in catalog properties, typos, etc.) harder to diagnose.
There are existing TODOs in TestDynConstructors documenting this gap:
common/src/test/java/org/apache/iceberg/common/TestDynConstructors.java:47 — string-based .impl(...) with an unrelated class
common/src/test/java/org/apache/iceberg/common/TestDynConstructors.java:60 — class-based .impl(...) with an unrelated class
Current behavior (both tests expect ClassCastException at newInstance() time):
DynConstructors.Ctor<MyInterface> ctor =
DynConstructors.builder(MyInterface.class)
.impl(MyUnrelatedClass.class) // does not implement MyInterface
.buildChecked();
ctor.newInstance(); // ClassCastException here
Proposed behavior
When baseClass is set on the builder, validate that the resolved implementation class is assignable to baseClass at build time (build() / buildChecked()), and fail fast with a clear, actionable error.
For example:
assertThatThrownBy(() ->
DynConstructors.builder(MyInterface.class)
.impl(MyUnrelatedClass.class)
.buildChecked())
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("does not implement");
The same validation should apply to both .impl(String, ...) and .impl(Class, ...) (and likely .hiddenImpl(...) variants as well).
Scope
- Module:
common (DynConstructors.java)
- Tests: Update
TestDynConstructors.testInterfaceWrongImplString and TestDynConstructors.testInterfaceWrongImplClass to assert build-time failure instead of ClassCastException at instantiation
- Impact: Improves error messages for all runtime plugin loading paths that use
DynConstructors.builder(SomeInterface.class) (Catalog, FileIO, AuthManager, etc.)
Notes
DynConstructors is copied from parquet-common; consider whether a similar upstream fix is warranted, but Iceberg can fix locally regardless.
- When
builder() is called without a baseClass, no assignability check is needed (existing behavior for DynConstructors.builder().impl(MyClass.class) should remain unchanged).
Query engine
No response
Willingness to contribute
Feature Request / Improvement
Problem
DynConstructors.Builderaccepts abaseClassviaDynConstructors.builder(Class<?> baseClass)and is used throughout Iceberg to load pluggable implementations at runtime (e.g.Catalog,FileIO,AuthManager,LockManager).When an implementation class does not implement or extend the expected base class, the error is deferred until
Ctor.newInstance()is called, where it surfaces as aClassCastException. This makes misconfiguration (wrong class name in catalog properties, typos, etc.) harder to diagnose.There are existing TODOs in
TestDynConstructorsdocumenting this gap:common/src/test/java/org/apache/iceberg/common/TestDynConstructors.java:47— string-based.impl(...)with an unrelated classcommon/src/test/java/org/apache/iceberg/common/TestDynConstructors.java:60— class-based.impl(...)with an unrelated classCurrent behavior (both tests expect
ClassCastExceptionatnewInstance()time):Proposed behavior
When
baseClassis set on the builder, validate that the resolved implementation class is assignable tobaseClassat build time (build()/buildChecked()), and fail fast with a clear, actionable error.For example:
The same validation should apply to both
.impl(String, ...)and.impl(Class, ...)(and likely.hiddenImpl(...)variants as well).Scope
common(DynConstructors.java)TestDynConstructors.testInterfaceWrongImplStringandTestDynConstructors.testInterfaceWrongImplClassto assert build-time failure instead ofClassCastExceptionat instantiationDynConstructors.builder(SomeInterface.class)(Catalog, FileIO, AuthManager, etc.)Notes
DynConstructorsis copied from parquet-common; consider whether a similar upstream fix is warranted, but Iceberg can fix locally regardless.builder()is called without abaseClass, no assignability check is needed (existing behavior forDynConstructors.builder().impl(MyClass.class)should remain unchanged).Query engine
No response
Willingness to contribute