This is an automated email from the ASF dual-hosted git repository.
siddteotia pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 3112cac380 [multistage][bugfix] fix group-by without agg call plan
failure (#9383)
3112cac380 is described below
commit 3112cac380218008ea50413eb9abeaef8ab814ce
Author: Rong Rong <[email protected]>
AuthorDate: Tue Sep 13 01:09:55 2022 -0700
[multistage][bugfix] fix group-by without agg call plan failure (#9383)
* add test
* add rel rule to allow distinct agg
Co-authored-by: Rong Rong <[email protected]>
---
.../rel/rules/PinotAggregateExchangeNodeInsertRule.java | 3 +--
.../calcite/rel/rules/PinotJoinExchangeNodeInsertRule.java | 3 +--
.../rel/rules/PinotLogicalSortFetchEliminationRule.java | 3 +--
.../java/org/apache/calcite/rel/rules/PinotRuleUtils.java | 9 +++++++++
.../calcite/rel/rules/PinotSortExchangeNodeInsertRule.java | 3 +--
.../main/java/org/apache/pinot/query/QueryEnvironment.java | 13 ++++++++-----
.../org/apache/pinot/query/QueryEnvironmentTestBase.java | 2 ++
.../org/apache/pinot/query/runtime/QueryRunnerTest.java | 4 ++++
8 files changed, 27 insertions(+), 13 deletions(-)
diff --git
a/pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotAggregateExchangeNodeInsertRule.java
b/pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotAggregateExchangeNodeInsertRule.java
index fba5f4fc12..5545798b48 100644
---
a/pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotAggregateExchangeNodeInsertRule.java
+++
b/pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotAggregateExchangeNodeInsertRule.java
@@ -33,7 +33,6 @@ import org.apache.calcite.rel.RelDistributions;
import org.apache.calcite.rel.RelNode;
import org.apache.calcite.rel.core.Aggregate;
import org.apache.calcite.rel.core.AggregateCall;
-import org.apache.calcite.rel.core.RelFactories;
import org.apache.calcite.rel.hint.RelHint;
import org.apache.calcite.rel.logical.LogicalAggregate;
import org.apache.calcite.rel.logical.LogicalExchange;
@@ -69,7 +68,7 @@ import
org.apache.pinot.query.planner.hints.PinotRelationalHints;
*/
public class PinotAggregateExchangeNodeInsertRule extends RelOptRule {
public static final PinotAggregateExchangeNodeInsertRule INSTANCE =
- new PinotAggregateExchangeNodeInsertRule(RelFactories.LOGICAL_BUILDER);
+ new
PinotAggregateExchangeNodeInsertRule(PinotRuleUtils.PINOT_REL_FACTORY);
private static final Set<SqlKind> SUPPORTED_AGG_KIND = ImmutableSet.of(
SqlKind.SUM, SqlKind.SUM0, SqlKind.MIN, SqlKind.MAX, SqlKind.COUNT);
diff --git
a/pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotJoinExchangeNodeInsertRule.java
b/pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotJoinExchangeNodeInsertRule.java
index 39af45cff7..dc623e3c8e 100644
---
a/pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotJoinExchangeNodeInsertRule.java
+++
b/pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotJoinExchangeNodeInsertRule.java
@@ -25,7 +25,6 @@ import org.apache.calcite.plan.RelOptRuleCall;
import org.apache.calcite.rel.RelDistributions;
import org.apache.calcite.rel.RelNode;
import org.apache.calcite.rel.core.Join;
-import org.apache.calcite.rel.core.RelFactories;
import org.apache.calcite.rel.hint.RelHint;
import org.apache.calcite.rel.logical.LogicalExchange;
import org.apache.calcite.rel.logical.LogicalJoin;
@@ -40,7 +39,7 @@ import
org.apache.pinot.query.planner.hints.PinotRelationalHints;
*/
public class PinotJoinExchangeNodeInsertRule extends RelOptRule {
public static final PinotJoinExchangeNodeInsertRule INSTANCE =
- new PinotJoinExchangeNodeInsertRule(RelFactories.LOGICAL_BUILDER);
+ new PinotJoinExchangeNodeInsertRule(PinotRuleUtils.PINOT_REL_FACTORY);
public PinotJoinExchangeNodeInsertRule(RelBuilderFactory factory) {
super(operand(LogicalJoin.class, any()), factory, null);
diff --git
a/pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotLogicalSortFetchEliminationRule.java
b/pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotLogicalSortFetchEliminationRule.java
index a85035b85b..e6a32c387b 100644
---
a/pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotLogicalSortFetchEliminationRule.java
+++
b/pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotLogicalSortFetchEliminationRule.java
@@ -20,7 +20,6 @@ package org.apache.calcite.rel.rules;
import org.apache.calcite.plan.RelOptRule;
import org.apache.calcite.plan.RelOptRuleCall;
-import org.apache.calcite.rel.core.RelFactories;
import org.apache.calcite.rel.core.Sort;
import org.apache.calcite.rel.logical.LogicalSort;
import org.apache.calcite.tools.RelBuilderFactory;
@@ -31,7 +30,7 @@ import org.apache.calcite.tools.RelBuilderFactory;
*/
public class PinotLogicalSortFetchEliminationRule extends RelOptRule {
public static final PinotLogicalSortFetchEliminationRule INSTANCE =
- new PinotLogicalSortFetchEliminationRule(RelFactories.LOGICAL_BUILDER);
+ new
PinotLogicalSortFetchEliminationRule(PinotRuleUtils.PINOT_REL_FACTORY);
public PinotLogicalSortFetchEliminationRule(RelBuilderFactory factory) {
super(operand(LogicalSort.class, any()), factory, null);
diff --git
a/pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotRuleUtils.java
b/pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotRuleUtils.java
index 97b1074b7f..d2634018ef 100644
---
a/pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotRuleUtils.java
+++
b/pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotRuleUtils.java
@@ -18,12 +18,21 @@
*/
package org.apache.calcite.rel.rules;
+import org.apache.calcite.plan.Contexts;
import org.apache.calcite.plan.hep.HepRelVertex;
import org.apache.calcite.rel.RelNode;
import org.apache.calcite.rel.core.Exchange;
+import org.apache.calcite.rel.core.RelFactories;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
public class PinotRuleUtils {
+ private static final RelBuilder.Config DEFAULT_CONFIG =
+
RelBuilder.Config.DEFAULT.withAggregateUnique(true).withPushJoinCondition(true);
+
+ public static final RelBuilderFactory PINOT_REL_FACTORY =
+ RelBuilder.proto(Contexts.of(RelFactories.DEFAULT_STRUCT,
DEFAULT_CONFIG));
private PinotRuleUtils() {
// do not instantiate.
diff --git
a/pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotSortExchangeNodeInsertRule.java
b/pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotSortExchangeNodeInsertRule.java
index c590f2b652..5e1d0fedb8 100644
---
a/pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotSortExchangeNodeInsertRule.java
+++
b/pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotSortExchangeNodeInsertRule.java
@@ -25,7 +25,6 @@ import org.apache.calcite.plan.hep.HepRelVertex;
import org.apache.calcite.rel.RelDistributions;
import org.apache.calcite.rel.RelNode;
import org.apache.calcite.rel.core.Exchange;
-import org.apache.calcite.rel.core.RelFactories;
import org.apache.calcite.rel.core.Sort;
import org.apache.calcite.rel.logical.LogicalExchange;
import org.apache.calcite.rel.logical.LogicalSort;
@@ -37,7 +36,7 @@ import org.apache.calcite.tools.RelBuilderFactory;
*/
public class PinotSortExchangeNodeInsertRule extends RelOptRule {
public static final PinotSortExchangeNodeInsertRule INSTANCE =
- new PinotSortExchangeNodeInsertRule(RelFactories.LOGICAL_BUILDER);
+ new PinotSortExchangeNodeInsertRule(PinotRuleUtils.PINOT_REL_FACTORY);
public PinotSortExchangeNodeInsertRule(RelBuilderFactory factory) {
super(operand(LogicalSort.class, any()), factory, null);
diff --git
a/pinot-query-planner/src/main/java/org/apache/pinot/query/QueryEnvironment.java
b/pinot-query-planner/src/main/java/org/apache/pinot/query/QueryEnvironment.java
index 1f84101e53..87156476e9 100644
---
a/pinot-query-planner/src/main/java/org/apache/pinot/query/QueryEnvironment.java
+++
b/pinot-query-planner/src/main/java/org/apache/pinot/query/QueryEnvironment.java
@@ -92,7 +92,12 @@ public class QueryEnvironment {
_config = Frameworks.newConfigBuilder().traitDefs()
.operatorTable(new
ChainedSqlOperatorTable(Arrays.asList(SqlStdOperatorTable.instance(),
_catalogReader)))
- .defaultSchema(_rootSchema.plus()).build();
+ .defaultSchema(_rootSchema.plus())
+ .sqlToRelConverterConfig(SqlToRelConverter.config()
+ .withHintStrategyTable(getHintStrategyTable())
+ .addRelBuilderConfigTransform(c -> c.withPushJoinCondition(true))
+ .addRelBuilderConfigTransform(c -> c.withAggregateUnique(true)))
+ .build();
// optimizer rules
_logicalRuleSet = PinotQueryRuleSets.LOGICAL_OPT_RULES;
@@ -191,8 +196,7 @@ public class QueryEnvironment {
RelOptCluster cluster =
RelOptCluster.create(plannerContext.getRelOptPlanner(), rexBuilder);
SqlToRelConverter sqlToRelConverter =
new SqlToRelConverter(plannerContext.getPlanner(),
plannerContext.getValidator(), _catalogReader, cluster,
- StandardConvertletTable.INSTANCE,
-
SqlToRelConverter.config().withHintStrategyTable(getHintStrategyTable(plannerContext)));
+ StandardConvertletTable.INSTANCE,
_config.getSqlToRelConverterConfig());
return sqlToRelConverter.convertQuery(parsed, false, true);
}
@@ -218,8 +222,7 @@ public class QueryEnvironment {
// utils
// --------------------------------------------------------------------------
- // TODO: add hint strategy table based on plannerContext.
- private HintStrategyTable getHintStrategyTable(PlannerContext
plannerContext) {
+ private HintStrategyTable getHintStrategyTable() {
return HintStrategyTable.builder().build();
}
}
diff --git
a/pinot-query-planner/src/test/java/org/apache/pinot/query/QueryEnvironmentTestBase.java
b/pinot-query-planner/src/test/java/org/apache/pinot/query/QueryEnvironmentTestBase.java
index d2939b1b7f..3d633e932f 100644
---
a/pinot-query-planner/src/test/java/org/apache/pinot/query/QueryEnvironmentTestBase.java
+++
b/pinot-query-planner/src/test/java/org/apache/pinot/query/QueryEnvironmentTestBase.java
@@ -65,6 +65,8 @@ public class QueryEnvironmentTestBase {
+ "AND AVG(a.col3) = 5"},
new Object[]{"SELECT dateTrunc('DAY', ts) FROM a LIMIT 10"},
new Object[]{"SELECT dateTrunc('DAY', a.ts + b.ts) FROM a JOIN b on
a.col1 = b.col1 AND a.col2 = b.col2"},
+ new Object[]{"SELECT a.col2, a.col3 FROM a JOIN b ON a.col1 = b.col1 "
+ + " WHERE a.col3 >= 0 GROUP BY a.col2, a.col3"},
};
}
}
diff --git
a/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTest.java
b/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTest.java
index 22d21106b7..d2ac8c95c8 100644
---
a/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTest.java
+++
b/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTest.java
@@ -174,6 +174,10 @@ public class QueryRunnerTest extends QueryRunnerTestBase {
// - on intermediate stage
new Object[]{"SELECT dateTrunc('DAY', round(a.ts, b.ts)) FROM a JOIN b
"
+ "ON a.col1 = b.col1 AND a.col2 = b.col2", 15},
+
+ // Distinct value done via GROUP BY with empty expr aggregation list.
+ new Object[]{"SELECT a.col2, a.col3 FROM a JOIN b ON a.col1 = b.col1 "
+ + " WHERE b.col3 > 0 GROUP BY a.col2, a.col3", 5},
};
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
