gianm commented on code in PR #12897:
URL: https://github.com/apache/druid/pull/12897#discussion_r944768188
##########
sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java:
##########
@@ -319,31 +345,34 @@ public PlannerResult plan() throws ValidationException
rootQueryRel = planner.rel(validatedQueryNode);
}
+ final boolean hasBindableTables = hasBindableTables(rootQueryRel.rel);
+
// the planner's type factory is not available until after parsing
this.rexBuilder = new RexBuilder(planner.getTypeFactory());
state = State.PLANNED;
try {
- return planWithDruidConvention(rootQueryRel, parsed.getExplainNode(),
parsed.getInsertOrReplace());
+ if (hasBindableTables) {
+ // Consider BINDABLE convention if necessary. Used for metadata tables.
+ if (!parsed.isSelect()) {
Review Comment:
That's a good question. Right now, it's equivalent, so your question is
really about future proofing. I thought about it, but BINDABLE only supports
SELECT and always will only support SELECT. So we do want to reject any
non-SELECT statements. If we add other statements beyond INSERT / REPLACE, then
the error message wouldn't make sense here, but I thought that was slightly
better than the logic not making sense. Either way, I think it's a very minor
point, since we'd need to update this code regardless when we add more
statements.
--
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]
