Shiva Jahangiri has posted comments on this change.

Change subject: [UI] Allow logical plan to be viewed as JSON / formatted JSON
......................................................................


Patch Set 24:

(8 comments)

https://asterix-gerrit.ics.uci.edu/#/c/1885/22/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:

Line 115:     private static final String lplan = "Logical plan";
> Right, that's all fine, but I don't see how it follows from what the sonarq
Oh I see. Right. Changed it.


Line 116:     private static final String oplan = "Optimized logical plan";
> I cannot change it cause this is what we write in <h4>
Done


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:

Line 115:     private static final String lplan = "Logical plan";
> MAJOR SonarQube violation:
Done


Line 116:     private static final String oplan = "Optimized logical plan";
> MAJOR SonarQube violation:
Done


PS24, Line 235: printPlanJson
> It seems that PlanPrettyPrinter should only need 1 method printPlan(...) th
Done


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.
Done


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 
It has to be static to be able to use static method reference in 
PlanPrettyPrinter.


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 prin
It was my first time using static method reference, but as far as I checked 
java was complaining that it needs the interface and I made it. Not sure if 
there is a better way for it. I made apply to throw exception. Thank you :)


-- 
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: [email protected]
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Ian Maxon <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Shiva Jahangiri <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-HasComments: Yes

Reply via email to