You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/09/07 23:19:55 UTC
[GitHub] [pinot] 61yao opened a new pull request, #9344: multi-thread query planning
61yao opened a new pull request, #9344:
URL: https://github.com/apache/pinot/pull/9344
Move unthread safe vars needed by query planning from QueryEnv to PlannerContext.
This includes PlannerImpl, Validator and LogicalPlanner.
In the future, we should probably have our own planner etc and move only the shared state to PlannerContext.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on pull request #9344: [Feature][multistage] Thread-safe query planning
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on PR #9344:
URL: https://github.com/apache/pinot/pull/9344#issuecomment-1242588602
@61yao - can you rebase please ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia merged pull request #9344: [Feature][multistage] Thread-safe query planning
Posted by GitBox <gi...@apache.org>.
siddharthteotia merged PR #9344:
URL: https://github.com/apache/pinot/pull/9344
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] 61yao commented on pull request #9344: [Feature][multistage] Thread-safe query planning
Posted by GitBox <gi...@apache.org>.
61yao commented on PR #9344:
URL: https://github.com/apache/pinot/pull/9344#issuecomment-1242596622
> > @61yao - can you rebase please ?
>
> Edit: Actually, the rebase is not done. Working on it.
I think I got the rebase done. Let me know if it works. Sorry, not very familiar with this..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] 61yao commented on a diff in pull request #9344: [Feature][multistage] Thread-safe query planning
Posted by GitBox <gi...@apache.org>.
61yao commented on code in PR #9344:
URL: https://github.com/apache/pinot/pull/9344#discussion_r967559828
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/context/PlannerContext.java:
##########
@@ -19,15 +19,51 @@
package org.apache.pinot.query.context;
import java.util.Map;
+import org.apache.calcite.plan.Contexts;
+import org.apache.calcite.plan.RelOptPlanner;
+import org.apache.calcite.plan.hep.HepProgram;
+import org.apache.calcite.prepare.PlannerImpl;
+import org.apache.calcite.prepare.Prepare;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.validate.SqlValidator;
+import org.apache.calcite.tools.FrameworkConfig;
+import org.apache.pinot.query.planner.logical.LogicalPlanner;
+import org.apache.pinot.query.validate.Validator;
/**
* PlannerContext is an object that holds all contextual information during planning phase.
*
- * TODO: currently the planner context is not used since we don't support option or query rewrite. This construct is
- * here as a placeholder for the parsed out options.
+ * TODO: currently we don't support option or query rewrite.
+ * It is used to hold per query context for query planning, which cannot be shared across queries.
*/
-public class PlannerContext {
+public class PlannerContext implements AutoCloseable {
+ public PlannerContext(FrameworkConfig config, Prepare.CatalogReader catalogReader, RelDataTypeFactory typeFactory,
+ HepProgram hepProgram) {
+ _planner = new PlannerImpl(config);
+ _validator = new Validator(SqlStdOperatorTable.instance(), catalogReader, typeFactory);
+ _relOptPlanner = new LogicalPlanner(hepProgram, Contexts.EMPTY_CONTEXT);
+ }
+
+ public PlannerImpl getPlanner() {
+ return _planner;
+ }
+
+ public SqlValidator getValidator() {
+ return _validator;
+ }
+
+ public RelOptPlanner getRelOptPlanner() {
+ return _relOptPlanner;
+ }
+
+ private PlannerImpl _planner;
+
+ private SqlValidator _validator;
+
+ private RelOptPlanner _relOptPlanner;
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #9344: [Feature][multistage] Thread-safe query planning
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9344:
URL: https://github.com/apache/pinot/pull/9344#discussion_r966525348
##########
pinot-query-planner/src/test/java/org/apache/pinot/query/QueryCompilationTest.java:
##########
@@ -181,24 +234,30 @@ private Object[][] provideQueriesWithException() {
@DataProvider(name = "testQueryPlanDataProvider")
private Object[][] provideQueriesWithExplainedPlan() {
- return new Object[][] {
- new Object[]{"EXPLAIN PLAN INCLUDING ALL ATTRIBUTES AS JSON FOR SELECT col1, col3 FROM a", "{\n"
- + " \"rels\": [\n" + " {\n" + " \"id\": \"0\",\n" + " \"relOp\": \"LogicalTableScan\",\n"
- + " \"table\": [\n" + " \"a\"\n" + " ],\n" + " \"inputs\": []\n" + " },\n"
- + " {\n" + " \"id\": \"1\",\n" + " \"relOp\": \"LogicalProject\",\n" + " \"fields\": [\n"
- + " \"col1\",\n" + " \"col3\"\n" + " ],\n" + " \"exprs\": [\n" + " {\n"
- + " \"input\": 2,\n" + " \"name\": \"$2\"\n" + " },\n" + " {\n"
- + " \"input\": 1,\n" + " \"name\": \"$1\"\n" + " }\n" + " ]\n" + " }\n"
- + " ]\n" + "}"},
- new Object[]{"EXPLAIN PLAN EXCLUDING ATTRIBUTES AS DOT FOR SELECT col1, COUNT(*) FROM a GROUP BY col1",
- "Execution Plan\n" + "digraph {\n" + "\"LogicalExchange\\n\" -> \"LogicalAggregate\\n\" [label=\"0\"]\n"
- + "\"LogicalAggregate\\n\" -> \"LogicalExchange\\n\" [label=\"0\"]\n"
- + "\"LogicalTableScan\\n\" -> \"LogicalAggregate\\n\" [label=\"0\"]\n" + "}\n"},
- new Object[]{"EXPLAIN PLAN FOR SELECT a.col1, b.col3 FROM a JOIN b ON a.col1 = b.col1", "Execution Plan\n"
- + "LogicalProject(col1=[$0], col3=[$1])\n" + " LogicalJoin(condition=[=($0, $2)], joinType=[inner])\n"
+ return new Object[][]{
+ new Object[]{
+ "EXPLAIN PLAN INCLUDING ALL ATTRIBUTES AS JSON FOR SELECT col1, col3 FROM a",
+ "{\n" + " \"rels\": [\n" + " {\n" + " \"id\": \"0\",\n"
Review Comment:
nit: why this code restructure occurs? if so combine the `"{\n" with the next line. actually if you can don't do any these refactoring in a feature PR.
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/context/PlannerContext.java:
##########
@@ -19,15 +19,51 @@
package org.apache.pinot.query.context;
import java.util.Map;
+import org.apache.calcite.plan.Contexts;
+import org.apache.calcite.plan.RelOptPlanner;
+import org.apache.calcite.plan.hep.HepProgram;
+import org.apache.calcite.prepare.PlannerImpl;
+import org.apache.calcite.prepare.Prepare;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.validate.SqlValidator;
+import org.apache.calcite.tools.FrameworkConfig;
+import org.apache.pinot.query.planner.logical.LogicalPlanner;
+import org.apache.pinot.query.validate.Validator;
/**
* PlannerContext is an object that holds all contextual information during planning phase.
*
- * TODO: currently the planner context is not used since we don't support option or query rewrite. This construct is
- * here as a placeholder for the parsed out options.
+ * TODO: currently we don't support option or query rewrite.
+ * It is used to hold per query context for query planning, which cannot be shared across queries.
*/
-public class PlannerContext {
+public class PlannerContext implements AutoCloseable {
+ public PlannerContext(FrameworkConfig config, Prepare.CatalogReader catalogReader, RelDataTypeFactory typeFactory,
+ HepProgram hepProgram) {
+ _planner = new PlannerImpl(config);
+ _validator = new Validator(SqlStdOperatorTable.instance(), catalogReader, typeFactory);
+ _relOptPlanner = new LogicalPlanner(hepProgram, Contexts.EMPTY_CONTEXT);
+ }
+
+ public PlannerImpl getPlanner() {
+ return _planner;
+ }
+
+ public SqlValidator getValidator() {
+ return _validator;
+ }
+
+ public RelOptPlanner getRelOptPlanner() {
+ return _relOptPlanner;
+ }
+
+ private PlannerImpl _planner;
+
+ private SqlValidator _validator;
+
+ private RelOptPlanner _relOptPlanner;
Review Comment:
nit: move these sections to the top of the class (in Pinot our class structure are member variables then functions)
also make it final so that it cannot be modify outside of constructor of the class / compiler knows how to optimize getter methods.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] 61yao commented on pull request #9344: [Feature][multistage] Thread-safe query planning
Posted by GitBox <gi...@apache.org>.
61yao commented on PR #9344:
URL: https://github.com/apache/pinot/pull/9344#issuecomment-1242593472
> lgtm
>
> ```
> > Thus when using {@code QueryEnvironment#planQuery(String)}, caller should ensure that no concurrent
> plan execution occurs.
> > Only one query query can be planned at a time. Afterwards planner needs to be reset in order to clear the
> state for the next planning.
> ```
>
> The above javadoc comments in QueryEnvironment can be updated
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] 61yao commented on a diff in pull request #9344: [Feature][multistage] Thread-safe query planning
Posted by GitBox <gi...@apache.org>.
61yao commented on code in PR #9344:
URL: https://github.com/apache/pinot/pull/9344#discussion_r966414537
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/context/PlannerContext.java:
##########
@@ -19,15 +19,50 @@
package org.apache.pinot.query.context;
import java.util.Map;
-
+import org.apache.calcite.plan.Contexts;
+import org.apache.calcite.plan.RelOptPlanner;
+import org.apache.calcite.plan.hep.HepProgram;
+import org.apache.calcite.prepare.PlannerImpl;
+import org.apache.calcite.prepare.Prepare;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.validate.SqlValidator;
+import org.apache.calcite.tools.FrameworkConfig;
+import org.apache.pinot.query.planner.logical.LogicalPlanner;
+import org.apache.pinot.query.validate.Validator;
/**
* PlannerContext is an object that holds all contextual information during planning phase.
*
- * TODO: currently the planner context is not used since we don't support option or query rewrite. This construct is
- * here as a placeholder for the parsed out options.
+ * TODO: currently we don't support option or query rewrite.
+ * It is used to hold per query context for query planning, which cannot be shared across queries.
*/
public class PlannerContext {
Review Comment:
Done.
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/context/PlannerContext.java:
##########
@@ -19,15 +19,50 @@
package org.apache.pinot.query.context;
import java.util.Map;
-
+import org.apache.calcite.plan.Contexts;
+import org.apache.calcite.plan.RelOptPlanner;
+import org.apache.calcite.plan.hep.HepProgram;
+import org.apache.calcite.prepare.PlannerImpl;
+import org.apache.calcite.prepare.Prepare;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.validate.SqlValidator;
+import org.apache.calcite.tools.FrameworkConfig;
+import org.apache.pinot.query.planner.logical.LogicalPlanner;
+import org.apache.pinot.query.validate.Validator;
/**
* PlannerContext is an object that holds all contextual information during planning phase.
*
- * TODO: currently the planner context is not used since we don't support option or query rewrite. This construct is
- * here as a placeholder for the parsed out options.
+ * TODO: currently we don't support option or query rewrite.
+ * It is used to hold per query context for query planning, which cannot be shared across queries.
*/
public class PlannerContext {
+ public PlannerContext(FrameworkConfig config, Prepare.CatalogReader catalogReader, RelDataTypeFactory typeFactory,
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] 61yao commented on a diff in pull request #9344: [Feature][multistage] Thread-safe query planning
Posted by GitBox <gi...@apache.org>.
61yao commented on code in PR #9344:
URL: https://github.com/apache/pinot/pull/9344#discussion_r967559047
##########
pinot-query-planner/src/test/java/org/apache/pinot/query/QueryCompilationTest.java:
##########
@@ -181,24 +234,30 @@ private Object[][] provideQueriesWithException() {
@DataProvider(name = "testQueryPlanDataProvider")
private Object[][] provideQueriesWithExplainedPlan() {
- return new Object[][] {
- new Object[]{"EXPLAIN PLAN INCLUDING ALL ATTRIBUTES AS JSON FOR SELECT col1, col3 FROM a", "{\n"
- + " \"rels\": [\n" + " {\n" + " \"id\": \"0\",\n" + " \"relOp\": \"LogicalTableScan\",\n"
- + " \"table\": [\n" + " \"a\"\n" + " ],\n" + " \"inputs\": []\n" + " },\n"
- + " {\n" + " \"id\": \"1\",\n" + " \"relOp\": \"LogicalProject\",\n" + " \"fields\": [\n"
- + " \"col1\",\n" + " \"col3\"\n" + " ],\n" + " \"exprs\": [\n" + " {\n"
- + " \"input\": 2,\n" + " \"name\": \"$2\"\n" + " },\n" + " {\n"
- + " \"input\": 1,\n" + " \"name\": \"$1\"\n" + " }\n" + " ]\n" + " }\n"
- + " ]\n" + "}"},
- new Object[]{"EXPLAIN PLAN EXCLUDING ATTRIBUTES AS DOT FOR SELECT col1, COUNT(*) FROM a GROUP BY col1",
- "Execution Plan\n" + "digraph {\n" + "\"LogicalExchange\\n\" -> \"LogicalAggregate\\n\" [label=\"0\"]\n"
- + "\"LogicalAggregate\\n\" -> \"LogicalExchange\\n\" [label=\"0\"]\n"
- + "\"LogicalTableScan\\n\" -> \"LogicalAggregate\\n\" [label=\"0\"]\n" + "}\n"},
- new Object[]{"EXPLAIN PLAN FOR SELECT a.col1, b.col3 FROM a JOIN b ON a.col1 = b.col1", "Execution Plan\n"
- + "LogicalProject(col1=[$0], col3=[$1])\n" + " LogicalJoin(condition=[=($0, $2)], joinType=[inner])\n"
+ return new Object[][]{
+ new Object[]{
+ "EXPLAIN PLAN INCLUDING ALL ATTRIBUTES AS JSON FOR SELECT col1, col3 FROM a",
+ "{\n" + " \"rels\": [\n" + " {\n" + " \"id\": \"0\",\n"
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #9344: [Feature][multistage] Thread-safe query planning
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9344:
URL: https://github.com/apache/pinot/pull/9344#discussion_r966214894
##########
pinot-query-planner/src/test/java/org/apache/pinot/query/QueryCompilationTest.java:
##########
@@ -148,6 +149,36 @@ public void testQueryProjectFilterPushDownForJoin() {
}
}
+ // Test that plan query can be run as multi-thread.
+ @Test
+ public void testPlanQueryMultiThread()
+ throws Exception {
+ Runnable planQuery = () -> {
+ String query = "SELECT a.col1, a.ts, b.col2, b.col3 FROM a JOIN b ON a.col1 = b.col2 "
+ + "WHERE a.col3 >= 0 AND a.col2 IN ('a', 'b') AND b.col3 < 0";
+ QueryPlan queryPlan = _queryEnvironment.planQuery(query);
+ List<StageNode> intermediateStageRoots =
+ queryPlan.getStageMetadataMap().entrySet().stream().filter(e -> e.getValue().getScannedTables().size() == 0)
+ .map(e -> queryPlan.getQueryStageMap().get(e.getKey())).collect(Collectors.toList());
+ // Assert that no project of filter node for any intermediate stage because all should've been pushed down.
+ for (StageNode roots : intermediateStageRoots) {
+ assertNodeTypeNotIn(roots, ImmutableList.of(ProjectNode.class, FilterNode.class));
+ }
+ };
Review Comment:
no need to assert this. we should test something like this.
- plan 2 type of SQL (say
- type a: SELECT * FROM tbl and
- type b: SELECT xxx JOIN)
- run 4 planning (a1, b1, a2, b2) at the same time
- assert that a1 plan == a2 plan, same for b type
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/context/PlannerContext.java:
##########
@@ -19,15 +19,50 @@
package org.apache.pinot.query.context;
import java.util.Map;
-
+import org.apache.calcite.plan.Contexts;
+import org.apache.calcite.plan.RelOptPlanner;
+import org.apache.calcite.plan.hep.HepProgram;
+import org.apache.calcite.prepare.PlannerImpl;
+import org.apache.calcite.prepare.Prepare;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.validate.SqlValidator;
+import org.apache.calcite.tools.FrameworkConfig;
+import org.apache.pinot.query.planner.logical.LogicalPlanner;
+import org.apache.pinot.query.validate.Validator;
/**
* PlannerContext is an object that holds all contextual information during planning phase.
*
- * TODO: currently the planner context is not used since we don't support option or query rewrite. This construct is
- * here as a placeholder for the parsed out options.
+ * TODO: currently we don't support option or query rewrite.
+ * It is used to hold per query context for query planning, which cannot be shared across queries.
*/
public class PlannerContext {
Review Comment:
let's make this closeable and make sure the planner is closed
```suggestion
public class PlannerContext implement Closeable {
```
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/context/PlannerContext.java:
##########
@@ -19,15 +19,50 @@
package org.apache.pinot.query.context;
import java.util.Map;
-
+import org.apache.calcite.plan.Contexts;
+import org.apache.calcite.plan.RelOptPlanner;
+import org.apache.calcite.plan.hep.HepProgram;
+import org.apache.calcite.prepare.PlannerImpl;
+import org.apache.calcite.prepare.Prepare;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.validate.SqlValidator;
+import org.apache.calcite.tools.FrameworkConfig;
+import org.apache.pinot.query.planner.logical.LogicalPlanner;
+import org.apache.pinot.query.validate.Validator;
/**
* PlannerContext is an object that holds all contextual information during planning phase.
*
- * TODO: currently the planner context is not used since we don't support option or query rewrite. This construct is
- * here as a placeholder for the parsed out options.
+ * TODO: currently we don't support option or query rewrite.
+ * It is used to hold per query context for query planning, which cannot be shared across queries.
*/
public class PlannerContext {
+ public PlannerContext(FrameworkConfig config, Prepare.CatalogReader catalogReader, RelDataTypeFactory typeFactory,
Review Comment:
great idea encoding these non-threadsafe objects into plannerContext!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #9344: [Feature][multistage] Thread-safe query planning
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #9344:
URL: https://github.com/apache/pinot/pull/9344#issuecomment-1240032118
# [Codecov](https://codecov.io/gh/apache/pinot/pull/9344?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#9344](https://codecov.io/gh/apache/pinot/pull/9344?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9d7437e) into [master](https://codecov.io/gh/apache/pinot/commit/334c978367182af58b0013e5e188d0af90e2ecb5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (334c978) will **decrease** coverage by `54.52%`.
> The diff coverage is `95.00%`.
```diff
@@ Coverage Diff @@
## master #9344 +/- ##
=============================================
- Coverage 69.76% 15.24% -54.53%
+ Complexity 4704 170 -4534
=============================================
Files 1882 1829 -53
Lines 100155 97893 -2262
Branches 15226 14967 -259
=============================================
- Hits 69874 14920 -54954
- Misses 25344 81845 +56501
+ Partials 4937 1128 -3809
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `15.24% <95.00%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/9344?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../java/org/apache/pinot/query/QueryEnvironment.java](https://codecov.io/gh/apache/pinot/pull/9344/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvUXVlcnlFbnZpcm9ubWVudC5qYXZh) | `81.48% <91.66%> (-1.28%)` | :arrow_down: |
| [...org/apache/pinot/query/context/PlannerContext.java](https://codecov.io/gh/apache/pinot/pull/9344/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvY29udGV4dC9QbGFubmVyQ29udGV4dC5qYXZh) | `90.90% <100.00%> (+15.90%)` | :arrow_up: |
| [...src/main/java/org/apache/pinot/sql/FilterKind.java](https://codecov.io/gh/apache/pinot/pull/9344/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvRmlsdGVyS2luZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/9344/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...in/java/org/apache/pinot/spi/utils/BytesUtils.java](https://codecov.io/gh/apache/pinot/pull/9344/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQnl0ZXNVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...n/java/org/apache/pinot/core/data/table/Table.java](https://codecov.io/gh/apache/pinot/pull/9344/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1RhYmxlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/9344/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/9344/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1JlY29yZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/util/GroupByUtils.java](https://codecov.io/gh/apache/pinot/pull/9344/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL0dyb3VwQnlVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/spi/trace/BaseRecording.java](https://codecov.io/gh/apache/pinot/pull/9344/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdHJhY2UvQmFzZVJlY29yZGluZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1420 more](https://codecov.io/gh/apache/pinot/pull/9344/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] 61yao commented on a diff in pull request #9344: [Feature][multistage] Thread-safe query planning
Posted by GitBox <gi...@apache.org>.
61yao commented on code in PR #9344:
URL: https://github.com/apache/pinot/pull/9344#discussion_r966414901
##########
pinot-query-planner/src/test/java/org/apache/pinot/query/QueryCompilationTest.java:
##########
@@ -148,6 +149,36 @@ public void testQueryProjectFilterPushDownForJoin() {
}
}
+ // Test that plan query can be run as multi-thread.
+ @Test
+ public void testPlanQueryMultiThread()
+ throws Exception {
+ Runnable planQuery = () -> {
+ String query = "SELECT a.col1, a.ts, b.col2, b.col3 FROM a JOIN b ON a.col1 = b.col2 "
+ + "WHERE a.col3 >= 0 AND a.col2 IN ('a', 'b') AND b.col3 < 0";
+ QueryPlan queryPlan = _queryEnvironment.planQuery(query);
+ List<StageNode> intermediateStageRoots =
+ queryPlan.getStageMetadataMap().entrySet().stream().filter(e -> e.getValue().getScannedTables().size() == 0)
+ .map(e -> queryPlan.getQueryStageMap().get(e.getKey())).collect(Collectors.toList());
+ // Assert that no project of filter node for any intermediate stage because all should've been pushed down.
+ for (StageNode roots : intermediateStageRoots) {
+ assertNodeTypeNotIn(roots, ImmutableList.of(ProjectNode.class, FilterNode.class));
+ }
+ };
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] 61yao commented on pull request #9344: [Feature][multistage] Thread-safe query planning
Posted by GitBox <gi...@apache.org>.
61yao commented on PR #9344:
URL: https://github.com/apache/pinot/pull/9344#issuecomment-1242593497
> @61yao - can you rebase please ?
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org