[ 
https://issues.apache.org/jira/browse/CALCITE-4544?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

James Starr updated CALCITE-4544:
---------------------------------
    Description: 
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.

  was:
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.  Finally, RelMetadataHandler.getDef() is never used and in some 
instances returns a def that does not actually represent what is metadata is 
handled, see RelMdPercentageOriginalRows.

So the following could be removed without the lose of any functionality:

* RelMetadataHandler.getDef() - not used, 
* 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.


> 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
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 40m
>  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)

Reply via email to