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 > >>>>>> > >>>>>> > >>>> > >>>> > >>