danny0405 commented on a change in pull request #1584: [CALCITE-3503] NPE at
VolcanoPlanner#isValid when DEBUG is enabled
URL: https://github.com/apache/calcite/pull/1584#discussion_r347078737
##########
File path:
core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java
##########
@@ -877,6 +877,10 @@ public RelSubset ensureRegistered(RelNode rel, RelNode
equivRel) {
* Checks internal consistency.
*/
protected boolean isValid(Litmus litmus) {
+ if (this.getRoot() == null) {
+ return true;
Review comment:
Then can you clarify your scenario where the root is null, there is already
code there we can fetch the cluster instance and why in your code just skip the
null root case check ?
There is no tests, no comment and just a one line fix, for which on one can
make sure if this fix is the right why to go. At least you should give evidence
that the fix is right.
I'm still agains to add a top level interface just for DEBUGGING.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
