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/21 18:08:17 UTC
[GitHub] [pinot] walterddr opened a new pull request, #9445: [stash] adding support for range
walterddr opened a new pull request, #9445:
URL: https://github.com/apache/pinot/pull/9445
Instructions:
1. The PR has to be tagged with at least one of the following labels (*):
1. `feature`
2. `bugfix`
3. `performance`
4. `ui`
5. `backward-incompat`
6. `release-notes` (**)
2. Remove these instructions before publishing the PR.
(*) Other labels to consider:
- `testing`
- `dependencies`
- `docker`
- `kubernetes`
- `observability`
- `security`
- `code-style`
- `extension-point`
- `refactor`
- `cleanup`
(**) Use `release-notes` label for scenarios like:
- New configuration options
- Deprecation of configurations
- Signature changes to public methods/interfaces
- New plugins added or old plugins removed
--
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 a diff in pull request #9445: [multistage] adding support for range predicate
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on code in PR #9445:
URL: https://github.com/apache/pinot/pull/9445#discussion_r978217599
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTest.java:
##########
@@ -200,6 +200,13 @@ private Object[][] provideTestSql() {
new Object[]{"SELECT a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 "
+ " WHERE a.col1 IN ('foo') AND b.col2 NOT IN ('')"},
+ // Range conditions with continuous and non-continuous range.
+ new Object[]{"SELECT a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 "
+ + " WHERE a.col3 IN (1, 2, 3) OR (a.col3 > 10 AND a.col3 < 50)"},
Review Comment:
is `col IN (<single value>)` getting rewritten to `col = <val>` ?
FWIW, in the current engine we do this rewrite but not really worth it
--
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 #9445: [multistage] adding support for range predicate
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9445:
URL: https://github.com/apache/pinot/pull/9445#discussion_r978262908
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTest.java:
##########
@@ -200,6 +200,16 @@ private Object[][] provideTestSql() {
new Object[]{"SELECT a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 "
+ " WHERE a.col1 IN ('foo') AND b.col2 NOT IN ('')"},
+ // Range conditions with continuous and non-continuous range.
+ new Object[]{"SELECT a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 "
+ + " WHERE a.col3 IN (1, 2, 3) OR (a.col3 > 10 AND a.col3 < 50)"},
+
+ new Object[]{"SELECT col1, SUM(col3) FROM a WHERE a.col3 BETWEEN 23 AND 36 "
+ + " GROUP BY col1 HAVING SUM(col3) > 10.0 AND MIN(col3) <> 123 AND MAX(col3) BETWEEN 10 AND 20"},
Review Comment:
yes. this is not done by the range predicate but by the compilation on server.
--
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 a diff in pull request #9445: [multistage] adding support for range predicate
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on code in PR #9445:
URL: https://github.com/apache/pinot/pull/9445#discussion_r978218291
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/stage/ValueNode.java:
##########
@@ -0,0 +1,54 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.query.planner.stage;
+
+import com.google.common.collect.ImmutableList;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.query.planner.logical.RexExpression;
+import org.apache.pinot.query.planner.serde.ProtoProperties;
+
+
+public class ValueNode extends AbstractStageNode {
+ @ProtoProperties
+ private List<List<RexExpression>> _literalRows;
+
+ public ValueNode(int stageId) {
+ super(stageId);
+ }
+
+ public ValueNode(int currentStageId, DataSchema dataSchema,
Review Comment:
General suggestion -- for every new operator / StageNode introduced, I think we should add a corresponding query compilation test to validate the plan, expressions etc. In the current engine, we have done that for quite some time in `CalciteSqlCompilerTest.java`
--
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 merged pull request #9445: [multistage] adding support for range predicate
Posted by GitBox <gi...@apache.org>.
walterddr merged PR #9445:
URL: https://github.com/apache/pinot/pull/9445
--
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 a diff in pull request #9445: [multistage] adding support for range predicate
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on code in PR #9445:
URL: https://github.com/apache/pinot/pull/9445#discussion_r978217599
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTest.java:
##########
@@ -200,6 +200,13 @@ private Object[][] provideTestSql() {
new Object[]{"SELECT a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 "
+ " WHERE a.col1 IN ('foo') AND b.col2 NOT IN ('')"},
+ // Range conditions with continuous and non-continuous range.
+ new Object[]{"SELECT a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 "
+ + " WHERE a.col3 IN (1, 2, 3) OR (a.col3 > 10 AND a.col3 < 50)"},
Review Comment:
is IN (<single value>) getting rewritten to = ?
FWIW, in the current engine we do this rewrite but not really worth it
--
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 #9445: [multistage] adding support for range predicate
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9445:
URL: https://github.com/apache/pinot/pull/9445#discussion_r977074669
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTest.java:
##########
@@ -200,6 +200,13 @@ private Object[][] provideTestSql() {
new Object[]{"SELECT a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 "
+ " WHERE a.col1 IN ('foo') AND b.col2 NOT IN ('')"},
+ // Range conditions with continuous and non-continuous range.
+ new Object[]{"SELECT a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 "
+ + " WHERE a.col3 IN (1, 2, 3) OR (a.col3 > 10 AND a.col3 < 50)"},
Review Comment:
yeah I was planning to split that into a separate PR but yeah sure let me add that to this one.
--
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 a diff in pull request #9445: [multistage] adding support for range predicate
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on code in PR #9445:
URL: https://github.com/apache/pinot/pull/9445#discussion_r978218291
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/stage/ValueNode.java:
##########
@@ -0,0 +1,54 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.query.planner.stage;
+
+import com.google.common.collect.ImmutableList;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.query.planner.logical.RexExpression;
+import org.apache.pinot.query.planner.serde.ProtoProperties;
+
+
+public class ValueNode extends AbstractStageNode {
+ @ProtoProperties
+ private List<List<RexExpression>> _literalRows;
+
+ public ValueNode(int stageId) {
+ super(stageId);
+ }
+
+ public ValueNode(int currentStageId, DataSchema dataSchema,
Review Comment:
General suggestion -- for every new `Operator` / `StageNode` introduced, I think we should add a corresponding query compilation test to validate the plan, expressions etc. In the current engine, we have done that for quite some time in `CalciteSqlCompilerTest.java`
--
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 #9445: [multistage] adding support for range predicate
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #9445:
URL: https://github.com/apache/pinot/pull/9445#issuecomment-1254382155
# [Codecov](https://codecov.io/gh/apache/pinot/pull/9445?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 [#9445](https://codecov.io/gh/apache/pinot/pull/9445?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4ae1067) into [master](https://codecov.io/gh/apache/pinot/commit/b6f3331e22af81e5b80a1524575989b7ccb1d4b5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b6f3331) will **decrease** coverage by `43.92%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #9445 +/- ##
=============================================
- Coverage 69.87% 25.94% -43.93%
+ Complexity 4790 44 -4746
=============================================
Files 1890 1892 +2
Lines 100961 101201 +240
Branches 15353 15379 +26
=============================================
- Hits 70543 26260 -44283
- Misses 25451 72302 +46851
+ Partials 4967 2639 -2328
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `25.94% <0.00%> (-0.10%)` | :arrow_down: |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `?` | |
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/9445?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...not/query/planner/logical/RelToStageConverter.java](https://codecov.io/gh/apache/pinot/pull/9445/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-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9sb2dpY2FsL1JlbFRvU3RhZ2VDb252ZXJ0ZXIuamF2YQ==) | `0.00% <0.00%> (-76.93%)` | :arrow_down: |
| [...rg/apache/pinot/query/planner/stage/ValueNode.java](https://codecov.io/gh/apache/pinot/pull/9445/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-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9zdGFnZS9WYWx1ZU5vZGUuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ot/query/runtime/executor/WorkerQueryExecutor.java](https://codecov.io/gh/apache/pinot/pull/9445/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-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9leGVjdXRvci9Xb3JrZXJRdWVyeUV4ZWN1dG9yLmphdmE=) | `0.00% <0.00%> (-95.24%)` | :arrow_down: |
| [...t/query/runtime/operator/LiteralValueOperator.java](https://codecov.io/gh/apache/pinot/pull/9445/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-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9vcGVyYXRvci9MaXRlcmFsVmFsdWVPcGVyYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...in/java/org/apache/pinot/spi/utils/BytesUtils.java](https://codecov.io/gh/apache/pinot/pull/9445/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: |
| [...java/org/apache/pinot/spi/trace/BaseRecording.java](https://codecov.io/gh/apache/pinot/pull/9445/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: |
| [...java/org/apache/pinot/spi/trace/NoOpRecording.java](https://codecov.io/gh/apache/pinot/pull/9445/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdHJhY2UvTm9PcFJlY29yZGluZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/config/table/FSTType.java](https://codecov.io/gh/apache/pinot/pull/9445/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0ZTVFR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/config/user/RoleType.java](https://codecov.io/gh/apache/pinot/pull/9445/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3VzZXIvUm9sZVR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/9445/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1390 more](https://codecov.io/gh/apache/pinot/pull/9445/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] walterddr commented on a diff in pull request #9445: [multistage] adding support for range predicate
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9445:
URL: https://github.com/apache/pinot/pull/9445#discussion_r977074669
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTest.java:
##########
@@ -200,6 +200,13 @@ private Object[][] provideTestSql() {
new Object[]{"SELECT a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 "
+ " WHERE a.col1 IN ('foo') AND b.col2 NOT IN ('')"},
+ // Range conditions with continuous and non-continuous range.
+ new Object[]{"SELECT a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 "
+ + " WHERE a.col3 IN (1, 2, 3) OR (a.col3 > 10 AND a.col3 < 50)"},
Review Comment:
yeah I was planning to split that into a separate PR but yeah sure let me create this one.
--
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 #9445: [multistage] adding support for range predicate
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9445:
URL: https://github.com/apache/pinot/pull/9445#discussion_r978262713
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/stage/ValueNode.java:
##########
@@ -0,0 +1,54 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.query.planner.stage;
+
+import com.google.common.collect.ImmutableList;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.query.planner.logical.RexExpression;
+import org.apache.pinot.query.planner.serde.ProtoProperties;
+
+
+public class ValueNode extends AbstractStageNode {
+ @ProtoProperties
+ private List<List<RexExpression>> _literalRows;
+
+ public ValueNode(int stageId) {
+ super(stageId);
+ }
+
+ public ValueNode(int currentStageId, DataSchema dataSchema,
Review Comment:
this one yes. `ValueNode` is a special node generated by calcite when the query evaluates back to a constant (regardless of underlying table)
value node replaces table scan as a root.
--
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 #9445: [multistage] adding support for range predicate
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9445:
URL: https://github.com/apache/pinot/pull/9445#discussion_r978262449
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTest.java:
##########
@@ -200,6 +200,13 @@ private Object[][] provideTestSql() {
new Object[]{"SELECT a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 "
+ " WHERE a.col1 IN ('foo') AND b.col2 NOT IN ('')"},
+ // Range conditions with continuous and non-continuous range.
+ new Object[]{"SELECT a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 "
+ + " WHERE a.col3 IN (1, 2, 3) OR (a.col3 > 10 AND a.col3 < 50)"},
Review Comment:
single IN yes will be rewrite
--
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 a diff in pull request #9445: [multistage] adding support for range predicate
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on code in PR #9445:
URL: https://github.com/apache/pinot/pull/9445#discussion_r978218291
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/stage/ValueNode.java:
##########
@@ -0,0 +1,54 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.query.planner.stage;
+
+import com.google.common.collect.ImmutableList;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.query.planner.logical.RexExpression;
+import org.apache.pinot.query.planner.serde.ProtoProperties;
+
+
+public class ValueNode extends AbstractStageNode {
+ @ProtoProperties
+ private List<List<RexExpression>> _literalRows;
+
+ public ValueNode(int stageId) {
+ super(stageId);
+ }
+
+ public ValueNode(int currentStageId, DataSchema dataSchema,
Review Comment:
General suggestion -- for every new operator / StageNode introduced, I think we should add a corresponding query compilation test to validate the plan, expressions etc. In the current engine, we have done that for quite some time in CalciteSqlCompilerTest.java
--
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 a diff in pull request #9445: [multistage] adding support for range predicate
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on code in PR #9445:
URL: https://github.com/apache/pinot/pull/9445#discussion_r978218723
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTest.java:
##########
@@ -200,6 +200,16 @@ private Object[][] provideTestSql() {
new Object[]{"SELECT a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 "
+ " WHERE a.col1 IN ('foo') AND b.col2 NOT IN ('')"},
+ // Range conditions with continuous and non-continuous range.
+ new Object[]{"SELECT a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 "
+ + " WHERE a.col3 IN (1, 2, 3) OR (a.col3 > 10 AND a.col3 < 50)"},
+
+ new Object[]{"SELECT col1, SUM(col3) FROM a WHERE a.col3 BETWEEN 23 AND 36 "
+ + " GROUP BY col1 HAVING SUM(col3) > 10.0 AND MIN(col3) <> 123 AND MAX(col3) BETWEEN 10 AND 20"},
Review Comment:
In the current engine non-literals (function expressions) on RHS get rewritten by moving to LHS. Do we support that here ?
`WHERE a > b + 20` -> `WHERE a - b > 20`
--
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] ankitsultana commented on a diff in pull request #9445: [multistage] adding support for range predicate
Posted by GitBox <gi...@apache.org>.
ankitsultana commented on code in PR #9445:
URL: https://github.com/apache/pinot/pull/9445#discussion_r977059132
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTest.java:
##########
@@ -200,6 +200,13 @@ private Object[][] provideTestSql() {
new Object[]{"SELECT a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 "
+ " WHERE a.col1 IN ('foo') AND b.col2 NOT IN ('')"},
+ // Range conditions with continuous and non-continuous range.
+ new Object[]{"SELECT a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 "
+ + " WHERE a.col3 IN (1, 2, 3) OR (a.col3 > 10 AND a.col3 < 50)"},
Review Comment:
Can you also check the case where someone tries an impossible range condition or conditions with overlaps? I was running into an issue with those edge-cases in my PR. Examples:
```
(a.col3 > 10 AND a.col3 < 20) AND (a.col3 > 30 AND a.col3 < 40) # Impossible range
(a.col3 > 10 AND a.col3 < 20) AND (a.col3 > 15 AND a.col3 < 25) # Overlapping range
```
--
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