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/10 09:02:23 UTC

[GitHub] [pinot] ankitsultana opened a new pull request, #9374: [multistage] Support IN Clause With 1 Argument

ankitsultana opened a new pull request, #9374:
URL: https://github.com/apache/pinot/pull/9374

   Related to #9345 
   
   Queries with a single IN clause argument is different from a query with multiple arguments in the IN clause. This PR adds support for the former. Will raise another PR soon for the multi-arg case (that PR will be a bit longer so I decoupled these)
   
   Query to verify:
   ```
   SELECT a.playerID, a.runs, a.yearID, b.runs, b.yearID
           FROM baseballStats_OFFLINE AS a JOIN baseballStats_OFFLINE AS b ON a.playerID = b.playerID
           WHERE a.runs > 160 AND b.teamID IN ('SFN')
   ```
   
   cc: @walterddr 


-- 
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 pull request #9374: [multistage] Support IN Clause With 1 Argument

Posted by GitBox <gi...@apache.org>.
ankitsultana commented on PR #9374:
URL: https://github.com/apache/pinot/pull/9374#issuecomment-1243236125

   @walterddr : I was working on the IN/NOT-IN/Range support. Does #9375 take care of that as well?


-- 
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 #9374: [multistage] Support IN and NOT-IN Clauses

Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9374:
URL: https://github.com/apache/pinot/pull/9374#discussion_r969876370


##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/logical/RexExpression.java:
##########
@@ -146,7 +180,7 @@ class Literal implements RexExpression {
     public Literal() {
     }
 
-    public Literal(FieldSpec.DataType dataType, SqlTypeName sqlTypeName, @Nullable Object value) {
+    public Literal(FieldSpec.DataType dataType, @Nullable Object value) {

Review Comment:
   good catch



##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/logical/RexExpression.java:
##########
@@ -106,6 +113,33 @@ static FieldSpec.DataType toDataType(RelDataType type) {
     }
   }
 
+  // POC Code. Will clean-up later.
+  static RexExpression handleSearch(RexCall rexCall) {

Review Comment:
   we can create a new util class call `RexExpressionUtils` to handle special SqlKinds



##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTest.java:
##########
@@ -100,6 +100,12 @@ private Object[][] provideTestSqlAndRowCount() {
         new Object[]{"SELECT a.col1, a.ts, b.col2, b.col3 FROM a JOIN b ON a.col1 = b.col2 "
             + " WHERE a.col3 >= 0 AND a.col2 = 'alice' AND b.col3 >= 0", 3},
 
+        // Join query with IN and Not-IN clause. Table A's side of join will return 9 rows and Table B's side will
+        // return 2 rows. Join will be only on col1=bar and since A will return 3 rows with that value and B will return
+        // 1 row, the final output will have 3 rows.
+        new Object[]{"SELECT a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 "

Review Comment:
   can we add more test. `IN` and `NOT IN` with 0, 1, and more than 1 values



-- 
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 #9374: [multistage] Support IN and NOT-IN Clauses

Posted by GitBox <gi...@apache.org>.
ankitsultana commented on code in PR #9374:
URL: https://github.com/apache/pinot/pull/9374#discussion_r971168114


##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTest.java:
##########
@@ -109,6 +109,23 @@ private Object[][] provideTestSqlAndRowCount() {
         new Object[]{"SELECT a.col1, a.ts, b.col2, b.col3 FROM a JOIN b ON a.col1 = b.col2 "
             + " WHERE a.col3 >= 0 AND a.col2 = 'alice' AND b.col3 >= 0", 3},
 
+        // Join query with IN and Not-IN clause. Table A's side of join will return 9 rows and Table B's side will
+        // return 2 rows. Join will be only on col1=bar and since A will return 3 rows with that value and B will return
+        // 1 row, the final output will have 3 rows.
+        new Object[]{"SELECT a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 "
+            + " WHERE a.col1 IN ('foo', 'bar', 'alice') AND b.col2 NOT IN ('foo', 'alice')", 3},

Review Comment:
   Done. Also added test for empty IN clause and range predicates to verify they are failing. The range test I'll move to testPlanWithoutException in the range PR.



-- 
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 #9374: [multistage] Support IN and NOT-IN Clauses

Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9374:
URL: https://github.com/apache/pinot/pull/9374#discussion_r969943442


##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTest.java:
##########
@@ -100,6 +100,12 @@ private Object[][] provideTestSqlAndRowCount() {
         new Object[]{"SELECT a.col1, a.ts, b.col2, b.col3 FROM a JOIN b ON a.col1 = b.col2 "
             + " WHERE a.col3 >= 0 AND a.col2 = 'alice' AND b.col3 >= 0", 3},
 
+        // Join query with IN and Not-IN clause. Table A's side of join will return 9 rows and Table B's side will
+        // return 2 rows. Join will be only on col1=bar and since A will return 3 rows with that value and B will return
+        // 1 row, the final output will have 3 rows.
+        new Object[]{"SELECT a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 "

Review Comment:
   also add test for implicit range search, such as 
   - `WHERE (a.col3 > 0 AND a.col3 < 10) OR a.col3 >= 30`
   - `WHERE a.col2 = 'foo' OR a.col2 = 'bar'`



-- 
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 #9374: [multistage] Support IN and NOT-IN Clauses

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #9374:
URL: https://github.com/apache/pinot/pull/9374#issuecomment-1243285187

   # [Codecov](https://codecov.io/gh/apache/pinot/pull/9374?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 [#9374](https://codecov.io/gh/apache/pinot/pull/9374?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8458f81) into [master](https://codecov.io/gh/apache/pinot/commit/454a1d87400b3033b72b5a6beb06432134ff3c5e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (454a1d8) will **decrease** coverage by `2.76%`.
   > The diff coverage is `78.26%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #9374      +/-   ##
   ============================================
   - Coverage     69.73%   66.97%   -2.77%     
   + Complexity     5081     4899     -182     
   ============================================
     Files          1884     1402     -482     
     Lines        100282    72929   -27353     
     Branches      15253    11686    -3567     
   ============================================
   - Hits          69935    48845   -21090     
   + Misses        25402    20551    -4851     
   + Partials       4945     3533    -1412     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `?` | |
   | integration2 | `?` | |
   | unittests1 | `66.97% <78.26%> (+<0.01%)` | :arrow_up: |
   | 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/9374?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/parser/CalciteRexExpressionParser.java](https://codecov.io/gh/apache/pinot/pull/9374/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-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGFyc2VyL0NhbGNpdGVSZXhFeHByZXNzaW9uUGFyc2VyLmphdmE=) | `50.00% <ø> (ø)` | |
   | [...che/pinot/query/planner/logical/RexExpression.java](https://codecov.io/gh/apache/pinot/pull/9374/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-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9sb2dpY2FsL1JleEV4cHJlc3Npb24uamF2YQ==) | `75.60% <77.27%> (-3.76%)` | :arrow_down: |
   | [...inot/query/runtime/operator/AggregateOperator.java](https://codecov.io/gh/apache/pinot/pull/9374/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-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9vcGVyYXRvci9BZ2dyZWdhdGVPcGVyYXRvci5qYXZh) | `84.21% <100.00%> (-0.17%)` | :arrow_down: |
   | [...va/org/apache/pinot/core/routing/RoutingTable.java](https://codecov.io/gh/apache/pinot/pull/9374/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yb3V0aW5nL1JvdXRpbmdUYWJsZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...va/org/apache/pinot/common/config/NettyConfig.java](https://codecov.io/gh/apache/pinot/pull/9374/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL05ldHR5Q29uZmlnLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/9374/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/9374/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/9374/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/9374/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...ache/pinot/server/access/AccessControlFactory.java](https://codecov.io/gh/apache/pinot/pull/9374/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VydmVyL2FjY2Vzcy9BY2Nlc3NDb250cm9sRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | ... and [741 more](https://codecov.io/gh/apache/pinot/pull/9374/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 pull request #9374: [multistage] Support IN Clause With 1 Argument

Posted by GitBox <gi...@apache.org>.
walterddr commented on PR #9374:
URL: https://github.com/apache/pinot/pull/9374#issuecomment-1242968647

   this should be included in https://github.com/apache/pinot/pull/9375 


-- 
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 #9374: [multistage] Support IN and NOT-IN Clauses

Posted by GitBox <gi...@apache.org>.
ankitsultana commented on code in PR #9374:
URL: https://github.com/apache/pinot/pull/9374#discussion_r969963755


##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTest.java:
##########
@@ -100,6 +100,12 @@ private Object[][] provideTestSqlAndRowCount() {
         new Object[]{"SELECT a.col1, a.ts, b.col2, b.col3 FROM a JOIN b ON a.col1 = b.col2 "
             + " WHERE a.col3 >= 0 AND a.col2 = 'alice' AND b.col3 >= 0", 3},
 
+        // Join query with IN and Not-IN clause. Table A's side of join will return 9 rows and Table B's side will
+        // return 2 rows. Join will be only on col1=bar and since A will return 3 rows with that value and B will return
+        // 1 row, the final output will have 3 rows.
+        new Object[]{"SELECT a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 "

Review Comment:
   I can add the range search test in the range PR that I'll raise later this week?
   
   This PR only focuses on IN/Not-In. Ranges won't work yet.



-- 
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 #9374: [multistage] Support IN and NOT-IN Clauses

Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9374:
URL: https://github.com/apache/pinot/pull/9374#discussion_r971147230


##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/logical/RexExpression.java:
##########
@@ -52,30 +51,32 @@ static RexExpression toRexExpression(RexNode rexNode) {
     } else if (rexNode instanceof RexLiteral) {
       RexLiteral rexLiteral = ((RexLiteral) rexNode);
       FieldSpec.DataType dataType = toDataType(rexLiteral.getType());
-      return new RexExpression.Literal(dataType, rexLiteral.getTypeName(),
-          toRexValue(dataType, rexLiteral.getValue()));
+      return new RexExpression.Literal(dataType, toRexValue(dataType, rexLiteral.getValue()));
     } else if (rexNode instanceof RexCall) {
       RexCall rexCall = (RexCall) rexNode;
-      List<RexExpression> operands = rexCall.getOperands().stream().map(RexExpression::toRexExpression)
-          .collect(Collectors.toList());
-      return toRexExpression(rexCall, operands);
+      return toRexExpression(rexCall);
     } else {
       throw new IllegalArgumentException("Unsupported RexNode type with SqlKind: " + rexNode.getKind());
     }
   }
 
-  static RexExpression toRexExpression(RexCall rexCall, List<RexExpression> operands) {
+  static RexExpression toRexExpression(RexCall rexCall) {
+    List<RexExpression> operands;
     switch (rexCall.getKind()) {
       case CAST:
         // CAST is being rewritten into "rexCall.CAST<targetType>(inputValue)",
         //   - e.g. result type has already been converted into the CAST RexCall, so we assert single operand.
+        operands = rexCall.getOperands().stream().map(RexExpression::toRexExpression).collect(Collectors.toList());
         Preconditions.checkState(operands.size() == 1, "CAST takes exactly 2 arguments");
         RelDataType castType = rexCall.getType();
         // add the 2nd argument as the source type info.
-        operands.add(new Literal(FieldSpec.DataType.STRING, rexCall.getOperands().get(0).getType().getSqlTypeName(),
+        operands.add(new Literal(FieldSpec.DataType.STRING,
             toPinotDataType(rexCall.getOperands().get(0).getType()).name()));
         return new RexExpression.FunctionCall(rexCall.getKind(), toDataType(rexCall.getType()), "CAST", operands);

Review Comment:
   can you also refactor this into `RexExpressionUtils.handleCast` ?



-- 
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 #9374: [multistage] Support IN and NOT-IN Clauses

Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9374:
URL: https://github.com/apache/pinot/pull/9374#discussion_r971148340


##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTest.java:
##########
@@ -109,6 +109,23 @@ private Object[][] provideTestSqlAndRowCount() {
         new Object[]{"SELECT a.col1, a.ts, b.col2, b.col3 FROM a JOIN b ON a.col1 = b.col2 "
             + " WHERE a.col3 >= 0 AND a.col2 = 'alice' AND b.col3 >= 0", 3},
 
+        // Join query with IN and Not-IN clause. Table A's side of join will return 9 rows and Table B's side will
+        // return 2 rows. Join will be only on col1=bar and since A will return 3 rows with that value and B will return
+        // 1 row, the final output will have 3 rows.
+        new Object[]{"SELECT a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 "
+            + " WHERE a.col1 IN ('foo', 'bar', 'alice') AND b.col2 NOT IN ('foo', 'alice')", 3},

Review Comment:
   can you also add a test in QueryCompilationTest (QueryEnvironmentTestBase) for the parsing? just one single entry should be enough by mixing all types of predicates together



-- 
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 pull request #9374: [multistage] Support IN and NOT-IN Clauses

Posted by GitBox <gi...@apache.org>.
walterddr commented on PR #9374:
URL: https://github.com/apache/pinot/pull/9374#issuecomment-1243824767

   > @walterddr : I was working on the IN/NOT-IN/Range support. Does #9375 take care of that as well?
   
   ah. no that only takes care of the `CHAR/VARCHAR` codepath to decode NlsString into regular String


-- 
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 #9374: [multistage] Support IN and NOT-IN Clauses

Posted by GitBox <gi...@apache.org>.
walterddr merged PR #9374:
URL: https://github.com/apache/pinot/pull/9374


-- 
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 #9374: [multistage] Support IN and NOT-IN Clauses

Posted by GitBox <gi...@apache.org>.
ankitsultana commented on code in PR #9374:
URL: https://github.com/apache/pinot/pull/9374#discussion_r970210306


##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTest.java:
##########
@@ -100,6 +100,12 @@ private Object[][] provideTestSqlAndRowCount() {
         new Object[]{"SELECT a.col1, a.ts, b.col2, b.col3 FROM a JOIN b ON a.col1 = b.col2 "
             + " WHERE a.col3 >= 0 AND a.col2 = 'alice' AND b.col3 >= 0", 3},
 
+        // Join query with IN and Not-IN clause. Table A's side of join will return 9 rows and Table B's side will
+        // return 2 rows. Join will be only on col1=bar and since A will return 3 rows with that value and B will return
+        // 1 row, the final output will have 3 rows.
+        new Object[]{"SELECT a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 "

Review Comment:
   Updated. Query fails parsing if there are no arguments to the IN clause, i.e. `col1 IN ()` fails



-- 
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 #9374: [multistage] Support IN and NOT-IN Clauses

Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9374:
URL: https://github.com/apache/pinot/pull/9374#discussion_r971147230


##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/logical/RexExpression.java:
##########
@@ -52,30 +51,32 @@ static RexExpression toRexExpression(RexNode rexNode) {
     } else if (rexNode instanceof RexLiteral) {
       RexLiteral rexLiteral = ((RexLiteral) rexNode);
       FieldSpec.DataType dataType = toDataType(rexLiteral.getType());
-      return new RexExpression.Literal(dataType, rexLiteral.getTypeName(),
-          toRexValue(dataType, rexLiteral.getValue()));
+      return new RexExpression.Literal(dataType, toRexValue(dataType, rexLiteral.getValue()));
     } else if (rexNode instanceof RexCall) {
       RexCall rexCall = (RexCall) rexNode;
-      List<RexExpression> operands = rexCall.getOperands().stream().map(RexExpression::toRexExpression)
-          .collect(Collectors.toList());
-      return toRexExpression(rexCall, operands);
+      return toRexExpression(rexCall);
     } else {
       throw new IllegalArgumentException("Unsupported RexNode type with SqlKind: " + rexNode.getKind());
     }
   }
 
-  static RexExpression toRexExpression(RexCall rexCall, List<RexExpression> operands) {
+  static RexExpression toRexExpression(RexCall rexCall) {
+    List<RexExpression> operands;
     switch (rexCall.getKind()) {
       case CAST:
         // CAST is being rewritten into "rexCall.CAST<targetType>(inputValue)",
         //   - e.g. result type has already been converted into the CAST RexCall, so we assert single operand.
+        operands = rexCall.getOperands().stream().map(RexExpression::toRexExpression).collect(Collectors.toList());
         Preconditions.checkState(operands.size() == 1, "CAST takes exactly 2 arguments");
         RelDataType castType = rexCall.getType();
         // add the 2nd argument as the source type info.
-        operands.add(new Literal(FieldSpec.DataType.STRING, rexCall.getOperands().get(0).getType().getSqlTypeName(),
+        operands.add(new Literal(FieldSpec.DataType.STRING,
             toPinotDataType(rexCall.getOperands().get(0).getType()).name()));
         return new RexExpression.FunctionCall(rexCall.getKind(), toDataType(rexCall.getType()), "CAST", operands);

Review Comment:
   can you also refactor this into RexExpressionUtils?



-- 
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