This is an automated email from the ASF dual-hosted git repository.
ibessonov pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new e896751ac6 IGNITE-21890 Fix BinaryTupleComparator for decimal values
(#3527)
e896751ac6 is described below
commit e896751ac6e5478944364f38b3c5aeb5ba432479
Author: Alexander Polovtcev <[email protected]>
AuthorDate: Tue Apr 2 10:42:24 2024 +0300
IGNITE-21890 Fix BinaryTupleComparator for decimal values (#3527)
---
.../internal/binarytuple/BinaryTupleReader.java | 7 ++-
.../apache/ignite/internal/lang/InternalTuple.java | 3 +-
.../storage/index/BinaryTupleComparator.java | 6 +--
.../storage/index/BinaryTupleComparatorTest.java | 4 +-
.../index/AbstractSortedIndexStorageTest.java | 27 +++++++----
.../internal/storage/index/impl/TestIndexRow.java | 55 ++++++++++++++++++----
6 files changed, 77 insertions(+), 25 deletions(-)
diff --git
a/modules/binary-tuple/src/main/java/org/apache/ignite/internal/binarytuple/BinaryTupleReader.java
b/modules/binary-tuple/src/main/java/org/apache/ignite/internal/binarytuple/BinaryTupleReader.java
index 050d26d9be..26cda8b375 100644
---
a/modules/binary-tuple/src/main/java/org/apache/ignite/internal/binarytuple/BinaryTupleReader.java
+++
b/modules/binary-tuple/src/main/java/org/apache/ignite/internal/binarytuple/BinaryTupleReader.java
@@ -255,7 +255,8 @@ public class BinaryTupleReader extends BinaryTupleParser
implements BinaryTupleP
* Reads value of specified element.
*
* @param index Element index.
- * @param scale Decimal scale.
+ * @param scale Decimal scale. If equal to {@link Integer#MIN_VALUE}, then
the value will be returned with whatever scale it is
+ * stored in.
* @return Element value.
*/
public @Nullable BigDecimal decimalValue(int index, int scale) {
@@ -266,7 +267,9 @@ public class BinaryTupleReader extends BinaryTupleParser
implements BinaryTupleP
short valScale = shortValue(begin, begin + 2);
- return new BigDecimal(numberValue(begin + 2, end),
valScale).setScale(scale, RoundingMode.UNNECESSARY);
+ BigDecimal decimalValue = new BigDecimal(numberValue(begin + 2, end),
valScale);
+
+ return scale < 0 ? decimalValue : decimalValue.setScale(scale,
RoundingMode.UNNECESSARY);
}
/**
diff --git
a/modules/core/src/main/java/org/apache/ignite/internal/lang/InternalTuple.java
b/modules/core/src/main/java/org/apache/ignite/internal/lang/InternalTuple.java
index 596e3a3a18..55e5a0866a 100644
---
a/modules/core/src/main/java/org/apache/ignite/internal/lang/InternalTuple.java
+++
b/modules/core/src/main/java/org/apache/ignite/internal/lang/InternalTuple.java
@@ -160,7 +160,8 @@ public interface InternalTuple {
* Reads value from specified column.
*
* @param col Column index.
- * @param decimalScale Decimal scale.
+ * @param decimalScale Decimal scale. If equal to {@link
Integer#MIN_VALUE}, then the value will be returned with whatever scale
+ * it is stored in.
* @return Column value.
*/
BigDecimal decimalValue(int col, int decimalScale);
diff --git
a/modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/BinaryTupleComparator.java
b/modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/BinaryTupleComparator.java
index de6f03bd0a..243492bb20 100644
---
a/modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/BinaryTupleComparator.java
+++
b/modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/BinaryTupleComparator.java
@@ -141,11 +141,11 @@ public class BinaryTupleComparator implements
Comparator<ByteBuffer> {
return
tuple1.stringValue(index).compareTo(tuple2.stringValue(index));
case NUMBER:
- case DECIMAL:
- // Floating point position is irrelevant during comparison.
- // The only real requirement is that it matches in both
arguments, and it does.
return
tuple1.numberValue(index).compareTo(tuple2.numberValue(index));
+ case DECIMAL:
+ return tuple1.decimalValue(index,
Integer.MIN_VALUE).compareTo(tuple2.decimalValue(index, Integer.MIN_VALUE));
+
case TIMESTAMP:
return
tuple1.timestampValue(index).compareTo(tuple2.timestampValue(index));
diff --git
a/modules/storage-api/src/test/java/org/apache/ignite/internal/storage/index/BinaryTupleComparatorTest.java
b/modules/storage-api/src/test/java/org/apache/ignite/internal/storage/index/BinaryTupleComparatorTest.java
index 0c66f72b2c..73c0a7ab14 100644
---
a/modules/storage-api/src/test/java/org/apache/ignite/internal/storage/index/BinaryTupleComparatorTest.java
+++
b/modules/storage-api/src/test/java/org/apache/ignite/internal/storage/index/BinaryTupleComparatorTest.java
@@ -199,11 +199,11 @@ public class BinaryTupleComparatorTest {
case DECIMAL: {
tuple1 = new BinaryTupleBuilder(1)
- .appendDecimal(BigDecimal.valueOf(-1), 4)
+ .appendDecimal(BigDecimal.valueOf(-1), 0)
.build();
tuple2 = new BinaryTupleBuilder(1)
- .appendDecimal(BigDecimal.valueOf(123456789.1234), 4)
+ .appendDecimal(BigDecimal.valueOf(123456789.12), 2)
.build();
break;
diff --git
a/modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/index/AbstractSortedIndexStorageTest.java
b/modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/index/AbstractSortedIndexStorageTest.java
index 2fbb534b02..10e0d198b0 100644
---
a/modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/index/AbstractSortedIndexStorageTest.java
+++
b/modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/index/AbstractSortedIndexStorageTest.java
@@ -35,6 +35,7 @@ import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.greaterThanOrEqualTo;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.sameInstance;
import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -70,6 +71,7 @@ import org.apache.ignite.internal.storage.RowId;
import
org.apache.ignite.internal.storage.index.StorageSortedIndexDescriptor.StorageSortedIndexColumnDescriptor;
import org.apache.ignite.internal.storage.index.impl.BinaryTupleRowSerializer;
import org.apache.ignite.internal.storage.index.impl.TestIndexRow;
+import
org.apache.ignite.internal.storage.index.impl.TestIndexRow.TestIndexPrefix;
import org.apache.ignite.internal.testframework.VariableSource;
import org.apache.ignite.internal.util.Cursor;
import org.apache.ignite.sql.ColumnType;
@@ -286,18 +288,19 @@ public abstract class AbstractSortedIndexStorageTest
extends AbstractIndexStorag
int firstIndex = 3;
int lastIndex = 8;
+ TestIndexPrefix first = entries.get(firstIndex).prefix(3);
+ TestIndexPrefix last = entries.get(lastIndex).prefix(5);
+
List<IndexRow> expected = entries.stream()
- .skip(firstIndex)
- .limit(lastIndex - firstIndex + 1)
+ .filter(row -> row.compareTo(first) >= 0 &&
row.compareTo(last) <= 0)
.collect(toList());
- BinaryTuplePrefix first = entries.get(firstIndex).prefix(3);
- BinaryTuplePrefix last = entries.get(lastIndex).prefix(5);
+ assertThat(expected, hasSize(greaterThanOrEqualTo(lastIndex -
firstIndex + 1)));
- try (Cursor<IndexRow> cursor = indexStorage.scan(first, last,
GREATER_OR_EQUAL | LESS_OR_EQUAL)) {
+ try (Cursor<IndexRow> cursor = indexStorage.scan(first.prefix(),
last.prefix(), GREATER_OR_EQUAL | LESS_OR_EQUAL)) {
List<IndexRow> actual = cursor.stream().collect(toList());
- assertThat(actual, hasSize(lastIndex - firstIndex + 1));
+ assertThat(actual, hasSize(expected.size()));
for (int i = firstIndex; i < actual.size(); ++i) {
assertThat(actual.get(i).rowId(),
is(equalTo(expected.get(i).rowId())));
@@ -413,7 +416,11 @@ public abstract class AbstractSortedIndexStorageTest
extends AbstractIndexStorag
put(indexStorage, entry1);
put(indexStorage, entry2);
- try (Cursor<IndexRow> cursor =
indexStorage.scan(entry2.prefix(indexSchema.size()),
entry1.prefix(indexSchema.size()), 0)) {
+ try (Cursor<IndexRow> cursor = indexStorage.scan(
+ entry2.prefix(indexSchema.size()).prefix(),
+ entry1.prefix(indexSchema.size()).prefix(),
+ 0
+ )) {
assertThat(cursor.stream().collect(toList()), is(empty()));
}
}
@@ -442,7 +449,11 @@ public abstract class AbstractSortedIndexStorageTest
extends AbstractIndexStorag
entry1 = t;
}
- try (Cursor<IndexRow> cursor = storage.scan(entry1.prefix(1),
entry2.prefix(1), GREATER_OR_EQUAL | LESS_OR_EQUAL)) {
+ try (Cursor<IndexRow> cursor = storage.scan(
+ entry1.prefix(1).prefix(),
+ entry2.prefix(1).prefix(),
+ GREATER_OR_EQUAL | LESS_OR_EQUAL
+ )) {
assertThat(
cursor.stream().map(row ->
row.indexColumns().byteBuffer()).collect(toList()),
contains(entry1.indexColumns().byteBuffer(),
entry2.indexColumns().byteBuffer())
diff --git
a/modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/index/impl/TestIndexRow.java
b/modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/index/impl/TestIndexRow.java
index 56d0eb71d8..36d4974666 100644
---
a/modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/index/impl/TestIndexRow.java
+++
b/modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/index/impl/TestIndexRow.java
@@ -24,7 +24,7 @@ import java.lang.reflect.Array;
import java.util.Arrays;
import java.util.BitSet;
import java.util.Comparator;
-import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
import java.util.function.Function;
import java.util.stream.IntStream;
import org.apache.ignite.internal.schema.BinaryTuple;
@@ -62,11 +62,9 @@ public class TestIndexRow implements IndexRow,
Comparable<TestIndexRow> {
* Creates an row with random column values that satisfies the given
schema.
*/
public static TestIndexRow randomRow(SortedIndexStorage indexStorage, int
partitionId) {
- var random = new Random();
-
Object[] columns = indexStorage.indexDescriptor().columns().stream()
.map(StorageSortedIndexColumnDescriptor::type)
- .map(type -> generateRandomValue(random, type))
+ .map(type -> generateRandomValue(ThreadLocalRandom.current(),
type))
.toArray();
var rowId = new RowId(partitionId);
@@ -78,11 +76,35 @@ public class TestIndexRow implements IndexRow,
Comparable<TestIndexRow> {
return new TestIndexRow(indexStorage, serializer, row, columns);
}
+ /**
+ * Class representing an Index Key prefix.
+ */
+ public static class TestIndexPrefix {
+ private final BinaryTuplePrefix prefix;
+
+ private final Object[] prefixColumns;
+
+ TestIndexPrefix(BinaryTuplePrefix prefix, Object[] prefixColumns) {
+ this.prefix = prefix;
+ this.prefixColumns = prefixColumns;
+ }
+
+ public BinaryTuplePrefix prefix() {
+ return prefix;
+ }
+
+ public Object[] prefixColumns() {
+ return prefixColumns;
+ }
+ }
+
/**
* Creates an Index Key prefix of the given length.
*/
- public BinaryTuplePrefix prefix(int length) {
- return serializer.serializeRowPrefix(Arrays.copyOf(columns, length));
+ public TestIndexPrefix prefix(int length) {
+ Object[] prefixColumns = Arrays.copyOf(columns, length);
+
+ return new
TestIndexPrefix(serializer.serializeRowPrefix(prefixColumns), prefixColumns);
}
@Override
@@ -103,10 +125,25 @@ public class TestIndexRow implements IndexRow,
Comparable<TestIndexRow> {
return sizeCompare;
}
- for (int i = 0; i < columns.length; ++i) {
- Comparator<Object> comparator = comparator(columns[i].getClass());
+ return compareColumns(columns, o.columns);
+ }
+
+ /**
+ * Compares the row with the given row prefix.
+ */
+ public int compareTo(TestIndexPrefix prefix) {
+ Object[] prefixColumns = prefix.prefixColumns();
+
+ assert prefixColumns.length <= columns.length;
+
+ return compareColumns(columns, prefixColumns);
+ }
+
+ private int compareColumns(Object[] a, Object[] b) {
+ for (int i = 0; i < Math.min(a.length, b.length); ++i) {
+ Comparator<Object> comparator = comparator(a[i].getClass());
- int compare = comparator.compare(columns[i], o.columns[i]);
+ int compare = comparator.compare(a[i], b[i]);
if (compare != 0) {
boolean asc =
indexStorage.indexDescriptor().columns().get(i).asc();
