wchevreuil commented on a change in pull request #2556:
URL: https://github.com/apache/hbase/pull/2556#discussion_r510007779



##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALPrettyPrinter.java
##########
@@ -301,15 +318,12 @@ public void processFile(final Configuration conf, final 
Path p)
         List<Map<String, Object>> actions = new ArrayList<>();
         for (Cell cell : edit.getCells()) {
           // add atomic operation to txn
-          Map<String, Object> op = new HashMap<>(toStringMap(cell, 
outputOnlyRowKey));
-          if (outputValues) {
-            op.put("value", Bytes.toStringBinary(CellUtil.cloneValue(cell)));
-          }
-          // check row output filter
-          if (row == null || ((String) op.get("row")).equals(row)) {

Review comment:
       Agree that moving the `if` check from this `processFile` method all the 
way to `toStringMap` may not be needed, but didn't think it's much of a 
concern, so kept my +1. Do you see this as a blocker, @Apache9 ?




----------------------------------------------------------------
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:
[email protected]


Reply via email to