ibessonov commented on code in PR #1175:
URL: https://github.com/apache/ignite-3/pull/1175#discussion_r990051723
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/tree/io/BplusInnerIo.java:
##########
@@ -81,23 +84,24 @@ public final long getLeft(long pageAddr, int idx) {
public final void setLeft(long pageAddr, int idx, long pageId) {
assertPageType(pageAddr);
- putLong(pageAddr, offset0(idx, SHIFT_LEFT), pageId);
+ writePartitionless(pageAddr + offset0(idx, SHIFT_LEFT), link(pageId,
0));
- assert pageId == getLeft(pageAddr, idx);
+ assert pageId == getLeft(pageAddr, idx, partitionId(pageId));
}
/**
- * Returns right page id for item.
+ * Returns right page ID for item.
*
* @param pageAddr Page address.
* @param idx Index of item.
+ * @param partId Partition ID.
*/
- public final long getRight(long pageAddr, int idx) {
- return getLong(pageAddr, offset0(idx, shiftRight));
+ public final long getRight(long pageAddr, int idx, int partId) {
+ return readPartitionless(partId, pageAddr, offset0(idx, shiftRight));
}
/**
- * Returns right page id for item.
+ * Sets right page ID for item.
Review Comment:
Same here, please leave "id"
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/util/PartitionlessLinks.java:
##########
@@ -29,25 +29,26 @@
import java.nio.ByteBuffer;
/**
- * Handling of <em>partitionless links</em>, that is, page memory links from
which partition ID is removed.
+ * Auxiliary class for working with links and page IDs without partition ID.
*
* <p>They are used to save storage space in cases when we know the partition
ID from the context.
*
* @see PageIdUtils#link(long, int)
+ * @see PageIdUtils#pageId(int, byte, int)
*/
public class PartitionlessLinks {
/** Number of bytes a partitionless link takes in storage. */
public static final int PARTITIONLESS_LINK_SIZE_BYTES = 6;
/**
- * Reads a partitionless link from the memory.
+ * Reads a partitionless link or page ID from the memory.
*
* @param partitionId Partition ID.
* @param pageAddr Page address.
* @param offset Data offset.
- * @return Partitionless link.
+ * @return Link or page ID with partition ID.
*/
- public static long readPartitionlessLink(int partitionId, long pageAddr,
int offset) {
+ public static long readPartitionless(int partitionId, long pageAddr, int
offset) {
Review Comment:
I also asked to update comment in this method. Line `Page with index 0 is
never a data page.` makes no sense now, it should be changed
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/tree/io/BplusIo.java:
##########
@@ -346,14 +357,15 @@ public abstract void copyItems(
L row,
@Nullable byte[] rowBytes,
long rightId,
- boolean needRowBytes
+ boolean needRowBytes,
+ int partId
) throws IgniteInternalCheckedException {
assertPageType(pageAddr);
int cnt = getCount(pageAddr);
// Move right all the greater elements to make a free slot for a new
row link.
- copyItems(pageAddr, pageAddr, idx, idx + 1, cnt - idx, false);
+ copyItems(pageAddr, pageAddr, idx, idx + 1, cnt - idx, false, partId);
Review Comment:
`partId` doesn't seem necessary for copying, why is it here?
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/tree/io/BplusInnerIo.java:
##########
@@ -81,23 +84,24 @@ public final long getLeft(long pageAddr, int idx) {
public final void setLeft(long pageAddr, int idx, long pageId) {
assertPageType(pageAddr);
- putLong(pageAddr, offset0(idx, SHIFT_LEFT), pageId);
+ writePartitionless(pageAddr + offset0(idx, SHIFT_LEFT), link(pageId,
0));
- assert pageId == getLeft(pageAddr, idx);
+ assert pageId == getLeft(pageAddr, idx, partitionId(pageId));
}
/**
- * Returns right page id for item.
+ * Returns right page ID for item.
Review Comment:
Please don't capitalize it. First of all, why would you touch a comment that
has nothing to do with your changes. Second, "id" is grammatically correct.
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/tree/io/BplusInnerIo.java:
##########
@@ -81,23 +84,24 @@ public final long getLeft(long pageAddr, int idx) {
public final void setLeft(long pageAddr, int idx, long pageId) {
assertPageType(pageAddr);
- putLong(pageAddr, offset0(idx, SHIFT_LEFT), pageId);
+ writePartitionless(pageAddr + offset0(idx, SHIFT_LEFT), link(pageId,
0));
Review Comment:
I believe that having a `link(pageId, 0)` is wrong here.
--
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]
