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 2020/01/14 02:02:16 UTC
[calcite] 02/02: [CALCITE-3593] JDBC adapter generates incorrect
HAVING clause for BigQuery (Jin Xing)
This is an automated email from the ASF dual-hosted git repository.
jhyde pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 3621b61ea0bca953652a2e6d1838e5a296c6a007
Author: jx158167 <jx...@antfin.com>
AuthorDate: Thu Jan 2 19:00:16 2020 +0800
[CALCITE-3593] JDBC adapter generates incorrect HAVING clause for BigQuery (Jin Xing)
BigQuery recognizes aliases in its HAVING clause (i.e. its
SqlConformance.isHavingAlias() returns true) and so for this case a new
sub-query is required.
Add SqlConformanceEnum.BIG_QUERY, which has similar behavior to PRAGMATIC_2003
except that isGroupByAlias() and isHavingAlias() return true.
Close apache/calcite#1716
---
.../apache/calcite/rel/rel2sql/SqlImplementor.java | 7 +++
.../java/org/apache/calcite/sql/SqlDialect.java | 2 +
.../calcite/sql/dialect/BigQuerySqlDialect.java | 8 ++-
.../calcite/sql/validate/SqlConformance.java | 19 ++++---
.../calcite/sql/validate/SqlConformanceEnum.java | 10 +++-
.../rel/rel2sql/RelToSqlConverterStructsTest.java | 5 +-
.../calcite/rel/rel2sql/RelToSqlConverterTest.java | 59 ++++++++++++++++++----
7 files changed, 86 insertions(+), 24 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java b/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java
index dc26f0d..16017bd 100644
--- a/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java
+++ b/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java
@@ -24,6 +24,7 @@ import org.apache.calcite.rel.core.Aggregate;
import org.apache.calcite.rel.core.AggregateCall;
import org.apache.calcite.rel.core.CorrelationId;
import org.apache.calcite.rel.core.JoinRelType;
+import org.apache.calcite.rel.core.Project;
import org.apache.calcite.rel.core.Window;
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeField;
@@ -1326,6 +1327,12 @@ public abstract class SqlImplementor {
}
}
+ if (rel instanceof Project
+ && this.clauses.contains(Clause.HAVING)
+ && dialect.getConformance().isHavingAlias()) {
+ return true;
+ }
+
if (rel instanceof Aggregate) {
final Aggregate agg = (Aggregate) rel;
final boolean hasNestedAgg = hasNestedAggregations(agg);
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlDialect.java b/core/src/main/java/org/apache/calcite/sql/SqlDialect.java
index 590cbba..2eef2dc 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlDialect.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlDialect.java
@@ -1117,6 +1117,8 @@ public class SqlDialect {
case UNKNOWN:
case CALCITE:
return SqlConformanceEnum.DEFAULT;
+ case BIG_QUERY:
+ return SqlConformanceEnum.BIG_QUERY;
case MYSQL:
return SqlConformanceEnum.MYSQL_5;
case ORACLE:
diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java
index c7332c3..eaa6ebf 100644
--- a/core/src/main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java
+++ b/core/src/main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java
@@ -221,11 +221,9 @@ public class BigQuerySqlDialect extends SqlDialect {
final SqlWriter.Frame trimFrame = writer.startFunCall(operatorName);
call.operand(2).unparse(writer, leftPrec, rightPrec);
- /**
- * If the trimmed character is non space character then add it to the target sql.
- * eg: TRIM(BOTH 'A' from 'ABCD'
- * Output Query: TRIM('ABC', 'A')
- * */
+ // If the trimmed character is a non-space character, add it to the target SQL.
+ // eg: TRIM(BOTH 'A' from 'ABCD'
+ // Output Query: TRIM('ABC', 'A')
if (!valueToTrim.toValue().matches("\\s+")) {
writer.literal(",");
call.operand(1).unparse(writer, leftPrec, rightPrec);
diff --git a/core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java b/core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java
index e8ee2f8..f8cbdeb 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java
@@ -74,6 +74,7 @@ public interface SqlConformance {
* <p>Among the built-in conformance levels, true in
* {@link SqlConformanceEnum#BABEL},
* {@link SqlConformanceEnum#LENIENT},
+ * {@link SqlConformanceEnum#BIG_QUERY},
* {@link SqlConformanceEnum#MYSQL_5};
* false otherwise.
*/
@@ -98,6 +99,7 @@ public interface SqlConformance {
* <p>Among the built-in conformance levels, true in
* {@link SqlConformanceEnum#BABEL},
* {@link SqlConformanceEnum#LENIENT},
+ * {@link SqlConformanceEnum#BIG_QUERY},
* {@link SqlConformanceEnum#MYSQL_5};
* false otherwise.
*/
@@ -410,19 +412,24 @@ public interface SqlConformance {
boolean allowPluralTimeUnits();
/**
- * Whether to allow qualifying common column in NATURAL join
- * or USING clause.
+ * Whether to allow a qualified common column in a query that has a
+ * NATURAL join or a join with a USING clause.
*
- * <p>For example, consider the query
+ * <p>For example, in the query
*
- * <blockquote><pre>SELECT emp.deptno emp join dept using (deptno)</pre></blockquote>
+ * <blockquote><pre>SELECT emp.deptno
+ * FROM emp
+ * JOIN dept USING (deptno)</pre></blockquote>
*
- * <p> {@code deptno} is the common column. Qualifying common column,
+ * <p>{@code deptno} is the common column. A qualified common column
* such as {@code emp.deptno} is not allowed in Oracle, but is allowed
* in PostgreSQL.
*
* <p>Among the built-in conformance levels, false in
- * {@link SqlConformanceEnum#ORACLE_10};
+ * {@link SqlConformanceEnum#STRICT_92},
+ * {@link SqlConformanceEnum#STRICT_99},
+ * {@link SqlConformanceEnum#STRICT_2003},
+ * {@link SqlConformanceEnum#ORACLE_10},
* {@link SqlConformanceEnum#ORACLE_12};
* true otherwise.
*/
diff --git a/core/src/main/java/org/apache/calcite/sql/validate/SqlConformanceEnum.java b/core/src/main/java/org/apache/calcite/sql/validate/SqlConformanceEnum.java
index be6f83e..106cac0 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/SqlConformanceEnum.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlConformanceEnum.java
@@ -45,6 +45,10 @@ public enum SqlConformanceEnum implements SqlConformance {
PRAGMATIC_99,
/** Conformance value that instructs Calcite to use SQL semantics
+ * consistent with BigQuery. */
+ BIG_QUERY,
+
+ /** Conformance value that instructs Calcite to use SQL semantics
* consistent with MySQL version 5.x. */
MYSQL_5,
@@ -84,6 +88,7 @@ public enum SqlConformanceEnum implements SqlConformance {
switch (this) {
case BABEL:
case LENIENT:
+ case BIG_QUERY:
case MYSQL_5:
return true;
default:
@@ -106,6 +111,7 @@ public enum SqlConformanceEnum implements SqlConformance {
switch (this) {
case BABEL:
case LENIENT:
+ case BIG_QUERY:
case MYSQL_5:
return true;
default:
@@ -118,6 +124,7 @@ public enum SqlConformanceEnum implements SqlConformance {
case DEFAULT:
case BABEL:
case LENIENT:
+ case BIG_QUERY:
case MYSQL_5:
case ORACLE_10:
case ORACLE_12:
@@ -218,6 +225,7 @@ public enum SqlConformanceEnum implements SqlConformance {
case LENIENT:
case PRAGMATIC_99:
case PRAGMATIC_2003:
+ case BIG_QUERY:
return true;
default:
return false;
@@ -282,6 +290,7 @@ public enum SqlConformanceEnum implements SqlConformance {
switch (this) {
case PRAGMATIC_99:
case PRAGMATIC_2003:
+ case BIG_QUERY:
case MYSQL_5:
case ORACLE_10:
case ORACLE_12:
@@ -327,5 +336,4 @@ public enum SqlConformanceEnum implements SqlConformance {
}
}
-
}
diff --git a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterStructsTest.java b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterStructsTest.java
index feb6619..0c863fe 100644
--- a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterStructsTest.java
+++ b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterStructsTest.java
@@ -34,6 +34,7 @@ import org.apache.calcite.schema.Table;
import org.apache.calcite.sql.SqlCall;
import org.apache.calcite.sql.SqlNode;
import org.apache.calcite.sql.dialect.CalciteSqlDialect;
+import org.apache.calcite.sql.parser.SqlParser;
import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.calcite.util.ImmutableBitSet;
@@ -180,8 +181,8 @@ public class RelToSqlConverterStructsTest {
private RelToSqlConverterTest.Sql sql(String sql) {
return new RelToSqlConverterTest.Sql(ROOT_SCHEMA, sql,
- CalciteSqlDialect.DEFAULT, RelToSqlConverterTest.DEFAULT_REL_CONFIG,
- ImmutableList.of());
+ CalciteSqlDialect.DEFAULT, SqlParser.Config.DEFAULT,
+ RelToSqlConverterTest.DEFAULT_REL_CONFIG, ImmutableList.of());
}
@Test public void testNestedSchemaSelectStar() {
diff --git a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
index b9025ab..d76aaf9 100644
--- a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
+++ b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
@@ -107,8 +107,8 @@ public class RelToSqlConverterTest {
/** Initiates a test case with a given SQL query. */
private Sql sql(String sql) {
return new Sql(CalciteAssert.SchemaSpec.JDBC_FOODMART, sql,
- CalciteSqlDialect.DEFAULT, DEFAULT_REL_CONFIG,
- ImmutableList.of());
+ CalciteSqlDialect.DEFAULT, SqlParser.Config.DEFAULT,
+ DEFAULT_REL_CONFIG, ImmutableList.of());
}
private static Planner getPlanner(List<RelTraitDef> traitDefs,
@@ -1008,7 +1008,7 @@ public class RelToSqlConverterTest {
/** Test case for
* <a href="https://issues.apache.org/jira/browse/CALCITE-3663">[CALCITE-3663]
- * Support for TRIM function in Bigquery dialect</a>. */
+ * Support for TRIM function in BigQuery dialect</a>. */
@Test public void testHiveAndBqTrim() {
final String query = "SELECT TRIM(' str ')\n"
@@ -4723,37 +4723,72 @@ public class RelToSqlConverterTest {
sql(query).ok(expected);
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-3593">[CALCITE-3593]
+ * RelToSqlConverter changes target of ambiguous HAVING clause with a Project
+ * on Filter on Aggregate</a>. */
+ @Test public void testBigQueryHaving() {
+ final String sql = ""
+ + "SELECT \"DEPTNO\" - 10 \"DEPTNO\"\n"
+ + "FROM \"EMP\"\n"
+ + "GROUP BY \"DEPTNO\"\n"
+ + "HAVING \"DEPTNO\" > 0";
+ final String expected = ""
+ + "SELECT DEPTNO - 10 AS DEPTNO\n"
+ + "FROM (SELECT DEPTNO\n"
+ + "FROM SCOTT.EMP\n"
+ + "GROUP BY DEPTNO\n"
+ + "HAVING DEPTNO > 0) AS t1";
+
+ // Parse the input SQL with PostgreSQL dialect,
+ // in which "isHavingAlias" is false.
+ final SqlParser.Config parserConfig =
+ PostgresqlSqlDialect.DEFAULT.configureParser(SqlParser.configBuilder())
+ .build();
+
+ // Convert rel node to SQL with BigQuery dialect,
+ // in which "isHavingAlias" is true.
+ sql(sql)
+ .parserConfig(parserConfig)
+ .schema(CalciteAssert.SchemaSpec.JDBC_SCOTT)
+ .withBigQuery()
+ .ok(expected);
+ }
+
/** Fluid interface to run tests. */
static class Sql {
private final SchemaPlus schema;
private final String sql;
private final SqlDialect dialect;
private final List<Function<RelNode, RelNode>> transforms;
+ private final SqlParser.Config parserConfig;
private final SqlToRelConverter.Config config;
Sql(CalciteAssert.SchemaSpec schemaSpec, String sql, SqlDialect dialect,
- SqlToRelConverter.Config config,
+ SqlParser.Config parserConfig, SqlToRelConverter.Config config,
List<Function<RelNode, RelNode>> transforms) {
final SchemaPlus rootSchema = Frameworks.createRootSchema(true);
this.schema = CalciteAssert.addSchema(rootSchema, schemaSpec);
this.sql = sql;
this.dialect = dialect;
this.transforms = ImmutableList.copyOf(transforms);
+ this.parserConfig = parserConfig;
this.config = config;
}
Sql(SchemaPlus schema, String sql, SqlDialect dialect,
- SqlToRelConverter.Config config,
+ SqlParser.Config parserConfig, SqlToRelConverter.Config config,
List<Function<RelNode, RelNode>> transforms) {
this.schema = schema;
this.sql = sql;
this.dialect = dialect;
this.transforms = ImmutableList.copyOf(transforms);
+ this.parserConfig = parserConfig;
this.config = config;
}
Sql dialect(SqlDialect dialect) {
- return new Sql(schema, sql, dialect, config, transforms);
+ return new Sql(schema, sql, dialect, parserConfig, config, transforms);
}
Sql withCalcite() {
@@ -4866,12 +4901,16 @@ public class RelToSqlConverterTest {
return dialect(oracleSqlDialect);
}
+ Sql parserConfig(SqlParser.Config parserConfig) {
+ return new Sql(schema, sql, dialect, parserConfig, config, transforms);
+ }
+
Sql config(SqlToRelConverter.Config config) {
- return new Sql(schema, sql, dialect, config, transforms);
+ return new Sql(schema, sql, dialect, parserConfig, config, transforms);
}
Sql optimize(final RuleSet ruleSet, final RelOptPlanner relOptPlanner) {
- return new Sql(schema, sql, dialect, config,
+ return new Sql(schema, sql, dialect, parserConfig, config,
FlatLists.append(transforms, r -> {
Program program = Programs.of(ruleSet);
final RelOptPlanner p =
@@ -4902,7 +4941,7 @@ public class RelToSqlConverterTest {
String exec() {
final Planner planner =
- getPlanner(null, SqlParser.Config.DEFAULT, schema, config);
+ getPlanner(null, parserConfig, schema, config);
try {
SqlNode parse = planner.parse(sql);
SqlNode validate = planner.validate(parse);
@@ -4917,7 +4956,7 @@ public class RelToSqlConverterTest {
}
public Sql schema(CalciteAssert.SchemaSpec schemaSpec) {
- return new Sql(schemaSpec, sql, dialect, config, transforms);
+ return new Sql(schemaSpec, sql, dialect, parserConfig, config, transforms);
}
}
}