[
https://issues.apache.org/jira/browse/CALCITE-4879?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17452168#comment-17452168
]
Jacques Nadeau commented on CALCITE-4879:
-----------------------------------------
> If people had been allowed to sub-class RelMetadataQuery they would have
People can and do sub-class RelMetadataQuery in its entirety. Nothing in the
code/construction of RelMetadataQuery or how RelOptCluster exposes a query
supplier restricts that. I've actually done it when implementing a new AOT
friendly RelMetadataQuery external to Calcite.
> RelMetadataQuery is a facade. It doesn't contain state or behavior.
I would disagree about it not containing state and behavior. It contains both.
It holds state around current versions of each handler as well as thread local
caches of janino compiled code and a cache of past metadata results. It also
contains behavior including: null canonicalization, cache management and
concepts around handler creation and revision. The latter is something that is
very specific concept to a runtime compilation pattern that doesn't match well
to an AOT pattern.
> and we would have a mess today
I think things are a bit of a mess now. This isn't a dig or a negative
statement (and I'm not trying to start a flame war here). It's an observation
after spending several weeks working with the existing code and building a
complete compatible system (passes all the same tests as the existing one) that
performs well and functions well in AOT environments. From my pov, there are
many good things that exist:
* it is relatively straightforward to add new metadata types (albeit verbose),
* performance is good if you're in an environment that can support runtime
compilation
* Virtually all of planning-side consumption is limited to the a constrained
portion of RelMetadataQuery that should be public.
But there are also multiple real challenges:
* Excessive runtime code that is not runtime specific/runtime compiled required
* Bindings between components are brittle and implicit (javac/ide won't help
identify many issues)
* Several internal concepts are exposed as public on RelMetadataQuery (see the
map, metadataProvider and THREAD_PROVIDER fields)
* There is a large amount of deprecated code supporting old patterns heavily
intertwined with the existing system
* Major caching bugs (according to comments by others, I haven't validated)
* Poor documentation
* Debugging and understanding the code is quite difficult
I agree that lack of freedom can be a feature. I don't think I'm altering the
current state of freedom, just cleaning up and formalizing the freedom that
already exists.
> Make RelMetadataQuery abstract
> ------------------------------
>
> Key: CALCITE-4879
> URL: https://issues.apache.org/jira/browse/CALCITE-4879
> Project: Calcite
> Issue Type: Improvement
> Components: core
> Reporter: Jacques Nadeau
> Priority: Major
>
> The RelOptCluster.setMetadataQuerySupplier() and RelMedataQuery abstraction
> are great at separating how planners and rules consume metadata versus how
> metadata is produced. While details about how metadata is produced is
> (mostly) not leaked in the api of RelMetadataQuery, the class does assume
> that metadata will be produced via the current mechanisms surrounding
> RelMetadataProviders and MetadataHandlers. This ticket targets separating the
> production of metadata from the consumption interface, by making
> RelMetadataQuery abstract (either as an abstract class or as a interface) and
> moving the handler and provider specific implementations to an implementation
> of RelMetadataQuery. This will allow a broader breadth of experimentation to
> be undertaken. For example, one example people have been evaluating is
> whether a lambda based system would be easier to understand and debug, as
> performant and more AOT friendly than the existing systems of chains and
> janino compilation.
> To accomplish this task, the first step will be to deprecate the existing
> constructors and inform people to use a concrete subtype. Once deprecated,
> the actual logic that currently exists in RelMetadataQuery can be extracted
> into the concrete subtype and the base class can be made either abstract or
> an interface (depending on what seems most appropriate at the time).
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
