[GitHub] [hbase] brfrn169 commented on a change in pull request #1114: HBASE-23146 Support CheckAndMutate with multi conditions

[GitBox <gi...@apache.org>]

brfrn169 commented on a change in pull request #1114: HBASE-23146 Support CheckAndMutate with multi conditions
URL: https://github.com/apache/hbase/pull/1114#discussion_r373902670
 
 

 ##########
 File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncTable.java
 ##########
 @@ -233,6 +234,28 @@
    */
   CheckAndMutateBuilder checkAndMutate(byte[] row, byte[] family);
 
+  /**
+   * Atomically checks if a row matches the specified filter. If it does, it adds the
+   * Put/Delete/RowMutations.
+   * <p>
+   * Use the returned {@link CheckAndMutateBuilder} to construct your request and then execute it.
+   * This is a fluent style API, the code is like:
+   *
+   * <pre>
+   * <code>
+   * table.checkAndMutate(row).ifMatches(filter).thenPut(put)
+   *     .thenAccept(succ -> {
+   *       if (succ) {
+   *         System.out.println("Check and put succeeded");
+   *       } else {
+   *         System.out.println("Check and put failed");
+   *       }
+   *     });
+   * </code>
+   * </pre>
+   */
+  CheckAndMutateBuilder checkAndMutate(byte[] row);
 
 Review comment:
   @Apache9 Thank you for reviewing this!
   
   > but here since we allow matching multiple qualifiers acrossing different families, we can accept null families as we can pass it through the filter implementation?
   
   Yes. With this new feature, we can specify multiple conditions across different families by setting a filter. So the family can be null.
   
   My intention was if we want to specify a filter by ifMatches(filter), we can use checkAndMutate(row). And if we want to specify a qualifier and a condition by qualifier()/ifNotExists()/ifEquals()/ifMatches(op, value), we can use checkAndMutate(row, family) in the existing way.
   
   And, if we call ifMatches(filter) after calling checkAndMutate(row, family), it will throw UnsupportedOperationException with the message "Please use checkAndMutate(row)":
   ```
   table.checkAndMutate(row, family).ifMatches(filter).thenPut(put); // UnsupportedOperationException: Please use checkAndMutate(row)
   ```
   
   And vice versa:
   ```
   table.checkAndMutate(row).qualifier(qualifier).ifMatches(op, value).thenPut(put); // UnsupportedOperationException: Please use checkAndMutate(row, family)
   ```
   
   Please see the following test for this:
   https://github.com/brfrn169/hbase/blob/HBASE-23146/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestCheckAndMutate.java#L380-L416
   
   I think, you are right, it might be a little bit confusing but I'm still feeling it's okay. What do you think?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services