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 !
