LuciferYang opened a new issue, #7647:
URL: https://github.com/apache/kyuubi/issues/7647
### Description
`DynConstructors.Ctor.newInstanceChecked` truncates arguments to the
resolved constructor's parameter count unconditionally:
```java
if (args.length > ctor.getParameterCount()) {
return ctor.newInstance(Arrays.copyOfRange(args, 0,
ctor.getParameterCount()));
}
```
`Constructor.newInstance` — like `Method.invoke` — never packs loose varargs
arguments: the only valid way to pass varargs through reflection is the packed
array as the last argument (loose trailing args throw
`IllegalArgumentException: wrong number of arguments`). So for a **varargs**
constructor the truncation can never produce a correct call; it has two failure
shapes:
- if the truncated prefix happens to type-match the formal parameters, the
trailing arguments are **silently dropped** and a wrong instance is
constructed. Example: for `Holder(String first, Object... rest)` (2 params),
`ctor.newInstance("one", new Object[]{"2a","2b"}, "three")` constructs
`Holder("one", ["2a","2b"])` — `"three"` vanishes with no error;
- otherwise the caller gets a misleading `argument type mismatch` (an
artifact of the truncated call) instead of the honest `wrong number of
arguments`.
### Upstream lineage
Although the class header says "Copied from parquet-common", the truncation
logic matches Apache Iceberg's copy, not parquet's: parquet-mr's
`newInstanceChecked` has never truncated (it passes `args` straight through,
and its `TestDynConstructors#testFirstImplReturned` pins
extra-arguments-must-throw), while Iceberg's copy carries the identical
unconditional truncation with no `isVarArgs` guard, no fixing commit in its
history, and no test covering extra or varargs arguments. The fixed-arity
truncation is long-standing in our copy and is load-bearing (cross-version
constructor compatibility in `TPCDSTable`, `HiveSessionManager`,
`HiveTableCatalog` and others), so it must be kept.
### Fix direction
Skip the truncation when `ctor.isVarArgs()`: varargs constructors' arguments
pass through unchanged, so callers get either a working packed-array invocation
or the same honest `IllegalArgumentException: wrong number of arguments` that
parquet-mr specifies upstream. Add unit tests for `DynConstructors` (none exist
today). Possibly worth reporting upstream against Iceberg's
`org.apache.iceberg.common.DynConstructors` separately.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
