walterddr commented on code in PR #9775:
URL: https://github.com/apache/pinot/pull/9775#discussion_r1019456017
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/HashJoinOperator.java:
##########
@@ -63,13 +64,18 @@ public class HashJoinOperator extends
BaseOperator<TransferableBlock> {
private KeySelector<Object[], Object[]> _leftKeySelector;
private KeySelector<Object[], Object[]> _rightKeySelector;
- // TODO: Fix inequi join bug. (https://github.com/apache/pinot/issues/9728)
- // TODO: Double check semi join logic.
public HashJoinOperator(Operator<TransferableBlock> leftTableOperator,
Operator<TransferableBlock> rightTableOperator,
DataSchema outputSchema, JoinNode.JoinKeys joinKeys, List<RexExpression>
joinClauses, JoinRelType joinType) {
- // TODO: Handle the case where _leftKeySelector and _rightKeySelector
could be null.
+ // TODO: Move precondition check to constructor and handle error in
upstream.
+ // TODO: Fix semi join bug. (https://github.com/apache/pinot/issues/9757)
+ Preconditions.checkState(joinType != JoinRelType.SEMI, "Semi join is not
supported");
+ Preconditions.checkState(joinType != JoinRelType.RIGHT, "Right join is not
supported");
+ Preconditions.checkState(joinType != JoinRelType.FULL, "Full join is not
supported");
+ Preconditions.checkState(joinType != JoinRelType.ANTI, "Anti join is not
supported");
Review Comment:
use static final SUPPORTED_JOIN_TYPE instead
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/QueryTestSet.java:
##########
@@ -26,6 +26,11 @@ public class QueryTestSet {
@DataProvider(name = "testSql")
public Object[][] provideTestSql() {
return new Object[][]{
+ // Failed left join. Not sure what's the expectation here.
+ // new Object[]{"SELECT a.col1 FROM a LEFT JOIN b ON (a.col1 =
'foo')"},
+ // new Object[]{"SELECT a.col1 FROM a LEFT JOIN b ON (a.col1 = b.col1
and a.col1 = 'foo)"},
+ // Failed semi join. Somehow it gets time out now
+ // new Object[]{"SELECT * FROM b WHERE b.col1 IN (SELECT a.col1 FROM
a)"},
Review Comment:
let's dont add these to the E2E test. but to the operator test
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
