[
https://issues.apache.org/jira/browse/IMPALA-7753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16664395#comment-16664395
]
Paul Rogers commented on IMPALA-7753:
-------------------------------------
Checked if an ORDER BY clause can contain an expression (if not, we'd not need
to do rewrites). Turns out it can:
{noformat}
order_by_elements ::=
order_by_element:e
...
order_by_element ::=
expr:e opt_order_param:o opt_nulls_order_param:n
{noformat}
Given that the parser allows an "expr" in the order by clause, applying
expression rewrite rules seems reasonable.
> Invalid logic when rewriting ORDER BY clause expressions
> --------------------------------------------------------
>
> Key: IMPALA-7753
> URL: https://issues.apache.org/jira/browse/IMPALA-7753
> Project: IMPALA
> Issue Type: Bug
> Components: Frontend
> Affects Versions: Impala 3.0
> Reporter: Paul Rogers
> Assignee: Paul Rogers
> Priority: Minor
>
> The select statement represents the ORDER BY clause in two distinct ways.
> First, there is a list of the "original" ordering expressions,
> {{orderByElements_}}. Second, there is an analyzed list in {{sortInfo_}}. The
> explanation is:
> {code:java}
> // create copies, we don't want to modify the original parse node, in
> case
> // we need to print it
> {code}
> Later, we apply rewrite rules to the {{ORDER BY}} expression, but we do so
> using the original version, not the copy:
> {code:java}
> for (OrderByElement orderByElem: orderByElements_) {
> orderByElem.setExpr(rewriteCheckOrdinalResult(rewriter,
> orderByElem.getExpr()));
> }
> {code}
> The result is that we apply rewrite rules to expressions which have not been
> analyzed, triggering the assertion mentioned above. This assertion is telling
> us something: we skipped a step. Here, it turns out we are rewriting the
> wrong set of expressions. Modifying the code to rewrite those in
> {{sortInfo_}} solves the problem. The current behavior is a bug as the
> rewrites currently do nothing, and the expressions we thought we were
> rewriting are never touched.
> The correct code would rewrite the expressions which are actually used when
> analyzing the query:
> {code} if (orderByElements_ != null) {
> List<Expr> sortExprs = sortInfo_.getSortExprs();
> for (int i = 0; i < sortExprs.size(); i++) {
> sortExprs.set(i, rewriteCheckOrdinalResult(rewriter,
> sortExprs.get(i)));
> }
> {code}
> We can, in addition, ask a more basic question: do we even need to do
> rewrites for {{ORDER BY}} expressions? The only valid expressions are column
> references, aren't they? Or, does Impala allow expressions in the {{ORDER
> BY}} clause?
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
