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