Hello Bharath Vissapragada, Greg Rahn, Zoltan Borok-Nagy, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11955
to look at the new patch set (#6).
Change subject: IMPALA-7844: HAVING clause cannot support ordinals
......................................................................
IMPALA-7844: HAVING clause cannot support ordinals
The SELECT statement has two clauses that take lists of columns and/or
aliases: ORDER BY and GROUP BY. Each element is a alias, a table.column
reference or a number, which represents the ordinal number of a column.
Thus, "GROUP BY a, 2, c" is unambiguous.
SELECT also has a number of predicate clauses: WHERE and HAVING. In
these, aliases are possible (though seldom suppored), but ordinals are
ambiguous: is "WHERE 1 = 2" a reference to two constants, two columns by
ordinal, or a combination? No SQL dialect supports ordinals in WHERE or
HAVING for this reason.
Impala seems to have adopted a rather odd convention: if the HAVING
predicate has only one element (no operators), then that one element can
be an ordinal or alias. Thus "HAVING a" and "HAVING 1" are valid, only
if alias a or the column at ordinal 1 are BOOLEAN. However,
"HAVING a = 1" and "HAVING 1 = 2" are not valid. This is unusual
because HAVING is normally a predicate: "HAVING a = 10".
This fix prepares to remove the attempt to support ordinals in the
HAVING clause, after which Impala will treat HAVING like WHERE, rather
than trying to treat it like ORDER BY or GROUP BY.
Review comments suggest that such a breaking change (even for such a
non-standard, undocumented, odd feature) should occur only in a major
version. So, for now, the no-ordinal support can be enabled via a
constant, but is turned off by default. (Perhaps a later patch can add
a session or runtime option instead of the constant.)
The fix retains the limited form of alias support.
Refactored the "ordinal or alias" code to make alias resolution
optional.
Reworded a few error messages for greater clarity.
Testing:
* Refactored AnalyzeStmtsTest to split apart the alias and ordinals
cases for easier debugging.
* Tests can validate either the current HAVING ordinal behavior or
the proposed new behavior. Test path is controlled by the constant
described above.
Change-Id: Ic2b9f9e8c60fe2b25e20c57c2ffc31d8e59d5861
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java
5 files changed, 331 insertions(+), 142 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/11955/6
--
To view, visit http://gerrit.cloudera.org:8080/11955
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic2b9f9e8c60fe2b25e20c57c2ffc31d8e59d5861
Gerrit-Change-Number: 11955
Gerrit-PatchSet: 6
Gerrit-Owner: Paul Rogers <[email protected]>
Gerrit-Reviewer: Bharath Vissapragada <[email protected]>
Gerrit-Reviewer: Greg Rahn <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Paul Rogers <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
