fix(isthmus): route native-image metadata queries through reflective provider - #1046
Open
nielspardon wants to merge 1 commit into
Open
fix(isthmus): route native-image metadata queries through reflective provider#1046nielspardon wants to merge 1 commit into
nielspardon wants to merge 1 commit into
Conversation
…provider Grouping-only `GROUP BY` and `SELECT DISTINCT` queries crashed in the isthmus native image. `RelBuilder.aggregate_` runs a column-uniqueness metadata query whose default `RelMetadataQuery` obtains handlers from `JaninoRelMetadataProvider`, which compiles a handler class at runtime with Janino. Runtime code generation is unavailable in a GraalVM native image (closed-world, ahead-of-time compiled), so the query failed with a Janino type-resolution error. An aggregate *with* a function skips the uniqueness check, so the existing smoke test (which only used `count(...)`) never exercised the broken path. `SqlConverterBase` already set a codegen-free `ProxyingMetadataHandlerProvider` on its cluster, but only the SQL-expression path used that cluster. The SQL-query path (`SubstraitSqlToCalcite`) and the Substrait->Calcite path (`ConverterProvider.getRelBuilder`, `SubstraitRelNodeConverter.convert`) built clusters with no metadata supplier, so they fell back to Janino. - Extract the proxying-provider setup into `Utils.useReflectiveMetadataProvider` and apply it to every cluster/`RelBuilder` used for Calcite conversion. - The reflective provider proxies the bare `BuiltInMetadata$*` interfaces, not just the `$Handler` sub-interfaces already registered, so register those too in `reachability-metadata.json`. - Add grouping-only `GROUP BY` and `SELECT DISTINCT` cases to `smoke.sh` so the native image exercises the uniqueness path in CI. Verified by building the native image locally (GraalVM 25) and running `smoke.sh` / `tpch_smoke.sh`. Closes substrait-io#251
nielspardon
marked this pull request as ready for review
July 29, 2026 08:35
nielspardon
requested review from
andrew-coleman,
benbellick,
bestbeforetoday and
vbarua
July 29, 2026 08:37
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Queries like the following crash in the isthmus native image (#251):
A grouping-only
GROUP BY(andSELECT DISTINCT) goes throughRelBuilder.aggregate_→alreadyUnique()→RelMetadataQuery.areColumnsUnique().Calcite's default
RelMetadataQueryobtains its handlers fromJaninoRelMetadataProvider, which compiles a handler class at runtime with Janino.Runtime code generation is unavailable in a GraalVM native image (closed-world,
ahead-of-time compiled), so the query fails:
An aggregate with a function (
count(...)) skips the uniqueness check, so theexisting smoke test never exercised the broken path — which is why this stayed
uncaught.
Fix
Calcite ships
ProxyingMetadataHandlerProvider, a reflection-based (dynamic-proxy)alternative to the Janino code generator. Dynamic proxies are supported in native
images once their interfaces are registered.
SqlConverterBasealready used thisprovider — but only the SQL-expression path used that cluster. The SQL-query path
(
SubstraitSqlToCalcite) and the Substrait→Calcite path(
ConverterProvider.getRelBuilder,SubstraitRelNodeConverter.convert) builtclusters with no metadata supplier and fell back to Janino.
Utils.useReflectiveMetadataProvider(RelOptCluster)andapply it to every cluster /
RelBuilderused for Calcite conversion (removing theduplicated inline lambda in
SqlConverterBase).BuiltInMetadata$*interfaces, notjust the
$Handlersub-interfaces already present, so register those inreachability-metadata.json(the safe superset — unused proxy registrations areharmless).
GROUP BYandSELECT DISTINCTcases tosmoke.shso thenative image exercises the uniqueness path in CI.
Closes #251
🤖 Generated with AI