From a7e83b095f8e4a198eb70e3c93669e4357bbbd6a Mon Sep 17 00:00:00 2001 From: Amanda Villarreal Date: Tue, 7 Jul 2026 14:03:02 -0500 Subject: [PATCH 1/9] Adding new Mutation constructor with ByteSequence parameter, removes 1 TODO from apache#2699 --- .../core/data/ConditionalMutation.java | 1 - .../apache/accumulo/core/data/Mutation.java | 48 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/accumulo/core/data/ConditionalMutation.java b/core/src/main/java/org/apache/accumulo/core/data/ConditionalMutation.java index 3c3e3894517..696c8c6f2d2 100644 --- a/core/src/main/java/org/apache/accumulo/core/data/ConditionalMutation.java +++ b/core/src/main/java/org/apache/accumulo/core/data/ConditionalMutation.java @@ -60,7 +60,6 @@ public ConditionalMutation(CharSequence row, Condition... conditions) { } public ConditionalMutation(ByteSequence row, Condition... conditions) { - // TODO add ByteSequence methods to mutations super(row.toArray()); init(conditions); } diff --git a/core/src/main/java/org/apache/accumulo/core/data/Mutation.java b/core/src/main/java/org/apache/accumulo/core/data/Mutation.java index 00e401218ea..71f35c30acc 100644 --- a/core/src/main/java/org/apache/accumulo/core/data/Mutation.java +++ b/core/src/main/java/org/apache/accumulo/core/data/Mutation.java @@ -134,6 +134,24 @@ private ByteBuffer serializedSnapshot() { } } + static byte[] copyIfNeeded(byte[] ba, int off, int len, boolean copyData) { + if (len == 0) { + return EMPTY_BYTES; + } + + if (!copyData && ba.length == len && off == 0) { + return ba; + } + + byte[] copy = new byte[len]; + System.arraycopy(ba, off, copy, 0, len); + return copy; + } + + private final void init(byte[] r, int rOff, int rLen, boolean copy) { + row = copyIfNeeded(r, rOff, rLen, copy); + } + /** * Creates a new mutation. A defensive copy is made. * @@ -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); + } + /** * Creates a new mutation from a Thrift mutation. * @@ -271,6 +309,16 @@ public byte[] getRow() { return row; } + /** + * Returns the row ID as a byte sequence. This method returns a pointer to the key's internal data + * and does not copy it. + * + * @return ByteSequence that points to the internal key row ID data + */ + public ByteSequence getRowData() { + return new ArrayByteSequence(row); + } + private void fill(byte[] b) { fill(b, b.length); } From 179fc8c6f3105b511bab38575d74760f5b74c50e Mon Sep 17 00:00:00 2001 From: Amanda Villarreal Date: Tue, 7 Jul 2026 15:22:41 -0500 Subject: [PATCH 2/9] formatted Mutation.java --- core/src/main/java/org/apache/accumulo/core/data/Mutation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/accumulo/core/data/Mutation.java b/core/src/main/java/org/apache/accumulo/core/data/Mutation.java index 71f35c30acc..4f590692a6a 100644 --- a/core/src/main/java/org/apache/accumulo/core/data/Mutation.java +++ b/core/src/main/java/org/apache/accumulo/core/data/Mutation.java @@ -265,7 +265,7 @@ public Mutation(ByteSequence row) { } rowLen = row.length(); - init(rowBytes, rowOffset, rowLen, true); + init(rowBytes, rowOffset, rowLen, true); } /** From 03c02dcff76eb30585978b2e9d605446d1b18c8e Mon Sep 17 00:00:00 2001 From: Amanda Villarreal Date: Wed, 8 Jul 2026 12:51:41 -0500 Subject: [PATCH 3/9] Adding test to MutationTest.java --- .../accumulo/core/data/MutationTest.java | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java b/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java index 2d7289a20a5..3f701c9bb06 100644 --- a/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java +++ b/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java @@ -1055,4 +1055,32 @@ public void testKeyColumnsAddingNullValue() { assertThrows(NullPointerException.class, () -> m.at().keyColumns(k).put(v)); } + private static class TestByteSequence extends ArrayByteSequence { + + private static final long serialVersionUID = 1234L; + + public TestByteSequence(String s) { + super(s); + } + + @Override + public boolean isBackedByArray() { + return false; + } + } + + @Test + public void testByteSequenceConstructor() { + var row1 = new ArrayByteSequence("Row"); + var row2 = new ArrayByteSequence("TheRowData").subSequence(3, 6); + var row3 = new TestByteSequence("Row"); + + var expectedMutation = new Mutation("Row"); + + for (var r : List.of(row1, row2, row3)) { + var actualMutation = new Mutation(r); + assertEquals(expectedMutation, actualMutation); + System.out.println(actualMutation); + } + } } From 4867f0fc138ab3bbb6ba929cc2092286cf17bb4c Mon Sep 17 00:00:00 2001 From: Amanda Villarreal Date: Wed, 8 Jul 2026 14:40:21 -0500 Subject: [PATCH 4/9] Fixing test --- .../java/org/apache/accumulo/core/data/MutationTest.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java b/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java index 3f701c9bb06..13d6d297460 100644 --- a/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java +++ b/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java @@ -1075,12 +1075,11 @@ public void testByteSequenceConstructor() { var row2 = new ArrayByteSequence("TheRowData").subSequence(3, 6); var row3 = new TestByteSequence("Row"); - var expectedMutation = new Mutation("Row"); + var expectedMutation = new Mutation(new ArrayByteSequence("Row".getBytes(UTF_8))); for (var r : List.of(row1, row2, row3)) { - var actualMutation = new Mutation(r); - assertEquals(expectedMutation, actualMutation); - System.out.println(actualMutation); + var actualMutation = new Mutation(new ArrayByteSequence(r)); + assertEquals(expectedMutation.getRowData(), actualMutation.getRowData()); } } } From a98fecacd9757adaffad13a2092a461ff947e2eb Mon Sep 17 00:00:00 2001 From: Amanda Villarreal Date: Thu, 9 Jul 2026 09:42:45 -0500 Subject: [PATCH 5/9] Calling new constructor --- .../java/org/apache/accumulo/core/data/ConditionalMutation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/accumulo/core/data/ConditionalMutation.java b/core/src/main/java/org/apache/accumulo/core/data/ConditionalMutation.java index 696c8c6f2d2..ba242089bf1 100644 --- a/core/src/main/java/org/apache/accumulo/core/data/ConditionalMutation.java +++ b/core/src/main/java/org/apache/accumulo/core/data/ConditionalMutation.java @@ -60,7 +60,7 @@ public ConditionalMutation(CharSequence row, Condition... conditions) { } public ConditionalMutation(ByteSequence row, Condition... conditions) { - super(row.toArray()); + super(row); init(conditions); } From 096de0bd25878c9ca03c898d58a6204dc9b6603d Mon Sep 17 00:00:00 2001 From: Amanda Villarreal Date: Thu, 9 Jul 2026 10:21:38 -0500 Subject: [PATCH 6/9] Changing new constructor, removing init() and copyIfNeeded() --- .../apache/accumulo/core/data/Mutation.java | 36 ++----------------- 1 file changed, 3 insertions(+), 33 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/data/Mutation.java b/core/src/main/java/org/apache/accumulo/core/data/Mutation.java index 4f590692a6a..4c644cc3950 100644 --- a/core/src/main/java/org/apache/accumulo/core/data/Mutation.java +++ b/core/src/main/java/org/apache/accumulo/core/data/Mutation.java @@ -134,24 +134,6 @@ private ByteBuffer serializedSnapshot() { } } - static byte[] copyIfNeeded(byte[] ba, int off, int len, boolean copyData) { - if (len == 0) { - return EMPTY_BYTES; - } - - if (!copyData && ba.length == len && off == 0) { - return ba; - } - - byte[] copy = new byte[len]; - System.arraycopy(ba, off, copy, 0, len); - return copy; - } - - private final void init(byte[] r, int rOff, int rLen, boolean copy) { - row = copyIfNeeded(r, rOff, rLen, copy); - } - /** * Creates a new mutation. A defensive copy is made. * @@ -249,23 +231,11 @@ public Mutation(CharSequence row, int initialBufferSize) { public Mutation() {} /** - * Creates a mutation with the specified row, This constructor creates a copy of the fields. + * Creates a mutation with the specified row */ 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); + this(row.isBackedByArray() ? row.getBackingArray() : row.toArray(), + row.isBackedByArray() ? row.offset() : 0, row.length()); } /** From 5ff4689299e09f7856ddf05cf13d28efdbf07759 Mon Sep 17 00:00:00 2001 From: Amanda Villarreal Date: Thu, 9 Jul 2026 13:31:04 -0500 Subject: [PATCH 7/9] Reverting changes, only removing TODO --- .../core/data/ConditionalMutation.java | 2 +- .../apache/accumulo/core/data/Mutation.java | 18 ------------ .../accumulo/core/data/MutationTest.java | 28 ------------------- 3 files changed, 1 insertion(+), 47 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/data/ConditionalMutation.java b/core/src/main/java/org/apache/accumulo/core/data/ConditionalMutation.java index ba242089bf1..696c8c6f2d2 100644 --- a/core/src/main/java/org/apache/accumulo/core/data/ConditionalMutation.java +++ b/core/src/main/java/org/apache/accumulo/core/data/ConditionalMutation.java @@ -60,7 +60,7 @@ public ConditionalMutation(CharSequence row, Condition... conditions) { } public ConditionalMutation(ByteSequence row, Condition... conditions) { - super(row); + super(row.toArray()); init(conditions); } diff --git a/core/src/main/java/org/apache/accumulo/core/data/Mutation.java b/core/src/main/java/org/apache/accumulo/core/data/Mutation.java index 4c644cc3950..00e401218ea 100644 --- a/core/src/main/java/org/apache/accumulo/core/data/Mutation.java +++ b/core/src/main/java/org/apache/accumulo/core/data/Mutation.java @@ -230,14 +230,6 @@ public Mutation(CharSequence row, int initialBufferSize) { */ public Mutation() {} - /** - * Creates a mutation with the specified row - */ - public Mutation(ByteSequence row) { - this(row.isBackedByArray() ? row.getBackingArray() : row.toArray(), - row.isBackedByArray() ? row.offset() : 0, row.length()); - } - /** * Creates a new mutation from a Thrift mutation. * @@ -279,16 +271,6 @@ public byte[] getRow() { return row; } - /** - * Returns the row ID as a byte sequence. This method returns a pointer to the key's internal data - * and does not copy it. - * - * @return ByteSequence that points to the internal key row ID data - */ - public ByteSequence getRowData() { - return new ArrayByteSequence(row); - } - private void fill(byte[] b) { fill(b, b.length); } diff --git a/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java b/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java index 13d6d297460..02afb497e5c 100644 --- a/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java +++ b/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java @@ -1054,32 +1054,4 @@ public void testKeyColumnsAddingNullValue() { assertThrows(NullPointerException.class, () -> m.at().keyColumns(k).put(v)); } - - private static class TestByteSequence extends ArrayByteSequence { - - private static final long serialVersionUID = 1234L; - - public TestByteSequence(String s) { - super(s); - } - - @Override - public boolean isBackedByArray() { - return false; - } - } - - @Test - public void testByteSequenceConstructor() { - var row1 = new ArrayByteSequence("Row"); - var row2 = new ArrayByteSequence("TheRowData").subSequence(3, 6); - var row3 = new TestByteSequence("Row"); - - var expectedMutation = new Mutation(new ArrayByteSequence("Row".getBytes(UTF_8))); - - for (var r : List.of(row1, row2, row3)) { - var actualMutation = new Mutation(new ArrayByteSequence(r)); - assertEquals(expectedMutation.getRowData(), actualMutation.getRowData()); - } - } } From 5f09e17e237f1ec87c9f98a07599aec37e0d9acc Mon Sep 17 00:00:00 2001 From: Amanda Villarreal Date: Thu, 9 Jul 2026 13:32:37 -0500 Subject: [PATCH 8/9] fix spacing --- .../test/java/org/apache/accumulo/core/data/MutationTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java b/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java index 02afb497e5c..ea4100cb26f 100644 --- a/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java +++ b/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java @@ -1054,4 +1054,5 @@ public void testKeyColumnsAddingNullValue() { assertThrows(NullPointerException.class, () -> m.at().keyColumns(k).put(v)); } + } From 8e5a6bb03c117d0fc07d66277fb9610d860a835e Mon Sep 17 00:00:00 2001 From: Amanda Villarreal Date: Thu, 9 Jul 2026 13:38:38 -0500 Subject: [PATCH 9/9] spacing --- .../test/java/org/apache/accumulo/core/data/MutationTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java b/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java index ea4100cb26f..2d7289a20a5 100644 --- a/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java +++ b/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java @@ -1054,5 +1054,5 @@ public void testKeyColumnsAddingNullValue() { assertThrows(NullPointerException.class, () -> m.at().keyColumns(k).put(v)); } - + }