From ff0de21fc523614e2ede451328ddc9f16e665b72 Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Mon, 5 Mar 2018 21:56:35 -0500 Subject: [PATCH] SQL: Fix assumption that AND, OR have two arguments. (#5470) Calcite can deliver an AND or OR operator with > 2 arguments. Fixes #5468. --- .../expression/BinaryOperatorConversion.java | 19 +++++++---- .../druid/sql/calcite/CalciteQueryTest.java | 33 +++++++++++++++++++ 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/sql/src/main/java/io/druid/sql/calcite/expression/BinaryOperatorConversion.java b/sql/src/main/java/io/druid/sql/calcite/expression/BinaryOperatorConversion.java index bf4f069e832..deb50c3bf7e 100644 --- a/sql/src/main/java/io/druid/sql/calcite/expression/BinaryOperatorConversion.java +++ b/sql/src/main/java/io/druid/sql/calcite/expression/BinaryOperatorConversion.java @@ -19,6 +19,7 @@ package io.druid.sql.calcite.expression; +import com.google.common.base.Joiner; import io.druid.java.util.common.ISE; import io.druid.java.util.common.StringUtils; import io.druid.sql.calcite.planner.PlannerContext; @@ -26,15 +27,17 @@ import io.druid.sql.calcite.table.RowSignature; import org.apache.calcite.rex.RexNode; import org.apache.calcite.sql.SqlOperator; +import java.util.stream.Collectors; + public class BinaryOperatorConversion implements SqlOperatorConversion { private final SqlOperator operator; - private final String druidOperator; + private final Joiner joiner; public BinaryOperatorConversion(final SqlOperator operator, final String druidOperator) { this.operator = operator; - this.druidOperator = druidOperator; + this.joiner = Joiner.on(" " + druidOperator + " "); } @Override @@ -55,16 +58,18 @@ public class BinaryOperatorConversion implements SqlOperatorConversion rowSignature, rexNode, operands -> { - if (operands.size() != 2) { + if (operands.size() < 2) { throw new ISE("WTF?! Got binary operator[%s] with %s args?", operator.getName(), operands.size()); } return DruidExpression.fromExpression( StringUtils.format( - "(%s %s %s)", - operands.get(0).getExpression(), - druidOperator, - operands.get(1).getExpression() + "(%s)", + joiner.join( + operands.stream() + .map(DruidExpression::getExpression) + .collect(Collectors.toList()) + ) ) ); } diff --git a/sql/src/test/java/io/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/io/druid/sql/calcite/CalciteQueryTest.java index 52967f9d917..62a5403e4db 100644 --- a/sql/src/test/java/io/druid/sql/calcite/CalciteQueryTest.java +++ b/sql/src/test/java/io/druid/sql/calcite/CalciteQueryTest.java @@ -1597,6 +1597,39 @@ public class CalciteQueryTest extends CalciteTestBase ); } + @Test + public void testGroupByCaseWhenOfTripleAnd() throws Exception + { + testQuery( + "SELECT\n" + + " CASE WHEN m1 > 1 AND m1 < 5 AND cnt = 1 THEN 'x' ELSE NULL END," + + " COUNT(*)\n" + + "FROM druid.foo\n" + + "GROUP BY 1", + ImmutableList.of( + GroupByQuery.builder() + .setDataSource(CalciteTests.DATASOURCE1) + .setInterval(QSS(Filtration.eternity())) + .setGranularity(Granularities.ALL) + .setVirtualColumns( + EXPRESSION_VIRTUAL_COLUMN( + "d0:v", + "case_searched(((\"m1\" > 1) && (\"m1\" < 5) && (\"cnt\" == 1)),'x','')", + ValueType.STRING + ) + ) + .setDimensions(DIMS(new DefaultDimensionSpec("d0:v", "d0"))) + .setAggregatorSpecs(AGGS(new CountAggregatorFactory("a0"))) + .setContext(QUERY_CONTEXT_DEFAULT) + .build() + ), + ImmutableList.of( + new Object[]{"", 3L}, + new Object[]{"x", 3L} + ) + ); + } + @Test public void testNullEmptyStringEquality() throws Exception {