Can someone review this patch for me https://github.com/apache/calcite/pull/1533 ? Thanks so much for that ~
Best, Danny Chan 在 2019年10月25日 +0800 AM9:02,Julian Hyde <jh...@apache.org>,写道: > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >