keith-turner commented on code in PR #5702:
URL: https://github.com/apache/accumulo/pull/5702#discussion_r2198273821


##########
core/src/main/java/org/apache/accumulo/core/data/Mutation.java:
##########
@@ -962,6 +965,23 @@ public QualifierOptions family(Text colFam) {
       return family(colFam.getBytes(), colFam.getLength());
     }
 
+    /**
+     * Sets the column family, column qualifier, and column visibility of a 
mutation.
+     *
+     * @param key key
+     * @return a TimestampOptions object, advancing the method chain

Review Comment:
   Need to add a `@since` javadoc tag.



##########
core/src/main/java/org/apache/accumulo/core/data/Mutation.java:
##########
@@ -962,6 +965,23 @@ public QualifierOptions family(Text colFam) {
       return family(colFam.getBytes(), colFam.getLength());
     }
 
+    /**
+     * Sets the column family, column qualifier, and column visibility of a 
mutation.

Review Comment:
   Would be helpful to mention that all other fields in the key are ignored



##########
core/src/main/java/org/apache/accumulo/core/data/Mutation.java:
##########
@@ -962,6 +965,23 @@ public QualifierOptions family(Text colFam) {
       return family(colFam.getBytes(), colFam.getLength());
     }
 
+    /**
+     * Sets the column family, column qualifier, and column visibility of a 
mutation.
+     *
+     * @param key key
+     * @return a TimestampOptions object, advancing the method chain
+     */
+    @Override
+    public TimestampOptions keyColumns(Key key) {
+      Objects.requireNonNull(key, "key cannot be null");
+
+      Text colFam = key.getColumnFamily();
+      Text colQual = key.getColumnQualifier();
+      Text colVis = key.getColumnVisibility();

Review Comment:
   Creating a hadoop Text object will copy the byte array in the key object.  
The following code will not copy the byte array in the key object.
   
   ```suggestion
         byte[] colFam = key.getColumnFamilyData().toArray();
         byte[] colQual = key.getColumnQualifierData().toArray();
         byte[] colVis = key.getColumnVisibilityData().toArray();
   ```



##########
core/src/main/java/org/apache/accumulo/core/data/Mutation.java:
##########
@@ -777,6 +778,8 @@ public interface FamilyOptions extends QualifierOptions {
     QualifierOptions family(CharSequence colFam);
 
     QualifierOptions family(Text colFam);
+
+    TimestampOptions keyColumns(Key key);

Review Comment:
   The javadoc is on the implementation of this menthod, it needs to be moved 
here instead.  When using this method in an IDE will see the javadoc from here.



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