LuciferYang opened a new pull request, #7706:
URL: https://github.com/apache/kyuubi/pull/7706
### Why are the changes needed?
Closes #7705.
`DynMethods.Builder` probes a chain of candidate classes and signatures,
swallowing each lookup failure so the next candidate gets a turn. When none
matches, `build()` and `buildChecked()` threw with the method name alone.
`HiveConnectorUtils.storageFormatApply` probes the seven-argument
`CatalogStorageFormat.apply` that SPARK-55645 added in Spark 4.2 and then the
six-argument one, so on a Spark whose signature matches neither, the entire
diagnosis was `Cannot find method: apply`. It now reads:
```
Cannot find method: apply
Missing
org.apache.spark.sql.catalyst.catalog.CatalogStorageFormat#apply(scala.Option,scala.Option,scala.Option,scala.Option,boolean,scala.collection.immutable.Map,scala.Option)
[java.lang.NoSuchMethodException:
org.apache.spark.sql.catalyst.catalog.CatalogStorageFormat.apply(scala.Option,scala.Option,scala.Option,scala.Option,boolean,scala.collection.immutable.Map,scala.Option)]
Missing
org.apache.spark.sql.catalyst.catalog.CatalogStorageFormat#apply(scala.Option,scala.Option,scala.Option,scala.Option,boolean,scala.collection.immutable.Map)
[java.lang.NoSuchMethodException:
org.apache.spark.sql.catalyst.catalog.CatalogStorageFormat.apply(scala.Option,scala.Option,scala.Option,scala.Option,boolean,scala.collection.immutable.Map)]
```
`RowSet.toHiveString` and `SparkUtilsHelper.getLocalDir` are the same shape.
`DynConstructors.Builder` in this package already reports its candidates this
way; this brings `DynMethods` in line, message and suppressed throwables both.
`DynConstructors` keys those entries by candidate name. A list is used here
instead, one entry per lookup, because two lookups can render the same name and
still miss for unrelated reasons: with a loader that cannot see a class,
`impl(name, ...)` records a `ClassNotFoundException` and a following
`impl(SomeClass.class, ...)` records a `NoSuchMethodException` under the same
name, and keeping either alone names the wrong reason for the failure. A
declared-member candidate is marked as such, so the `hiddenImpl` then `impl`
pair that `ReflectUtils.invokeAs` writes reads as the two different questions
it asks: `getDeclaredMethod` searches one class at every visibility,
`getMethod` searches the whole hierarchy at public only, and neither miss
implies the other.
Naming a candidate has to tolerate a null argument class. `SparkUtilsHelper`
passes
`DynClasses.builder().impl("org.apache.spark.ReadOnlySparkConf").orNull().build()`
straight into `impl` as its first candidate, and that is null below Spark 4.1.
Reflection accepts a null element and renders it as `null` in its own message,
so it counts as an ordinary miss, and building a key from it must not turn that
into a throw. Without the guard the fallback chain breaks on the default
`-Pspark-3.5` profile and `SparkSQLEngine.setupConf()` never gets a local dir.
`DynConstructors.methodName` had the identical gap, reached through `ctorImpl`
and through its own builder, so it gets the same guard; on Flink 2.x that one
is currently saved only by an earlier candidate matching first.
`ctorImpl` has no caller in this repository outside its tests.
`ctorImpl(Class, ...)` hands its target class to the constructor builder it
delegates to, so the aggregate that builder produces names the class. The
`String` overload cannot do the same, having no loaded class to hand over, so
its delegated text still reads `Cannot find constructor for null`; that wording
belongs to `DynConstructors` and is left alone here. Because a delegated
aggregate carries newlines of its own, it is indented one level further,
keeping its entries from reading as candidates of the outer builder.
Two spelling corrections come along: the class javadoc claimed the file was
copied from parquet-common, but it matches iceberg-common and carries iceberg's
own note about parquet, so it now says what `DynConstructors` says; and two
javadoc mentions of `RuntimeError`, which is not a Java type, now read
`RuntimeException`.
This sits on top of #7689 and only adds to the `ctorImpl(String, ...)` body
that PR introduced. One note for whoever merges second: #7687 adds a `catch
(RuntimeException e)` to `hiddenImpl` for strongly encapsulated members, and
that clause will need a recording call of its own, or the `module ... does not
opens ...` text it exists to surface stays discarded.
Two things are left for their own changes. `NoClassDefFoundError` still
escapes `impl(String, ...)` and `hiddenImpl(String, ...)` instead of counting
as a miss, which is what parquet fixed in GH-3644 for 1.18.0. And `DynFields`
discards its causes entirely, which `ReflectUtils.getField` reaches through the
same shape `invokeAs` uses.
### How was this patch tested?
19 tests in `DynMethodsTest` and 1 in `DynConstructorsTest`, covering each
of the six recording sites, the message layout and entry order, the pairing
between an entry and the throwable it names, the suppressed order, a declared
and a public lookup of one signature, one candidate name covering two unrelated
reasons, a null argument class and a null argument array on both the method and
the constructor paths, a `ClassNotFoundException` with no detail message of its
own, a nested cause whose message spans lines, and `orNoop` succeeding with
misses already recorded.
Each was checked by mutation: the production source was mutated, recompiled
and the suite rerun, then restored from a copy. Every property the tests claim
has at least one mutant that only one test catches, down to substituting a
fabricated throwable for the one the lookup actually raised.
```
build/mvn test -pl kyuubi-util -am -DwildcardSuites=none
-Dtest='DynMethodsTest,DynConstructorsTest'
build/mvn spotless:check -pl kyuubi-util
```
### Was this patch authored or co-authored using generative AI tooling?
Assisted-by: Claude Opus 5
--
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]
