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


##########
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:
   instead of this creative reflectioned approach ; how about overriding the 
`createSelectNamespace`;
   in that method you will see:
   * the current `SqlSelect` node
   * the encosingNode - which will be `SqlInsert` or something...so can give 
access the catalog orderby somehow
   * full control on hiw the `SelectNamespace` is being created - it might be 
even possible to supply an alternate implementation



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