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/12/02 00:15:31 UTC
[GitHub] [pinot] 61yao opened a new pull request, #9895: [multistage] [feature] Support right outer join
61yao opened a new pull request, #9895:
URL: https://github.com/apache/pinot/pull/9895
1. Broadcast left table and probe the broadcast table using right table
2. This PR assumes that partitioned left join table can fit in memory for right join
--
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 #9895: [multistage] [feature] Support right outer join
Posted by GitBox <gi...@apache.org>.
61yao commented on code in PR #9895:
URL: https://github.com/apache/pinot/pull/9895#discussion_r1041419948
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/HashJoinOperator.java:
##########
@@ -114,14 +125,98 @@ protected TransferableBlock getNextBlock() {
return TransferableBlockUtils.getNoOpTransferableBlock();
}
// JOIN each left block with the right block.
- return buildJoinedDataBlock(_leftTableOperator.nextBlock());
+ return buildJoinedDataBlock(getProbeOperator().nextBlock());
} catch (Exception e) {
return TransferableBlockUtils.getErrorTransferableBlock(e);
}
}
+ private Operator<TransferableBlock> getBroadcastOperator() {
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] agavra commented on pull request #9895: [multistage] [feature] Support right outer join
Posted by GitBox <gi...@apache.org>.
agavra commented on PR #9895:
URL: https://github.com/apache/pinot/pull/9895#issuecomment-1338193048
> The result should be order by col1 without re-write.
why would that be the case? ordering by a constant shouldn't guarantee anything right?
--
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 closed pull request #9895: [multistage] [feature] Support right outer join
Posted by GitBox <gi...@apache.org>.
61yao closed pull request #9895: [multistage] [feature] Support right outer join
URL: https://github.com/apache/pinot/pull/9895
--
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 #9895: [multistage] [feature] Support right outer join
Posted by GitBox <gi...@apache.org>.
61yao commented on PR #9895:
URL: https://github.com/apache/pinot/pull/9895#issuecomment-1335825931
> > > WDYT about implementing this as a rewriter that just swaps the left and right tables?
> >
> >
> > That's a good idea. but then we need to deal the situation where we output the rows in the right column order. Let me check the code to see how complicated it is
>
> I can probably write a calcite rule to do that I guess. I check that calcite doesn't have builtin rule for this.
I tried to code the re-write and seems it is not very easy to do:
https://github.com/apache/pinot/pull/9903/files
Calcite will throw exception on transform call when we changed the join type
I didn't update the inequi join condition (join.condition) in 9903 PR. but we have to solve the exception problem first. So I'll stick to this PR's method for now.
@agavra
--
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 #9895: [multistage] [feature] Support right outer join
Posted by GitBox <gi...@apache.org>.
61yao commented on code in PR #9895:
URL: https://github.com/apache/pinot/pull/9895#discussion_r1044035634
##########
pinot-query-runtime/src/test/resources/queries/FromExpressions.json:
##########
@@ -57,9 +57,14 @@
},
{
"psql": "7.2.1.1",
- "ignored": true,
+ "comments": "select all doesn't work because h2 output the rows in different order from Pinot and test framework doesn't consider the column order for rows and blindly assume they are in the same order",
"description": "RIGHT OUTER JOIN",
- "sql": "SELECT * FROM {tbl1} RIGHT OUTER JOIN {tbl2} ON {tbl1}.col1 = {tbl2}.col1"
+ "sql": "SELECT {tbl1}.num, {tbl1}.name, {tbl2}.num, {tbl2}.value FROM {tbl1} RIGHT OUTER JOIN {tbl2} ON {tbl1}.num = {tbl2}.num"
Review Comment:
Addressed in the other PR.
Nice test and caught a bug.
Unfortunately, the inequality join sometimes doesn't work because we don't handle the null value. This is not a regression because left join has the same problem. :(
--
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 #9895: [multistage] [feature] Support right outer join
Posted by GitBox <gi...@apache.org>.
61yao commented on PR #9895:
URL: https://github.com/apache/pinot/pull/9895#issuecomment-1334654361
> > WDYT about implementing this as a rewriter that just swaps the left and right tables?
>
> That's a good idea. but then we need to deal the situation where we output the rows in the right column order. Let me check the code to see how complicated it is
I can probably write a calcite rule to do that I guess. I check that calcite doesn't have builtin rule for 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 #9895: [multistage] [feature] Support right outer join
Posted by GitBox <gi...@apache.org>.
61yao commented on code in PR #9895:
URL: https://github.com/apache/pinot/pull/9895#discussion_r1041420841
##########
pinot-query-runtime/src/test/resources/queries/FromExpressions.json:
##########
@@ -57,9 +57,14 @@
},
{
"psql": "7.2.1.1",
- "ignored": true,
+ "comments": "select all doesn't work because h2 output the rows in different order from Pinot and test framework doesn't consider the column order for rows and blindly assume they are in the same order",
Review Comment:
Unfortunately no because ts columns is also selected, that value cannot be specified.
--
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] agavra commented on pull request #9895: [multistage] [feature] Support right outer join
Posted by GitBox <gi...@apache.org>.
agavra commented on PR #9895:
URL: https://github.com/apache/pinot/pull/9895#issuecomment-1338275216
Update: just realized that `order by 1` orders by the first column, not the constant "1"...
--
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 #9895: [multistage] [feature] Support right outer join
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9895:
URL: https://github.com/apache/pinot/pull/9895#discussion_r1042903441
##########
pinot-query-runtime/src/test/resources/queries/FromExpressions.json:
##########
@@ -57,9 +57,14 @@
},
{
"psql": "7.2.1.1",
- "ignored": true,
+ "comments": "select all doesn't work because h2 output the rows in different order from Pinot and test framework doesn't consider the column order for rows and blindly assume they are in the same order",
"description": "RIGHT OUTER JOIN",
- "sql": "SELECT * FROM {tbl1} RIGHT OUTER JOIN {tbl2} ON {tbl1}.col1 = {tbl2}.col1"
+ "sql": "SELECT {tbl1}.num, {tbl1}.name, {tbl2}.num, {tbl2}.value FROM {tbl1} RIGHT OUTER JOIN {tbl2} ON {tbl1}.num = {tbl2}.num"
Review Comment:
could you add tests
1. with inequality conditions?
2. with mixed column selections such as `{tbl1}.num, {tbl2}.num, {tbl1}.name, {tbl2}.value`
3. with mixed transform functions `{tbl1}.num > {tbl2}.num} + 1`
--
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 #9895: [multistage] [feature] Support right outer join
Posted by GitBox <gi...@apache.org>.
61yao commented on PR #9895:
URL: https://github.com/apache/pinot/pull/9895#issuecomment-1334619423
> WDYT about implementing this as a rewriter that just swaps the left and right tables?
That's a good idea. but then we need to deal the situation where we output the rows in the right column order. Let me check the code to see how complicated it is
--
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] agavra commented on pull request #9895: [multistage] [feature] Support right outer join
Posted by GitBox <gi...@apache.org>.
agavra commented on PR #9895:
URL: https://github.com/apache/pinot/pull/9895#issuecomment-1335927107
@61yao instead of an optimization rule it should be implemented as a rewriter. I tested this and it works:
```java
public class RightJoinRewriter extends SqlShuttle {
public SqlNode visit(SqlCall call) {
if (call instanceof SqlJoin) {
SqlJoin join = (SqlJoin) call;
if (join.getJoinType() == JoinType.RIGHT) {
return super.visit(new SqlJoin(
call.getParserPosition(),
join.getRight(),
join.isNaturalNode(),
SqlLiteral.createSymbol(JoinType.LEFT, join.getJoinTypeNode().getParserPosition()),
join.getLeft(),
join.getConditionTypeNode(),
join.getCondition()));
}
}
return super.visit(call);
}
}
```
And then I modified `CalciteSqlParser#extractSqlNodeAndOptions` to apply this rewriter:
```java
return new SqlNodeAndOptions(statementNode.accept(new RightJoinRewriter()), sqlType, options);
```
--
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 #9895: [multistage] [feature] Support right outer join
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9895:
URL: https://github.com/apache/pinot/pull/9895#discussion_r1042904694
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/HashJoinOperator.java:
##########
@@ -42,51 +43,98 @@
/**
* This basic {@code BroadcastJoinOperator} implement a basic broadcast join algorithm.
+ * This algorithm assumes that the broadcast table has to fit in memory since we are not supporting any spilling.
*
+ * For left join and inner join,
* <p>It takes the right table as the broadcast side and materialize a hash table. Then for each of the left table row,
* it looks up for the corresponding row(s) from the hash table and create a joint row.
*
* <p>For each of the data block received from the left table, it will generate a joint data block.
*
- * We currently support left join, inner join and semi join.
+ * For right join,
+ * We broadcast the left table and probe the hash table using right table.
+ *
+ * We currently support left join, inner join and right join.
* The output is in the format of [left_row, right_row]
*/
// TODO: Move inequi out of hashjoin. (https://github.com/apache/pinot/issues/9728)
public class HashJoinOperator extends BaseOperator<TransferableBlock> {
+ private static class JoinResolver {
+ public static JoinResolver create(JoinRelType joinType, Operator<TransferableBlock> leftTableOperator,
+ Operator<TransferableBlock> rightTableOperator, KeySelector leftKeySelector, KeySelector rightKeySelector) {
+ JoinResolver resolver = new JoinResolver();
+ switch (joinType) {
+ case LEFT:
+ case INNER:
+ resolver._broadcastOperator = rightTableOperator;
+ resolver._probeOperator = leftTableOperator;
+ resolver._probeKeySelector = leftKeySelector;
+ resolver._broadcastKeySelector = rightKeySelector;
+ resolver._getLeftRow = (Object[] probeRow, Object[] broadcastRow) -> probeRow;
+ resolver._getRightRow = (Object[] probeRow, Object[] broadcastRow) -> broadcastRow;
+ break;
+ case RIGHT:
+ resolver._broadcastOperator = leftTableOperator;
+ resolver._probeOperator = rightTableOperator;
+ resolver._probeKeySelector = rightKeySelector;
+ resolver._broadcastKeySelector = leftKeySelector;
+ resolver._getLeftRow = (Object[] probeRow, Object[] broadcastRow) -> broadcastRow;
+ resolver._getRightRow = (Object[] probeRow, Object[] broadcastRow) -> probeRow;
+ break;
Review Comment:
i felt like this approach is a bit too complex and wasn't sure if this is the most appropriate approach.
for a more straight forward way to handle this we can simple do the exact same algorithm
1. if LEFT JOIN || FULL OUTER, and right-side hashmap match not found, return left side + right null
2. if RIGHT JOIN || FULL OUTER, and right-side hashmap match has not been used, return left null + right side.
this way we can also address #9907 together. which you will have to do it anyway.
for a more concrete / optimized solution we should consider rewriting relational correlations.
--
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 #9895: [multistage] [feature] Support right outer join
Posted by GitBox <gi...@apache.org>.
61yao commented on PR #9895:
URL: https://github.com/apache/pinot/pull/9895#issuecomment-1336281494
>
This works pretty well except one corner case:
Select * From tbl1 LEFT join tbl2 order by 1;
This column reference 1 is referring to different thing after rewrite.
H2 actually has the same issue I guess they are also doing this rewrite trick. I tested it in psql and it works correctly (i.e. it returns the value in right order).
We have a couple options:
1) Rewrite the order by 1 as well to handle the corner case. (this is kinda tricky because have to know how many rows for left and right table during sql rewrite
2) Re-order the columns after join. we somehow need to pass an option to join implementation to let join know the re-ordering is happening.
3) We just ignore the error and write in the documentation for the behavior expectation.
4) Write the implementation for right join
Maybe let's do 4) for now and we can do the rewrite in next iteration? I do prefer the rewrite version if it works correctly because it is way more elegant than implementing everything twice.
--
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 #9895: [multistage] [feature] Support right outer join
Posted by GitBox <gi...@apache.org>.
61yao commented on PR #9895:
URL: https://github.com/apache/pinot/pull/9895#issuecomment-1337999081
> https://github.com/apache/pinot/pull/9903/files#diff-0ab607c4916711c3c741979eaf2e14eb04559c500b3a78ef648e3bba1923f461
Say tbl1 has col1, col2, tbl2 has col3 and col4
Select * From tbl1 LEFT join tbl2 order by 1;
The result should be order by col1 without re-write.
Applying the rewrite will order the result by col3.
@agavra ^
--
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] agavra commented on pull request #9895: [multistage] [feature] Support right outer join
Posted by GitBox <gi...@apache.org>.
agavra commented on PR #9895:
URL: https://github.com/apache/pinot/pull/9895#issuecomment-1334613980
WDYT about implementing this as a rewriter that just swaps the left and right tables?
--
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 #9895: [multistage] [feature] Support right outer join
Posted by GitBox <gi...@apache.org>.
61yao commented on PR #9895:
URL: https://github.com/apache/pinot/pull/9895#issuecomment-1335978224
> ```java
> SqlShuttle
> ```
I see. Let me try!
--
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 #9895: [multistage] [feature] Support right outer join
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #9895:
URL: https://github.com/apache/pinot/pull/9895#issuecomment-1339936380
# [Codecov](https://codecov.io/gh/apache/pinot/pull/9895?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 [#9895](https://codecov.io/gh/apache/pinot/pull/9895?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (eef85a1) into [master](https://codecov.io/gh/apache/pinot/commit/041865a80f7a2359270571a2343049bb1f294fe5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (041865a) will **decrease** coverage by `43.54%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #9895 +/- ##
=============================================
- Coverage 68.65% 25.10% -43.55%
+ Complexity 5049 44 -5005
=============================================
Files 1973 1970 -3
Lines 106008 106075 +67
Branches 16060 16087 +27
=============================================
- Hits 72775 26631 -46144
- Misses 28110 76724 +48614
+ Partials 5123 2720 -2403
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `25.10% <0.00%> (+0.05%)` | :arrow_up: |
| 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/9895?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...pinot/query/runtime/operator/HashJoinOperator.java](https://codecov.io/gh/apache/pinot/pull/9895/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-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9vcGVyYXRvci9IYXNoSm9pbk9wZXJhdG9yLmphdmE=) | `0.00% <0.00%> (-93.34%)` | :arrow_down: |
| [.../pinot/query/runtime/plan/PhysicalPlanVisitor.java](https://codecov.io/gh/apache/pinot/pull/9895/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-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9wbGFuL1BoeXNpY2FsUGxhblZpc2l0b3IuamF2YQ==) | `0.00% <0.00%> (-96.88%)` | :arrow_down: |
| [...rg/apache/pinot/query/service/QueryDispatcher.java](https://codecov.io/gh/apache/pinot/pull/9895/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-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvc2VydmljZS9RdWVyeURpc3BhdGNoZXIuamF2YQ==) | `0.00% <ø> (-84.70%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/spi/utils/LoopUtils.java](https://codecov.io/gh/apache/pinot/pull/9895/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvTG9vcFV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...in/java/org/apache/pinot/spi/utils/BytesUtils.java](https://codecov.io/gh/apache/pinot/pull/9895/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/9895/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/9895/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/9895/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/9895/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/9895/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/9895/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 #9895: [multistage] [feature] Support right outer join
Posted by GitBox <gi...@apache.org>.
61yao commented on code in PR #9895:
URL: https://github.com/apache/pinot/pull/9895#discussion_r1041420234
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/service/QueryConfig.java:
##########
@@ -22,7 +22,7 @@
* Configuration for setting up query runtime.
*/
public class QueryConfig {
- public static final long DEFAULT_TIMEOUT_NANO = 10_000_000_000L;
+ public static final long DEFAULT_TIMEOUT_NANO = 10_000_000_000_000L;
Review Comment:
Ah. yeah. thanks for catching 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 pull request #9895: [multistage] [feature] Support right outer join
Posted by GitBox <gi...@apache.org>.
61yao commented on PR #9895:
URL: https://github.com/apache/pinot/pull/9895#issuecomment-1336281788
> ```java
> RightJoinRewriter
> ```
@agavra See the test case here for rewrite problem:
https://github.com/apache/pinot/pull/9903/files#diff-0ab607c4916711c3c741979eaf2e14eb04559c500b3a78ef648e3bba1923f461
--
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] agavra commented on pull request #9895: [multistage] [feature] Support right outer join
Posted by GitBox <gi...@apache.org>.
agavra commented on PR #9895:
URL: https://github.com/apache/pinot/pull/9895#issuecomment-1337810988
@61yao I'm not totally sure I understand, what is ordering by a constant supposed do?
--
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] agavra commented on a diff in pull request #9895: [multistage] [feature] Support right outer join
Posted by GitBox <gi...@apache.org>.
agavra commented on code in PR #9895:
URL: https://github.com/apache/pinot/pull/9895#discussion_r1040180019
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/HashJoinOperator.java:
##########
@@ -114,14 +125,98 @@ protected TransferableBlock getNextBlock() {
return TransferableBlockUtils.getNoOpTransferableBlock();
}
// JOIN each left block with the right block.
- return buildJoinedDataBlock(_leftTableOperator.nextBlock());
+ return buildJoinedDataBlock(getProbeOperator().nextBlock());
} catch (Exception e) {
return TransferableBlockUtils.getErrorTransferableBlock(e);
}
}
+ private Operator<TransferableBlock> getBroadcastOperator() {
Review Comment:
nit (suggestion): might be nicer to wrap all of these in a single Tuple class so that we can have only once switch statement:
```
static class JoinResolver {
Operator<TransferableBlock> broadcastOperator;
Operator<TransferableBlock> probeOperator;
KeySelector broadcastSelector;
KeySelector probOperator;
BiFunction<Object[], Object[]> getLeftRow;
BiFunction<Object[], Object[]> getRightRow;
}
```
and then just:
```
switch(_joinType) {
case Left:
return new JoinResolver(...)
}
```
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/service/QueryConfig.java:
##########
@@ -22,7 +22,7 @@
* Configuration for setting up query runtime.
*/
public class QueryConfig {
- public static final long DEFAULT_TIMEOUT_NANO = 10_000_000_000L;
+ public static final long DEFAULT_TIMEOUT_NANO = 10_000_000_000_000L;
Review Comment:
(MAJOR) let's not forget to revert this!
##########
pinot-query-runtime/src/test/resources/queries/FromExpressions.json:
##########
@@ -57,9 +57,14 @@
},
{
"psql": "7.2.1.1",
- "ignored": true,
+ "comments": "select all doesn't work because h2 output the rows in different order from Pinot and test framework doesn't consider the column order for rows and blindly assume they are in the same order",
Review Comment:
can we add a test for this with the outputs specified?
--
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