Sure.

Let’s have a test that creates such a sub-class. Then people can use it as an 
example. 

Also, let’s make sure that a sub-sub-class of RelMetadataQuery also works.

Perhaps split RelMetadataQuery into a base class (containing the essential 
mechanism) and a derived class (containing the handlers and methods for each 
kind of built-in metadata). It will be interesting to see which code needs the 
derived class and how which code needs only the base class.

Julian


> On Oct 24, 2019, at 4:56 PM, Haisheng Yuan <h.y...@alibaba-inc.com> wrote:
> 
> Sounds like we have reached a consensus, I guess.
> Can we create a JIRA to allow users to create their own sub-class of 
> RelMetadataQuery?
> 
> - Haisheng
> 
> ------------------------------------------------------------------
> 发件人:Julian Hyde<jh...@apache.org>
> 日 期:2019年10月22日 09:58:34
> 收件人:dev<dev@calcite.apache.org>
> 主 题:Re: [DISCUSSION] Extension of Metadata Query
> 
> Yes, the division of labor between MetadataFactory and RelMetadataQuery is a 
> good one, and we should keep them intact. One provides the raw metadata, and 
> the other provides a typed interface and transactions/caching.
> 
> It might be allowable for a user to create their own sub-class of 
> RelMetadataQuery that adds only handler fields and metadata methods, provided 
> that it follows the existing pattern. We could add a new thread-local in 
> RelMetadataQuery.instance() that is a factory to create the required 
> sub-class of RelMetadataQuery.
> 
> The process by which metadata factories re-generate themselves is delicate 
> and subtle:
> 
> public Double getMaxRowCount(RelNode rel) {
> for (;;) {
> try {
> return maxRowCountHandler.getMaxRowCount(rel, this);
> } catch (JaninoRelMetadataProvider.NoHandler e) {
> maxRowCountHandler =
> revise(e.relClass, BuiltInMetadata.MaxRowCount.DEF);
> }
> }
> }
> 
> (Note that “revise” generates a new class, creating bytecode via janino, and 
> the generated code throws “NoHandler" when it needs to extend itself.) I 
> don’t trust the typical Calcite user to be able to write a sub-class that 
> works correctly and efficiently.
> 
> Julian
> 
> 
> 
> > On Oct 18, 2019, at 3:55 AM, XING JIN <jinxing.co...@gmail.com> wrote:
> > 
> > +1 on Danny's comment.
> > If we use MedataFactory to customize and use RelMetadataQuery for
> > convenience, that will make user confused.
> > 
> > Danny Chan <yuzhao....@gmail.com> 于2019年10月18日周五 下午12:33写道:
> > 
> >> That is the point, we should supply a way to extend the RelMetadataQuery
> >> conveniently for Calcite, because in most of the RelOptRules, user would
> >> use the code like:
> >> 
> >> RelOptRuleCall.getMetadataQuery
> >> 
> >> To get a RMQ instead of using AbstractRelNode.metadata() to fetch a
> >> MedataFactory.
> >> 
> >> We should at lest unity the metadata query entrance/interfaces, or it
> >> would confuse a lot.
> >> 
> >> Best,
> >> Danny Chan
> >> 在 2019年10月18日 +0800 AM12:23,Seliverstov Igor <gvvinbl...@gmail.com>,写道:
> >>> At least in our project (Apache Ignite) we use
> >> AbstractRelNode.metadata().
> >>> 
> >>> But it so because there is no way to put our metadata type into
> >>> RelMetadataQuery without changes in Calcite.
> >>> 
> >>> Regards,
> >>> Igor
> >>> 
> >>> чт, 17 окт. 2019 г., 19:16 Xiening Dai <xndai....@gmail.com>:
> >>> 
> >>>> MetadataFactory is still useful. It provides a way to access Metadata
> >>>> directly. If someone creates a new type of Metadata class, it can be
> >>>> accessed through AbstractRelNode.metadata(). This way you don’t need to
> >>>> update RelMetadataQuery interface to include the getter for this new
> >> meta.
> >>>> Although I don’t see this pattern being used often, but I do think it
> >> is
> >>>> still useful and shouldn’t be removed.
> >>>> 
> >>>> 
> >>>> For your second point, I think you would still need a way to keep
> >>>> RelMetadataQuery object during a rule call. If you choose to create new
> >>>> instance, you will have to pass it around while applying the rule. That
> >>>> actually complicates things a lot.
> >>>> 
> >>>> 
> >>>>> On Oct 17, 2019, at 12:49 AM, XING JIN <jinxing.co...@gmail.com>
> >> wrote:
> >>>>> 
> >>>>> 1. RelMetadataQuery covers the functionality of MetadataFactory, why
> >>>> should
> >>>>> we keep/maintain both of them ? shall we just deprecate
> >> MetadataFactory.
> >>>> I
> >>>>> see MetadataFactory is rarely used in current code. Also I
> >>>>> think MetadataFactory is not good place to offering customized
> >> metadata,
> >>>>> which will make user confused for the difference between
> >> RelMetadataQuery
> >>>>> and MetadataFactory.
> >>>>> 
> >>>>>> Customized RelMetadataQuery with code generated meta handler for
> >>>>> customized metadata, also can provide convenient way to get metadata.
> >>>>> It makes sense for me.
> >>>>> 
> >>>>> 2. If the natural lifespan of a RelMetadataQuery is a RelOptCall,
> >> shall
> >>>> we
> >>>>> deprecate RelOptCluster#getMetadataQuery ? If a user wants to get the
> >>>>> metadata but without a RelOptCall, he/she will need to create a new
> >>>>> instance of RelMetadataQuery.
> >>>>> 
> >>>>> Xiening Dai <xndai....@gmail.com> 于2019年10月17日周四 上午2:27写道:
> >>>>> 
> >>>>>> I have seen both patterns in current code base. In most places, for
> >>>>>> example SubQueryRemoveRule, AggregateUnionTrasposeRule
> >>>>>> SortJoinTransposeRule, etc., RelOptCluster.getMetadataQuery() is
> >> used.
> >>>> And
> >>>>>> there are a few other places where new RelMetadataQuery instance is
> >>>>>> created, which Haisheng attempts to fix.
> >>>>>> 
> >>>>>> Currently RelOptCluster.invalidateMetadataQuery() is called at the
> >> end
> >>>> of
> >>>>>> RelOptRuleCall.transformTo(). So the lifespan of RelMetadataQuery
> >> is
> >>>>>> guaranteed to be within a RelOptCall. I think Haisheng’s fix is
> >> safe.
> >>>>>> 
> >>>>>> 
> >>>>>>> On Oct 16, 2019, at 1:53 AM, Danny Chan <yuzhao....@gmail.com>
> >> wrote:
> >>>>>>> 
> >>>>>>> This is the reason I was struggling for the discussion.
> >>>>>>> 
> >>>>>>> Best,
> >>>>>>> Danny Chan
> >>>>>>> 在 2019年10月16日 +0800 AM11:23,dev@calcite.apache.org,写道:
> >>>>>>>> 
> >>>>>>>> RelMetadataQuery
> >>>>>> 
> >>>>>> 
> >>>> 
> >>>> 
> >> 

Reply via email to