Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ public ConditionalMutation(CharSequence row, Condition... conditions) {
}

public ConditionalMutation(ByteSequence row, Condition... conditions) {
// TODO add ByteSequence methods to mutations
super(row.toArray());
super(row);
init(conditions);
}

Expand Down
48 changes: 48 additions & 0 deletions core/src/main/java/org/apache/accumulo/core/data/Mutation.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -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);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Suggested change
init(rowBytes, rowOffset, rowLen, true);
this(row.isBackedByArray() ? row.getBackingArray() : row.toArray(),
row.isBackedByArray() ? row.offset() : 0, row.length());

}

/**
* Creates a new mutation from a Thrift mutation.
*
Expand Down Expand Up @@ -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);
}
Expand Down
27 changes: 27 additions & 0 deletions core/src/test/java/org/apache/accumulo/core/data/MutationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1055,4 +1055,31 @@ 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());
}
}
}