zachjsh commented on code in PR #16260:
URL: https://github.com/apache/druid/pull/16260#discussion_r1569365887


##########
sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidSqlValidator.java:
##########
@@ -348,6 +398,42 @@ private DatasourceTable validateInsertTarget(
     }
   }
 
+  /**
+   * Clustering is a kind of ordering. We push the CLUSTERED BY clause into 
the source query as
+   * an ORDER BY clause. In an ideal world, clustering would be outside of 
SELECT: it is an operation
+   * applied after the data is selected. For now, we push it into the SELECT, 
then MSQ pulls it back
+   * out. This is unfortunate as errors will be attributed to ORDER BY, which 
the user did not
+   * actually specify (it is an error to do so.) However, with the current 
hybrid structure, it is
+   * not possible to add the ORDER by later: doing so requires access to the 
order by namespace
+   * which is not visible to subclasses.
+   */
+  private void rewriteClusteringToOrderBy(SqlNode source, DruidSqlIngest 
ingestNode, @Nullable SqlNodeList catalogClustering)

Review Comment:
   Thank you for the suggestion, this worked great! Let me know if good now.



##########
sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidSqlValidator.java:
##########
@@ -88,6 +101,39 @@ public class DruidSqlValidator extends BaseDruidSqlValidator
 
   private final PlannerContext plannerContext;
 
+  // This part is a bit sad. By the time we get here, the validator will have 
created
+  // the ORDER BY namespace if we had a real ORDER BY. We have to "catch up" 
and do the
+  // work that registerQuery() should have done. That's kind of OK. But, the 
orderScopes
+  // variable is private, so we have to play dirty tricks to get at it.
+  //
+  // Warning: this may no longer work if Java forbids access to private fields 
in a
+  // future release.
+  private static final Field CLAUSE_SCOPES_FIELD;
+  private static final Object ORDER_CLAUSE;
+
+  static {
+    try {
+      CLAUSE_SCOPES_FIELD = FieldUtils.getDeclaredField(
+          SqlValidatorImpl.class,
+          "clauseScopes",
+          true
+      );
+    }
+    catch (RuntimeException e) {
+      throw new ISE(e, "SqlValidatorImpl.clauseScopes is not accessible");
+    }
+
+    try {
+      Class<?> innerClass = 
Class.forName("org.apache.calcite.sql.validate.SqlValidatorImpl$Clause");
+      Method method = innerClass.getMethod("valueOf", Class.class, 
String.class);
+      method.setAccessible(true);
+      ORDER_CLAUSE = method.invoke(null, innerClass, "ORDER");

Review Comment:
   Took your suggestion here, great tip!



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