[ 
https://issues.apache.org/jira/browse/CALCITE-4514?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17292265#comment-17292265
 ] 

Botong Huang commented on CALCITE-4514:
---------------------------------------

Hi [~vladimirsitnikov]
{quote}Is it really needed to compute the set?
{quote}
Yes I believe so, detecting the parent child relset relationship is an 
optimization to disable useless cycles in the memo. It is added in 
CALCITE-3819, adding [~hyuan] to verify just in case.
{quote}caching the Set<...> looks like to be "always faster", however, it might 
result in an unintentional memory leak.
{quote}
I did a quick test over some heavy queries. With the patch, there is no perf 
downgrade, at the same time the improvement is barely un-noticeable as well. 
This is expected because the patch does not change the behavior for most merge 
cases. Again, we did find noticeable perf difference in the query that hits the 
special case, which is resolved by this patch.

That said, I agree that adding the cache comes at the cost of possibly pinning 
some stale RelSets in memory. I don't have a strong preference between cache vs 
compute from scratch every time. [~julianhyde] What do you think?

> Fine tune the merge order of two RelSets, cache RelSet's childSet computation
> -----------------------------------------------------------------------------
>
>                 Key: CALCITE-4514
>                 URL: https://issues.apache.org/jira/browse/CALCITE-4514
>             Project: Calcite
>          Issue Type: Improvement
>            Reporter: Botong Huang
>            Priority: Minor
>              Labels: pull-request-available
>          Time Spent: 0.5h
>  Remaining Estimate: 0h
>
> When merging two relsets, we have two preferences: 
> 1. Merge parent relset into child relset
> 2. Merge newer relset into older relset
> Currently, when the two relsets are parent set of each other, we randomly 
> pick a merge order without checking the second condition above. For 
> performance reasons, we should, to avoid unnecessary churn. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to