Hey all,

I've been working on AOT compilation with Graal (Janino not usable) and I'm
struggling with the hierarchy of classes related to RelMetadataQuery. Right
now it feels like there is tight coupling between how the
JaninoRelMetadataProvider works and RelMetdataQuery. In a perfect world, it
seems like the current implementation should be separated into
JaninoRelMetdataQuery and RelMetdataQuery.
- JaninoRelMetdataQuery: The current version of RelMetdataQuery (roughly)
- RelMetdataQuery: An abstract class with minimal implementation and
doesn't extend RelMetadataQueryBase (which is fully coupled
to JaninoRelMetadataProvider)

Existing users who extend RelMetadatQuery today would move to extending
JaninoRelMetdataQuery. Some examples of the current problematic
encapsulation:
- Weird behavior in RelOptCluster: [1]
- Several internal concerns leaked via public fields in
RelMetadataQueryBase [2]
- No way to override "initialHandler" in RelMetadataQueryBase because it is
declared as a static method for [3]

Does anyone have strong opinions about this type of change?

FYI, I know one of the key points of focus on this code was performance but
unfortunately I don't notice any benchmarks in Calcite focused on this code
(did I miss some?).

[1]
https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/plan/RelOptCluster.java#L156
[2]
https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/rel/metadata/RelMetadataQueryBase.java#L69
[3]
https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/rel/metadata/RelMetadataQueryBase.java#L84

Reply via email to