You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/12/02 11:55:14 UTC
[GitHub] [druid] abhishekagarwal87 commented on a change in pull request #12017: Fix the error case when there are multi top level unions
abhishekagarwal87 commented on a change in pull request #12017:
URL: https://github.com/apache/druid/pull/12017#discussion_r761021647
##########
File path: sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java
##########
@@ -431,25 +431,26 @@ private PlannerResult planExplanation(
*/
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
ObjectMapper jsonMapper = plannerContext.getJsonMapper();
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));
+ try {
+ druidQueryList = flattenOutermostRel(rel)
+ .stream()
+ .map(druidRel -> druidRel.toDruidQuery(false))
+ .collect(Collectors.toList());
+
+ }
+ catch (UnsupportedOperationException unsupportedOperationException) {
+ 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.");
+ }
+ catch (Exception e) {
Review comment:
we should log the exception details though.
##########
File path: sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java
##########
@@ -431,25 +431,26 @@ private PlannerResult planExplanation(
*/
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
ObjectMapper jsonMapper = plannerContext.getJsonMapper();
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));
+ try {
+ druidQueryList = flattenOutermostRel(rel)
+ .stream()
+ .map(druidRel -> druidRel.toDruidQuery(false))
+ .collect(Collectors.toList());
+
+ }
+ catch (UnsupportedOperationException unsupportedOperationException) {
Review comment:
are you throwing `unsupportedOperationException` somewhere in the code path? I am wondering why is it being caught separately.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org