[GitHub] [ignite-3] ibessonov commented on a diff in pull request #1213: IGNITE-17671 Implement BinaryTuple inlining in a sorted index B+Tree

[GitBox <gi...@apache.org>]

ibessonov commented on code in PR #1213:
URL: https://github.com/apache/ignite-3/pull/1213#discussion_r997060259


##########
modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/index/AbstractSortedIndexStorageTest.java:
##########
@@ -280,20 +275,15 @@ void testSingleColumnIndex(ColumnDefinition columnDefinition) throws Exception {
      */
     @Test
     void testPutIdempotence() throws Exception {
-        List<ColumnDefinition> columns = List.of(
-                column(ColumnType.string().typeSpec().name(), ColumnType.string()).asNullable(false).build(),
-                column(ColumnType.INT32.typeSpec().name(), ColumnType.INT32).asNullable(false).build()
-        );
-
         SortedIndexDefinition indexDefinition = SchemaBuilders.sortedIndex("TEST_INDEX")
-                .addIndexColumn(columns.get(0).name()).asc().done()
-                .addIndexColumn(columns.get(1).name()).asc().done()
+                .addIndexColumn(ColumnType.string().typeSpec().name()).asc().done()

Review Comment:
   Why don't you use a constant, like `ColumnTypeSpec.STRING`?



##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/index/sorted/io/SortedIndexTreeIo.java:
##########
@@ -117,39 +166,48 @@ default int compare(
             int idx,
             SortedIndexRowKey rowKey
     ) throws IgniteInternalCheckedException {
-        int off = offset(idx);
+        final int off = offset(idx);
+
+        int indexColumnsSize = getShort(pageAddr + off, SIZE_OFFSET);
 
-        long link = readPartitionless(partitionId, pageAddr, off + INDEX_COLUMNS_LINK_OFFSET);
+        ByteBuffer firstBinaryTupleBuffer;
 
-        //TODO Add in-place compare in IGNITE-17671
-        ReadIndexColumnsValue indexColumnsTraversal = new ReadIndexColumnsValue();
+        if (indexColumnsSize != NOT_FULLY_INLINE) {

Review Comment:
   This line is very hard to read. Double negation should be avoided at all cost. You could have swapped two branches, for example, or created a method with a good name



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org