This is an automated email from the ASF dual-hosted git repository.
soumyava pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new 6784e9c507b Fix summary row issues in case postaggregations are
happening (#15232)
6784e9c507b is described below
commit 6784e9c507be43aee1b7596ee0275db717132a67
Author: Zoltan Haindrich <[email protected]>
AuthorDate: Wed Oct 25 05:33:59 2023 +0200
Fix summary row issues in case postaggregations are happening (#15232)
* fix-1/2
* add message v1
* extend test to cover for IOB issue
* move stuff around
* change message
* fix testcase string
* compute postaggs (thank you Clint!)
* enable feature for test
* ignore tests in msq
---------
Co-authored-by: Soumyava Das <[email protected]>
---
.../druid/msq/test/CalciteSelectQueryMSQTest.java | 12 ++++++++++++
.../org/apache/druid/query/groupby/GroupingEngine.java | 14 +++++++++++---
.../druid/query/groupby/GroupByQueryRunnerTest.java | 7 ++++++-
.../druid/sql/calcite/planner/CalcitePlanner.java | 3 ++-
.../druid/sql/calcite/planner/DruidSqlValidator.java | 17 ++++++++++++++++-
.../druid/sql/calcite/planner/PlannerFactory.java | 7 +++++--
.../org/apache/druid/sql/calcite/CalciteQueryTest.java | 11 +++++++++++
.../apache/druid/sql/calcite/CalciteSysQueryTest.java | 3 +++
8 files changed, 66 insertions(+), 8 deletions(-)
diff --git
a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/test/CalciteSelectQueryMSQTest.java
b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/test/CalciteSelectQueryMSQTest.java
index e9c54cfc54b..c83ec91f454 100644
---
a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/test/CalciteSelectQueryMSQTest.java
+++
b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/test/CalciteSelectQueryMSQTest.java
@@ -157,6 +157,18 @@ public class CalciteSelectQueryMSQTest extends
CalciteQueryTest
}
+ @Ignore
+ @Override
+ public void testUnSupportedNullsFirst()
+ {
+ }
+
+ @Ignore
+ @Override
+ public void testUnSupportedNullsLast()
+ {
+ }
+
/**
* Same query as {@link
CalciteQueryTest#testArrayAggQueryOnComplexDatatypes}. ARRAY_AGG is not
supported in MSQ currently.
* Once support is added, this test can be removed and msqCompatible() can
be added to the one in CalciteQueryTest.
diff --git
a/processing/src/main/java/org/apache/druid/query/groupby/GroupingEngine.java
b/processing/src/main/java/org/apache/druid/query/groupby/GroupingEngine.java
index b242ff98555..40c336f7a71 100644
---
a/processing/src/main/java/org/apache/druid/query/groupby/GroupingEngine.java
+++
b/processing/src/main/java/org/apache/druid/query/groupby/GroupingEngine.java
@@ -788,11 +788,19 @@ public class GroupingEngine
private static Iterator<ResultRow> summaryRowIterator(GroupByQuery q)
{
List<AggregatorFactory> aggSpec = q.getAggregatorSpecs();
- Object[] values = new Object[aggSpec.size()];
+ ResultRow resultRow =
ResultRow.create(q.getResultRowSizeWithPostAggregators());
for (int i = 0; i < aggSpec.size(); i++) {
- values[i] = aggSpec.get(i).factorize(new
AllNullColumnSelectorFactory()).get();
+ resultRow.set(i, aggSpec.get(i).factorize(new
AllNullColumnSelectorFactory()).get());
}
- return Collections.singleton(ResultRow.of(values)).iterator();
+ Map<String, Object> map = resultRow.toMap(q);
+ for (int i = 0; i < q.getPostAggregatorSpecs().size(); i++) {
+ final PostAggregator postAggregator = q.getPostAggregatorSpecs().get(i);
+ final Object value = postAggregator.compute(map);
+
+ resultRow.set(q.getResultRowPostAggregatorStart() + i, value);
+ map.put(postAggregator.getName(), value);
+ }
+ return Collections.singleton(resultRow).iterator();
}
}
diff --git
a/processing/src/test/java/org/apache/druid/query/groupby/GroupByQueryRunnerTest.java
b/processing/src/test/java/org/apache/druid/query/groupby/GroupByQueryRunnerTest.java
index c8f5aa7dfca..e9cd4d0c85e 100644
---
a/processing/src/test/java/org/apache/druid/query/groupby/GroupByQueryRunnerTest.java
+++
b/processing/src/test/java/org/apache/druid/query/groupby/GroupByQueryRunnerTest.java
@@ -12962,6 +12962,9 @@ public class GroupByQueryRunnerTest extends
InitializedNullHandlingTest
new FloatSumAggregatorFactory("idxFloat", "indexFloat"),
new DoubleSumAggregatorFactory("idxDouble", "index")
)
+ .setPostAggregatorSpecs(
+ ImmutableList.of(
+ new ExpressionPostAggregator("post", "idx * 2", null,
TestExprMacroTable.INSTANCE)))
.setGranularity(QueryRunnerTestHelper.ALL_GRAN)
.build();
@@ -12976,7 +12979,9 @@ public class GroupByQueryRunnerTest extends
InitializedNullHandlingTest
"idxFloat",
NullHandling.replaceWithDefault() ? 0.0 : null,
"idxDouble",
- NullHandling.replaceWithDefault() ? 0.0 : null
+ NullHandling.replaceWithDefault() ? 0.0 : null,
+ "post",
+ NullHandling.replaceWithDefault() ? 0L : null
)
);
diff --git
a/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalcitePlanner.java
b/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalcitePlanner.java
index 1abec772e31..2cda011848b 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalcitePlanner.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalcitePlanner.java
@@ -407,7 +407,8 @@ public class CalcitePlanner implements Planner, ViewExpander
opTab,
catalogReader,
getTypeFactory(),
- validatorConfig
+ validatorConfig,
+ context.unwrapOrThrow(PlannerContext.class)
);
}
diff --git
a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidSqlValidator.java
b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidSqlValidator.java
index bbc4f99a131..2844350241b 100644
---
a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidSqlValidator.java
+++
b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidSqlValidator.java
@@ -30,6 +30,8 @@ import org.apache.calcite.sql.SqlNode;
import org.apache.calcite.sql.SqlOperatorTable;
import org.apache.calcite.sql.parser.SqlParserPos;
import org.apache.calcite.sql.validate.SqlValidatorScope;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.sql.calcite.run.EngineFeature;
/**
* Druid extended SQL validator. (At present, it doesn't actually
@@ -37,19 +39,32 @@ import org.apache.calcite.sql.validate.SqlValidatorScope;
*/
class DruidSqlValidator extends BaseDruidSqlValidator
{
+ private final PlannerContext plannerContext;
+
protected DruidSqlValidator(
SqlOperatorTable opTab,
CalciteCatalogReader catalogReader,
JavaTypeFactory typeFactory,
- Config validatorConfig
+ Config validatorConfig,
+ PlannerContext plannerContext
)
{
super(opTab, catalogReader, typeFactory, validatorConfig);
+ this.plannerContext = plannerContext;
}
@Override
public void validateCall(SqlCall call, SqlValidatorScope scope)
{
+ if (call.getKind() == SqlKind.OVER) {
+ if (!plannerContext.featureAvailable(EngineFeature.WINDOW_FUNCTIONS)) {
+ throw buildCalciteContextException(
+ StringUtils.format(
+ "The query contains window functions; To run these window
functions, enable [%s] in query context.",
+ EngineFeature.WINDOW_FUNCTIONS),
+ call);
+ }
+ }
if (call.getKind() == SqlKind.NULLS_FIRST) {
SqlNode op0 = call.getOperandList().get(0);
if (op0.getKind() == SqlKind.DESCENDING) {
diff --git
a/sql/src/main/java/org/apache/druid/sql/calcite/planner/PlannerFactory.java
b/sql/src/main/java/org/apache/druid/sql/calcite/planner/PlannerFactory.java
index 8cd7151dfb7..a5cdc028e7f 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/PlannerFactory.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/PlannerFactory.java
@@ -182,9 +182,12 @@ public class PlannerFactory extends PlannerToolbox
return DruidConformance.instance();
}
};
- } else {
- return null;
}
+ if (aClass.equals(PlannerContext.class)) {
+ return (C) plannerContext;
+ }
+
+ return null;
}
});
diff --git
a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
index 482b02f61ab..948eea4c1ff 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
@@ -14293,6 +14293,17 @@ public class CalciteQueryTest extends
BaseCalciteQueryTest
assertThat(e, invalidSqlIs("ASCENDING ordering with NULLS LAST is not
supported! (line [1], column [41])"));
}
+ @Test
+ public void testWindowingErrorWithoutFeatureFlag()
+ {
+ DruidException e = assertThrows(DruidException.class, () -> testBuilder()
+ .queryContext(ImmutableMap.of(PlannerContext.CTX_ENABLE_WINDOW_FNS,
false))
+ .sql("SELECT dim1,ROW_NUMBER() OVER () from druid.foo")
+ .run());
+
+ assertThat(e, invalidSqlIs("The query contains window functions; To run
these window functions, enable [WINDOW_FUNCTIONS] in query context. (line [1],
column [13])"));
+ }
+
@Test
public void testInGroupByLimitOutGroupByOrderBy()
{
diff --git
a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteSysQueryTest.java
b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteSysQueryTest.java
index 0ebd26e6553..5b0a5dd82e3 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteSysQueryTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteSysQueryTest.java
@@ -20,8 +20,10 @@
package org.apache.druid.sql.calcite;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import org.apache.druid.sql.calcite.NotYetSupported.Modes;
import org.apache.druid.sql.calcite.NotYetSupported.NotYetSupportedProcessor;
+import org.apache.druid.sql.calcite.planner.PlannerContext;
import org.junit.Rule;
import org.junit.Test;
@@ -53,6 +55,7 @@ public class CalciteSysQueryTest extends BaseCalciteQueryTest
msqIncompatible();
testBuilder()
+ .queryContext(ImmutableMap.of(PlannerContext.CTX_ENABLE_WINDOW_FNS,
true))
.sql("select datasource, sum(duration) over () from sys.tasks group by
datasource")
.expectedResults(ImmutableList.of(
new Object[]{"foo", 11L},
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
