Copilot commented on code in PR #7406:
URL: https://github.com/apache/ignite-3/pull/7406#discussion_r2689227438
##########
modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItKeyValueBinaryViewApiTest.java:
##########
@@ -582,6 +598,302 @@ public void schemaMismatch(TestCase testCase) {
);
}
+ @ParameterizedTest
+ @MethodSource("typeCastTestCases")
+ public void testWriteAsByte(TestCase testCase) {
+ KeyValueView<Tuple, Tuple> tbl = testCase.view();
+ String keyName = testCase.keyColumnName(0);
+ String valName = testCase.valColumnName(0);
+ Tuple key = Tuple.create().set(keyName, 1L);
+ ColumnType targetType = ColumnType.INT8;
+
+ // Put short value.
+ {
+ Tuple val = Tuple.create().set(valName, (short) Byte.MAX_VALUE);
+
+ tbl.put(null, key, val);
+ assertThat(tbl.get(null, key).byteValue(valName),
is(Byte.MAX_VALUE));
+
+ Tuple outOfRange = Tuple.create().set(valName, (short)
(Byte.MAX_VALUE + 1));
+ expectTypeMismatch(() -> tbl.put(null, key, outOfRange), valName,
targetType, ColumnType.INT16);
+ }
+
+ // Put int value.
+ {
+ Tuple val = Tuple.create().set(valName, (int) Byte.MAX_VALUE);
+
+ tbl.put(null, key, val);
+ assertThat(tbl.get(null, key).byteValue(valName),
is(Byte.MAX_VALUE));
+
+ Tuple outOfRange = Tuple.create().set(valName, Byte.MAX_VALUE + 1);
+ expectTypeMismatch(() -> tbl.put(null, key, outOfRange), valName,
targetType, ColumnType.INT32);
+ }
+
+ // Put long value.
+ {
+ Tuple val = Tuple.create().set(valName, (long) Byte.MAX_VALUE);
+
+ tbl.put(null, key, val);
+ assertThat(tbl.get(null, key).byteValue(valName),
is(Byte.MAX_VALUE));
+
+ Tuple outOfRange = Tuple.create().set(valName, (long)
(Byte.MAX_VALUE + 1));
+ expectTypeMismatch(() -> tbl.put(null, key, outOfRange), valName,
targetType, ColumnType.INT64);
+ }
+
+ // Wrong (floating point) types
+ {
+ Tuple floatValue = Tuple.create().set(valName, Float.MAX_VALUE);
+ expectTypeMismatch(() -> tbl.put(null, key, floatValue), valName,
targetType, ColumnType.FLOAT);
+
+ Tuple doubleValue = Tuple.create().set(valName, Double.MAX_VALUE);
+ expectTypeMismatch(() -> tbl.put(null, key, doubleValue), valName,
targetType, ColumnType.DOUBLE);
+ }
+
+ // Wrong (decimal) type
+ {
+ Tuple decimalValue = Tuple.create().set(valName, new
BigDecimal(1));
+ expectTypeMismatch(() -> tbl.put(null, key, decimalValue),
valName, targetType, ColumnType.DECIMAL);
+ }
+ }
+
+ @ParameterizedTest
+ @MethodSource("typeCastTestCases")
+ public void testWriteAsShort(TestCase testCase) {
+ KeyValueView<Tuple, Tuple> tbl = testCase.view();
+ String keyName = testCase.keyColumnName(0);
+ String valName = testCase.valColumnName(1);
+ Tuple key = Tuple.create().set(keyName, 1L);
+ ColumnType targetType = ColumnType.INT16;
+
+ // Put byte value.
+ {
+ Tuple val = Tuple.create().set(valName, Byte.MAX_VALUE);
+
+ tbl.put(null, key, val);
+ assertThat(tbl.get(null, key).shortValue(valName), is((short)
Byte.MAX_VALUE));
+ }
+
+ // Put int value.
+ {
+ Tuple val = Tuple.create().set(valName, (int) Short.MAX_VALUE);
+
+ tbl.put(null, key, val);
+ assertThat(tbl.get(null, key).shortValue(valName),
is(Short.MAX_VALUE));
+
+ Tuple outOfRange = Tuple.create().set(valName, Short.MAX_VALUE +
1);
+ expectTypeMismatch(() -> tbl.put(null, key, outOfRange), valName,
targetType, ColumnType.INT32);
+ }
+
+ // Put long value.
+ {
+ Tuple val = Tuple.create().set(valName, (long) Short.MAX_VALUE);
+
+ tbl.put(null, key, val);
+ assertThat(tbl.get(null, key).shortValue(valName),
is(Short.MAX_VALUE));
+
+ Tuple outOfRange = Tuple.create().set(valName, (long)
(Short.MAX_VALUE + 1));
+ expectTypeMismatch(() -> tbl.put(null, key, outOfRange), valName,
targetType, ColumnType.INT64);
+ }
+
+ // Wrong (floating point) types
+ {
+ Tuple floatValue = Tuple.create().set(valName, Float.MAX_VALUE);
+ expectTypeMismatch(() -> tbl.put(null, key, floatValue), valName,
targetType, ColumnType.FLOAT);
+
+ Tuple doubleValue = Tuple.create().set(valName, Double.MAX_VALUE);
+ expectTypeMismatch(() -> tbl.put(null, key, doubleValue), valName,
targetType, ColumnType.DOUBLE);
+ }
+
+ // Wrong (decimal) type
+ {
+ Tuple decimalValue = Tuple.create().set(valName, new
BigDecimal(1));
+ expectTypeMismatch(() -> tbl.put(null, key, decimalValue),
valName, targetType, ColumnType.DECIMAL);
+ }
+ }
+
+ @ParameterizedTest
+ @MethodSource("typeCastTestCases")
+ public void testWriteAsInt(TestCase testCase) {
+ KeyValueView<Tuple, Tuple> tbl = testCase.view();
+ String keyName = testCase.keyColumnName(0);
+ String valName = testCase.valColumnName(2);
+ Tuple key = Tuple.create().set(keyName, 1L);
+ ColumnType targetType = ColumnType.INT32;
+
+ // Put byte value.
+ {
+ Tuple val = Tuple.create().set(valName, Byte.MAX_VALUE);
+
+ tbl.put(null, key, val);
+ assertThat(tbl.get(null, key).intValue(valName), is((int)
Byte.MAX_VALUE));
+ }
+
+ // Put short value.
+ {
+ Tuple val = Tuple.create().set(valName, Short.MAX_VALUE);
+
+ tbl.put(null, key, val);
+ assertThat(tbl.get(null, key).intValue(valName), is((int)
Short.MAX_VALUE));
+ }
+
+ // Put long value.
+ {
+ Tuple val = Tuple.create().set(valName, (long) Integer.MAX_VALUE);
+
+ tbl.put(null, key, val);
+ assertThat(tbl.get(null, key).intValue(valName),
is(Integer.MAX_VALUE));
+
+ Tuple outOfRange = Tuple.create().set(valName, ((long)
Integer.MAX_VALUE) + 1);
+ expectTypeMismatch(() -> tbl.put(null, key, outOfRange), valName,
targetType, ColumnType.INT64);
+ }
+
+ // Wrong (floating point) types
+ {
+ Tuple floatValue = Tuple.create().set(valName, Float.MAX_VALUE);
+ expectTypeMismatch(() -> tbl.put(null, key, floatValue), valName,
targetType, ColumnType.FLOAT);
+
+ Tuple doubleValue = Tuple.create().set(valName, Double.MAX_VALUE);
+ expectTypeMismatch(() -> tbl.put(null, key, doubleValue), valName,
targetType, ColumnType.DOUBLE);
+ }
+
+ // Wrong (decimal) type
+ {
+ Tuple decimalValue = Tuple.create().set(valName, new
BigDecimal(1));
+ expectTypeMismatch(() -> tbl.put(null, key, decimalValue),
valName, targetType, ColumnType.DECIMAL);
+ }
+ }
+
+ @ParameterizedTest
+ @MethodSource("typeCastTestCases")
+ public void testWriteAsLong(TestCase testCase) {
+ KeyValueView<Tuple, Tuple> tbl = testCase.view();
+ String keyName = testCase.keyColumnName(0);
+ String valName = testCase.valColumnName(3);
+ Tuple key = Tuple.create().set(keyName, 1L);
+
+ // Put byte value.
+ {
+ Tuple val = Tuple.create().set(valName, Byte.MAX_VALUE);
+
+ tbl.put(null, key, val);
+ assertThat(tbl.get(null, key).longValue(valName), is((long)
Byte.MAX_VALUE));
+ }
+
+ // Put short value.
+ {
+ Tuple val = Tuple.create().set(valName, Short.MAX_VALUE);
+
+ tbl.put(null, key, val);
+ assertThat(tbl.get(null, key).longValue(valName), is((long)
Short.MAX_VALUE));
+ }
+
+ // Put int value.
+ {
+ Tuple val = Tuple.create().set(valName, Integer.MAX_VALUE);
+
+ tbl.put(null, key, val);
+ assertThat(tbl.get(null, key).longValue(valName), is((long)
Integer.MAX_VALUE));
+ }
+
+ ColumnType targetType = ColumnType.INT64;
+
+ // Wrong (floating point) types
+ {
+ Tuple floatValue = Tuple.create().set(valName, Float.MAX_VALUE);
+ expectTypeMismatch(() -> tbl.put(null, key, floatValue), valName,
targetType, ColumnType.FLOAT);
+
+ Tuple doubleValue = Tuple.create().set(valName, Double.MAX_VALUE);
+ expectTypeMismatch(() -> tbl.put(null, key, doubleValue), valName,
targetType, ColumnType.DOUBLE);
+ }
+
+ // Wrong (decimal) type
+ {
+ Tuple decimalValue = Tuple.create().set(valName, new
BigDecimal(1));
+ expectTypeMismatch(() -> tbl.put(null, key, decimalValue),
valName, targetType, ColumnType.DECIMAL);
+ }
+ }
+
+ @ParameterizedTest
+ @MethodSource("typeCastTestCases")
+ public void testWriteAsFloat(TestCase testCase) {
+ KeyValueView<Tuple, Tuple> tbl = testCase.view();
+ String keyName = testCase.keyColumnName(0);
+ String valName = testCase.valColumnName(4);
+ Tuple key = Tuple.create().set(keyName, 1L);
+ ColumnType targetType = ColumnType.FLOAT;
+
+ // Put double value.
+ {
+ Tuple val = Tuple.create().set(valName, (double) Float.MAX_VALUE);
+
+ tbl.put(null, key, val);
+ assertThat(tbl.get(null, key).floatValue(valName),
is(Float.MAX_VALUE));
+
+ Tuple outOfRange = Tuple.create().set(valName, Double.MAX_VALUE);
+ expectTypeMismatch(() -> tbl.put(null, key, outOfRange), valName,
targetType, ColumnType.DOUBLE);
+ }
+
+ // Wrong (integer) types
+ {
+ Tuple byteValue = Tuple.create().set(valName, Byte.MAX_VALUE);
+ expectTypeMismatch(() -> tbl.put(null, key, byteValue), valName,
targetType, ColumnType.INT8);
+
+ Tuple shortValue = Tuple.create().set(valName, Short.MAX_VALUE);
+ expectTypeMismatch(() -> tbl.put(null, key, shortValue), valName,
targetType, ColumnType.INT16);
+
+ Tuple intValue = Tuple.create().set(valName, Integer.MAX_VALUE);
+ expectTypeMismatch(() -> tbl.put(null, key, intValue), valName,
targetType, ColumnType.INT32);
+
+ Tuple longValue = Tuple.create().set(valName, Long.MAX_VALUE);
+ expectTypeMismatch(() -> tbl.put(null, key, longValue), valName,
targetType, ColumnType.INT64);
+ }
+
+ // Wrong (decimal) type
+ {
+ Tuple decimalValue = Tuple.create().set(valName, new
BigDecimal(1));
+ expectTypeMismatch(() -> tbl.put(null, key, decimalValue),
valName, targetType, ColumnType.DECIMAL);
+ }
+ }
Review Comment:
The test coverage for floating-point type casting doesn't include special
cases like NaN, positive infinity, and negative infinity. These are important
edge cases that should be tested to ensure the type casting logic handles them
correctly when converting between Float and Double types.
##########
modules/core/src/main/java/org/apache/ignite/internal/util/TupleTypeCastUtils.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.util;
+
+import java.util.EnumSet;
+import java.util.Set;
+import org.apache.ignite.internal.lang.InternalTuple;
+import org.apache.ignite.sql.ColumnType;
+
+/**
+ * Helper methods for reading values from {@link InternalTuple} with allowed
numeric type conversions.
+ *
+ * <p>The following conversions are supported:
+ * <ul>
+ * <li>Any integer type to any other integer type with range checks (e.g.
{@link ColumnType#INT64} to {@link ColumnType#INT8}
+ * may throw {@link ArithmeticException} if the value doesn't fit into
{@link ColumnType#INT8}).</li>
+ * <li>{@link ColumnType#FLOAT} to {@link ColumnType#DOUBLE} are always
allowed.</li>
+ * <li>{@link ColumnType#DOUBLE} to {@link ColumnType#FLOAT} with
precision checks (may throw {@link ArithmeticException}
+ * if the value cannot be represented as FLOAT without precision
loss).</li>
Review Comment:
The javadoc mentions throwing ArithmeticException for narrowing conversions,
but the actual implementation returns false for invalid casts and doesn't throw
any exception. The documentation should be updated to accurately reflect that
the method returns a boolean value indicating whether the cast is allowed.
```suggestion
* is considered invalid (and {@link #isCastAllowed(ColumnType,
ColumnType, Object)} returns {@code false}) if the value
* doesn't fit into {@link ColumnType#INT8}).</li>
* <li>{@link ColumnType#FLOAT} to {@link ColumnType#DOUBLE} are always
allowed.</li>
* <li>{@link ColumnType#DOUBLE} to {@link ColumnType#FLOAT} with
precision checks (the cast is considered invalid, and
* {@link #isCastAllowed(ColumnType, ColumnType, Object)} returns {@code
false}, if the value cannot be represented as
* {@link ColumnType#FLOAT} without precision loss).</li>
```
##########
modules/client-common/src/main/java/org/apache/ignite/internal/client/proto/ClientBinaryTupleUtils.java:
##########
@@ -288,6 +288,165 @@ public static void appendValue(BinaryTupleBuilder
builder, ColumnType type, Stri
}
}
+ private static void appendByteValue(BinaryTupleBuilder builder, Object
val) {
+ if (val.getClass() == Byte.class) {
+ builder.appendByte((byte) val);
+ return;
+ }
+
+ if (val.getClass() == Short.class) {
+ short shortVal = (short) val;
+ byte byteVal = (byte) shortVal;
+
+ if (shortVal == byteVal) {
+ builder.appendByte(byteVal);
+ return;
+ }
+ }
+
+ if (val.getClass() == Integer.class) {
+ int intVal = (int) val;
+ byte byteVal = (byte) intVal;
+
+ if (intVal == byteVal) {
+ builder.appendByte(byteVal);
+ return;
+ }
+ }
+
+ if (val.getClass() == Long.class) {
+ long longVal = (long) val;
+ byte byteVal = (byte) longVal;
+
+ if (longVal == byteVal) {
+ builder.appendByte(byteVal);
+ return;
+ }
+ }
+
+ throw new ClassCastException("Cannot cast to byte: " + val.getClass());
+ }
+
+ private static void appendShortValue(BinaryTupleBuilder builder, Object
val) {
+ if (val.getClass() == Short.class) {
+ builder.appendShort((short) val);
+ return;
+ }
+
+ if (val.getClass() == Byte.class) {
+ builder.appendShort((byte) val);
+ return;
+ }
+
+ if (val.getClass() == Integer.class) {
+ int intVal = (int) val;
+ short shortVal = (short) intVal;
+
+ if (intVal == shortVal) {
+ builder.appendShort(shortVal);
+ return;
+ }
+ }
+
+ if (val.getClass() == Long.class) {
+ long longVal = (long) val;
+ short shortVal = (short) longVal;
+
+ if (longVal == shortVal) {
+ builder.appendShort(shortVal);
+ return;
+ }
+ }
+
+ throw new ClassCastException();
Review Comment:
The error message in appendShortValue is inconsistent with other methods.
While appendByteValue and appendIntValue include the class name in the error
message, this method doesn't. For consistency, consider changing this to:
"Cannot cast to short: " + val.getClass()
```suggestion
throw new ClassCastException("Cannot cast to short: " +
val.getClass());
```
##########
modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItRecordBinaryViewApiTest.java:
##########
@@ -791,6 +807,308 @@ public void bytesTypeMatch(BinTestCase testCase) {
}
}
+ @ParameterizedTest
+ @MethodSource("typeCastTestCases")
+ public void testWriteAsByte(BinTestCase testCase) {
+ RecordView<Tuple> recordView = testCase.view();
+ String keyName = DEFAULT_KEY[0].name();
+ String valName = "C_BYTE";
+ ColumnType targetType = ColumnType.INT8;
+
+ Tuple key = Tuple.create().set(keyName, 1L);
+
+ // Put short value.
+ {
+ Tuple val = Tuple.copy(key).set(valName, (short) Byte.MAX_VALUE);
+
+ recordView.upsert(null, val);
+ assertThat(recordView.get(null, key).byteValue(valName),
is(Byte.MAX_VALUE));
+
+ Tuple outOfRange = Tuple.copy(key).set(valName, (short)
(Byte.MAX_VALUE + 1));
+ expectTypeMismatch(() -> recordView.upsert(null, outOfRange),
valName, targetType, ColumnType.INT16);
+ }
+
+ // Put int value.
+ {
+ Tuple val = Tuple.copy(key).set(valName, (int) Byte.MAX_VALUE);
+
+ recordView.upsert(null, val);
+ assertThat(recordView.get(null, key).byteValue(valName),
is(Byte.MAX_VALUE));
+
+ Tuple outOfRange = Tuple.copy(key).set(valName, Byte.MAX_VALUE +
1);
+ expectTypeMismatch(() -> recordView.upsert(null, outOfRange),
valName, targetType, ColumnType.INT32);
+ }
+
+ // Put long value.
+ {
+ Tuple val = Tuple.copy(key).set(valName, (long) Byte.MAX_VALUE);
+
+ recordView.upsert(null, val);
+ assertThat(recordView.get(null, key).byteValue(valName),
is(Byte.MAX_VALUE));
+
+ Tuple outOfRange = Tuple.copy(key).set(valName, (long)
(Byte.MAX_VALUE + 1));
+ expectTypeMismatch(() -> recordView.upsert(null, outOfRange),
valName, targetType, ColumnType.INT64);
+ }
+
+ // Wrong (floating point) types
+ {
+ Tuple floatValue = Tuple.copy(key).set(valName, Float.MAX_VALUE);
+ expectTypeMismatch(() -> recordView.upsert(null, floatValue),
valName, targetType, ColumnType.FLOAT);
+
+ Tuple doubleValue = Tuple.copy(key).set(valName, Double.MAX_VALUE);
+ expectTypeMismatch(() -> recordView.upsert(null, doubleValue),
valName, targetType, ColumnType.DOUBLE);
+ }
+
+ // Wrong (decimal) type
+ {
+ Tuple decimalValue = Tuple.copy(key).set(valName, new
BigDecimal(1));
+ expectTypeMismatch(() -> recordView.upsert(null, decimalValue),
valName, targetType, ColumnType.DECIMAL);
+ }
+ }
Review Comment:
The test cases only verify MAX_VALUE boundaries but don't test MIN_VALUE
boundaries for narrowing conversions. For example, when casting Long to Byte,
negative values like Byte.MIN_VALUE should also be tested to ensure the range
checking works correctly for both positive and negative bounds.
##########
modules/client-common/src/main/java/org/apache/ignite/internal/client/proto/ClientBinaryTupleUtils.java:
##########
@@ -288,6 +288,165 @@ public static void appendValue(BinaryTupleBuilder
builder, ColumnType type, Stri
}
}
+ private static void appendByteValue(BinaryTupleBuilder builder, Object
val) {
+ if (val.getClass() == Byte.class) {
+ builder.appendByte((byte) val);
+ return;
+ }
+
+ if (val.getClass() == Short.class) {
+ short shortVal = (short) val;
+ byte byteVal = (byte) shortVal;
+
+ if (shortVal == byteVal) {
+ builder.appendByte(byteVal);
+ return;
+ }
+ }
+
+ if (val.getClass() == Integer.class) {
+ int intVal = (int) val;
+ byte byteVal = (byte) intVal;
+
+ if (intVal == byteVal) {
+ builder.appendByte(byteVal);
+ return;
+ }
+ }
+
+ if (val.getClass() == Long.class) {
+ long longVal = (long) val;
+ byte byteVal = (byte) longVal;
+
+ if (longVal == byteVal) {
+ builder.appendByte(byteVal);
+ return;
+ }
+ }
+
+ throw new ClassCastException("Cannot cast to byte: " + val.getClass());
+ }
+
+ private static void appendShortValue(BinaryTupleBuilder builder, Object
val) {
+ if (val.getClass() == Short.class) {
+ builder.appendShort((short) val);
+ return;
+ }
+
+ if (val.getClass() == Byte.class) {
+ builder.appendShort((byte) val);
+ return;
+ }
+
+ if (val.getClass() == Integer.class) {
+ int intVal = (int) val;
+ short shortVal = (short) intVal;
+
+ if (intVal == shortVal) {
+ builder.appendShort(shortVal);
+ return;
+ }
+ }
+
+ if (val.getClass() == Long.class) {
+ long longVal = (long) val;
+ short shortVal = (short) longVal;
+
+ if (longVal == shortVal) {
+ builder.appendShort(shortVal);
+ return;
+ }
+ }
+
+ throw new ClassCastException();
+ }
+
+ private static void appendIntValue(BinaryTupleBuilder builder, Object val)
{
+ if (val.getClass() == Integer.class) {
+ builder.appendInt((int) val);
+ return;
+ }
+
+ if (val.getClass() == Short.class) {
+ builder.appendInt((short) val);
+ return;
+ }
+
+ if (val.getClass() == Byte.class) {
+ builder.appendInt((byte) val);
+ return;
+ }
+
+ if (val.getClass() == Long.class) {
+ long longVal = (long) val;
+ int intVal = (int) longVal;
+
+ if (longVal == intVal) {
+ builder.appendInt(intVal);
+ return;
+ }
+ }
+
+ throw new ClassCastException("Cannot cast to int: " + val.getClass());
+ }
+
+ private static void appendLongValue(BinaryTupleBuilder builder, Object
val) {
+ if (val.getClass() == Integer.class) {
+ builder.appendLong((int) val);
+ return;
+ }
+
+ if (val.getClass() == Short.class) {
+ builder.appendLong((short) val);
+ return;
+ }
+
+ if (val.getClass() == Byte.class) {
+ builder.appendLong((byte) val);
+ return;
+ }
+
+ if (val.getClass() == Long.class) {
+ builder.appendLong((long) val);
+ return;
+ }
+
+ throw new ClassCastException("Cannot cast to long: " + val.getClass());
+ }
+
+ private static void appendFloatValue(BinaryTupleBuilder builder, Object
val) {
+ if (val.getClass() == Float.class) {
+ builder.appendFloat((float) val);
+ return;
+ }
+
+ if (val.getClass() == Double.class) {
+ double doubleVal = (double) val;
+ float floatVal = (float) doubleVal;
+
+ if (doubleVal == floatVal) {
Review Comment:
The comparison `doubleVal == floatVal` doesn't handle special floating-point
values correctly. For example, Double.NaN cast to float will produce Float.NaN,
and NaN == NaN is always false, which would incorrectly reject valid NaN
values. Consider using Double.compare() or Float.isNaN() to handle special
cases like NaN and infinity correctly.
```suggestion
if (Double.compare(doubleVal, floatVal) == 0) {
```
##########
modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItKeyValueBinaryViewApiTest.java:
##########
@@ -582,6 +598,302 @@ public void schemaMismatch(TestCase testCase) {
);
}
+ @ParameterizedTest
+ @MethodSource("typeCastTestCases")
+ public void testWriteAsByte(TestCase testCase) {
+ KeyValueView<Tuple, Tuple> tbl = testCase.view();
+ String keyName = testCase.keyColumnName(0);
+ String valName = testCase.valColumnName(0);
+ Tuple key = Tuple.create().set(keyName, 1L);
+ ColumnType targetType = ColumnType.INT8;
+
+ // Put short value.
+ {
+ Tuple val = Tuple.create().set(valName, (short) Byte.MAX_VALUE);
+
+ tbl.put(null, key, val);
+ assertThat(tbl.get(null, key).byteValue(valName),
is(Byte.MAX_VALUE));
+
+ Tuple outOfRange = Tuple.create().set(valName, (short)
(Byte.MAX_VALUE + 1));
+ expectTypeMismatch(() -> tbl.put(null, key, outOfRange), valName,
targetType, ColumnType.INT16);
+ }
+
+ // Put int value.
+ {
+ Tuple val = Tuple.create().set(valName, (int) Byte.MAX_VALUE);
+
+ tbl.put(null, key, val);
+ assertThat(tbl.get(null, key).byteValue(valName),
is(Byte.MAX_VALUE));
+
+ Tuple outOfRange = Tuple.create().set(valName, Byte.MAX_VALUE + 1);
+ expectTypeMismatch(() -> tbl.put(null, key, outOfRange), valName,
targetType, ColumnType.INT32);
+ }
+
+ // Put long value.
+ {
+ Tuple val = Tuple.create().set(valName, (long) Byte.MAX_VALUE);
+
+ tbl.put(null, key, val);
+ assertThat(tbl.get(null, key).byteValue(valName),
is(Byte.MAX_VALUE));
+
+ Tuple outOfRange = Tuple.create().set(valName, (long)
(Byte.MAX_VALUE + 1));
+ expectTypeMismatch(() -> tbl.put(null, key, outOfRange), valName,
targetType, ColumnType.INT64);
+ }
+
+ // Wrong (floating point) types
+ {
+ Tuple floatValue = Tuple.create().set(valName, Float.MAX_VALUE);
+ expectTypeMismatch(() -> tbl.put(null, key, floatValue), valName,
targetType, ColumnType.FLOAT);
+
+ Tuple doubleValue = Tuple.create().set(valName, Double.MAX_VALUE);
+ expectTypeMismatch(() -> tbl.put(null, key, doubleValue), valName,
targetType, ColumnType.DOUBLE);
+ }
+
+ // Wrong (decimal) type
+ {
+ Tuple decimalValue = Tuple.create().set(valName, new
BigDecimal(1));
+ expectTypeMismatch(() -> tbl.put(null, key, decimalValue),
valName, targetType, ColumnType.DECIMAL);
+ }
+ }
Review Comment:
The test cases only verify MAX_VALUE boundaries but don't test MIN_VALUE
boundaries for narrowing conversions. For example, when casting Long to Byte,
negative values like Byte.MIN_VALUE should also be tested to ensure the range
checking works correctly for both positive and negative bounds.
##########
modules/core/src/main/java/org/apache/ignite/internal/util/TupleTypeCastUtils.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.util;
+
+import java.util.EnumSet;
+import java.util.Set;
+import org.apache.ignite.internal.lang.InternalTuple;
+import org.apache.ignite.sql.ColumnType;
+
+/**
+ * Helper methods for reading values from {@link InternalTuple} with allowed
numeric type conversions.
+ *
+ * <p>The following conversions are supported:
+ * <ul>
+ * <li>Any integer type to any other integer type with range checks (e.g.
{@link ColumnType#INT64} to {@link ColumnType#INT8}
+ * may throw {@link ArithmeticException} if the value doesn't fit into
{@link ColumnType#INT8}).</li>
+ * <li>{@link ColumnType#FLOAT} to {@link ColumnType#DOUBLE} are always
allowed.</li>
+ * <li>{@link ColumnType#DOUBLE} to {@link ColumnType#FLOAT} with
precision checks (may throw {@link ArithmeticException}
+ * if the value cannot be represented as FLOAT without precision
loss).</li>
+ * </ul>
+ */
+public class TupleTypeCastUtils {
+ /** Integer column types. */
+ private static final Set<ColumnType> INT_TYPES =
+ EnumSet.of(ColumnType.INT8, ColumnType.INT16, ColumnType.INT32,
ColumnType.INT64);
+
+ /**
+ * Checks whether a cast is possible between two types for the given value.
+ *
+ * <p>Widening casts between integer types and between floating-point
types are always allowed.
+ *
+ * <p>Narrowing casts between integer types and between floating-point
types are allowed only
+ * when the provided value can be represented in the target type.
+ *
+ * @param from Source column type
+ * @param to Target column type
+ * @param val The value to be cast
+ * @return {@code True} if the cast is possible without data loss, {@code
false} otherwise.
+ */
+ public static boolean isCastAllowed(ColumnType from, ColumnType to, Object
val) {
+ if (!(floatingPointType(from) && floatingPointType(to))
+ && !(integerType(from) && integerType(to))) {
+ return false;
+ }
+
+ Number number = (Number) val;
+
+ switch (to) {
+ case INT8:
+ return number.byteValue() == number.longValue();
+ case INT16:
+ return number.shortValue() == number.longValue();
+ case INT32:
+ return number.intValue() == number.longValue();
+ case FLOAT:
+ return number.floatValue() == number.doubleValue();
Review Comment:
The comparison `number.floatValue() == number.doubleValue()` doesn't handle
special floating-point values correctly. For example, Double.NaN will fail the
comparison since NaN == NaN is always false. This would incorrectly reject
valid NaN values when casting from Double to Float. Consider handling special
cases like NaN and infinity explicitly.
```suggestion
case FLOAT: {
double d = number.doubleValue();
float f = number.floatValue();
if (Double.isNaN(d)) {
// NaN is representable as Float.NaN without precision
loss.
return true;
}
// For non-NaN values, keep the existing precision check
logic.
return f == d;
}
```
##########
modules/core/src/main/java/org/apache/ignite/internal/util/TupleTypeCastUtils.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.util;
+
+import java.util.EnumSet;
+import java.util.Set;
+import org.apache.ignite.internal.lang.InternalTuple;
+import org.apache.ignite.sql.ColumnType;
+
+/**
+ * Helper methods for reading values from {@link InternalTuple} with allowed
numeric type conversions.
Review Comment:
The javadoc states "Helper methods for reading values" but this utility is
actually used for writing/validating values during tuple insertion. The comment
should be updated to reflect the actual usage, such as "Helper methods for
validating numeric type conversions when writing values to tuples".
```suggestion
* Helper methods for validating numeric type conversions when writing
values to {@link InternalTuple}.
```
##########
modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItRecordBinaryViewApiTest.java:
##########
@@ -791,6 +807,308 @@ public void bytesTypeMatch(BinTestCase testCase) {
}
}
+ @ParameterizedTest
+ @MethodSource("typeCastTestCases")
+ public void testWriteAsByte(BinTestCase testCase) {
+ RecordView<Tuple> recordView = testCase.view();
+ String keyName = DEFAULT_KEY[0].name();
+ String valName = "C_BYTE";
+ ColumnType targetType = ColumnType.INT8;
+
+ Tuple key = Tuple.create().set(keyName, 1L);
+
+ // Put short value.
+ {
+ Tuple val = Tuple.copy(key).set(valName, (short) Byte.MAX_VALUE);
+
+ recordView.upsert(null, val);
+ assertThat(recordView.get(null, key).byteValue(valName),
is(Byte.MAX_VALUE));
+
+ Tuple outOfRange = Tuple.copy(key).set(valName, (short)
(Byte.MAX_VALUE + 1));
+ expectTypeMismatch(() -> recordView.upsert(null, outOfRange),
valName, targetType, ColumnType.INT16);
+ }
+
+ // Put int value.
+ {
+ Tuple val = Tuple.copy(key).set(valName, (int) Byte.MAX_VALUE);
+
+ recordView.upsert(null, val);
+ assertThat(recordView.get(null, key).byteValue(valName),
is(Byte.MAX_VALUE));
+
+ Tuple outOfRange = Tuple.copy(key).set(valName, Byte.MAX_VALUE +
1);
+ expectTypeMismatch(() -> recordView.upsert(null, outOfRange),
valName, targetType, ColumnType.INT32);
+ }
+
+ // Put long value.
+ {
+ Tuple val = Tuple.copy(key).set(valName, (long) Byte.MAX_VALUE);
+
+ recordView.upsert(null, val);
+ assertThat(recordView.get(null, key).byteValue(valName),
is(Byte.MAX_VALUE));
+
+ Tuple outOfRange = Tuple.copy(key).set(valName, (long)
(Byte.MAX_VALUE + 1));
+ expectTypeMismatch(() -> recordView.upsert(null, outOfRange),
valName, targetType, ColumnType.INT64);
+ }
+
+ // Wrong (floating point) types
+ {
+ Tuple floatValue = Tuple.copy(key).set(valName, Float.MAX_VALUE);
+ expectTypeMismatch(() -> recordView.upsert(null, floatValue),
valName, targetType, ColumnType.FLOAT);
+
+ Tuple doubleValue = Tuple.copy(key).set(valName, Double.MAX_VALUE);
+ expectTypeMismatch(() -> recordView.upsert(null, doubleValue),
valName, targetType, ColumnType.DOUBLE);
+ }
+
+ // Wrong (decimal) type
+ {
+ Tuple decimalValue = Tuple.copy(key).set(valName, new
BigDecimal(1));
+ expectTypeMismatch(() -> recordView.upsert(null, decimalValue),
valName, targetType, ColumnType.DECIMAL);
+ }
+ }
+
+ @ParameterizedTest
+ @MethodSource("typeCastTestCases")
+ public void testWriteAsShort(BinTestCase testCase) {
+ RecordView<Tuple> recordView = testCase.view();
+ String keyName = DEFAULT_KEY[0].name();
+ String valName = "C_SHORT";
+ ColumnType targetType = ColumnType.INT16;
+
+ Tuple key = Tuple.create().set(keyName, 1L);
+
+ // Put byte value.
+ {
+ Tuple val = Tuple.copy(key).set(valName, Byte.MAX_VALUE);
+
+ recordView.upsert(null, val);
+ assertThat(recordView.get(null, key).shortValue(valName),
is((short) Byte.MAX_VALUE));
+ }
+
+ // Put int value.
+ {
+ Tuple val = Tuple.copy(key).set(valName, (int) Short.MAX_VALUE);
+
+ recordView.upsert(null, val);
+ assertThat(recordView.get(null, key).shortValue(valName),
is(Short.MAX_VALUE));
+
+ Tuple outOfRange = Tuple.copy(key).set(valName, Short.MAX_VALUE +
1);
+ expectTypeMismatch(() -> recordView.upsert(null, outOfRange),
valName, targetType, ColumnType.INT32);
+ }
+
+ // Put long value.
+ {
+ Tuple val = Tuple.copy(key).set(valName, (long) Short.MAX_VALUE);
+
+ recordView.upsert(null, val);
+ assertThat(recordView.get(null, key).shortValue(valName),
is(Short.MAX_VALUE));
+
+ Tuple outOfRange = Tuple.copy(key).set(valName, (long)
(Short.MAX_VALUE + 1));
+ expectTypeMismatch(() -> recordView.upsert(null, outOfRange),
valName, targetType, ColumnType.INT64);
+ }
+
+ // Wrong (floating point) types
+ {
+ Tuple floatValue = Tuple.copy(key).set(valName, Float.MAX_VALUE);
+ expectTypeMismatch(() -> recordView.upsert(null, floatValue),
valName, targetType, ColumnType.FLOAT);
+
+ Tuple doubleValue = Tuple.copy(key).set(valName, Double.MAX_VALUE);
+ expectTypeMismatch(() -> recordView.upsert(null, doubleValue),
valName, targetType, ColumnType.DOUBLE);
+ }
+
+ // Wrong (decimal) type
+ {
+ Tuple decimalValue = Tuple.copy(key).set(valName, new
BigDecimal(1));
+ expectTypeMismatch(() -> recordView.upsert(null, decimalValue),
valName, targetType, ColumnType.DECIMAL);
+ }
+ }
+
+ @ParameterizedTest
+ @MethodSource("typeCastTestCases")
+ public void testWriteAsInt(BinTestCase testCase) {
+ RecordView<Tuple> recordView = testCase.view();
+ String keyName = DEFAULT_KEY[0].name();
+ String valName = "C_INT";
+ ColumnType targetType = ColumnType.INT32;
+
+ Tuple key = Tuple.create().set(keyName, 1L);
+
+ // Put byte value.
+ {
+ Tuple val = Tuple.copy(key).set(valName, Byte.MAX_VALUE);
+
+ recordView.upsert(null, val);
+ assertThat(recordView.get(null, key).intValue(valName), is((int)
Byte.MAX_VALUE));
+ }
+
+ // Put short value.
+ {
+ Tuple val = Tuple.copy(key).set(valName, Short.MAX_VALUE);
+
+ recordView.upsert(null, val);
+ assertThat(recordView.get(null, key).intValue(valName), is((int)
Short.MAX_VALUE));
+ }
+
+ // Put long value.
+ {
+ Tuple val = Tuple.copy(key).set(valName, (long) Integer.MAX_VALUE);
+
+ recordView.upsert(null, val);
+ assertThat(recordView.get(null, key).intValue(valName),
is(Integer.MAX_VALUE));
+
+ Tuple outOfRange = Tuple.copy(key).set(valName, ((long)
Integer.MAX_VALUE) + 1);
+ expectTypeMismatch(() -> recordView.upsert(null, outOfRange),
valName, targetType, ColumnType.INT64);
+ }
+
+ // Wrong (floating point) types
+ {
+ Tuple floatValue = Tuple.copy(key).set(valName, Float.MAX_VALUE);
+ expectTypeMismatch(() -> recordView.upsert(null, floatValue),
valName, targetType, ColumnType.FLOAT);
+
+ Tuple doubleValue = Tuple.copy(key).set(valName, Double.MAX_VALUE);
+ expectTypeMismatch(() -> recordView.upsert(null, doubleValue),
valName, targetType, ColumnType.DOUBLE);
+ }
+
+ // Wrong (decimal) type
+ {
+ Tuple decimalValue = Tuple.copy(key).set(valName, new
BigDecimal(1));
+ expectTypeMismatch(() -> recordView.upsert(null, decimalValue),
valName, targetType, ColumnType.DECIMAL);
+ }
+ }
+
+ @ParameterizedTest
+ @MethodSource("typeCastTestCases")
+ public void testWriteAsLong(BinTestCase testCase) {
+ RecordView<Tuple> recordView = testCase.view();
+ String keyName = DEFAULT_KEY[0].name();
+ String valName = "C_LONG";
+
+ Tuple key = Tuple.create().set(keyName, 1L);
+
+ // Put byte value.
+ {
+ Tuple val = Tuple.copy(key).set(valName, Byte.MAX_VALUE);
+
+ recordView.upsert(null, val);
+ assertThat(recordView.get(null, key).longValue(valName), is((long)
Byte.MAX_VALUE));
+ }
+
+ // Put short value.
+ {
+ Tuple val = Tuple.copy(key).set(valName, Short.MAX_VALUE);
+
+ recordView.upsert(null, val);
+ assertThat(recordView.get(null, key).longValue(valName), is((long)
Short.MAX_VALUE));
+ }
+
+ // Put int value.
+ {
+ Tuple val = Tuple.copy(key).set(valName, Integer.MAX_VALUE);
+
+ recordView.upsert(null, val);
+ assertThat(recordView.get(null, key).longValue(valName), is((long)
Integer.MAX_VALUE));
+ }
+
+ ColumnType targetType = ColumnType.INT64;
+
+ // Wrong (floating point) types
+ {
+ Tuple floatValue = Tuple.copy(key).set(valName, Float.MAX_VALUE);
+ expectTypeMismatch(() -> recordView.upsert(null, floatValue),
valName, targetType, ColumnType.FLOAT);
+
+ Tuple doubleValue = Tuple.copy(key).set(valName, Double.MAX_VALUE);
+ expectTypeMismatch(() -> recordView.upsert(null, doubleValue),
valName, targetType, ColumnType.DOUBLE);
+ }
+
+ // Wrong (decimal) type
+ {
+ Tuple decimalValue = Tuple.copy(key).set(valName, new
BigDecimal(1));
+ expectTypeMismatch(() -> recordView.upsert(null, decimalValue),
valName, targetType, ColumnType.DECIMAL);
+ }
+ }
+
+ @ParameterizedTest
+ @MethodSource("typeCastTestCases")
+ public void testWriteAsFloat(BinTestCase testCase) {
+ RecordView<Tuple> recordView = testCase.view();
+ String keyName = DEFAULT_KEY[0].name();
+ String valName = "C_FLOAT";
+ ColumnType targetType = ColumnType.FLOAT;
+
+ Tuple key = Tuple.create().set(keyName, 1L);
+
+ // Put double value.
+ {
+ Tuple val = Tuple.copy(key).set(valName, (double) Float.MAX_VALUE);
+
+ recordView.upsert(null, val);
+ assertThat(recordView.get(null, key).floatValue(valName),
is(Float.MAX_VALUE));
+
+ Tuple outOfRange = Tuple.copy(key).set(valName, Double.MAX_VALUE);
+ expectTypeMismatch(() -> recordView.upsert(null, outOfRange),
valName, targetType, ColumnType.DOUBLE);
+ }
+
+ // Wrong (integer) types
+ {
+ Tuple byteValue = Tuple.copy(key).set(valName, Byte.MAX_VALUE);
+ expectTypeMismatch(() -> recordView.upsert(null, byteValue),
valName, targetType, ColumnType.INT8);
+
+ Tuple shortValue = Tuple.copy(key).set(valName, Short.MAX_VALUE);
+ expectTypeMismatch(() -> recordView.upsert(null, shortValue),
valName, targetType, ColumnType.INT16);
+
+ Tuple intValue = Tuple.copy(key).set(valName, Integer.MAX_VALUE);
+ expectTypeMismatch(() -> recordView.upsert(null, intValue),
valName, targetType, ColumnType.INT32);
+
+ Tuple longValue = Tuple.copy(key).set(valName, Long.MAX_VALUE);
+ expectTypeMismatch(() -> recordView.upsert(null, longValue),
valName, targetType, ColumnType.INT64);
+ }
+
+ // Wrong (decimal) type
+ {
+ Tuple decimalValue = Tuple.copy(key).set(valName, new
BigDecimal(1));
+ expectTypeMismatch(() -> recordView.upsert(null, decimalValue),
valName, targetType, ColumnType.DECIMAL);
+ }
+ }
Review Comment:
The test coverage for floating-point type casting doesn't include special
cases like NaN, positive infinity, and negative infinity. These are important
edge cases that should be tested to ensure the type casting logic handles them
correctly when converting between Float and Double types.
--
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]
