korlov42 commented on code in PR #5013:
URL: https://github.com/apache/ignite-3/pull/5013#discussion_r1912819663
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java:
##########
@@ -1367,7 +1367,7 @@ private CompletableFuture<List<BinaryRow>>
scanSortedIndex(
int flags = request.flags();
- var comparator = new
BinaryTupleComparator(indexStorage.indexDescriptor().columns());
+ var comparator =
StorageUtils.binaryTupleComparator(indexStorage.indexDescriptor().columns());
Review Comment:
```suggestion
BinaryTupleComparator comparator =
StorageUtils.binaryTupleComparator(indexStorage.indexDescriptor().columns());
```
##########
modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/index/impl/TestSortedIndexStorage.java:
##########
@@ -54,7 +54,7 @@ public class TestSortedIndexStorage extends
AbstractTestIndexStorage implements
public TestSortedIndexStorage(int partitionId,
StorageSortedIndexDescriptor descriptor) {
super(partitionId, descriptor);
- BinaryTupleComparator binaryTupleComparator = new
BinaryTupleComparator(descriptor.columns());
+ var binaryTupleComparator =
StorageUtils.binaryTupleComparator(descriptor.columns());
Review Comment:
```suggestion
BinaryTupleComparator binaryTupleComparator =
StorageUtils.binaryTupleComparator(descriptor.columns());
```
##########
modules/schema/src/main/java/org/apache/ignite/internal/schema/BinaryTupleComparator.java:
##########
@@ -89,74 +95,88 @@ public int compare(ByteBuffer buffer1, ByteBuffer buffer2) {
}
/**
- * Compares individual fields of two tuples.
+ * Compares two tuples by column using given column index.
+ */
+ protected int compareField(int colIdx, BinaryTupleReader tuple1,
BinaryTupleReader tuple2) {
Review Comment:
should it be private? the same question for similar method below
##########
modules/storage-api/src/main/java/org/apache/ignite/internal/storage/util/StorageUtils.java:
##########
@@ -253,4 +258,17 @@ public static void
throwExceptionIfIndexIsNotBuilt(@Nullable RowId nextRowIdToBu
throw new IndexNotBuiltException("Index not built yet: [{}]",
storageInfoSupplier.get());
}
}
+
+ /**
+ * Creates a comparator for a Sorted Index identified by the given columns
descriptors.
+ */
+ public static BinaryTupleComparator
binaryTupleComparator(List<StorageSortedIndexColumnDescriptor> columns) {
+ CatalogColumnCollation[] collations = columns.stream()
+ .map(c -> CatalogColumnCollation.get(c.asc(), !c.asc()))
+ .toArray(CatalogColumnCollation[]::new);
+
+ NativeType[] columnTypes =
columns.stream().map(StorageSortedIndexColumnDescriptor::type).toArray(NativeType[]::new);
Review Comment:
I would replaced 2 streams with single `for-loop` due to performance reason
(currently, comparator is created for every ScanRetrieve operation per each
partition)
--
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]
