cshannon commented on code in PR #4486:
URL: https://github.com/apache/accumulo/pull/4486#discussion_r1581219889
##########
server/base/src/main/java/org/apache/accumulo/server/metadata/iterators/SetEqualityIterator.java:
##########
@@ -79,14 +87,26 @@ public void seek(Range range, Collection<ByteSequence>
columnFamilies, boolean i
int count = 0;
while (source.hasTop()) {
+ byte[] bytesToWrite = null;
byte[] ba = source.getTopKey().getColumnQualifierData().toArray();
- dos.writeInt(ba.length);
- dos.write(ba, 0, ba.length);
+ if (concat) {
+ byte[] val = source.getTopValue().get();
Review Comment:
Do we need to worry about checking for null when calling
source.getTopValue() ?
##########
server/base/src/main/java/org/apache/accumulo/server/metadata/iterators/SetEqualityIterator.java:
##########
@@ -179,10 +200,11 @@ private static <T> byte[] encode(Set<T> set,
Function<T,byte[]> encoder) {
private static final Text EMPTY = new Text();
public static <T> Condition createCondition(Collection<T> set,
Function<T,byte[]> encoder,
Review Comment:
You could create a second createCondition() method that leaves off the
concatValue and delegates to the other method with a default value of false,
this might be nice as then you don't need to update any of the conditions that
use this except FILES
##########
server/base/src/main/java/org/apache/accumulo/server/metadata/iterators/SetEqualityIterator.java:
##########
@@ -79,14 +87,26 @@ public void seek(Range range, Collection<ByteSequence>
columnFamilies, boolean i
int count = 0;
while (source.hasTop()) {
+ byte[] bytesToWrite = null;
byte[] ba = source.getTopKey().getColumnQualifierData().toArray();
- dos.writeInt(ba.length);
- dos.write(ba, 0, ba.length);
+ if (concat) {
+ byte[] val = source.getTopValue().get();
+ bytesToWrite = new byte[ba.length + VALUE_SEPARATOR_BYTES_LENGTH +
val.length];
+ System.arraycopy(ba, 0, bytesToWrite, 0, ba.length);
+ System.arraycopy(VALUE_SEPARATOR_BYTES, 0, bytesToWrite, ba.length,
+ VALUE_SEPARATOR_BYTES_LENGTH);
+ System.arraycopy(val, 0, bytesToWrite, ba.length +
VALUE_SEPARATOR_BYTES_LENGTH,
+ val.length);
+ } else {
+ bytesToWrite = ba;
+ }
+ dos.writeInt(bytesToWrite.length);
+ dos.write(bytesToWrite, 0, bytesToWrite.length);
Review Comment:
I went through each line of the arrary copy logic, it looks correct to me.
##########
server/base/src/main/java/org/apache/accumulo/server/metadata/iterators/SetEqualityIterator.java:
##########
@@ -79,14 +87,26 @@ public void seek(Range range, Collection<ByteSequence>
columnFamilies, boolean i
int count = 0;
while (source.hasTop()) {
+ byte[] bytesToWrite = null;
Review Comment:
```suggestion
final byte[] bytesToWrite;
```
##########
server/base/src/main/java/org/apache/accumulo/server/metadata/ConditionalTabletMutatorImpl.java:
##########
@@ -207,13 +209,13 @@ private void requireSameSingle(TabletMetadata
tabletMetadata, ColumnType type) {
break;
case LOADED: {
Condition c =
SetEqualityIterator.createCondition(tabletMetadata.getLoaded().keySet(),
- stf -> stf.getMetadata().getBytes(UTF_8),
BulkFileColumnFamily.NAME);
+ stf -> stf.getMetadata().getBytes(UTF_8),
BulkFileColumnFamily.NAME, false);
mutation.addCondition(c);
}
break;
case COMPACTED: {
Condition c =
SetEqualityIterator.createCondition(tabletMetadata.getCompacted(),
- fTid -> fTid.canonical().getBytes(UTF_8),
CompactedColumnFamily.NAME);
+ fTid -> fTid.canonical().getBytes(UTF_8),
CompactedColumnFamily.NAME, false);
Review Comment:
This is an example related to my other comment where this line wouldn't need
to change if we just had a second helper createCondition method without the
argument that defaults to false
--
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]
