soumyakanti3578 commented on code in PR #6230:
URL: https://github.com/apache/hive/pull/6230#discussion_r2604456878
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainTask.java:
##########
@@ -628,6 +621,42 @@ public int execute() {
}
}
+ /**
+ * Augments the JSON plan with the outputs names of ReduceSink operators.
+ * @param jsonPlan the JSON plan object that is augmented
+ */
+ private void augmentJSONWithRSOutputs(JSONObject jsonPlan) throws Exception {
+ if ("tez".equals(HiveConf.getVar(conf,
HiveConf.ConfVars.HIVE_EXECUTION_ENGINE))) {
+ // The JSON object is augmented via side effect of TezJsonParser.print()
+ new TezJsonParser().print(jsonPlan, null);
+ }
+ }
+
+ /**
+ * Creates the appropriate JsonParser based on the ExplainWork configuration.
+ * @return a JsonParser
+ */
+ private JsonParser createParser() {
+ final JsonParser defaultParser;
+ if (HiveConf.getBoolVar(conf, ConfVars.HIVE_EXPLAIN_FORMATTED_INDENT)) {
+ defaultParser = (json, out) -> out.print(json.toString(2));
+ } else {
+ defaultParser = (json, out) -> out.print(json.toString());
+ }
+ if (work.isCbo() || work.isLogical() || work.isAuthorize() ||
work.getDependency() || work.isLocks()) {
+ return defaultParser;
+ } else if (work.isUserLevelExplain()) {
+ if (HiveConf.getVar(conf,
HiveConf.ConfVars.HIVE_EXECUTION_ENGINE).equals("tez")) {
Review Comment:
nit: This may not be an issue here, but flipping this `equals` may be safer
wrt NPE.
edit: Just saw that you did this in `augmentJSONWithRSOutputs`, so it would
be a good idea to flip it here too.
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainTask.java:
##########
@@ -603,13 +601,8 @@ public int execute() {
} else {
JSONObject jsonPlan = getJSONPlan(out, work);
if (work.isFormatted()) {
- // use the parser to get the output operators of RS
- JsonParser jsonParser = JsonParserFactory.getParser(conf);
- if (jsonParser != null) {
- jsonParser.print(jsonPlan, null);
- LOG.info("JsonPlan is augmented to {}", jsonPlan);
Review Comment:
We are missing this `INFO` in the new code. It's probably a good idea to not
print the `jsonPlan`, as it can be large. But do you think we should have a
`DEBUG` log to replace it?
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainTask.java:
##########
@@ -628,6 +621,42 @@ public int execute() {
}
}
+ /**
+ * Augments the JSON plan with the outputs names of ReduceSink operators.
+ * @param jsonPlan the JSON plan object that is augmented
+ */
+ private void augmentJSONWithRSOutputs(JSONObject jsonPlan) throws Exception {
+ if ("tez".equals(HiveConf.getVar(conf,
HiveConf.ConfVars.HIVE_EXECUTION_ENGINE))) {
+ // The JSON object is augmented via side effect of TezJsonParser.print()
+ new TezJsonParser().print(jsonPlan, null);
+ }
+ }
+
+ /**
+ * Creates the appropriate JsonParser based on the ExplainWork configuration.
+ * @return a JsonParser
+ */
+ private JsonParser createParser() {
+ final JsonParser defaultParser;
+ if (HiveConf.getBoolVar(conf, ConfVars.HIVE_EXPLAIN_FORMATTED_INDENT)) {
+ defaultParser = (json, out) -> out.print(json.toString(2));
Review Comment:
It's not immediately clear that `2` represents the number of indent spaces.
Using a descriptive variable would improve readability.
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainTask.java:
##########
@@ -628,6 +621,42 @@ public int execute() {
}
}
+ /**
+ * Augments the JSON plan with the outputs names of ReduceSink operators.
+ * @param jsonPlan the JSON plan object that is augmented
+ */
+ private void augmentJSONWithRSOutputs(JSONObject jsonPlan) throws Exception {
+ if ("tez".equals(HiveConf.getVar(conf,
HiveConf.ConfVars.HIVE_EXECUTION_ENGINE))) {
+ // The JSON object is augmented via side effect of TezJsonParser.print()
+ new TezJsonParser().print(jsonPlan, null);
+ }
+ }
+
+ /**
+ * Creates the appropriate JsonParser based on the ExplainWork configuration.
+ * @return a JsonParser
+ */
+ private JsonParser createParser() {
+ final JsonParser defaultParser;
Review Comment:
nit: rename to `parser`
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
