You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2017/01/25 21:25:18 UTC
[4/9] calcite git commit: [CALCITE-1535] Give error if column
referenced in ORDER BY is ambiguous (Zhen Wang)
[CALCITE-1535] Give error if column referenced in ORDER BY is ambiguous (Zhen Wang)
Close apache/calcite#359
Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/c3288a14
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/c3288a14
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/c3288a14
Branch: refs/heads/master
Commit: c3288a1443f1934db6e081ef691c251ba063edc8
Parents: 0f45481
Author: zhen wang <zi...@gmail.com>
Authored: Sat Jan 21 19:18:42 2017 +0800
Committer: Julian Hyde <jh...@apache.org>
Committed: Wed Jan 25 11:00:12 2017 -0800
----------------------------------------------------------------------
.../calcite/sql/validate/OrderByScope.java | 26 +++++++-
.../apache/calcite/test/SqlValidatorTest.java | 64 +++++++++++++++-----
2 files changed, 72 insertions(+), 18 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/calcite/blob/c3288a14/core/src/main/java/org/apache/calcite/sql/validate/OrderByScope.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/validate/OrderByScope.java b/core/src/main/java/org/apache/calcite/sql/validate/OrderByScope.java
index 117390f..e4256c4 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/OrderByScope.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/OrderByScope.java
@@ -25,6 +25,8 @@ import org.apache.calcite.sql.SqlSelect;
import java.util.List;
+import static org.apache.calcite.util.Static.RESOURCE;
+
/**
* Represents the name-resolution context for expressions in an ORDER BY clause.
*
@@ -69,14 +71,20 @@ public class OrderByScope extends DelegatingScope {
// If it's a simple identifier, look for an alias.
if (identifier.isSimple()
&& validator.getConformance().isSortByAlias()) {
- String name = identifier.names.get(0);
+ final String name = identifier.names.get(0);
final SqlValidatorNamespace selectNs =
validator.getNamespace(select);
final RelDataType rowType = selectNs.getRowType();
final SqlNameMatcher nameMatcher = validator.catalogReader.nameMatcher();
final RelDataTypeField field = nameMatcher.field(rowType, name);
- if (field != null && !field.isDynamicStar()) {
+ final int aliasCount = aliasCount(nameMatcher, name);
+ if (aliasCount > 1) {
+ // More than one column has this alias.
+ throw validator.newValidationError(identifier,
+ RESOURCE.columnAmbiguous(name));
+ }
+ if (field != null && !field.isDynamicStar() && aliasCount == 1) {
// if identifier is resolved to a dynamic star, use super.fullyQualify() for such case.
return SqlQualified.create(this, 1, selectNs, identifier);
}
@@ -84,6 +92,20 @@ public class OrderByScope extends DelegatingScope {
return super.fullyQualify(identifier);
}
+ /** Returns the number of columns in the SELECT clause that have {@code name}
+ * as their implicit (e.g. {@code t.name}) or explicit (e.g.
+ * {@code t.c as name}) alias. */
+ private int aliasCount(SqlNameMatcher nameMatcher, String name) {
+ int n = 0;
+ for (SqlNode s : select.getSelectList()) {
+ final String alias = SqlValidatorUtil.getAlias(s, -1);
+ if (alias != null && nameMatcher.matches(alias, name)) {
+ n++;
+ }
+ }
+ return n;
+ }
+
public RelDataType resolveColumn(String name, SqlNode ctx) {
final SqlValidatorNamespace selectNs = validator.getNamespace(select);
final RelDataType rowType = selectNs.getRowType();
http://git-wip-us.apache.org/repos/asf/calcite/blob/c3288a14/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
index 898ce80..6c7ccd7 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
@@ -4430,6 +4430,36 @@ public class SqlValidatorTest extends SqlValidatorTestCase {
.fails("Column 'DEPTNO' is ambiguous");
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-1535">[CALCITE-1535]
+ * Give error if column referenced in ORDER BY is ambiguous</a>. */
+ @Test public void testOrderByColumn() {
+ sql("select emp.deptno from emp, dept order by emp.deptno")
+ .ok();
+ // Not ambiguous. There are two columns which could be referenced as
+ // "deptno", but the one in the SELECT clause takes priority.
+ sql("select emp.deptno from emp, dept order by deptno")
+ .ok();
+ sql("select emp.deptno as deptno from emp, dept order by deptno")
+ .ok();
+ sql("select emp.empno as deptno from emp, dept order by deptno")
+ .ok();
+ sql("select emp.deptno as n, dept.deptno as n from emp, dept order by ^n^")
+ .fails("Column 'N' is ambiguous");
+ sql("select emp.empno as deptno, dept.deptno from emp, dept\n"
+ + "order by ^deptno^")
+ .fails("Column 'DEPTNO' is ambiguous");
+ sql("select emp.empno as deptno, dept.deptno from emp, dept\n"
+ + "order by emp.deptno")
+ .ok();
+ sql("select emp.empno as deptno, dept.deptno from emp, dept order by 1, 2")
+ .ok();
+ sql("select empno as \"deptno\", deptno from emp order by deptno")
+ .ok();
+ sql("select empno as \"deptno\", deptno from emp order by \"deptno\"")
+ .ok();
+ }
+
@Test public void testWindowNegative() {
// Do not fail when window has negative size. Allow
final String negSize = null;
@@ -7375,9 +7405,9 @@ public class SqlValidatorTest extends SqlValidatorTestCase {
}
@Test public void testRewriteWithColumnReferenceExpansion() {
- // NOTE jvs 9-Apr-2007: This tests illustrates that
- // ORDER BY is still a special case. Update expected
- // output if that gets fixed in the future.
+ // The names in the ORDER BY clause are not qualified.
+ // This is because ORDER BY references columns in the SELECT clause
+ // in preference to columns in tables in the FROM clause.
SqlValidator validator = tester.getValidator();
validator.setIdentifierExpansion(true);
@@ -7395,25 +7425,27 @@ public class SqlValidatorTest extends SqlValidatorTestCase {
}
@Test public void testRewriteWithColumnReferenceExpansionAndFromAlias() {
- // NOTE jvs 9-Apr-2007: This tests illustrates that
- // ORDER BY is still a special case. Update expected
- // output if that gets fixed in the future.
+ // In the ORDER BY clause, 'ename' is not qualified but 'deptno' and 'sal'
+ // are. This is because 'ename' appears as an alias in the SELECT clause.
+ // 'sal' is qualified in the ORDER BY clause, so remains qualified.
SqlValidator validator = tester.getValidator();
validator.setIdentifierExpansion(true);
validator.setColumnReferenceExpansion(true);
tester.checkRewrite(
validator,
- "select name from (select * from dept)"
- + " where name = 'Moonracer' group by name"
- + " having sum(deptno) > 3 order by name",
- "SELECT `EXPR$0`.`NAME`\n"
- + "FROM (SELECT `DEPT`.`DEPTNO`, `DEPT`.`NAME`\n"
- + "FROM `CATALOG`.`SALES`.`DEPT` AS `DEPT`) AS `EXPR$0`\n"
- + "WHERE `EXPR$0`.`NAME` = 'Moonracer'\n"
- + "GROUP BY `EXPR$0`.`NAME`\n"
- + "HAVING SUM(`EXPR$0`.`DEPTNO`) > 3\n"
- + "ORDER BY `NAME`");
+ "select ename, sal from (select * from emp) as e"
+ + " where ename = 'Moonracer' group by ename, deptno, sal"
+ + " having sum(deptno) > 3 order by ename, deptno, e.sal",
+ "SELECT `E`.`ENAME`, `E`.`SAL`\n"
+ + "FROM (SELECT `EMP`.`EMPNO`, `EMP`.`ENAME`, `EMP`.`JOB`,"
+ + " `EMP`.`MGR`, `EMP`.`HIREDATE`, `EMP`.`SAL`, `EMP`.`COMM`,"
+ + " `EMP`.`DEPTNO`, `EMP`.`SLACKER`\n"
+ + "FROM `CATALOG`.`SALES`.`EMP` AS `EMP`) AS `E`\n"
+ + "WHERE `E`.`ENAME` = 'Moonracer'\n"
+ + "GROUP BY `E`.`ENAME`, `E`.`DEPTNO`, `E`.`SAL`\n"
+ + "HAVING SUM(`E`.`DEPTNO`) > 3\n"
+ + "ORDER BY `ENAME`, `E`.`DEPTNO`, `E`.`SAL`");
}
@Test public void testCoalesceWithoutRewrite() {