jacques-n commented on a change in pull request #2378:
URL: https://github.com/apache/calcite/pull/2378#discussion_r742424624
##########
File path:
core/src/main/java/org/apache/calcite/rel/metadata/RelMetadataQuery.java
##########
@@ -886,10 +919,59 @@ public Boolean isVisibleInExplain(RelNode rel,
for (;;) {
try {
return lowerBoundCostHandler.getLowerBoundCost(rel, this, planner);
- } catch (JaninoRelMetadataProvider.NoHandler e) {
+ } catch (MetadataHandlerProvider.NoHandler e) {
lowerBoundCostHandler =
- revise(BuiltInMetadata.LowerBoundCost.Handler.class);
+
metadataHandlerProvider.revise(BuiltInMetadata.LowerBoundCost.Handler.class);
}
}
}
+
+ public static Builder<RelMetadataQuery> builder() {
Review comment:
If we want to go with a builder, I think we should make a couple
changes. (Not entirely sure a builder is yet necessary.)
- This should probably be a method called `supplierBuilder()` or similar
since it doesn't actually build the type of class it is contained within. I'd
expect `RelMetdataQuery.builder()` to build a `RelMetadataQuery`, not
`Supplier<RelMetadataQuery>`.
- I think that we may want to add other ways of building in the future. For
example I'm exploring the introduction of a lambda based metadata handling
systems. In that situation, we may want to construct one of those using the
same initial builder. As such I suggest a specialization strategy where calling
withProviders returns a subtype of builder that has configuration options
related to a provider based builder. Maybe in the future we have a separate one
that works based on lambdas and we call withLambdas() to use it (and a call to
that would specialize the builder to that type of metadataquery supplier.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
