haridsv commented on code in PR #2239:
URL: https://github.com/apache/phoenix/pull/2239#discussion_r2253237639


##########
phoenix-core-client/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java:
##########
@@ -1945,6 +1948,152 @@ public KeySlots visitLeave(InListExpression node, 
List<KeySlots> childParts) {
       return newKeyParts(childSlot, node, new ArrayList<KeyRange>(ranges));
     }
 
+    /**
+     * If {@link ArrayAnyComparisonExpression} is of the form:
+     *
+     * <pre>
+     * COL = ANY(ARR)
+     * </pre>
+     *
+     * then we can extract the scan ranges for the COL, given COL is a PK 
column. This syntactical
+     * pattern can be used as a replacement for a IN expression. So, instead 
of following IN
+     * expression:
+     *
+     * <pre>
+     * COL IN (VAL1, VAL2, ... VALN)
+     * </pre>
+     *
+     * we can use the following ANY expression:
+     *
+     * <pre>
+     * try (Connection conn = DriverManager.getConnection(url)) {
+     *      conn.createArrayOf("CHAR", new String[] {"VAL1", "VAL2", ... 
"VALN"});
+     *      try (PreparedStatement stmt = conn.prepareStatement(
+     *          "SELECT ... FROM TABLE WHERE COL = ANY(?)")) {
+     *          stmt.setArray(1, arr);
+     *          ResultSet rs = stmt.executeQuery();
+     *      }
+     * }
+     * </pre>
+     *
+     * This will help in saving the query parsing time as on using IN list 
query parsing time
+     * increases with the size of IN list but in case of ANY expression it is 
constant. Below we
+     * account for cases where COL is on the LHS or RHS of the comparison 
expression.
+     * @param node           {@link ArrayAnyComparisonExpression} node for 
which scan ranges are to
+     *                       be extracted
+     * @param keyExpressions {@link RowKeyColumnExpression} for the PK column 
for which scan ranges
+     *                       are to be extracted
+     * @return true if the scan ranges can be extracted, false otherwise
+     */
+    private boolean 
shouldExtractKeyRangesForArrayAnyExpr(ArrayAnyComparisonExpression node,

Review Comment:
   Nit: Method  name feels like it is a readonly operation, doesn't clearly 
indicate that `keyExpressions` will be modified.



##########
phoenix-core-client/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java:
##########
@@ -1945,6 +1948,152 @@ public KeySlots visitLeave(InListExpression node, 
List<KeySlots> childParts) {
       return newKeyParts(childSlot, node, new ArrayList<KeyRange>(ranges));
     }
 
+    /**
+     * If {@link ArrayAnyComparisonExpression} is of the form:
+     *
+     * <pre>
+     * COL = ANY(ARR)
+     * </pre>
+     *
+     * then we can extract the scan ranges for the COL, given COL is a PK 
column. This syntactical
+     * pattern can be used as a replacement for a IN expression. So, instead 
of following IN
+     * expression:
+     *
+     * <pre>
+     * COL IN (VAL1, VAL2, ... VALN)
+     * </pre>
+     *
+     * we can use the following ANY expression:
+     *
+     * <pre>
+     * try (Connection conn = DriverManager.getConnection(url)) {
+     *      conn.createArrayOf("CHAR", new String[] {"VAL1", "VAL2", ... 
"VALN"});
+     *      try (PreparedStatement stmt = conn.prepareStatement(
+     *          "SELECT ... FROM TABLE WHERE COL = ANY(?)")) {
+     *          stmt.setArray(1, arr);
+     *          ResultSet rs = stmt.executeQuery();
+     *      }
+     * }

Review Comment:
   ```suggestion
        * we can use the above ANY expression, like this:
        *
        * <pre>
        * try (Connection conn = DriverManager.getConnection(url)) {
        *      Array arr = conn.createArrayOf("CHAR", new String[] {"VAL1", 
"VAL2", ... "VALN"});
        *      try (PreparedStatement stmt = conn.prepareStatement(
        *          "SELECT ... FROM TABLE WHERE COL = ANY(?)")) {
        *          stmt.setArray(1, arr);
        *          ResultSet rs = stmt.executeQuery();
        *      }
        * }



##########
phoenix-core-client/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java:
##########
@@ -1945,6 +1948,152 @@ public KeySlots visitLeave(InListExpression node, 
List<KeySlots> childParts) {
       return newKeyParts(childSlot, node, new ArrayList<KeyRange>(ranges));
     }
 
+    /**
+     * If {@link ArrayAnyComparisonExpression} is of the form:
+     *
+     * <pre>
+     * COL = ANY(ARR)
+     * </pre>
+     *
+     * then we can extract the scan ranges for the COL, given COL is a PK 
column. This syntactical
+     * pattern can be used as a replacement for a IN expression. So, instead 
of following IN
+     * expression:
+     *
+     * <pre>
+     * COL IN (VAL1, VAL2, ... VALN)
+     * </pre>
+     *
+     * we can use the following ANY expression:
+     *
+     * <pre>
+     * try (Connection conn = DriverManager.getConnection(url)) {
+     *      conn.createArrayOf("CHAR", new String[] {"VAL1", "VAL2", ... 
"VALN"});
+     *      try (PreparedStatement stmt = conn.prepareStatement(
+     *          "SELECT ... FROM TABLE WHERE COL = ANY(?)")) {
+     *          stmt.setArray(1, arr);
+     *          ResultSet rs = stmt.executeQuery();
+     *      }
+     * }
+     * </pre>
+     *
+     * This will help in saving the query parsing time as on using IN list 
query parsing time

Review Comment:
   It will also make the `PreparedStatement` highly reusable unlike in case of 
`IN` list where you need the correct number of bind parameters for one to be 
reusable.



##########
phoenix-core-client/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java:
##########
@@ -1945,6 +1948,152 @@ public KeySlots visitLeave(InListExpression node, 
List<KeySlots> childParts) {
       return newKeyParts(childSlot, node, new ArrayList<KeyRange>(ranges));
     }
 
+    /**
+     * If {@link ArrayAnyComparisonExpression} is of the form:
+     *
+     * <pre>
+     * COL = ANY(ARR)
+     * </pre>
+     *
+     * then we can extract the scan ranges for the COL, given COL is a PK 
column. This syntactical
+     * pattern can be used as a replacement for a IN expression. So, instead 
of following IN
+     * expression:
+     *
+     * <pre>
+     * COL IN (VAL1, VAL2, ... VALN)
+     * </pre>
+     *
+     * we can use the following ANY expression:
+     *
+     * <pre>
+     * try (Connection conn = DriverManager.getConnection(url)) {
+     *      conn.createArrayOf("CHAR", new String[] {"VAL1", "VAL2", ... 
"VALN"});
+     *      try (PreparedStatement stmt = conn.prepareStatement(
+     *          "SELECT ... FROM TABLE WHERE COL = ANY(?)")) {
+     *          stmt.setArray(1, arr);
+     *          ResultSet rs = stmt.executeQuery();
+     *      }
+     * }
+     * </pre>
+     *
+     * This will help in saving the query parsing time as on using IN list 
query parsing time
+     * increases with the size of IN list but in case of ANY expression it is 
constant. Below we
+     * account for cases where COL is on the LHS or RHS of the comparison 
expression.
+     * @param node           {@link ArrayAnyComparisonExpression} node for 
which scan ranges are to
+     *                       be extracted
+     * @param keyExpressions {@link RowKeyColumnExpression} for the PK column 
for which scan ranges
+     *                       are to be extracted
+     * @return true if the scan ranges can be extracted, false otherwise
+     */
+    private boolean 
shouldExtractKeyRangesForArrayAnyExpr(ArrayAnyComparisonExpression node,
+      List<Expression> keyExpressions) {
+      // {@link ArrayAnyComparisonExpression} has two children, and the second 
child is
+      // comparison expression
+      Expression childExpr = node.getChildren().get(1);
+      if (!(childExpr instanceof ComparisonExpression)) {
+        return false;
+      }
+      ComparisonExpression comparisonExpr = (ComparisonExpression) childExpr;
+
+      // Replacing IN() with =ANY() is only valid if the comparison operator 
is EQUAL
+      if (comparisonExpr.getFilterOp() != CompareOperator.EQUAL) {
+        return false;
+      }
+
+      // {@link ComparisonExpression} will have two children in this case, we 
need to make
+      // sure that one of them is a {@link RowKeyColumnExpression} and the 
other is a {@link
+      // ArrayElemRefExpression}. Further, the first child of {@link 
ArrayElemRefExpression}
+      // must be a {@link LiteralExpression}. The first child of {@link
+      // ArrayElemRefExpression} is same as the first child of {@link
+      // ArrayAnyComparisonExpression}.
+      Expression lhs = comparisonExpr.getChildren().get(0);
+      Expression rhs = comparisonExpr.getChildren().get(1);
+      if (lhs instanceof RowKeyColumnExpression && rhs instanceof 
ArrayElemRefExpression) {
+        ArrayElemRefExpression arrayElemRefExpr = (ArrayElemRefExpression) rhs;
+        if (!(arrayElemRefExpr.getChildren().get(0) instanceof 
LiteralExpression)) {
+          return false;
+        }
+        // Capture {@link RowKeyColumnExpression} for the generation of key 
slots.
+        keyExpressions.add(lhs);
+
+      } else if (lhs instanceof ArrayElemRefExpression && rhs instanceof 
RowKeyColumnExpression) {
+        ArrayElemRefExpression arrayElemRefExpr = (ArrayElemRefExpression) lhs;
+        if (!(arrayElemRefExpr.getChildren().get(0) instanceof 
LiteralExpression)) {
+          return false;
+        }
+        // Capture {@link RowKeyColumnExpression} for the generation of key 
slots.
+        keyExpressions.add(rhs);
+      } else {

Review Comment:
   Nit: you can avoid repetitiveness using 2 pairs of (lhs, rhs) and (rhs, lhs).



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

Reply via email to