Till Westmann has posted comments on this change. Change subject: [UI] Allow logical plan to be viewed as JSON / formatted JSON ......................................................................
Patch Set 24: (4 comments) https://asterix-gerrit.ics.uci.edu/#/c/1885/24/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/common/APIFramework.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/common/APIFramework.java: PS24, Line 235: printPlanJson It seems that PlanPrettyPrinter should only need 1 method printPlan(...) that leaves all the decisions concerning the plan format to the visitor. That way there would be no format-specific methods on the PlanPrettyPrinter. https://asterix-gerrit.ics.uci.edu/#/c/1885/24/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/AbstractLogicalOperator.java File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/AbstractLogicalOperator.java: PS24, Line 68: String id = ""; I think that this should not be needed anymore. https://asterix-gerrit.ics.uci.edu/#/c/1885/24/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/prettyprint/LogicalOperatorPrettyPrintVisitor.java File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/prettyprint/LogicalOperatorPrettyPrintVisitor.java: PS24, Line 91: printPlan If we make printOperator and printPlan non-static methods, I think that we can pull printPlan up to AbstractLogicalOperatorPrettyPrintVisitor. https://asterix-gerrit.ics.uci.edu/#/c/1885/24/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/prettyprint/PlanPrettyPrinter.java File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/prettyprint/PlanPrettyPrinter.java: PS24, Line 27: interface I'm not sure if we still need/use this interface, if printOperator and printPlan are methods on the visitors. But if we still need it, apply(...) should probably throw an AlgebricksException to avoid the swallowing of exceptions that we have in these static methods. That would also make SonarQube much happier :) -- To view, visit https://asterix-gerrit.ics.uci.edu/1885 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4dd62e355048a5b8a02e074049fe41e73e74e357 Gerrit-PatchSet: 24 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: shi...@uci.edu Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Ian Maxon <ima...@apache.org> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Shiva Jahangiri <shi...@uci.edu> Gerrit-Reviewer: Till Westmann <ti...@apache.org> Gerrit-HasComments: Yes
