LakshSingla commented on a change in pull request #11908:
URL: https://github.com/apache/druid/pull/11908#discussion_r756037412



##########
File path: 
sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java
##########
@@ -366,24 +374,76 @@ public void cleanup(EnumeratorIterator<Object[]> 
iterFromMake)
    */
   private PlannerResult planExplanation(
       final RelNode rel,
-      final SqlExplain explain
+      final SqlExplain explain,
+      final boolean isDruidConventionExplanation
   )
   {
-    final String explanation = RelOptUtil.dumpPlan("", rel, 
explain.getFormat(), explain.getDetailLevel());
+    String explanation = RelOptUtil.dumpPlan("", rel, explain.getFormat(), 
explain.getDetailLevel());
     String resources;
     try {
+      if (isDruidConventionExplanation && rel instanceof DruidRel) {
+        // Show the native queries instead of Calcite's explain if the legacy 
flag is turned off
+        if (!plannerContext.getPlannerConfig().isUseLegacyDruidExplain()) {
+          DruidRel<?> druidRel = (DruidRel<?>) rel;
+          explanation = explainSqlPlanAsNativeQueries(druidRel);
+        }
+      }
       resources = jsonMapper.writeValueAsString(plannerContext.getResources());
     }
     catch (JsonProcessingException jpe) {
       // this should never happen, we create the Resources here, not a user
       log.error(jpe, "Encountered exception while serializing Resources for 
explain output");
       resources = null;
     }
+    catch (ISE ise) {
+      log.error(ise, "Unable to translate to a native Druid query. Resorting 
to legacy Druid explain plan");
+      resources = null;
+    }
     final Supplier<Sequence<Object[]>> resultsSupplier = Suppliers.ofInstance(
         Sequences.simple(ImmutableList.of(new Object[]{explanation, 
resources})));
     return new PlannerResult(resultsSupplier, 
getExplainStructType(rel.getCluster().getTypeFactory()));
   }
 
+  /**
+   * This method doesn't utilize the Calcite's internal {@link 
RelOptUtil#dumpPlan} since that tends to be verbose
+   * and not indicative of the native Druid Queries which will get executed
+   * This method assumes that the Planner has converted the RelNodes to 
DruidRels, and thereby we can implictly cast it
+   *
+   * @param rel Instance of the root {@link DruidRel} which is formed by 
running the planner transformations on it
+   * @return A string representing an array of native queries that correspond 
to the given SQL query, in JSON format
+   * @throws JsonProcessingException
+   */
+  private String explainSqlPlanAsNativeQueries(DruidRel<?> rel) throws 
JsonProcessingException
+  {
+    // Only if rel is an instance of DruidUnionRel, do we run multiple native 
queries corresponding to single SQL query
+    // Also, DruidUnionRel can only be a top level node, so we don't need to 
check for this condition in the subsequent
+    // child nodes
+    List<DruidQuery> druidQueryList;
+    if (rel instanceof DruidUnionRel) {
+      druidQueryList = rel.getInputs().stream().map(childRel -> (DruidRel<?>) 
childRel).map(childRel -> {
+        if (childRel instanceof DruidUnionRel) {
+          log.error("DruidUnionRel can only be the outermost RelNode. This 
error shouldn't be encountered");
+          throw new ISE("DruidUnionRel is only supported at the outermost 
RelNode.");
+        }
+        return childRel.toDruidQuery(false);
+      }).collect(Collectors.toList());
+    } else {
+      druidQueryList = ImmutableList.of(rel.toDruidQuery(false));
+    }
+
+    ArrayNode nativeQueriesArrayNode = jsonMapper.createArrayNode();
+
+    for (DruidQuery druidQuery : druidQueryList) {
+      Query<?> nativeQuery = druidQuery.getQuery();
+      ObjectNode objectNode = jsonMapper.createObjectNode();
+      objectNode.put("query", jsonMapper.convertValue(nativeQuery, 
ObjectNode.class));
+      objectNode.put("signature", 
druidQuery.getOutputRowSignature().toString());

Review comment:
       Added the JSON representation of the row signature. Output of the 
signature is similar to 
https://github.com/apache/druid/pull/11959/files#diff-baa9c67c8599846d87cd9205a5b18e6d407aad799c8457d542216129f22c07e2R90,
 so the testcases needn't be changed once Gian's changes gets merged. 




-- 
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]

Reply via email to