[
https://issues.apache.org/jira/browse/CALCITE-4544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17434993#comment-17434993
]
Brian Hulette commented on CALCITE-4544:
----------------------------------------
Thanks for the quick response. I'm not that familiar with this code, but it
does look like we already had logic to work around insufficient cache
invalidation ([for
example|https://github.com/apache/beam/blob/f8d3c1bed4a8b1f500fad61001cfa24d2da6afc3/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/planner/RelMdNodeStats.java#L66]).
Could you provide any pointers on how to use the Janino API for custom
metadata? All I've found are the directions
[here|https://calcite.apache.org/javadocAggregate/org/apache/calcite/rel/metadata/RelMetadataQuery.html]
for adding standard metadata to core.
Also, I'm curious why you didn't move away from using
CachingRelMetadataProvider
[here|https://github.com/apache/calcite/blob/2280879f38c8347801749998f2801aaf7364c6f0/core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java#L369)
when it was deprecated? This seems to be the path that results in Beam using
it.
> Deprecate Metadata API backed by Java Reflection
> ------------------------------------------------
>
> Key: CALCITE-4544
> URL: https://issues.apache.org/jira/browse/CALCITE-4544
> Project: Calcite
> Issue Type: Improvement
> Reporter: James Starr
> Assignee: James Starr
> Priority: Major
> Labels: pull-request-available
> Fix For: 1.28.0
>
> Time Spent: 1.5h
> Remaining Estimate: 0h
>
> A large portion of the metadata api is functionally deprecated, we should
> make it explicit.
> Apache Calcite currently has 2 ways deriving metadata about rel node, one
> through runtime generated handler classes that are compiled using Janino, and
> the other using java reflection. The former will be referred to as Janino
> and the latter as java reflection for brevity. Java reflection is not used
> due to performance reason in calcite main, nor is it tested, and using it can
> lead to degraded performance due to interactions with caching of the janino
> handlers. Both share a common way of registering classes with implementation
> by using ReflectiveRelMetadataProvider and ChainedRelMetadataProvider. There
> are also 2 different subsystems for supporting metadata caching, one where a
> reference to a cache is stored in the metadata query and the other where
> caching is a wrapper pattern around the providers. There are also 2 separate
> ways for invalidating the caching, one with a counter stored in the planner
> used by java reflection api, and another where volcano planner tracks the
> parents of all rel nodes that it mutates and then explicitly invalidates
> those parents used by the janino api.
> So the following could be removed without the lose of any functionality:
> * CachingRelMetadataProvider - used for java reflection base metadata caching
> * RelOptPlanner.getRelMetadataTimestamp - used by java reflection to
> invalidate the metadata cache
> * RelOptPlanner.registerMetadataProviders(List<RelMetadataProvider> list) -
> used by java reflection API to support of Hep and Volcano nodes.
> * MetadataFactory - used to caching an internal artifact of the
> implementation of java reflection API
> * HepRelMetadataProvider - used by java reflection to support Hep node
> delegation to a child node.
> * VolcanoRelMetadataProvider - used by java reflection to support RelSubset
> node delegation to a child node.
> * AbstractRelNode.metadata - i think was an even older api that now bridges
> to the java reflection api
> * RelNode.metadata - see above
> * MetadataDef.metadtaClass - used by java reflection and for a unique
> descriptive name of the janino generated handler. Janino dependency on it is
> easily broken.
> * ReflectiveRelMetadataProvider. map and metadataClass0 - used by java
> reflection implementation
> * RelMetadataProvider.apply() - most of the actual implementation java
> reflection API
> Metadata and MetadataDef could also be removed, however removing it would
> require reworking portions of janino code do so.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
