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