Thanks! Can you improve the JIRA subject, so that everyone reading the release notes will understand it? (I had to look up COLLECTION_TABLE.)
> On Jun 30, 2021, at 5:12 AM, stanilovsky evgeny <estanilovs...@gridgain.com> > wrote: > > Guys, i fill the ticket [1] and PR is ready for check, can anyone take a look > on it ? > Additional test appended, all tests locally passed. > > [1] https://issues.apache.org/jira/browse/CALCITE-4673 > > thanks ! > >>> The PR needs to fix a bug or implement a feature. So, first you should log >>> a JIRA case describing what doesn’t work. Write tests for what doesn’t work >>> that you want to make work. (Or maybe you can refactor/generalize existing >>> tests.) Then submit a PR, and we will review that PR on the basis of >>> whether it fixes the bug. >>> >>> That may sound like a lot of process. But the alternative is people just >>> randomly changing code. >> >> No, this sounds like a proper way ) >> Ok, i understand you ! >> >>> >>>> On Jun 3, 2021, at 7:37 AM, stanilovsky evgeny >>>> <estanilovs...@gridgain.com> wrote: >>>> >>>> Julian, thanks for reply and comments. >>>> Can you explain, is it would possible to commit PR containing >>>> deduplication code moved upper this flag ? >>>> If so - i will create PR and rerun all existing tests, of course. >>>> Thanks ! >>>> >>>>> Master is a moving target. Apparently you are using a version of the >>>>> code from sometime in March. These links work better: >>>>> >>>>> [1] >>>>> https://github.com/apache/calcite/blob/796675c9b33e0461bc45a72780162d474a4b098b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java#L2881 >>>>> >>>>> [2] >>>>> https://github.com/apache/calcite/blob/796675c9b33e0461bc45a72780162d474a4b098b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java#L6209 >>>>> >>>>> I don't know the exact cause of what you are seeing, but when you set >>>>> "expand=false" you are choosing a newer (and better) code path. We do >>>>> not have feature parity between the code paths. Nor do we write tests >>>>> for both code paths. >>>>> >>>>> Some day I'd like to officially deprecate, then obsolete, >>>>> "expand=true". It is de facto deprecated because we put more effort >>>>> into the "expand=false" path. >>>>> >>>>> Julian >>>>> >>>>> >>>>> On Fri, May 28, 2021 at 1:19 AM stanilovsky evgeny >>>>> <estanilovs...@gridgain.com> wrote: >>>>>> >>>>>> Hi, calciters ) >>>>>> >>>>>> I am trying to figure out why DeduplicateCorrelateVariables [1] is called >>>>>> only if withExpand [2] flag is true ? Why we don`t need to deduplicate in >>>>>> appropriate case ? >>>>>> >>>>>> [1] >>>>>> https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java#L2881 >>>>>> >>>>>> [2] >>>>>> https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java#L6209 >>>>>> >>>>>> Thanks !
