jihoonson commented on a change in pull request #11911:
URL: https://github.com/apache/druid/pull/11911#discussion_r759744852
##########
File path:
sql/src/main/java/org/apache/druid/sql/calcite/planner/UnsupportedSQLQueryException.java
##########
@@ -24,14 +24,14 @@
/**
* This class is different from {@link
org.apache.calcite.plan.RelOptPlanner.CannotPlanException} in that the error
- * messages are user-friendly unlike it's parent class. This exception class
be used instead of
+ * messages are user-friendly unlike its parent class. This exception class
should be used instead of
* {@link org.apache.druid.java.util.common.ISE} or {@link
org.apache.druid.java.util.common.IAE} when processing is
- * to be halted during planning. Similarly, Druid planner can catch these
exception and know that the error
- * can be directly exposed to end-user.
+ * to be halted during planning. Similarly, Druid planner can catch this
exception and know that the error
+ * can be directly exposed to end-users.
*/
-public class UnsupportedQueryFeatureException extends
RelOptPlanner.CannotPlanException
+public class UnsupportedSQLQueryException extends
RelOptPlanner.CannotPlanException
Review comment:
:+1:
##########
File path:
sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java
##########
@@ -209,23 +212,27 @@ public PlannerResult plan() throws SqlParseException,
ValidationException, RelCo
// Not a CannotPlanningException, rethrow without trying with bindable
throw e;
}
+ if (parsed.getInsertNode() != null) {
+ // Cannot INSERT with BINDABLE.
+ throw e;
+ }
// Try again with BINDABLE convention. Used for querying Values and
metadata tables.
try {
- return planWithBindableConvention(explain, root);
+ return planWithBindableConvention(rootQueryRel,
parsed.getExplainNode());
}
catch (Exception e2) {
e.addSuppressed(e2);
- log.noStackTrace().warn(e, "Failed to plan the query '%s'", sql);
+ log.noStackTrace().warn(e, "Failed to plan the query '%s'",
plannerContext.getSql());
Review comment:
https://github.com/apache/druid/pull/11911#discussion_r755584263
##########
File path:
sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java
##########
@@ -209,23 +212,27 @@ public PlannerResult plan() throws SqlParseException,
ValidationException, RelCo
// Not a CannotPlanningException, rethrow without trying with bindable
throw e;
}
+ if (parsed.getInsertNode() != null) {
+ // Cannot INSERT with BINDABLE.
+ throw e;
+ }
// Try again with BINDABLE convention. Used for querying Values and
metadata tables.
try {
- return planWithBindableConvention(explain, root);
+ return planWithBindableConvention(rootQueryRel,
parsed.getExplainNode());
}
catch (Exception e2) {
e.addSuppressed(e2);
- log.noStackTrace().warn(e, "Failed to plan the query '%s'", sql);
+ log.noStackTrace().warn(e, "Failed to plan the query '%s'",
plannerContext.getSql());
Review comment:
Also, warning seems reasonable to me as this is not a system failure but
is inconsistent with
https://github.com/apache/druid/blob/master/server/src/main/java/org/apache/druid/server/QueryLifecycle.java#L323
--
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]
