Thanks, yes, that’s better.

> On Jun 30, 2021, at 10:34 PM, stanilovsky evgeny <estanilovs...@gridgain.com> 
> wrote:
> 
> thanks! done, i hope )
> 
>> 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 !

Reply via email to