sashapolo commented on code in PR #847:
URL: https://github.com/apache/ignite-3/pull/847#discussion_r888771461
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/mv/io/RowVersionDataIo.java:
##########
@@ -60,25 +60,54 @@ protected void writeRowData(long pageAddr, int dataOff, int
payloadSize, RowVers
putShort(addr, 0, (short) payloadSize);
addr += 2;
- addr += Timestamps.writeTimestamp(addr, 0, row.timestamp());
+ addr += Timestamps.writeTimestampToMemory(addr, 0, row.timestamp());
addr += PartitionlessLinks.writeToMemory(addr, row.nextLink());
- ByteBuffer value = row.value();
- putInt(addr, 0, value.limit());
+ putInt(addr, 0, row.valueSize());
addr += 4;
- putByteBuffer(addr, 0, value);
+ putByteBuffer(addr, 0, row.value());
}
/** {@inheritDoc} */
@Override
protected void writeFragmentData(RowVersion row, ByteBuffer buf, int
rowOff, int payloadSize) {
assertPageType(buf);
- // TODO: IGNITE-17009 - implement
+ if (rowOff == 0) {
+ // first fragment
+ assert row.headerSize() <= payloadSize : "Header must entirely fit
first fragment, but header size is " + row.headerSize()
Review Comment:
```suggestion
assert row.headerSize() <= payloadSize : "Header must entirely
fit in the first fragment, but header size is " + row.headerSize()
```
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/datapage/ReadPageMemoryRowValue.java:
##########
@@ -86,8 +86,10 @@ private long readFully(long pageAddr, DataPagePayload
payload) {
}
private void readValueFragmentToArray(long pageAddr, DataPagePayload
payload, int offsetToValue) {
- PageUtils.getBytes(pageAddr, payload.offset() + offsetToValue,
allValueBytes, transferredBytes, payload.payloadSize());
- transferredBytes += payload.payloadSize();
+ int valueBytesToRead = payload.payloadSize() - offsetToValue;
Review Comment:
Shall we add an assertion that `allValueBytes != null`?
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/mv/io/RowVersionDataIo.java:
##########
@@ -60,25 +60,54 @@ protected void writeRowData(long pageAddr, int dataOff, int
payloadSize, RowVers
putShort(addr, 0, (short) payloadSize);
addr += 2;
- addr += Timestamps.writeTimestamp(addr, 0, row.timestamp());
+ addr += Timestamps.writeTimestampToMemory(addr, 0, row.timestamp());
addr += PartitionlessLinks.writeToMemory(addr, row.nextLink());
- ByteBuffer value = row.value();
- putInt(addr, 0, value.limit());
+ putInt(addr, 0, row.valueSize());
addr += 4;
- putByteBuffer(addr, 0, value);
+ putByteBuffer(addr, 0, row.value());
}
/** {@inheritDoc} */
@Override
protected void writeFragmentData(RowVersion row, ByteBuffer buf, int
rowOff, int payloadSize) {
Review Comment:
Should we validate the `rowOff` against the `row` capacity and `payloadSize`
against the `buf` capacity?
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/mv/Timestamps.java:
##########
@@ -64,6 +65,19 @@ public static int writeTimestamp(long addr, int offset,
@Nullable Timestamp time
return 2 * Long.BYTES;
}
+ /**
+ * Writes a {@link Timestamp} to a buffer.
+ *
+ * @param buffer buffer to which to write
+ * @param timestamp the timestamp to write
+ */
+ public static void writeTimestampToBuffer(ByteBuffer buffer, @Nullable
Timestamp timestamp) {
Review Comment:
I wonder why aren't these classes present in the `Timestamp` class itself?
--
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]
