sashapolo commented on code in PR #2728:
URL: https://github.com/apache/ignite-3/pull/2728#discussion_r1370143022
##########
modules/schema/src/main/java/org/apache/ignite/internal/schema/BinaryTuplePrefix.java:
##########
@@ -73,6 +97,88 @@ public static BinaryTuplePrefix fromBinaryTuple(BinaryTuple
tuple) {
return new BinaryTuplePrefix(tuple.elementCount(), prefixBuffer);
}
+ private static BinaryTuplePrefix expandTuple(int numElements, BinaryTuple
tuple) {
+ assert numElements > tuple.elementCount();
+
+ if (tuple.elementCount() == 0) {
+ return new BinaryTuplePrefix(
+ numElements, new BinaryTuplePrefixBuilder(0, numElements,
0).build()
+ );
+ }
+
+ int[] dataBeginOffsetHolder = new int[1];
+ int[] dataEndOffsetHolder = new int[1];
+
+ tuple.fetch(0, (index, begin, end) -> dataBeginOffsetHolder[0] =
begin);
+ tuple.fetch(tuple.elementCount() - 1, (index, begin, end) ->
dataEndOffsetHolder[0] = end);
+
+ ByteBuffer tupleBuffer = tuple.byteBuffer();
+
+ byte flags = tupleBuffer.get(0);
+ int entrySize = BinaryTupleCommon.flagsToEntrySize(flags);
+
+ ByteBuffer prefixBuffer = ByteBuffer.allocate(
+ tupleBuffer.remaining()
Review Comment:
Please extract this expression into a local variable, it's hard to read
##########
modules/schema/src/test/java/org/apache/ignite/internal/schema/BinaryTuplePrefixTest.java:
##########
@@ -76,6 +78,81 @@ public void testInternalBufferReallocation() {
assertThat(prefix.intValue(0), is(Integer.MAX_VALUE));
}
+ @Test
+ public void testCreatePrefixFromBinaryTupleWhichSizeIsLesserThanRequired()
{
+ int sourceTupleSize = 1;
+
+ ByteBuffer buffer = new BinaryTupleBuilder(sourceTupleSize)
+ .appendInt(10)
+ .build();
+
+ BinaryTuplePrefix prefix = BinaryTuplePrefix.fromBinaryTuple(4, new
BinaryTuple(sourceTupleSize, buffer));
+
+ assertThat(prefix.elementCount(), equalTo(sourceTupleSize));
+ assertThat(prefix.intValue(0), equalTo(10));
+ assertThat(prefix.hasNullValue(1), equalTo(true));
+ assertThat(prefix.hasNullValue(2), equalTo(true));
+ assertThat(prefix.hasNullValue(3), equalTo(true));
+ }
+
+ @Test
+ public void testCreatePrefixFromBinaryTupleWhichSizeIsEqualsToRequired() {
Review Comment:
```suggestion
public void testCreatePrefixFromBinaryTupleWhichSizeIsEqualToRequired() {
```
##########
modules/schema/src/main/java/org/apache/ignite/internal/schema/BinaryTuplePrefix.java:
##########
@@ -73,6 +97,88 @@ public static BinaryTuplePrefix fromBinaryTuple(BinaryTuple
tuple) {
return new BinaryTuplePrefix(tuple.elementCount(), prefixBuffer);
}
+ private static BinaryTuplePrefix expandTuple(int numElements, BinaryTuple
tuple) {
+ assert numElements > tuple.elementCount();
+
+ if (tuple.elementCount() == 0) {
+ return new BinaryTuplePrefix(
+ numElements, new BinaryTuplePrefixBuilder(0, numElements,
0).build()
+ );
+ }
+
+ int[] dataBeginOffsetHolder = new int[1];
+ int[] dataEndOffsetHolder = new int[1];
+
+ tuple.fetch(0, (index, begin, end) -> dataBeginOffsetHolder[0] =
begin);
+ tuple.fetch(tuple.elementCount() - 1, (index, begin, end) ->
dataEndOffsetHolder[0] = end);
+
+ ByteBuffer tupleBuffer = tuple.byteBuffer();
+
+ byte flags = tupleBuffer.get(0);
+ int entrySize = BinaryTupleCommon.flagsToEntrySize(flags);
+
+ ByteBuffer prefixBuffer = ByteBuffer.allocate(
+ tupleBuffer.remaining()
+ + (entrySize * (numElements -
tuple.elementCount()))
+ + Integer.BYTES)
+ .order(ORDER)
+ .put(tupleBuffer.slice().limit(dataBeginOffsetHolder[0])); //
header
Review Comment:
Small nitpick: `duplicate` is more suitable here than `slice`
##########
modules/schema/src/test/java/org/apache/ignite/internal/schema/BinaryTuplePrefixTest.java:
##########
@@ -76,6 +78,81 @@ public void testInternalBufferReallocation() {
assertThat(prefix.intValue(0), is(Integer.MAX_VALUE));
}
+ @Test
+ public void testCreatePrefixFromBinaryTupleWhichSizeIsLesserThanRequired()
{
Review Comment:
```suggestion
public void testCreatePrefixFromBinaryTupleWhichSizeIsLessThanRequired()
{
```
##########
modules/schema/src/main/java/org/apache/ignite/internal/schema/BinaryTuplePrefix.java:
##########
@@ -73,6 +97,102 @@ public static BinaryTuplePrefix fromBinaryTuple(BinaryTuple
tuple) {
return new BinaryTuplePrefix(tuple.elementCount(), prefixBuffer);
}
+ private static BinaryTuplePrefix expandTuple(int size, BinaryTuple tuple) {
+ assert size > tuple.elementCount();
+
+ var stats = new Sink() {
+ int dataBeginOffset = 0;
+ int dataEndOffset = 0;
+
+ @Override
+ public void nextElement(int index, int begin, int end) {
+ if (index == 0) {
+ dataBeginOffset = begin;
+ }
+
+ dataEndOffset = end;
+ }
+ };
+
+ tuple.parse(stats);
+
+ ByteBuffer tupleBuffer = tuple.byteBuffer();
+
+ byte flags = tupleBuffer.get(0);
+ int entrySize = BinaryTupleCommon.flagsToEntrySize(flags);
+
+ ByteBuffer prefixBuffer = ByteBuffer.allocate(
+ tupleBuffer.remaining()
+ + (entrySize * (size - tuple.elementCount()))
+ + Integer.BYTES)
+ .order(ORDER)
+ .put(tupleBuffer.slice().limit(stats.dataBeginOffset)); //
header
+
+ int payloadEndPosition = stats.dataEndOffset - stats.dataBeginOffset;
+ for (int idx = tuple.elementCount(); idx < size; idx++) {
+ switch (entrySize) {
+ case Byte.BYTES:
+ prefixBuffer.put((byte) payloadEndPosition);
+ break;
+ case Short.BYTES:
+ prefixBuffer.putShort((short) payloadEndPosition);
+ break;
+ case Integer.BYTES:
+ prefixBuffer.putInt(payloadEndPosition);
+ break;
+ default:
+ assert false;
+ }
+ }
+
+ prefixBuffer
+
.put(tupleBuffer.slice().position(stats.dataBeginOffset).limit(stats.dataEndOffset))
// payload
+ .putInt(tuple.elementCount())
+ .flip();
+
+ prefixBuffer.put(0, (byte) (flags | PREFIX_FLAG));
+
+ return new BinaryTuplePrefix(size, prefixBuffer);
+ }
+
+ private static BinaryTuplePrefix truncateTuple(int size, BinaryTuple
tuple) {
+ assert size < tuple.elementCount();
+
+ var stats = new Sink() {
+ int dataBeginOffset = 0;
+ int dataEndOffset = 0;
+
+ @Override
+ public void nextElement(int index, int begin, int end) {
+ if (index == 0) {
+ dataBeginOffset = begin;
+ }
+
+ if (index < size) {
+ dataEndOffset = end;
+ }
+ }
+ };
+
+ tuple.parse(stats);
+
+ BinaryTuplePrefixBuilder builder = new BinaryTuplePrefixBuilder(size,
size, stats.dataEndOffset - stats.dataBeginOffset);
+
+ tuple.parse((index, begin, end) -> {
Review Comment:
My first comment was about replacing `fetch` with a `for` cycle... Do you
think that my proposal is not correct? Here's the proposed code again:
```
for (int i = 0; i < size; i++) {
byte[] valueBytes = tuple.bytesValue(i);
builder.appendBytes(valueBytes);
}
```
##########
modules/schema/src/test/java/org/apache/ignite/internal/schema/BinaryTuplePrefixTest.java:
##########
@@ -76,6 +78,101 @@ public void testInternalBufferReallocation() {
assertThat(prefix.intValue(0), is(Integer.MAX_VALUE));
}
+ @Test
+ public void testCreatePrefixFromBinaryTuple1() {
Review Comment:
Much better now, thanks
--
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]
