[
https://issues.apache.org/jira/browse/CALCITE-3870?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17066415#comment-17066415
]
Jiatao Tao edited comment on CALCITE-3870 at 3/25/20, 6:43 AM:
---------------------------------------------------------------
HI [~julianhyde]
Thanks for your comment.
Seems there are many tests failed because we contract SqlToRelConverter.Config
with the default "expand" config(true), like in PlannerImpl#rel.
!image-2020-03-25-14-05-04-011.png|width=543,height=205!
And here comes two solutions to me:
* add `withExpand(true)` in every failed UT, as a workaround, mainly for
passing UT.
** advantage: less workload
** disadvantage:
*** still have lots of `withExpand(true)` in our code
*** will impact on existing Calcite users if they didn't claim "expand" to
false when they use SqlToRelConverter, it is a breaking change.
* add the expand process(SubQueryRemoveRule, may be more rules) to the process
of org.apache.calcite.sql2rel.RelDecorrelator#decorrelateQuery.
** advantage: if the existing users do `decorrelateQuery` after
SqlToRelConverter(I think most will do that), This change has little effect on
them.
** disadvantage: more workload compare solutions1.
This change's impact seems much bigger than I first thought(The impact to the
existing user who without setting expand explicitly), hope to hear more voice
about this.
Thanks again
was (Author: aron.tao):
[~julianhyde]
Ok, And seems there are many tests failed because we contract
SqlToRelConverter.Config with the default "expand" config(true), like in
PlannerImpl#rel.
!image-2020-03-25-14-05-04-011.png|width=543,height=205!
And here comes two solutions to me:
* add `withExpand(true)` in every failed UT, as a workaround, mainly for
passing UT.
** advantage: less workload
** disadvantage:
*** still have lots of `withExpand(true)` in our code
*** will impact on existing Calcite users if they didn't claim "expand" to
false when they use SqlToRelConverter, it is a breaking change.
* add the expand process(SubQueryRemoveRule, may be more rules) to the process
of org.apache.calcite.sql2rel.RelDecorrelator#decorrelateQuery.
** advantage: if the existing users do `decorrelateQuery` after
SqlToRelConverter(I think most will do that), This change has little effect on
them.
** disadvantage: more workload compare solutions1.
This change's impact seems much bigger than I first thought(The impact to the
existing user who without setting expand explicitly), hope to hear more voice
about this.
Thanks again
> set "SqlToRelConverter.ConfigBuilder#expand" default to false
> -------------------------------------------------------------
>
> Key: CALCITE-3870
> URL: https://issues.apache.org/jira/browse/CALCITE-3870
> Project: Calcite
> Issue Type: Bug
> Components: core
> Reporter: Jiatao Tao
> Priority: Minor
> Labels: pull-request-available
> Attachments: image-2020-03-25-14-05-04-011.png
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Now the default "expand" in SqlToRelConverter.ConfigBuilder is true but in
> calcite's main process, actually, it is false(
> `withExpand(THREAD_EXPAND.get())`)
>
> That leads we need to explicitly set `withExpand` to false when we use
> SqlToRelConverter.
>
> So I think we should change the default to "false" in
> SqlToRelConverter.ConfigBuilder.
>
>
> !https://mail.google.com/mail/u/0?ui=2&ik=09d5de6bf3&attid=0.1&permmsgid=msg-a:r-1571046416348666034&th=17106774a5349d1d&view=fimg&sz=s0-l75-ft&attbid=ANGjdJ_IjYbiwT8IKlaiAIDzbkq_ehCBktd4DzCYr14Ppst6lBftqMbrrWu8hzGpN2wpCz46a3F1FKvfD_miGHkC-EMx509KPC4FEovxtN2M0WcU1up209hyLtKoZqE&disp=emb&realattid=ii_k8460q950!
>
> !https://mail.google.com/mail/u/0?ui=2&ik=09d5de6bf3&attid=0.2&permmsgid=msg-a:r-1571046416348666034&th=17106774a5349d1d&view=fimg&sz=s0-l75-ft&attbid=ANGjdJ_K-EyE_x0Vo66h6sHRzpEOxIE5qTzuujqG8WqZVPdprgC3khwcO_8OLLai-8ikrMRC9ksO-xAzqe2HFr-QYM1lzQDLYfSlD5x-eOXbww9CNsb5cgu-979N1lU&disp=emb&realattid=ii_k846zj601!
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
