You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by bh...@apache.org on 2018/12/19 18:54:16 UTC
[2/3] impala git commit: 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
Reviewed-on: http://gerrit.cloudera.org:8080/11955
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/d51aa522
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/d51aa522
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/d51aa522
Branch: refs/heads/master
Commit: d51aa522dd0e636d0de5a8d423cd3b689bdd39cb
Parents: fb3f7e6
Author: Paul Rogers <pr...@cloudera.com>
Authored: Sun Nov 18 17:02:45 2018 -0800
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Wed Dec 19 02:37:23 2018 +0000
----------------------------------------------------------------------
.../java/org/apache/impala/analysis/Expr.java | 8 +
.../org/apache/impala/analysis/QueryStmt.java | 108 +++++----
.../org/apache/impala/analysis/SelectStmt.java | 75 +++++--
.../impala/analysis/AnalyzeStmtsTest.java | 221 ++++++++++++-------
.../impala/analysis/AnalyzeSubqueriesTest.java | 59 ++++-
5 files changed, 330 insertions(+), 141 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/d51aa522/fe/src/main/java/org/apache/impala/analysis/Expr.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/Expr.java b/fe/src/main/java/org/apache/impala/analysis/Expr.java
index a5ed1f7..488773e 100644
--- a/fe/src/main/java/org/apache/impala/analysis/Expr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/Expr.java
@@ -293,6 +293,14 @@ abstract public class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl
}
};
+ public final static com.google.common.base.Predicate<Expr> IS_INT_LITERAL =
+ new com.google.common.base.Predicate<Expr>() {
+ @Override
+ public boolean apply(Expr arg) {
+ return IS_LITERAL.apply(arg) && arg.getType().isIntegerType();
+ }
+ };
+
// id that's unique across the entire query statement and is assigned by
// Analyzer.registerConjuncts(); only assigned for the top-level terms of a
// conjunction, and therefore null for most Exprs
http://git-wip-us.apache.org/repos/asf/impala/blob/d51aa522/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java b/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
index e75beca..1c03a9e 100644
--- a/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
@@ -317,32 +317,6 @@ public abstract class QueryStmt extends StatementBase {
}
/**
- * Substitutes an ordinal or an alias. An ordinal is an integer NumericLiteral
- * that refers to a select-list expression by ordinal. An alias is a SlotRef
- * that matches the alias of a select-list expression (tracked by 'aliasMap_').
- * We should substitute by ordinal or alias but not both to avoid an incorrect
- * double substitution.
- * Returns clone() of 'expr' if it is not an ordinal, nor an alias.
- * The returned expr is analyzed regardless of whether substitution was performed.
- */
- protected Expr substituteOrdinalOrAlias(Expr expr, String errorPrefix,
- Analyzer analyzer) throws AnalysisException {
- Expr substituteExpr = trySubstituteOrdinal(expr, errorPrefix, analyzer);
- if (substituteExpr != null) return substituteExpr;
- if (ambiguousAliasList_.contains(expr)) {
- throw new AnalysisException("Column '" + expr.toSql() +
- "' in " + errorPrefix + " clause is ambiguous");
- }
- if (expr instanceof SlotRef) {
- substituteExpr = expr.trySubstitute(aliasSmap_, analyzer, false);
- } else {
- expr.analyze(analyzer);
- substituteExpr = expr;
- }
- return substituteExpr;
- }
-
- /**
* Substitutes top-level ordinals and aliases. Does not substitute ordinals and
* aliases in subexpressions.
* Modifies the 'exprs' list in-place.
@@ -352,35 +326,79 @@ public abstract class QueryStmt extends StatementBase {
protected void substituteOrdinalsAndAliases(List<Expr> exprs, String errorPrefix,
Analyzer analyzer) throws AnalysisException {
for (int i = 0; i < exprs.size(); ++i) {
- exprs.set(i, substituteOrdinalOrAlias(exprs.get(i), errorPrefix, analyzer));
+ exprs.set(i, resolveReferenceExpr(exprs.get(i), errorPrefix,
+ analyzer, true));
}
}
- // Attempt to replace an expression of form "<number>" with the corresponding
- // select list items. Return null if not an ordinal expression.
- private Expr trySubstituteOrdinal(Expr expr, String errorPrefix,
- Analyzer analyzer) throws AnalysisException {
- if (!(expr instanceof NumericLiteral)) return null;
- expr.analyze(analyzer);
- if (!expr.getType().isIntegerType()) return null;
- long pos = ((NumericLiteral) expr).getLongValue();
- if (pos < 1) {
- throw new AnalysisException(
- errorPrefix + ": ordinal must be >= 1: " + expr.toSql());
+ /**
+ * Substitutes an ordinal or an alias. An ordinal is an integer NumericLiteral
+ * that refers to a select-list expression by ordinal. An alias is a SlotRef
+ * that matches the alias of a select-list expression (tracked by 'aliasMap_').
+ * We should substitute by ordinal or alias but not both to avoid an incorrect
+ * double substitution.
+ *
+ * Logic is a bit tricky. The SlotRef, if it exists, cannot be resolved until we
+ * check for an alias. (Resolving the SlotRef may find a column, or trigger an
+ * error, which is not what we want.)
+ *
+ * After the alias check, then we can resolve (analyze) the expression.Then, if
+ * the expression is an ordinal, replace it. Else, the expression is "ordinary"
+ * and can be rewritten by the caller.
+ *
+ * @param expr the expression on which to perform substitution
+ * @param allowOrdinal whether the context of the expression allows ordinals.
+ * lists (ORDER BY, GROUP BY) allow ordinals, expressions (HAVING) do not.
+ * (In the 3.x series, for backward compatibility, HAVING continues to allow
+ * ordinals, but the goal is to remove this non-standard support in the future)
+ * @return the rewritten or substituted expression, with analysis completed
+ */
+ protected Expr resolveReferenceExpr(Expr expr,
+ String errorPrefix, Analyzer analyzer, boolean allowOrdinal)
+ throws AnalysisException {
+ // Check for a SlotRef (representing an alias) before analysis. Since
+ // the slot does not reference a real column, the analysis will fail.
+ // TODO: Seems an odd state of affairs. Consider revisiting by putting
+ // alias in a namespace that can be resolved more easily.
+ if (expr instanceof SlotRef) {
+ if (ambiguousAliasList_.contains(expr)) {
+ // Reference to an ambiguous alias
+ // SELECT foo AS a, bar AS a ORDER BY a
+ throw new AnalysisException(errorPrefix +
+ ": ambiguous alias: '" + expr.toSql() + "'");
+ }
+ // Look the name up in the alias substitution map
+ // Returns a clone of the expression if not found
+ return expr.trySubstitute(aliasSmap_, analyzer_, false);
}
- if (pos > resultExprs_.size()) {
- throw new AnalysisException(
- errorPrefix + ": ordinal exceeds number of items in select list: "
- + expr.toSql());
+
+ // Ordinal reference?
+ // Only want values that started as numeric literals
+ boolean wasNumber = expr instanceof NumericLiteral;
+ expr.analyze(analyzer);
+ if (allowOrdinal && wasNumber && Expr.IS_INT_LITERAL.apply(expr)) {
+ long pos = ((NumericLiteral) expr).getLongValue();
+ if (pos < 1) {
+ throw new AnalysisException(
+ errorPrefix + ": ordinal must be >= 1: " + expr.toSql());
+ }
+ if (pos > resultExprs_.size()) {
+ throw new AnalysisException(
+ errorPrefix + ": ordinal exceeds the number of items in the SELECT list: " +
+ expr.toSql());
+ }
+
+ // Create copy to protect against accidentally shared state.
+ return resultExprs_.get((int) pos - 1).clone();
}
- // Create copy to protect against accidentally shared state.
- return resultExprs_.get((int) pos - 1).clone();
+ // Ordinary expression..
+ return expr;
}
/**
* Returns the materialized tuple ids of the output of this stmt.
- * Used in case this stmt is part of an @InlineViewRef,
+ * Used in case this stmt is part of an InlineViewRef,
* since we need to know the materialized tupls ids of a TableRef.
* This call must be idempotent because it may be called more than once for Union stmt.
* TODO: The name of this function has become outdated due to analytics
http://git-wip-us.apache.org/repos/asf/impala/blob/d51aa522/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java b/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
index 0f6b8cd..8c99714 100644
--- a/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
@@ -49,6 +49,39 @@ import com.google.common.collect.Lists;
public class SelectStmt extends QueryStmt {
private final static Logger LOG = LoggerFactory.getLogger(SelectStmt.class);
+ /**
+ * Impala 3.x and earlier provide non-standard support for a single ordinal
+ * in the HAVING clause:
+ *
+ * SELECT region, count(*), count(*) > 10 FROM sales
+ * HAVING 3
+ *
+ * Most other SQL dialects do not allow ordinals in expressions (such as
+ * HAVING), only in lists (GROUP BY, ORDER BY). The standard way to express
+ * the above is:
+ *
+ * SELECT region, count(*) FROM sales
+ * HAVING count(*) > 10
+ *
+ * Or, better (not currently supported by IMPALA):
+ *
+ * SELECT region, count(*) c FROM SALES
+ * HAVING c > 10
+ *
+ * We intend to disable this non-standard feature in the next major
+ * release. For now, we leave the feature/bug enabled, controlled by
+ * the following constant.
+ *
+ * In that same release, we will enable the third example above.
+ * At present, all that is supported is the odd:
+ *
+ * SELECT region, count(*), count(*) > 10 p FROM sales
+ * HAVING p
+ *
+ * See IMPALA-7844.
+ */
+ public static final boolean ALLOW_ORDINALS_IN_HAVING = true;
+
/////////////////////////////////////////
// BEGIN: Members that need to be reset()
@@ -537,25 +570,37 @@ public class SelectStmt extends QueryStmt {
verifyAggregation();
}
+ /**
+ * Analyze the HAVING clause. The HAVING clause is a predicate, not a list
+ * (like GROUP BY or ORDER BY) and so cannot contain ordinals.
+ *
+ * At present, Impala's alias substitution logic only works for top-level
+ * expressions in a list. Since HAVING is a predicate, alias substitution
+ * does not work (the top-level predicate in HAVING is a Boolean expression.)
+ *
+ * TODO: Modify alias substitution to work like column resolution so that aliases
+ * can appear anywhere in the expression. Then, enable alias substitution in HAVING.
+ */
private void analyzeHavingClause() throws AnalysisException {
// Analyze the HAVING clause first so we can check if it contains aggregates.
// We need to analyze/register it even if we are not computing aggregates.
- if (havingClause_ != null) {
- // can't contain subqueries
- if (havingClause_.contains(Predicates.instanceOf(Subquery.class))) {
- throw new AnalysisException(
- "Subqueries are not supported in the HAVING clause.");
- }
- havingPred_ = substituteOrdinalOrAlias(havingClause_, "HAVING", analyzer_);
- // can't contain analytic exprs
- Expr analyticExpr = havingPred_.findFirstOf(AnalyticExpr.class);
- if (analyticExpr != null) {
- throw new AnalysisException(
- "HAVING clause must not contain analytic expressions: "
- + analyticExpr.toSql());
- }
- havingPred_.checkReturnsBool("HAVING clause", true);
+ if (havingClause_ == null) return;
+ // can't contain subqueries
+ if (havingClause_.contains(Predicates.instanceOf(Subquery.class))) {
+ throw new AnalysisException(
+ "Subqueries are not supported in the HAVING clause.");
+ }
+ // Resolve (top-level) aliases and analyzes
+ havingPred_ = resolveReferenceExpr(havingClause_, "HAVING", analyzer_,
+ ALLOW_ORDINALS_IN_HAVING);
+ // can't contain analytic exprs
+ Expr analyticExpr = havingPred_.findFirstOf(AnalyticExpr.class);
+ if (analyticExpr != null) {
+ throw new AnalysisException(
+ "HAVING clause must not contain analytic expressions: "
+ + analyticExpr.toSql());
}
+ havingPred_.checkReturnsBool("HAVING clause", true);
}
private boolean checkForAggregates() throws AnalysisException {
http://git-wip-us.apache.org/repos/asf/impala/blob/d51aa522/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
index d83f9dd..a46cff8 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
@@ -980,100 +980,161 @@ public class AnalyzeStmtsTest extends AnalyzerTest {
}
@Test
- public void TestAliasesAndOrdinals() throws AnalysisException {
- AnalysisError("select * from functional.alltypes group by 1",
- "cannot combine '*' in select list with grouping or aggregation");
- AnalysisError("select * from functional.alltypes order by 14",
- "ORDER BY: ordinal exceeds number of items in select list: 14");
- AnalyzesOk("select t.* from functional.alltypes t order by 1");
- AnalyzesOk("select t2.* from functional.alltypes t1, " +
- "functional.alltypes t2 order by 1");
- AnalyzesOk("select * from (select max(id) from functional.testtbl) t1 order by 1");
- AnalysisError("select * from (select max(id) from functional.testtbl) t1 order by 2",
- "ORDER BY: ordinal exceeds number of items in select list: 2");
-
- // IMPALA-5191: In GROUP BY, HAVING, ORDER BY, aliases and ordinals must only be
+ public void TestGroupByAliases() throws AnalysisException {
+ // IMPALA-5191: GROUP BY, aliases must only be
// substituted at the top level
- // Ambiguous alias in HAVING
- AnalysisError("select not bool_col m, min(smallint_col) m, max(bigint_col) m "
- + "from functional.alltypes group by bool_col having m",
- "Column 'm' in HAVING clause is ambiguous");
- // GROUP BY, ORDER BY, HAVING contains top-level SlotRef or ordinal
AnalyzesOk("select int_col / 2 as x from functional.alltypes group by x");
- AnalyzesOk("select int_col / 2 as x from functional.alltypes order by x");
- AnalyzesOk("select not bool_col as nb from functional.alltypes having nb");
- AnalyzesOk("select int_col / 2 as x from functional.alltypes group by 1");
- AnalyzesOk("select int_col / 2 as x from functional.alltypes order by 1");
- AnalyzesOk("select not bool_col as nb from functional.alltypes having 1");
- // GROUP BY, ORDER BY, HAVING contains alias in subexpression
+ // GROUP BY contains alias in subexpression
AnalysisError(
"select int_col / 2 as x from functional.alltypes group by x / 2",
"Could not resolve column/field reference: 'x'");
- AnalysisError(
- "select int_col / 2 as x from functional.alltypes order by -x",
- "Could not resolve column/field reference: 'x'");
- AnalysisError(
- "select int_col / 2 as x from functional.alltypes having x > 0",
- "Could not resolve column/field reference: 'x'");
- // Alias or ordinal referring to aggregation output in GROUP BY, ORDER BY, HAVING
+ // Alias referring to aggregation output in GROUP BY
AnalysisError("select count(*) a from functional.alltypes group by a",
"GROUP BY expression must not contain aggregate functions: a");
- AnalyzesOk("select count(*) a from functional.alltypes order by a");
- AnalysisError("select count(*) a from functional.alltypes having a",
- "HAVING clause 'count(*)' requires return type 'BOOLEAN'. " +
- "Actual type is 'BIGINT'.");
- AnalysisError("select count(*) from functional.alltypes group by 1",
- "GROUP BY expression must not contain aggregate functions: 1");
- AnalyzesOk("select count(*) from functional.alltypes order by 1");
- AnalysisError("select count(*) from functional.alltypes having 1",
- "HAVING clause 'count(*)' requires return type 'BOOLEAN'. " +
- "Actual type is 'BIGINT'.");
- // Alias or ordinal referring to predicate in GROUP BY, ORDER BY, HAVING
+ // Alias referring to predicate in GROUP BY
AnalysisError("select count(*) > 10 a from functional.alltypes group by a",
"GROUP BY expression must not contain aggregate functions: a");
- AnalyzesOk("select count(*) > 10 a from functional.alltypes order by a");
- AnalyzesOk("select count(*) > 10 a from functional.alltypes having a");
- AnalysisError("select count(*) > 10 from functional.alltypes group by 1",
- "GROUP BY expression must not contain aggregate functions: 1");
- AnalyzesOk("select count(*) > 10 from functional.alltypes order by 1");
- AnalyzesOk("select count(*) > 10 from functional.alltypes having 1");
- // Alias or ordinal referring to analytic output in GROUP BY, ORDER BY, HAVING
+ // Alias referring to analytic output in GROUP BY
AnalysisError("select sum(id) over(order by id) a " +
"from functional.alltypes group by a",
"GROUP BY expression must not contain analytic expressions: " +
"sum(id) OVER (ORDER BY id ASC)");
- AnalyzesOk("select sum(id) over(order by id) a from functional.alltypes order by a");
- AnalysisError("select sum(id) over(order by id) a from functional.alltypes having a",
- "HAVING clause must not contain analytic expressions: " +
- "sum(id) OVER (ORDER BY id ASC)");
- AnalysisError("select sum(id) over(order by id) " +
- "from functional.alltypes group by 1",
- "GROUP BY expression must not contain analytic expressions: " +
- "sum(id) OVER (ORDER BY id ASC)");
- AnalyzesOk("select sum(id) over(order by id) from functional.alltypes order by 1");
- AnalysisError("select sum(id) over(order by id) from functional.alltypes having 1",
- "HAVING clause must not contain analytic expressions: " +
- "sum(id) OVER (ORDER BY id ASC)");
+ // Alias referring to analytic output in GROUP BY
AnalyzesOk("with w_test as (select '1' as `one`, 2 as two, '3' as three) " +
"select `one` as `one`, substring(cast(two as string), 1, 1) as two, " +
"three as three, count(1) as cnt " +
"from w_test " +
"group by `one`, substring(cast(two as string), 1, 1), three");
+ }
+
+ @Test
+ public void TestGroupByOrdinals() throws AnalysisException {
+ AnalysisError("select * from functional.alltypes group by 1",
+ "cannot combine '*' in select list with grouping or aggregation");
+ // IMPALA-5191: ORDER BY, ordinals must only be
+ // substituted at the top level
+ AnalyzesOk("select int_col / 2 as x from functional.alltypes group by 1");
+ // Ordinal referring to aggregation output in GROUP BY
+ AnalysisError("select count(*) from functional.alltypes group by 1",
+ "GROUP BY expression must not contain aggregate functions: 1");
+ // Ordinal referring to predicate in GROUP BY
+ AnalysisError("select count(*) > 10 from functional.alltypes group by 1",
+ "GROUP BY expression must not contain aggregate functions: 1");
+ // Ordinal referring to analytic output in GROUP BY
+ AnalysisError("select sum(id) over(order by id) " +
+ "from functional.alltypes group by 1",
+ "GROUP BY expression must not contain analytic expressions: " +
+ "sum(id) OVER (ORDER BY id ASC)");
// Constant exprs should not be interpreted as ordinals
AnalyzesOk("select int_col, count(*) from functional.alltypes group by 1, 1 * 2");
- AnalyzesOk("select int_col, bigint_col from functional.alltypes order by 1 + 4");
- AnalysisError("select int_col, bool_col, count(*) from functional.alltypes " +
- "group by 1, 2 having 1 + 1",
- "HAVING clause '1 + 1' requires return type 'BOOLEAN'. " +
- "Actual type is 'SMALLINT'.");
AnalyzesOk("select int_col, count(*) from functional.alltypes group by 1, " +
"if(true, 2, int_col)");
+ }
+
+ @Test
+ public void TestOrderByAliases() throws AnalysisException {
+ // IMPALA-5191: ORDER BY, aliases must only be
+ // substituted at the top level
+ AnalyzesOk("select int_col / 2 as x from functional.alltypes order by x");
+ // ORDER BY contains alias in subexpression
+ AnalysisError(
+ "select int_col / 2 as x from functional.alltypes order by -x",
+ "Could not resolve column/field reference: 'x'");
+ // Alias referring to aggregation output in ORDER BY
+ AnalyzesOk("select count(*) a from functional.alltypes order by a");
+ // Alias referring to predicate in ORDER BY
+ AnalyzesOk("select count(*) > 10 a from functional.alltypes order by a");
+ // Alias referring to analytic output in ORDER BY
+ AnalyzesOk("select sum(id) over(order by id) a from functional.alltypes order by a");
+ }
+
+ @Test
+ public void TestOrderByOrdinals() throws AnalysisException {
+ AnalysisError("select * from functional.alltypes order by 14",
+ "ORDER BY: ordinal exceeds the number of items in the SELECT list: 14");
+ AnalyzesOk("select t.* from functional.alltypes t order by 1");
+ AnalyzesOk("select t2.* from functional.alltypes t1, " +
+ "functional.alltypes t2 order by 1");
+ AnalyzesOk("select * from (select max(id) from functional.testtbl) t1 order by 1");
+ AnalysisError("select * from (select max(id) from functional.testtbl) t1 order by 2",
+ "ORDER BY: ordinal exceeds the number of items in the SELECT list: 2");
+ // IMPALA-5191: ORDER BY, ordinals must only be
+ // substituted at the top level
+ AnalyzesOk("select int_col / 2 as x from functional.alltypes order by 1");
+ // Ordinal referring to aggregation output in ORDER BY
+ AnalyzesOk("select count(*) from functional.alltypes order by 1");
+ // Ordinal referring to predicate in ORDER BY
+ AnalyzesOk("select count(*) > 10 from functional.alltypes order by 1");
+ // Ordinal referring to analytic output in ORDER BY
+ AnalyzesOk("select sum(id) over(order by id) from functional.alltypes order by 1");
+ // Constant exprs should not be interpreted as ordinals
+ AnalyzesOk("select int_col, bigint_col from functional.alltypes order by 1 + 4");
AnalyzesOk("select int_col, bigint_col from functional.alltypes order by " +
"if(true, 7, int_col)");
- AnalysisError("select int_col, bool_col, count(*) from functional.alltypes " +
- "group by 1, 2 having if(TRUE, 2, int_col)",
- "HAVING clause 'if(TRUE, 2, int_col)' requires return type 'BOOLEAN'. " +
- "Actual type is 'INT'.");
+ }
+
+ @Test
+ public void TestHavingAliases() throws AnalysisException {
+ // Ambiguous alias in HAVING
+ AnalysisError("select not bool_col m, min(smallint_col) m, max(bigint_col) m "
+ + "from functional.alltypes group by bool_col having m",
+ "HAVING: ambiguous alias: 'm'");
+ // IMPALA-5191: HAVING, aliases must only be
+ // substituted at the top level
+ AnalyzesOk("select not bool_col as nb from functional.alltypes having nb");
+ // HAVING contains alias in subexpression
+ AnalysisError(
+ "select int_col / 2 as x from functional.alltypes having x > 0",
+ "Could not resolve column/field reference: 'x'");
+ // Alias referring to aggregation output in HAVING
+ AnalysisError("select count(*) a from functional.alltypes having a",
+ "HAVING clause 'count(*)' requires return type 'BOOLEAN'. " +
+ "Actual type is 'BIGINT'.");
+ // Alias referring to predicate in HAVING
+ AnalyzesOk("select count(*) > 10 a from functional.alltypes having a");
+ // Alias referring to analytic output in HAVING
+ AnalysisError("select sum(id) over(order by id) a from functional.alltypes having a",
+ "HAVING clause must not contain analytic expressions: " +
+ "sum(id) OVER (ORDER BY id ASC)");
+ }
+
+ // IMPALA-7844: Ordinals not supported in HAVING since
+ // HAVING is an expression, not a list like GROUP BY or ORDER BY.
+ // Verify that an integer is treated as a constant, not an ordinal.
+ @Test
+ public void TestHavingIntegers() throws AnalysisException {
+ if (SelectStmt.ALLOW_ORDINALS_IN_HAVING) {
+ // Legacy 3.x functionality
+ AnalyzesOk("select not bool_col as nb from functional.alltypes having 1");
+ AnalysisError("select count(*) from functional.alltypes having 1",
+ "HAVING clause 'count(*)' requires return type 'BOOLEAN'. " +
+ "Actual type is 'BIGINT'.");
+ AnalyzesOk("select count(*) > 10 from functional.alltypes having 1");
+ AnalysisError("select sum(id) over(order by id) from functional.alltypes having 1",
+ "HAVING clause must not contain analytic expressions: " +
+ "sum(id) OVER (ORDER BY id ASC)");
+ AnalysisError("select sum(id) over(order by id) from functional.alltypes having -1",
+ "HAVING: ordinal must be >= 1: -1");
+ } else {
+ // Forward-looking, post 3.x functionality
+ // IMPALA-7844: Ordinals not supported in HAVING since
+ // HAVING is an expression, not a list like GROUP BY or ORDER BY.
+ AnalysisError("select not bool_col as nb from functional.alltypes having 1",
+ "HAVING clause '1' requires return type 'BOOLEAN'. " +
+ "Actual type is 'TINYINT'.");
+ AnalysisError("select not bool_col as nb from functional.alltypes having -1",
+ "HAVING clause '1' requires return type 'BOOLEAN'. " +
+ "Actual type is 'TINYINT'.");
+ // Constant exprs should not be interpreted as ordinals
+ AnalysisError("select int_col, bool_col, count(*) from functional.alltypes " +
+ "group by 1, 2 having 1 + 1",
+ "HAVING clause '1 + 1' requires return type 'BOOLEAN'. " +
+ "Actual type is 'SMALLINT'.");
+ AnalysisError("select int_col, bool_col, count(*) from functional.alltypes " +
+ "group by 1, 2 having if(TRUE, 2, int_col)",
+ "HAVING clause 'if(TRUE, 2, int_col)' requires return type 'BOOLEAN'. " +
+ "Actual type is 'INT'.");
+ }
}
@Test
@@ -1168,7 +1229,7 @@ public class AnalyzeStmtsTest extends AnalyzerTest {
"(missing from GROUP BY clause?)");
AnalysisError("select * from " +
"(select zip, count(*) from functional.testtbl group by 3) x",
- "GROUP BY: ordinal exceeds number of items in select list");
+ "GROUP BY: ordinal exceeds the number of items in the SELECT list");
AnalysisError("select * from " +
"(select * from functional.alltypes group by 1) x",
"cannot combine '*' in select list with grouping or aggregation");
@@ -2392,7 +2453,7 @@ public class AnalyzeStmtsTest extends AnalyzerTest {
AnalyzesOk("select zip, count(*) from functional.testtbl group by 1");
AnalyzesOk("select count(*), zip from functional.testtbl group by 2");
AnalysisError("select zip, count(*) from functional.testtbl group by 3",
- "GROUP BY: ordinal exceeds number of items in select list: 3");
+ "GROUP BY: ordinal exceeds the number of items in the SELECT list: 3");
AnalysisError("select * from functional.alltypes group by 1",
"cannot combine '*' in select list with grouping or aggregation");
// picks up select item alias
@@ -2405,9 +2466,9 @@ public class AnalyzeStmtsTest extends AnalyzerTest {
"from functional.alltypes group by a, b");
// ambiguous alias
AnalysisError("select zip a, id a, count(*) from functional.testtbl group by a",
- "Column 'a' in GROUP BY clause is ambiguous");
+ "GROUP BY: ambiguous alias: 'a'");
AnalysisError("select zip id, id, count(*) from functional.testtbl group by id",
- "Column 'id' in GROUP BY clause is ambiguous");
+ "GROUP BY: ambiguous alias: 'id'");
// can't group by aggregate
AnalysisError("select zip, count(*) from functional.testtbl group by count(*)",
@@ -2464,7 +2525,7 @@ public class AnalyzeStmtsTest extends AnalyzerTest {
AnalysisError("select zip, id from functional.testtbl order by 0",
"ORDER BY: ordinal must be >= 1");
AnalysisError("select zip, id from functional.testtbl order by 3",
- "ORDER BY: ordinal exceeds number of items in select list: 3");
+ "ORDER BY: ordinal exceeds the number of items in the SELECT list: 3");
AnalyzesOk("select * from functional.alltypes order by 1");
// picks up select item alias
AnalyzesOk("select zip z, id C, id D from functional.testtbl order by z, C, d");
@@ -2500,10 +2561,10 @@ public class AnalyzeStmtsTest extends AnalyzerTest {
// ambiguous alias causes error
AnalysisError("select string_col a, int_col a from " +
"functional.alltypessmall order by a limit 1",
- "Column 'a' in ORDER BY clause is ambiguous");
+ "ORDER BY: ambiguous alias: 'a'");
AnalysisError("select string_col a, int_col A from " +
"functional.alltypessmall order by a limit 1",
- "Column 'a' in ORDER BY clause is ambiguous");
+ "ORDER BY: ambiguous alias: 'a'");
// Test if an ignored order by produces the expected warning.
AnalyzesOk("select * from (select * from functional.alltypes order by int_col) A",
@@ -2666,12 +2727,12 @@ public class AnalyzeStmtsTest extends AnalyzerTest {
// Invalid ordinal in order by.
AnalysisError("(select int_col from functional.alltypes) " +
"union (select int_col from functional.alltypessmall) order by 2",
- "ORDER BY: ordinal exceeds number of items in select list: 2");
+ "ORDER BY: ordinal exceeds the number of items in the SELECT list: 2");
// Ambiguous order by.
AnalysisError("(select int_col a, string_col a from functional.alltypes) " +
"union (select int_col a, string_col a " +
"from functional.alltypessmall) order by a",
- "Column 'a' in ORDER BY clause is ambiguous");
+ "ORDER BY: ambiguous alias: 'a'");
// Ambiguous alias in the second union operand should work.
AnalyzesOk("(select int_col a, string_col b from functional.alltypes) " +
"union (select int_col a, string_col a " +
@@ -2680,7 +2741,7 @@ public class AnalyzeStmtsTest extends AnalyzerTest {
// (the corresponding in exprs in the other operand are different)
AnalysisError("select int_col a, int_col a from functional.alltypes " +
"union all (select 1, bigint_col from functional.alltypessmall) order by a",
- "Column 'a' in ORDER BY clause is ambiguous");
+ "ORDER BY: ambiguous alias: 'a'");
// Column labels are inherited from first select block.
// Order by references an invalid column
http://git-wip-us.apache.org/repos/asf/impala/blob/d51aa522/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java
index 6e19189..2a3d274 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java
@@ -1140,11 +1140,15 @@ public class AnalyzeSubqueriesTest extends AnalyzerTest {
}
@Test
- public void TestSubqueries() throws AnalysisException {
+ public void testColResolution() throws AnalysisException {
// Test resolution of column references inside subqueries.
// Correlated column references can be qualified or unqualified.
AnalyzesOk("select * from functional.jointbl t where exists " +
"(select id from functional.alltypes where id = test_id and id = t.test_id)");
+ }
+
+ @Test
+ public void testCoorelatedColRef() throws AnalysisException {
// Correlated column references are invalid outside of WHERE and ON clauses.
AnalysisError("select * from functional.jointbl t where exists " +
"(select t.test_id = id from functional.alltypes)",
@@ -1155,11 +1159,18 @@ public class AnalyzeSubqueriesTest extends AnalyzerTest {
AnalysisError("select * from functional.jointbl t where exists " +
"(select 1 from functional.alltypes order by t.test_id limit 1)",
"Could not resolve column/field reference: 't.test_id'");
+ }
+
+ @Test
+ public void testParentWildcard() throws AnalysisException {
// Star exprs cannot reference an alias from a parent block.
AnalysisError("select * from functional.jointbl t where exists " +
"(select t.* from functional.alltypes)",
"Could not resolve star expression: 't.*'");
+ }
+ @Test
+ public void testScalarSubqueries() throws AnalysisException {
// Scalar subquery check is done at runtime, not during analysis
for (String cmpOp: cmpOperators) {
AnalyzesOk(String.format(
@@ -1182,7 +1193,10 @@ public class AnalyzeSubqueriesTest extends AnalyzerTest {
"(select id from functional.alltypes t2 where t1.int_col = t2.int_col)",
"Unsupported correlated subquery with runtime scalar check: " +
"SELECT id FROM functional.alltypes t2 WHERE t1.int_col = t2.int_col");
+ }
+ @Test
+ public void testCorrelatedTableRef() throws AnalysisException {
// Test resolution of correlated table references inside subqueries. The testing
// here is rather basic, because the analysis goes through the same codepath
// as the analysis of correlated inline views, which are more thoroughly tested.
@@ -1192,7 +1206,10 @@ public class AnalyzeSubqueriesTest extends AnalyzerTest {
"where id in (select item cnt from t.int_array_col)");
AnalyzesOk("select id from functional.allcomplextypes t " +
"where id < (select count(*) from t.int_array_col)");
+ }
+ @Test
+ public void testAliases() throws AnalysisException {
// Test behavior of aliases in subqueries with correlated table references.
// Inner reference resolves to the base table, not the implicit parent alias.
AnalyzesOk("select id from functional.allcomplextypes t " +
@@ -1202,6 +1219,10 @@ public class AnalyzeSubqueriesTest extends AnalyzerTest {
AnalyzesOk("select id from functional.allcomplextypes " +
"where id < (select count(1) cnt from allcomplextypes)",
createAnalysisCtx("functional"));
+ }
+
+ @Test
+ public void testSubqueryTableRef() throws AnalysisException {
// Illegal correlated table references.
AnalysisError("select id from (select * from functional.alltypestiny) t " +
"where t.int_col = (select count(*) from t)",
@@ -1233,7 +1254,10 @@ public class AnalyzeSubqueriesTest extends AnalyzerTest {
// "'v1.arr' correlated with an outer block as well as an " +
// "uncorrelated one 'functional.alltypestiny':\n" +
// "SELECT item FROM v1.arr, functional.alltypestiny");
+ }
+ @Test
+ public void testExists() throws AnalysisException {
// EXISTS, IN and aggregate subqueries
AnalyzesOk("select * from functional.alltypes t where exists " +
"(select * from functional.alltypesagg a where a.int_col = " +
@@ -1258,7 +1282,10 @@ public class AnalyzeSubqueriesTest extends AnalyzerTest {
"(select id from functional.alltypessmall s where exists (select " +
"* from functional.alltypestiny a where a.int_col = s.int_col and " +
"a.bigint_col < 10)))");
+ }
+ @Test
+ public void testInsertSelect() throws AnalysisException {
// INSERT SELECT
AnalyzesOk("insert into functional.alltypessmall partition (year, month) " +
"select * from functional.alltypes where id in (select id from " +
@@ -1271,7 +1298,10 @@ public class AnalyzeSubqueriesTest extends AnalyzerTest {
AnalyzesOk("insert into functional.alltypessmall partition (year, month) " +
"select * from functional.alltypestiny where id = (select 1) " +
"union select * from functional.alltypestiny where id = (select 2)");
+ }
+ @Test
+ public void testUpsert() throws AnalysisException {
// UPSERT
AnalyzesOk("upsert into functional_kudu.testtbl select * from " +
"functional_kudu.testtbl where id in (select id from functional_kudu.testtbl " +
@@ -1279,7 +1309,10 @@ public class AnalyzeSubqueriesTest extends AnalyzerTest {
AnalyzesOk("upsert into functional_kudu.testtbl select * from " +
"functional_kudu.testtbl union select bigint_col, string_col, int_col from " +
"functional.alltypes");
+ }
+ @Test
+ public void testCtasSubquery() throws AnalysisException {
// CTAS with correlated subqueries
AnalyzesOk("create table functional.test_tbl as select * from " +
"functional.alltypes t where t.id in (select id from functional.alltypesagg " +
@@ -1289,7 +1322,10 @@ public class AnalyzeSubqueriesTest extends AnalyzerTest {
AnalyzesOk("create table functional.test_tbl as " +
"select * from functional.alltypestiny where id = (select 1) " +
"union select * from functional.alltypestiny where id = (select 2)");
+ }
+ @Test
+ public void TestIllegalSubquery() throws AnalysisException {
// Predicate with a child subquery in the HAVING clause
AnalysisError("select id, count(*) from functional.alltypestiny t group by " +
"id having count(*) > (select count(*) from functional.alltypesagg)",
@@ -1312,7 +1348,10 @@ public class AnalyzeSubqueriesTest extends AnalyzerTest {
AnalysisError("select id from functional.alltypestiny " +
"order by (select int_col from functional.alltypestiny)",
"Subqueries are not supported in the ORDER BY clause.");
+ }
+ @Test
+ public void testInlineView() throws AnalysisException {
// Subquery with an inline view
AnalyzesOk("select id from functional.alltypestiny t where exists " +
"(select * from (select id, int_col from functional.alltypesagg) a where " +
@@ -1324,11 +1363,17 @@ public class AnalyzeSubqueriesTest extends AnalyzerTest {
// Same view referenced in both the inner and outer block
AnalyzesOk("select * from functional.alltypes_view a where exists " +
"(select * from functional.alltypes_view b where a.id = b.id)");
+ }
+ @Test
+ public void testTableRefCollection() throws AnalysisException {
// Subquery with collection table ref.
AnalyzesOk("select int_col from functional.alltypes where int_col < " +
"(select count(a.item) from functional.allcomplextypes t, t.int_array_col a)");
+ }
+ @Test
+ public void testUnionWithSubquery() throws AnalysisException {
// Union query with subqueries
AnalyzesOk("select * from functional.alltypes where id = " +
"(select max(id) from functional.alltypestiny) union " +
@@ -1350,13 +1395,19 @@ public class AnalyzeSubqueriesTest extends AnalyzerTest {
"SELECT id FROM functional.alltypesagg)");
AnalysisError("select * from functional.alltypes where exists (values(1))",
"A subquery must contain a single select block: (VALUES(1))");
+ }
+ @Test
+ public void testSubqueryInLimit() throws AnalysisException {
// Subquery in LIMIT
AnalysisError("select * from functional.alltypes limit " +
"(select count(*) from functional.alltypesagg)",
"LIMIT expression must be a constant expression: " +
"(SELECT count(*) FROM functional.alltypesagg)");
+ }
+ @Test
+ public void testSubqueryExprs() throws AnalysisException {
// NOT predicates in conjunction with subqueries
AnalyzesOk("select * from functional.alltypes t where t.id not in " +
"(select id from functional.alltypesagg g where g.bool_col = false) " +
@@ -1370,7 +1421,10 @@ public class AnalyzeSubqueriesTest extends AnalyzerTest {
// IS NULL with a BinaryPredicate that contains a subquery
AnalyzesOk("select * from functional.alltypestiny where (id = " +
"(select max(id) from functional.alltypessmall)) is null");
+ }
+ @Test
+ public void testBetween() throws AnalysisException {
// between predicates with subqueries
AnalyzesOk("select * from functional.alltypestiny where " +
"(select avg(id) from functional.alltypesagg where bool_col = true) " +
@@ -1400,7 +1454,10 @@ public class AnalyzeSubqueriesTest extends AnalyzerTest {
AnalyzesOk("select * from functional.alltypestiny a where " +
"double_col between cast(1 as double) and cast(10 as double) and " +
"exists (select 1 from functional.alltypessmall b where a.id = b.id)");
+ }
+ @Test
+ public void testWhereSubqueries() throws AnalysisException {
AnalyzesOk("select count(1) from functional.alltypes " +
"where (select int_col > 10 from functional.alltypes)");
AnalyzesOk("select count(1) from functional.alltypes " +