[Impala-ASF-CR] IMPALA-7844: HAVING clause cannot support ordinals

["Paul Rogers (Code Review)" <ge...@cloudera.org>]

Paul Rogers has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/11955 )

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 name, 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 opeators), than 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 odd because HAVING is normally a predicate: "HAVING a = 10".

This fix removes the attempt to support ordinals in the HAVING clause,
and treats HAVING like WHERE, rather than trying to treat it like
ORDER BY or GROUP BY. The fix retains the limited form of alias
support.

A future change can add full alias support, but must do so inside any
valid expression so it works with all column types. That is,
"HAVING a = 10" must work if a is an alias.

The existing "ordinal or alias" code was a bit messy, and not in good
shape to support the eventual goal of integrating rewrites into the
analysis phase. Refactored the code to allow resolution of only
aliases (for HAVING) and to return whether the result expression needs
a rewrite (for future rewrite integration.)

Reworded a few error messages for greater clarity.

Testing:

* Refactored AnalyzeStmtsTest to split apart the alias and ordinals
  cases for easier debugging.
* Disabled the tests for ordinals in HAVING.
* Added new tests to verify that integers in HAVING act like
  (unsupported) integer constants.

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, 306 insertions(+), 132 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/11955/2
-- 
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: 2
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>