[
https://issues.apache.org/jira/browse/CALCITE-3503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16977955#comment-16977955
]
Xiening Dai commented on CALCITE-3503:
--------------------------------------
{quote}I don't think it is meaningless to do `isValid()` check if root is NULL
which is the only chance that the root node can be checked.
{quote}
This is incorrect. Root is checked every time in the subsequent calls.
{quote}but the check not only check the cost right ? It also check the cyclic
reference of memos.
{quote}
No, it doesn't. The cyclic exceptions are caught and ignored.
{quote}And we always do the `isValid()` check for "logical nodes".
{quote}
True. And the previous fix doesn't change this. It only skip checks when
setRoot() is not returned.
Like [~zabetak] mentioned, to avoid spend more time on this one, I send out a
PR ([https://github.com/apache/calcite/pull/1597/)] which makes the behavior
exactly same as it was before, although personally I don't feel it's worthy.
Please take a look, and I will let you decide which one is better.
> NPE at VolcanoPlanner#isValid when DEBUG is enabled
> ---------------------------------------------------
>
> Key: CALCITE-3503
> URL: https://issues.apache.org/jira/browse/CALCITE-3503
> Project: Calcite
> Issue Type: Bug
> Components: core
> Affects Versions: 1.21.0
> Reporter: Stamatis Zampetakis
> Priority: Blocker
> Labels: pull-request-available
> Fix For: 1.22.0
>
> Time Spent: 3h
> Remaining Estimate: 0h
>
> The stacktrace is the following:
> {noformat}
> java.lang.NullPointerException
> at
> org.apache.calcite.plan.volcano.VolcanoPlanner.isValid(VolcanoPlanner.java:880)
> at
> org.apache.calcite.plan.volcano.VolcanoPlanner.ensureRegistered(VolcanoPlanner.java:870)
> at
> org.apache.calcite.plan.volcano.VolcanoPlanner.ensureRegistered(VolcanoPlanner.java:92)
> at
> org.apache.calcite.rel.AbstractRelNode.onRegister(AbstractRelNode.java:321)
> at
> org.apache.calcite.plan.volcano.VolcanoPlanner.registerImpl(VolcanoPlanner.java:1701)
> at
> org.apache.calcite.plan.volcano.VolcanoPlanner.setRoot(VolcanoPlanner.java:298)
> at
> org.apache.calcite.tools.Programs.lambda$standard$3(Programs.java:269)
> {noformat}
> According to [~volodymyr] this is a regression caused by CALCITE-3487.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
