You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by stanilovsky evgeny <es...@gridgain.com> on 2021/05/28 08:19:27 UTC
Deduplicate correlate variables question.
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 !
Re: Deduplicate correlate variables question.
Posted by Julian Hyde <jh...@gmail.com>.
Thanks, yes, that’s better.
> On Jun 30, 2021, at 10:34 PM, stanilovsky evgeny <es...@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 <es...@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 <es...@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
>>>>>>> <es...@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 !
Re: Deduplicate correlate variables question.
Posted by stanilovsky evgeny <es...@gridgain.com>.
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
>> <es...@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
>>>>> <es...@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
>>>>>> <es...@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 !
Re: Deduplicate correlate variables question.
Posted by Julian Hyde <jh...@gmail.com>.
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 <es...@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 <es...@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
>>>>> <es...@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 !
Re: Deduplicate correlate variables question.
Posted by stanilovsky evgeny <es...@gridgain.com>.
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
>>> <es...@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
>>>> <es...@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 !
Re: Deduplicate correlate variables question.
Posted by stanilovsky evgeny <es...@gridgain.com>.
> 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
>> <es...@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
>>> <es...@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 !
Re: Deduplicate correlate variables question.
Posted by Julian Hyde <jh...@gmail.com>.
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.
> On Jun 3, 2021, at 7:37 AM, stanilovsky evgeny <es...@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
>> <es...@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 !
Re: Deduplicate correlate variables question.
Posted by stanilovsky evgeny <es...@gridgain.com>.
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
> <es...@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 !
Re: Deduplicate correlate variables question.
Posted by Julian Hyde <jh...@gmail.com>.
Sure. Contributions welcome.
> On Jun 2, 2021, at 1:23 PM, James Starr <ja...@gmail.com> wrote:
>
> If it is de facto deprecated, then could it not just be deprecated with a
> comment stating why it has not been deleted? Calcite seems to have several
> API that are defacto deprecated and there is no way of knowing by reading
> the code. This would personally have made me much simpler for myself to
> get ramped on the calcite code base.
>
> James
>
> On Wed, Jun 2, 2021 at 12:07 PM Julian Hyde <jh...@apache.org> wrote:
>
>> 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
>> <es...@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 !
>>
Re: Deduplicate correlate variables question.
Posted by James Starr <ja...@gmail.com>.
If it is de facto deprecated, then could it not just be deprecated with a
comment stating why it has not been deleted? Calcite seems to have several
API that are defacto deprecated and there is no way of knowing by reading
the code. This would personally have made me much simpler for myself to
get ramped on the calcite code base.
James
On Wed, Jun 2, 2021 at 12:07 PM Julian Hyde <jh...@apache.org> wrote:
> 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
> <es...@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 !
>
Re: Deduplicate correlate variables question.
Posted by Julian Hyde <jh...@apache.org>.
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
<es...@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 !