You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "yashmayya (via GitHub)" <gi...@apache.org> on 2025/04/24 11:09:41 UTC
[PR] ASOF JOIN [pinot]
yashmayya opened a new pull request, #15630:
URL: https://github.com/apache/pinot/pull/15630
- Adds support for `ASOF JOIN` (and `LEFT ASOF JOIN`) in the multi-stage engine with syntax and semantics similar to Snowflake's - https://docs.snowflake.com/en/sql-reference/constructs/asof-join.
- Calcite recently added support for this join type (in the parser, validator, and rel logic) in `1.38.0` - https://github.com/apache/calcite/pull/3883 (we recently upgraded from `1.37.0` to `1.39.0` - https://github.com/apache/pinot/pull/15263).
- Note that Calcite currently doesn't support `ASOF JOIN`s without an `ON` clause even though Snowflake does - https://docs.snowflake.com/en/sql-reference/constructs/asof-join#parameters. Currently, the workaround would be to use a clause like `ON TRUE` - https://github.com/apache/calcite/pull/3883#discussion_r2057922563. Note that Calcite enforces the `ON` clause in these joins to be a conjunction of equalities (apart from literal conditions).
- The `MATCH_CONDITION` clause is mandatory (in both Calcite and Snowflake).
- We don't use the `MATCH_CONDITION` to determine the exchange / distribution logic here in Pinot, we only use the actual join keys (from the `ON` clause) to implement the usual hash exchange. The reason is that `MATCH_CONDITION` can only be one out of `>`, `>=`, `<`, `<=` and the semantics of the join (finding the closest match) make it such that we can't use buckets to distribute the data accurately either. The hash exchange will be based on the join keys from the `ON` clause. If the clause is `ON TRUE`, we'll use a random + broadcast distribution strategy, similar to regular joins without equality based join conditions.
- This patch also refactors the physical `BaseJoinOperator` and existing implementations to increase reused logic and adds a new `AsofJoinOperator` implementation that uses a tree map for efficient (binary search based) computation of the closest match as per the `MATCH_CONDITION`.
- Since H2 doesn't support `ASOF JOIN` queries, the test cases added here only have manually validated logical correctness.
--
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
Re: [PR] ASOF JOIN [pinot]
Posted by "yashmayya (via GitHub)" <gi...@apache.org>.
yashmayya merged PR #15630:
URL: https://github.com/apache/pinot/pull/15630
--
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
Re: [PR] ASOF JOIN [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on code in PR #15630:
URL: https://github.com/apache/pinot/pull/15630#discussion_r2106797766
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/AsofJoinOperator.java:
##########
@@ -0,0 +1,183 @@
+/**
+ * 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.runtime.operator;
+
+import com.google.common.base.Preconditions;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.NavigableMap;
+import java.util.TreeMap;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.query.planner.logical.RexExpression;
+import org.apache.pinot.query.planner.partitioning.KeySelector;
+import org.apache.pinot.query.planner.partitioning.KeySelectorFactory;
+import org.apache.pinot.query.planner.plannode.JoinNode;
+import org.apache.pinot.query.runtime.blocks.MseBlock;
+import org.apache.pinot.query.runtime.plan.OpChainExecutionContext;
+
+
+public class AsofJoinOperator extends BaseJoinOperator {
+ private static final String EXPLAIN_NAME = "ASOF_JOIN";
+
+ private final Map<Object, NavigableMap<Comparable<?>, Object[]>> _rightTable;
+ private final KeySelector<?> _leftKeySelector;
+ private final KeySelector<?> _rightKeySelector;
+ private final MatchConditionType _matchConditionType;
+ private final int _leftMatchKeyIndex;
+ private final int _rightMatchKeyIndex;
+
+ public AsofJoinOperator(OpChainExecutionContext context, MultiStageOperator leftInput, DataSchema leftSchema,
+ MultiStageOperator rightInput, JoinNode node) {
+ super(context, leftInput, leftSchema, rightInput, node);
+ Preconditions.checkState(node.getNonEquiConditions().isEmpty(),
+ "ASOF_JOIN operator cannot have non-equi join keys");
+ Preconditions.checkState(node.getMatchCondition() != null, "ASOF_JOIN operator must have a match condition");
+ _rightTable = new HashMap<>();
+ List<Integer> leftKeys = node.getLeftKeys();
+
+ _leftKeySelector = KeySelectorFactory.getKeySelector(leftKeys);
+ _rightKeySelector = KeySelectorFactory.getKeySelector(node.getRightKeys());
+
+ RexExpression matchCondition = node.getMatchCondition();
+ Preconditions.checkState(matchCondition instanceof RexExpression.FunctionCall,
+ "ASOF_JOIN operator only supports function call match condition");
+
+ try {
+ _matchConditionType =
+ MatchConditionType.valueOf(((RexExpression.FunctionCall) matchCondition).getFunctionName().toUpperCase());
+ } catch (IllegalArgumentException e) {
+ throw new UnsupportedOperationException("Unsupported match condition: " + matchCondition);
+ }
+ List<RexExpression> matchKeys = ((RexExpression.FunctionCall) matchCondition).getFunctionOperands();
+ // TODO: Add support for MATCH_CONDITION containing two columns of different types. In that case, there would be
+ // a CAST RexExpression.FunctionCall on top of the RexExpression.InputRef, and we'd need to add the
+ // appropriate type casts to make sure that the Comparable based comparisons in this class don't throw
+ // ClassCastException.
Review Comment:
Aren't we able to fail during planning in this case?
--
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
Re: [PR] ASOF JOIN [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #15630:
URL: https://github.com/apache/pinot/pull/15630#issuecomment-2827344373
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/15630?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `0%` with `162 lines` in your changes missing coverage. Please review.
> Project coverage is 33.51%. Comparing base [(`9f030bb`)](https://app.codecov.io/gh/apache/pinot/commit/9f030bb0f20de69595156c18188946b2d2876e1c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`8216e08`)](https://app.codecov.io/gh/apache/pinot/commit/8216e08f107f1f1ba46ac7243967f7ff5acd0d72?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 11 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/15630?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...pinot/query/runtime/operator/AsofJoinOperator.java](https://app.codecov.io/gh/apache/pinot/pull/15630?src=pr&el=tree&filepath=pinot-query-runtime%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fquery%2Fruntime%2Foperator%2FAsofJoinOperator.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9vcGVyYXRvci9Bc29mSm9pbk9wZXJhdG9yLmphdmE=) | 0.00% | [65 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/15630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...pinot/query/runtime/operator/BaseJoinOperator.java](https://app.codecov.io/gh/apache/pinot/pull/15630?src=pr&el=tree&filepath=pinot-query-runtime%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fquery%2Fruntime%2Foperator%2FBaseJoinOperator.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9vcGVyYXRvci9CYXNlSm9pbk9wZXJhdG9yLmphdmE=) | 0.00% | [27 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/15630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../query/planner/logical/RelToPlanNodeConverter.java](https://app.codecov.io/gh/apache/pinot/pull/15630?src=pr&el=tree&filepath=pinot-query-planner%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fquery%2Fplanner%2Flogical%2FRelToPlanNodeConverter.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9sb2dpY2FsL1JlbFRvUGxhbk5vZGVDb252ZXJ0ZXIuamF2YQ==) | 0.00% | [24 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/15630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../apache/pinot/query/planner/plannode/JoinNode.java](https://app.codecov.io/gh/apache/pinot/pull/15630?src=pr&el=tree&filepath=pinot-query-planner%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fquery%2Fplanner%2Fplannode%2FJoinNode.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9wbGFubm9kZS9Kb2luTm9kZS5qYXZh) | 0.00% | [8 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/15630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../pinot/query/planner/serde/PlanNodeSerializer.java](https://app.codecov.io/gh/apache/pinot/pull/15630?src=pr&el=tree&filepath=pinot-query-planner%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fquery%2Fplanner%2Fserde%2FPlanNodeSerializer.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9zZXJkZS9QbGFuTm9kZVNlcmlhbGl6ZXIuamF2YQ==) | 0.00% | [8 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/15630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...he/pinot/query/runtime/plan/PlanNodeToOpChain.java](https://app.codecov.io/gh/apache/pinot/pull/15630?src=pr&el=tree&filepath=pinot-query-runtime%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fquery%2Fruntime%2Fplan%2FPlanNodeToOpChain.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9wbGFuL1BsYW5Ob2RlVG9PcENoYWluLmphdmE=) | 0.00% | [7 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/15630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...inot/query/planner/serde/PlanNodeDeserializer.java](https://app.codecov.io/gh/apache/pinot/pull/15630?src=pr&el=tree&filepath=pinot-query-planner%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fquery%2Fplanner%2Fserde%2FPlanNodeDeserializer.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9zZXJkZS9QbGFuTm9kZURlc2VyaWFsaXplci5qYXZh) | 0.00% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/15630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../query/planner/logical/PlanNodeToRelConverter.java](https://app.codecov.io/gh/apache/pinot/pull/15630?src=pr&el=tree&filepath=pinot-query-planner%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fquery%2Fplanner%2Flogical%2FPlanNodeToRelConverter.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9sb2dpY2FsL1BsYW5Ob2RlVG9SZWxDb252ZXJ0ZXIuamF2YQ==) | 0.00% | [5 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/15630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ite/rel/rules/PinotJoinExchangeNodeInsertRule.java](https://app.codecov.io/gh/apache/pinot/pull/15630?src=pr&el=tree&filepath=pinot-query-planner%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcalcite%2Frel%2Frules%2FPinotJoinExchangeNodeInsertRule.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2FsY2l0ZS9yZWwvcnVsZXMvUGlub3RKb2luRXhjaGFuZ2VOb2RlSW5zZXJ0UnVsZS5qYXZh) | 0.00% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/15630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...pinot/query/runtime/operator/HashJoinOperator.java](https://app.codecov.io/gh/apache/pinot/pull/15630?src=pr&el=tree&filepath=pinot-query-runtime%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fquery%2Fruntime%2Foperator%2FHashJoinOperator.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9vcGVyYXRvci9IYXNoSm9pbk9wZXJhdG9yLmphdmE=) | 0.00% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/15630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| ... and [1 more](https://app.codecov.io/gh/apache/pinot/pull/15630?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
> :exclamation: There is a different number of reports uploaded between BASE (9f030bb) and HEAD (8216e08). Click for more details.
>
> <details><summary>HEAD has 67 uploads less than BASE</summary>
>
>| Flag | BASE (9f030bb) | HEAD (8216e08) |
>|------|------|------|
>|java-21|10|1|
>|unittests|6|1|
>|unittests1|3|0|
>|skip-bytebuffers-true|4|0|
>|temurin|15|1|
>|skip-bytebuffers-false|8|1|
>|unittests2|3|1|
>|java-11|5|0|
>|integration|9|0|
>|integration2|3|0|
>|integration1|3|0|
>|custom-integration1|3|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #15630 +/- ##
=============================================
- Coverage 62.77% 33.51% -29.26%
+ Complexity 1384 690 -694
=============================================
Files 2864 2865 +1
Lines 162734 162887 +153
Branches 24917 24933 +16
=============================================
- Hits 102151 54597 -47554
- Misses 52875 103926 +51051
+ Partials 7708 4364 -3344
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/15630/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/15630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/15630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/15630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/15630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/15630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/15630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `33.51% <0.00%> (-29.22%)` | :arrow_down: |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/15630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `33.51% <0.00%> (-29.25%)` | :arrow_down: |
| [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/15630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/15630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `33.51% <0.00%> (-29.26%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/15630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `33.51% <0.00%> (-29.25%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/15630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/15630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `33.51% <0.00%> (-0.03%)` | :arrow_down: |
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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/15630?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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
Re: [PR] ASOF JOIN [pinot]
Posted by "yashmayya (via GitHub)" <gi...@apache.org>.
yashmayya commented on code in PR #15630:
URL: https://github.com/apache/pinot/pull/15630#discussion_r2106997370
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/AsofJoinOperator.java:
##########
@@ -0,0 +1,183 @@
+/**
+ * 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.runtime.operator;
+
+import com.google.common.base.Preconditions;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.NavigableMap;
+import java.util.TreeMap;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.query.planner.logical.RexExpression;
+import org.apache.pinot.query.planner.partitioning.KeySelector;
+import org.apache.pinot.query.planner.partitioning.KeySelectorFactory;
+import org.apache.pinot.query.planner.plannode.JoinNode;
+import org.apache.pinot.query.runtime.blocks.MseBlock;
+import org.apache.pinot.query.runtime.plan.OpChainExecutionContext;
+
+
+public class AsofJoinOperator extends BaseJoinOperator {
+ private static final String EXPLAIN_NAME = "ASOF_JOIN";
+
+ private final Map<Object, NavigableMap<Comparable<?>, Object[]>> _rightTable;
+ private final KeySelector<?> _leftKeySelector;
+ private final KeySelector<?> _rightKeySelector;
+ private final MatchConditionType _matchConditionType;
+ private final int _leftMatchKeyIndex;
+ private final int _rightMatchKeyIndex;
+
+ public AsofJoinOperator(OpChainExecutionContext context, MultiStageOperator leftInput, DataSchema leftSchema,
+ MultiStageOperator rightInput, JoinNode node) {
+ super(context, leftInput, leftSchema, rightInput, node);
+ Preconditions.checkState(node.getNonEquiConditions().isEmpty(),
+ "ASOF_JOIN operator cannot have non-equi join keys");
+ Preconditions.checkState(node.getMatchCondition() != null, "ASOF_JOIN operator must have a match condition");
+ _rightTable = new HashMap<>();
+ List<Integer> leftKeys = node.getLeftKeys();
+
+ _leftKeySelector = KeySelectorFactory.getKeySelector(leftKeys);
+ _rightKeySelector = KeySelectorFactory.getKeySelector(node.getRightKeys());
+
+ RexExpression matchCondition = node.getMatchCondition();
+ Preconditions.checkState(matchCondition instanceof RexExpression.FunctionCall,
+ "ASOF_JOIN operator only supports function call match condition");
+
+ try {
+ _matchConditionType =
+ MatchConditionType.valueOf(((RexExpression.FunctionCall) matchCondition).getFunctionName().toUpperCase());
+ } catch (IllegalArgumentException e) {
+ throw new UnsupportedOperationException("Unsupported match condition: " + matchCondition);
+ }
+ List<RexExpression> matchKeys = ((RexExpression.FunctionCall) matchCondition).getFunctionOperands();
+ // TODO: Add support for MATCH_CONDITION containing two columns of different types. In that case, there would be
+ // a CAST RexExpression.FunctionCall on top of the RexExpression.InputRef, and we'd need to add the
+ // appropriate type casts to make sure that the Comparable based comparisons in this class don't throw
+ // ClassCastException.
Review Comment:
Good point, I've moved all the checks from this operator into the planning phase (I hadn't realized that some of the checks were duplicated across both the places).
--
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
Re: [PR] ASOF JOIN [pinot]
Posted by "albertobastos (via GitHub)" <gi...@apache.org>.
albertobastos commented on code in PR #15630:
URL: https://github.com/apache/pinot/pull/15630#discussion_r2063684134
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/AsofJoinOperator.java:
##########
@@ -0,0 +1,183 @@
+/**
+ * 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.runtime.operator;
+
+import com.google.common.base.Preconditions;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.NavigableMap;
+import java.util.TreeMap;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.query.planner.logical.RexExpression;
+import org.apache.pinot.query.planner.partitioning.KeySelector;
+import org.apache.pinot.query.planner.partitioning.KeySelectorFactory;
+import org.apache.pinot.query.planner.plannode.JoinNode;
+import org.apache.pinot.query.runtime.blocks.MseBlock;
+import org.apache.pinot.query.runtime.plan.OpChainExecutionContext;
+
+
+public class AsofJoinOperator extends BaseJoinOperator {
+ private static final String EXPLAIN_NAME = "ASOF_JOIN";
+
+ private final Map<Object, NavigableMap<Comparable<?>, Object[]>> _rightTable;
+ private final KeySelector<?> _leftKeySelector;
+ private final KeySelector<?> _rightKeySelector;
+ private final MatchConditionType _matchConditionType;
+ private final int _leftMatchKeyIndex;
+ private final int _rightMatchKeyIndex;
+
+ public AsofJoinOperator(OpChainExecutionContext context, MultiStageOperator leftInput, DataSchema leftSchema,
+ MultiStageOperator rightInput, JoinNode node) {
+ super(context, leftInput, leftSchema, rightInput, node);
+ Preconditions.checkState(node.getNonEquiConditions().isEmpty(),
+ "ASOF_JOIN operator cannot have non-equi join keys");
+ Preconditions.checkState(node.getMatchCondition() != null, "ASOF_JOIN operator must have a match condition");
+ _rightTable = new HashMap<>();
+ List<Integer> leftKeys = node.getLeftKeys();
+
+ _leftKeySelector = KeySelectorFactory.getKeySelector(leftKeys);
+ _rightKeySelector = KeySelectorFactory.getKeySelector(node.getRightKeys());
+
+ RexExpression matchCondition = node.getMatchCondition();
+ Preconditions.checkState(matchCondition instanceof RexExpression.FunctionCall,
+ "ASOF_JOIN operator only supports function call match condition");
+
+ try {
+ _matchConditionType =
+ MatchConditionType.valueOf(((RexExpression.FunctionCall) matchCondition).getFunctionName().toUpperCase());
+ } catch (IllegalArgumentException e) {
+ throw new UnsupportedOperationException("Unsupported match condition: " + matchCondition);
+ }
+ List<RexExpression> matchKeys = ((RexExpression.FunctionCall) matchCondition).getFunctionOperands();
+ // TODO: Add support for MATCH_CONDITION containing two columns of different types. In that case, there would be
+ // a CAST RexExpression.FunctionCall on top of the RexExpression.InputRef, and we'd need to add the
+ // appropriate type casts to make sure that the Comparable based comparisons in this class don't throw
+ // ClassCastException.
+ Preconditions.checkState(
+ matchKeys.size() == 2 && matchKeys.get(0) instanceof RexExpression.InputRef
+ && matchKeys.get(1) instanceof RexExpression.InputRef,
+ "ASOF_JOIN operator only supports match conditions with a comparison between two columns of the same type");
Review Comment:
How is this checking that both columns are of the same type? I'm only seeing it checking they are instances of type `InputRef`, but maybe I'm missing something.
--
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
Re: [PR] ASOF JOIN [pinot]
Posted by "yashmayya (via GitHub)" <gi...@apache.org>.
yashmayya commented on code in PR #15630:
URL: https://github.com/apache/pinot/pull/15630#discussion_r2063712803
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/AsofJoinOperator.java:
##########
@@ -0,0 +1,183 @@
+/**
+ * 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.runtime.operator;
+
+import com.google.common.base.Preconditions;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.NavigableMap;
+import java.util.TreeMap;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.query.planner.logical.RexExpression;
+import org.apache.pinot.query.planner.partitioning.KeySelector;
+import org.apache.pinot.query.planner.partitioning.KeySelectorFactory;
+import org.apache.pinot.query.planner.plannode.JoinNode;
+import org.apache.pinot.query.runtime.blocks.MseBlock;
+import org.apache.pinot.query.runtime.plan.OpChainExecutionContext;
+
+
+public class AsofJoinOperator extends BaseJoinOperator {
+ private static final String EXPLAIN_NAME = "ASOF_JOIN";
+
+ private final Map<Object, NavigableMap<Comparable<?>, Object[]>> _rightTable;
+ private final KeySelector<?> _leftKeySelector;
+ private final KeySelector<?> _rightKeySelector;
+ private final MatchConditionType _matchConditionType;
+ private final int _leftMatchKeyIndex;
+ private final int _rightMatchKeyIndex;
+
+ public AsofJoinOperator(OpChainExecutionContext context, MultiStageOperator leftInput, DataSchema leftSchema,
+ MultiStageOperator rightInput, JoinNode node) {
+ super(context, leftInput, leftSchema, rightInput, node);
+ Preconditions.checkState(node.getNonEquiConditions().isEmpty(),
+ "ASOF_JOIN operator cannot have non-equi join keys");
+ Preconditions.checkState(node.getMatchCondition() != null, "ASOF_JOIN operator must have a match condition");
+ _rightTable = new HashMap<>();
+ List<Integer> leftKeys = node.getLeftKeys();
+
+ _leftKeySelector = KeySelectorFactory.getKeySelector(leftKeys);
+ _rightKeySelector = KeySelectorFactory.getKeySelector(node.getRightKeys());
+
+ RexExpression matchCondition = node.getMatchCondition();
+ Preconditions.checkState(matchCondition instanceof RexExpression.FunctionCall,
+ "ASOF_JOIN operator only supports function call match condition");
+
+ try {
+ _matchConditionType =
+ MatchConditionType.valueOf(((RexExpression.FunctionCall) matchCondition).getFunctionName().toUpperCase());
+ } catch (IllegalArgumentException e) {
+ throw new UnsupportedOperationException("Unsupported match condition: " + matchCondition);
+ }
+ List<RexExpression> matchKeys = ((RexExpression.FunctionCall) matchCondition).getFunctionOperands();
+ // TODO: Add support for MATCH_CONDITION containing two columns of different types. In that case, there would be
+ // a CAST RexExpression.FunctionCall on top of the RexExpression.InputRef, and we'd need to add the
+ // appropriate type casts to make sure that the Comparable based comparisons in this class don't throw
+ // ClassCastException.
+ Preconditions.checkState(
+ matchKeys.size() == 2 && matchKeys.get(0) instanceof RexExpression.InputRef
+ && matchKeys.get(1) instanceof RexExpression.InputRef,
+ "ASOF_JOIN operator only supports match conditions with a comparison between two columns of the same type");
Review Comment:
The TODO above mentions the reason - basically, if the two columns are of different but coercible types, Calcite will introduce a cast function call on one side of the match condition. So one of the keys will no longer be an `InputRef`, but a cast `FunctionCall` (with the `InputRef` being its first operand).
--
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
Re: [PR] ASOF JOIN [pinot]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #15630:
URL: https://github.com/apache/pinot/pull/15630#discussion_r2106575716
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/AsofJoinOperator.java:
##########
@@ -0,0 +1,183 @@
+/**
+ * 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.runtime.operator;
+
+import com.google.common.base.Preconditions;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.NavigableMap;
+import java.util.TreeMap;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.query.planner.logical.RexExpression;
+import org.apache.pinot.query.planner.partitioning.KeySelector;
+import org.apache.pinot.query.planner.partitioning.KeySelectorFactory;
+import org.apache.pinot.query.planner.plannode.JoinNode;
+import org.apache.pinot.query.runtime.blocks.MseBlock;
+import org.apache.pinot.query.runtime.plan.OpChainExecutionContext;
+
+
+public class AsofJoinOperator extends BaseJoinOperator {
+ private static final String EXPLAIN_NAME = "ASOF_JOIN";
+
+ private final Map<Object, NavigableMap<Comparable<?>, Object[]>> _rightTable;
+ private final KeySelector<?> _leftKeySelector;
+ private final KeySelector<?> _rightKeySelector;
+ private final MatchConditionType _matchConditionType;
+ private final int _leftMatchKeyIndex;
+ private final int _rightMatchKeyIndex;
+
+ public AsofJoinOperator(OpChainExecutionContext context, MultiStageOperator leftInput, DataSchema leftSchema,
+ MultiStageOperator rightInput, JoinNode node) {
+ super(context, leftInput, leftSchema, rightInput, node);
+ Preconditions.checkState(node.getNonEquiConditions().isEmpty(),
+ "ASOF_JOIN operator cannot have non-equi join keys");
+ Preconditions.checkState(node.getMatchCondition() != null, "ASOF_JOIN operator must have a match condition");
+ _rightTable = new HashMap<>();
+ List<Integer> leftKeys = node.getLeftKeys();
+
+ _leftKeySelector = KeySelectorFactory.getKeySelector(leftKeys);
+ _rightKeySelector = KeySelectorFactory.getKeySelector(node.getRightKeys());
+
+ RexExpression matchCondition = node.getMatchCondition();
+ Preconditions.checkState(matchCondition instanceof RexExpression.FunctionCall,
+ "ASOF_JOIN operator only supports function call match condition");
+
+ try {
+ _matchConditionType =
+ MatchConditionType.valueOf(((RexExpression.FunctionCall) matchCondition).getFunctionName().toUpperCase());
+ } catch (IllegalArgumentException e) {
+ throw new UnsupportedOperationException("Unsupported match condition: " + matchCondition);
+ }
+ List<RexExpression> matchKeys = ((RexExpression.FunctionCall) matchCondition).getFunctionOperands();
+ // TODO: Add support for MATCH_CONDITION containing two columns of different types. In that case, there would be
+ // a CAST RexExpression.FunctionCall on top of the RexExpression.InputRef, and we'd need to add the
+ // appropriate type casts to make sure that the Comparable based comparisons in this class don't throw
+ // ClassCastException.
+ Preconditions.checkState(
+ matchKeys.size() == 2 && matchKeys.get(0) instanceof RexExpression.InputRef
+ && matchKeys.get(1) instanceof RexExpression.InputRef,
+ "ASOF_JOIN operator only supports match conditions with a comparison between two columns of the same type");
+ _leftMatchKeyIndex = ((RexExpression.InputRef) matchKeys.get(0)).getIndex();
+ _rightMatchKeyIndex = ((RexExpression.InputRef) matchKeys.get(1)).getIndex() - leftSchema.size();
Review Comment:
Clarify in the comments that subtracting leftSchema.size() is based on the assumption that the right input's column indices start immediately after the left schema. This will help future maintainers understand the rationale behind the index adjustment.
--
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