DomGarguilo commented on code in PR #6463:
URL: https://github.com/apache/accumulo/pull/6463#discussion_r3552611463


##########
core/src/main/java/org/apache/accumulo/core/data/Mutation.java:
##########
@@ -230,6 +248,26 @@ public Mutation(CharSequence row, int initialBufferSize) {
    */
   public Mutation() {}
 
+  /**
+   * Creates a mutation with the specified row, This constructor creates a 
copy of the fields.
+   */
+  public Mutation(ByteSequence row) {
+    byte[] rowBytes;
+    int rowOffset;
+    int rowLen;
+
+    if (row.isBackedByArray()) {
+      rowBytes = row.getBackingArray();
+      rowOffset = row.offset();
+    } else {
+      rowBytes = row.toArray();
+      rowOffset = 0;
+    }
+    rowLen = row.length();
+
+    init(rowBytes, rowOffset, rowLen, true);

Review Comment:
   I think there is a bug here. I think init() and copyIfNeeded() will only 
copy the row bytes leaving the other fields not set (`buffer` and 
`estRowAndLargeValSize`). I think we can just use the existing byte-array 
constructor which sets things up properly:
   ```suggestion
       this(row.isBackedByArray() ? row.getBackingArray() : row.toArray(),
           row.isBackedByArray() ? row.offset() : 0, row.length());
   ```



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