Jackie-Jiang commented on a change in pull request #4648: Enable DISTINCT and
throw exceptions for not-supported cases
URL: https://github.com/apache/incubator-pinot/pull/4648#discussion_r328404769
##########
File path:
pinot-common/src/main/java/org/apache/pinot/pql/parsers/Pql2Compiler.java
##########
@@ -152,6 +157,69 @@ public BrokerRequest compileToBrokerRequest(String
expression)
}
}
+ /**
+ * Currently we don't support DISTINCT queries that have ORDER BY
+ * Until support for ORDER BY with DISTINCT is implemented,
+ * throw compilation error
+ *
+ * We walk the AST to detect if the AST has a FunctionCallAstNode for
+ * DISTINCT function along with OrderByAstNode.
+ *
+ * The following indicates how AST generally looks like for DISTINCT
+ * queries. Under the root node (SelectAstNode), we don't need to check
+ * for all kinds of children (like WhereAstNode, GroupByAstNode,
+ * StarColumnListAstNode).
+ *
+ * Only check for 2 children: OutputColumnListAstNode (and ensure output
column
+ * inside this list is for DISTINCT function) and OrderByAstNode
+ *
+ * SelectAstNode{_tableName='foo', _resourceName='foo', _recordLimit=-1,
_offset=-1}
+ * OutputColumnListAstNode
+ * OutputColumnAstNode
+ * FunctionCallAstNode{_name='DISTINCT'}
+ * IdentifierAstNode{_name='col1'}
+ * IdentifierAstNode{_name='col2'}
+ * OrderByAstNode
+ * OrderByExpressionAstNode{_column='col1', _ordering='asc'}
+ * IdentifierAstNode{_name='col1'}
+ * OrderByExpressionAstNode{_column='col2', _ordering='asc'}
+ * IdentifierAstNode{_name='col2'}
+ * @param root root node of AST
+ */
+ private void checkForOrderByWithDistinct(AstNode root) {
+ if (root instanceof SelectAstNode) {
+ SelectAstNode selectAstNode = (SelectAstNode) root;
+ List<? extends AstNode> selectChildren = selectAstNode.getChildren();
+ boolean distinctExists = false;
+ for (AstNode selectChild : selectChildren) {
+ if (selectChild instanceof OutputColumnListAstNode) {
+ List<? extends AstNode> outputColumnAstNodes =
selectChild.getChildren();
+ if (outputColumnAstNodes.size() > 1) {
+ // there has to be exactly one OutputColumnAstNode under
+ // OutputColumnListAstNode. if more than 1, we know this
+ // can't be a DISTINCT query and so break early
+ break;
+ }
+ for (AstNode outputColumnAstNode : outputColumnAstNodes) {
+ if (outputColumnAstNode instanceof OutputColumnAstNode) {
Review comment:
This check is redundant
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
