AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r682827447
##########
File path:
modules/schema/src/main/java/org/apache/ignite/internal/schema/row/RowAssembler.java
##########
@@ -389,6 +405,90 @@ public RowAssembler appendDouble(double val) {
return this;
}
+ /**
+ * Appends BigInteger value for the current column to the chunk.
+ *
+ * @param val Column value.
+ * @return {@code this} for chaining.
+ */
+ public RowAssembler appendNumber(BigInteger val) {
+ checkType(NativeTypeSpec.NUMBER);
+
+ Column col = curCols.column(curCol);
+
+ NumberNativeType type = (NumberNativeType)col.type();
+
+ if (NumericTypeUtils.precisionDoesNotFit(val, type.precision()))
+ throw new IllegalArgumentException("Failed to set number value for
column '" + col.name() + "' " +
+ "(max precision exceeds allocated precision) " +
+ "[number=" + val + ", max precision=" + type.precision() +
"]");
+
+ byte[] bytes = val.toByteArray();
+
+ buf.putBytes(curOff, bytes);
+
+ if (isKeyChunk())
+ keyHash = 31 * keyHash + Arrays.hashCode(bytes);
+
+ writeVarlenOffset(curVartblEntry, curOff - dataOff);
+
+ curVartblEntry++;
+
+ shiftColumn(bytes.length);
+
+ return this;
+ }
+
+ /**
+ * Calculates byte size for BigInteger value.
+ */
+ public static int sizeInBytes(BigInteger val) {
+ return val.bitLength() / 8 + 1;
+ }
+
+ /**
+ * Appends BigDecimal value for the current column to the chunk.
+ *
+ * @param val Column value.
+ * @return {@code this} for chaining.
+ */
+ public RowAssembler appendDecimal(BigDecimal val) {
+ checkType(NativeTypeSpec.DECIMAL);
+
+ Column col = curCols.column(curCol);
+
+ DecimalNativeType type = (DecimalNativeType)col.type();
+
+ val = val.setScale(type.scale(), RoundingMode.HALF_UP);
+
+ if (val.precision() > type.precision())
+ throw new IllegalArgumentException("Failed to set decimal value
for column '" + col.name() + "' " +
+ "(max precision exceeds allocated precision)" +
+ " [decimal=" + val + ", max precision=" + type.precision() +
"]");
+
+ byte[] bytes = val.unscaledValue().toByteArray();
+
+ buf.putBytes(curOff, bytes);
+
+ if (isKeyChunk())
+ keyHash = 31 * keyHash + Arrays.hashCode(bytes);
+
+ writeVarlenOffset(curVartblEntry, curOff - dataOff);
+
+ curVartblEntry++;
+
+ shiftColumn(bytes.length);
+
+ return this;
+ }
+
+ /**
+ * Calculates byte size for BigDecimal value.
+ */
+ public static int sizeInBytes(BigDecimal val) {
Review comment:
Static method must be declared before the constructor.
##########
File path:
modules/schema/src/main/java/org/apache/ignite/internal/schema/row/RowAssembler.java
##########
@@ -389,6 +405,90 @@ public RowAssembler appendDouble(double val) {
return this;
}
+ /**
+ * Appends BigInteger value for the current column to the chunk.
+ *
+ * @param val Column value.
+ * @return {@code this} for chaining.
+ */
+ public RowAssembler appendNumber(BigInteger val) {
+ checkType(NativeTypeSpec.NUMBER);
+
+ Column col = curCols.column(curCol);
+
+ NumberNativeType type = (NumberNativeType)col.type();
+
+ if (NumericTypeUtils.precisionDoesNotFit(val, type.precision()))
+ throw new IllegalArgumentException("Failed to set number value for
column '" + col.name() + "' " +
+ "(max precision exceeds allocated precision) " +
+ "[number=" + val + ", max precision=" + type.precision() +
"]");
+
+ byte[] bytes = val.toByteArray();
+
+ buf.putBytes(curOff, bytes);
+
+ if (isKeyChunk())
+ keyHash = 31 * keyHash + Arrays.hashCode(bytes);
+
+ writeVarlenOffset(curVartblEntry, curOff - dataOff);
+
+ curVartblEntry++;
+
+ shiftColumn(bytes.length);
+
+ return this;
+ }
+
+ /**
+ * Calculates byte size for BigInteger value.
+ */
+ public static int sizeInBytes(BigInteger val) {
Review comment:
Static method must be declared before the constructor.
##########
File path:
modules/table/src/test/java/org/apache/ignite/internal/table/type/NumericTypesSerializerTest.java
##########
@@ -0,0 +1,257 @@
+/*
+ * 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.table.type;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import java.math.RoundingMode;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Random;
+import java.util.UUID;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.InvalidTypeException;
+import org.apache.ignite.internal.schema.NativeTypes;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.marshaller.TupleMarshaller;
+import org.apache.ignite.internal.schema.row.Row;
+import org.apache.ignite.internal.table.TupleBuilderImpl;
+import org.apache.ignite.internal.table.TupleMarshallerImpl;
+import org.apache.ignite.internal.table.impl.DummySchemaManagerImpl;
+import org.apache.ignite.internal.util.Pair;
+import org.apache.ignite.table.Tuple;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+/** */
+public class NumericTypesSerializerTest {
+ /** Random. */
+ private Random rnd = new Random();
+
+ /** Schema descriptor. */
+ private SchemaDescriptor schema;
+
+ /**
+ * @return List of BigInteger pairs for test.
+ */
+ private static List<Pair<BigInteger, BigInteger>> numbers() {
+ return Arrays.asList(
+ new Pair<>(BigInteger.valueOf(10L), BigInteger.valueOf(10)),
+ new Pair<>(BigInteger.valueOf(-10L), BigInteger.valueOf(-10)),
+ new Pair<>(new BigInteger("10"), BigInteger.valueOf(10)),
+ new Pair<>(new BigInteger("1000").divide(BigInteger.TEN),
BigInteger.valueOf(10).multiply(BigInteger.TEN)),
+ new Pair<>(new BigInteger("999999999"),
BigInteger.valueOf(999999999L)),
+ new Pair<>(new BigInteger("+999999999"),
BigInteger.valueOf(999999999L)),
+ new Pair<>(new BigInteger("-999999999"),
BigInteger.valueOf(-999999999L))
+ );
+ }
+
+ /**
+ * @return List of string decimal representations for test.
+ */
+ private static String[] stringDecimalRepresentation() {
+ return new String[]{"0", "0.00", "123", "-123", "1.23E3", "1.23E+3",
"12.3E+7", "12.0", "12.3", "0.00123",
+ "-1.23E-12", "1234.5E-4", "0E+7", "-0", "123456789.0123",
"123456789.1", "123456789.112312315413",
+ "123456789.0123", "123.123456789", "123456789.3210"};
+ }
+
+ @BeforeEach
+ public void setup() {
+ long seed = System.currentTimeMillis();
+
+ rnd = new Random(seed);
+ }
+
+ /**
+ *
+ */
+ @ParameterizedTest
+ @MethodSource("numbers")
+ public void testNumber(Pair<BigInteger, BigInteger> pair) {
+ schema = new SchemaDescriptor(
+ UUID.randomUUID(),
+ 42,
+ new Column[] {new Column("key", NativeTypes.INT64, false)},
+ new Column[] {
+ new Column("number1", NativeTypes.numberOf(19), false),
+ new Column("number2", NativeTypes.numberOf(10), false)
+ }
+ );
+
+ final TupleBuilderImpl tup = new TupleBuilderImpl(schema);
+
+ tup.set("key", rnd.nextLong());
+ tup.set("number1", pair.getFirst());
+ tup.set("number2", pair.getSecond());
+
+ Tuple keyTuple = new TupleBuilderImpl(schema).set("key",
rnd.nextLong()).build();
+
+ TupleMarshaller marshaller = new TupleMarshallerImpl(new
DummySchemaManagerImpl(schema));
+
+ final Row row = marshaller.marshal(keyTuple, tup.build());
+
+ assertEquals(row.numberValue(1), row.numberValue(2));
+ }
+
+ /**
+ *
+ */
+ @Test
+ public void testPrecisionRestrictionsForNumbers() {
+ schema = new SchemaDescriptor(
+ UUID.randomUUID(),
+ 42,
+ new Column[] {new Column("key", NativeTypes.INT64, false)},
+ new Column[] {new Column("number1", NativeTypes.numberOf(5),
false)}
+ );
+
+ final TupleBuilderImpl badTup = new TupleBuilderImpl(schema);
+
+ badTup.set("key", rnd.nextLong());
+ assertThrows(InvalidTypeException.class, () -> badTup.set("number1",
BigInteger.valueOf(999991L)), "Column's type mismatch");
+ assertThrows(InvalidTypeException.class, () -> badTup.set("number1",
new BigInteger("111111")), "Column's type mismatch");
+ assertThrows(InvalidTypeException.class, () -> badTup.set("number1",
BigInteger.valueOf(-999991L)), "Column's type mismatch");
+ assertThrows(InvalidTypeException.class, () -> badTup.set("number1",
new BigInteger("-111111")), "Column's type mismatch");
+ }
+
+ /**
+ *
+ */
+ @Test
+ public void testPrecisionRestrictionsForDecimal() {
+ schema = new SchemaDescriptor(
+ UUID.randomUUID(),
+ 42,
+ new Column[] {new Column("key", NativeTypes.INT64, false)},
+ new Column[] {
+ new Column("decimalCol", NativeTypes.decimalOf(9, 3), false),
+ }
+ );
+
+ final TupleBuilderImpl badTup = new TupleBuilderImpl(schema);
+
+ Tuple keyTuple = new TupleBuilderImpl(schema).set("key",
rnd.nextLong()).build();
+
+ TupleMarshaller marshaller = new TupleMarshallerImpl(new
DummySchemaManagerImpl(schema));
+
+ badTup.set("key", rnd.nextLong());
+ assertThrows(IllegalArgumentException.class,
+ () -> marshaller.marshal(keyTuple, badTup.set("decimalCol", new
BigDecimal("123456789.0123")).build()),
+ "Failed to set decimal value for column"
+ );
+ assertThrows(IllegalArgumentException.class,
+ () -> marshaller.marshal(keyTuple, badTup.set("decimalCol", new
BigDecimal("-1234567890123")).build()),
+ "Failed to set decimal value for column"
+ );
+ assertThrows(IllegalArgumentException.class,
+ () -> marshaller.marshal(keyTuple, badTup.set("decimalCol", new
BigDecimal("123456789.0123")).build()),
Review comment:
```suggestion
() -> marshaller.marshal(keyTuple, badTup.set("decimalCol", new
BigDecimal("1234567")).build()),
```
##########
File path:
modules/schema/src/main/java/org/apache/ignite/internal/schema/NumericTypeUtils.java
##########
@@ -0,0 +1,43 @@
+/*
+ * 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.schema;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import org.apache.ignite.schema.ColumnType;
+
+/**
+ * Numeric types utility class.
+ * */
+public class NumericTypeUtils {
+ /**
+ * check precision of BigInteger value.
+ */
+ public static boolean precisionDoesNotFit(BigInteger val, int precision) {
Review comment:
I think method can be dropped and check can be inlined as
`if (type.precision > 0 && val.precision <= type.precision())`
Can we use zero as magic value for 'unlimited' as it is done for
string/bytes?
Seems, zero precision make no sense and forbidden by SQL standard and will
be compliant with others varlen types.
##########
File path:
modules/table/src/test/java/org/apache/ignite/internal/table/type/NumericTypesSerializerTest.java
##########
@@ -0,0 +1,257 @@
+/*
+ * 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.table.type;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import java.math.RoundingMode;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Random;
+import java.util.UUID;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.InvalidTypeException;
+import org.apache.ignite.internal.schema.NativeTypes;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.marshaller.TupleMarshaller;
+import org.apache.ignite.internal.schema.row.Row;
+import org.apache.ignite.internal.table.TupleBuilderImpl;
+import org.apache.ignite.internal.table.TupleMarshallerImpl;
+import org.apache.ignite.internal.table.impl.DummySchemaManagerImpl;
+import org.apache.ignite.internal.util.Pair;
+import org.apache.ignite.table.Tuple;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+/** */
+public class NumericTypesSerializerTest {
+ /** Random. */
+ private Random rnd = new Random();
+
+ /** Schema descriptor. */
+ private SchemaDescriptor schema;
+
+ /**
+ * @return List of BigInteger pairs for test.
+ */
+ private static List<Pair<BigInteger, BigInteger>> numbers() {
+ return Arrays.asList(
+ new Pair<>(BigInteger.valueOf(10L), BigInteger.valueOf(10)),
+ new Pair<>(BigInteger.valueOf(-10L), BigInteger.valueOf(-10)),
+ new Pair<>(new BigInteger("10"), BigInteger.valueOf(10)),
+ new Pair<>(new BigInteger("1000").divide(BigInteger.TEN),
BigInteger.valueOf(10).multiply(BigInteger.TEN)),
+ new Pair<>(new BigInteger("999999999"),
BigInteger.valueOf(999999999L)),
+ new Pair<>(new BigInteger("+999999999"),
BigInteger.valueOf(999999999L)),
+ new Pair<>(new BigInteger("-999999999"),
BigInteger.valueOf(-999999999L))
+ );
+ }
+
+ /**
+ * @return List of string decimal representations for test.
+ */
+ private static String[] stringDecimalRepresentation() {
+ return new String[]{"0", "0.00", "123", "-123", "1.23E3", "1.23E+3",
"12.3E+7", "12.0", "12.3", "0.00123",
+ "-1.23E-12", "1234.5E-4", "0E+7", "-0", "123456789.0123",
"123456789.1", "123456789.112312315413",
+ "123456789.0123", "123.123456789", "123456789.3210"};
+ }
+
+ @BeforeEach
+ public void setup() {
+ long seed = System.currentTimeMillis();
+
+ rnd = new Random(seed);
+ }
+
+ /**
+ *
+ */
+ @ParameterizedTest
+ @MethodSource("numbers")
+ public void testNumber(Pair<BigInteger, BigInteger> pair) {
+ schema = new SchemaDescriptor(
+ UUID.randomUUID(),
+ 42,
+ new Column[] {new Column("key", NativeTypes.INT64, false)},
+ new Column[] {
+ new Column("number1", NativeTypes.numberOf(19), false),
+ new Column("number2", NativeTypes.numberOf(10), false)
+ }
+ );
+
+ final TupleBuilderImpl tup = new TupleBuilderImpl(schema);
+
+ tup.set("key", rnd.nextLong());
+ tup.set("number1", pair.getFirst());
+ tup.set("number2", pair.getSecond());
+
+ Tuple keyTuple = new TupleBuilderImpl(schema).set("key",
rnd.nextLong()).build();
+
+ TupleMarshaller marshaller = new TupleMarshallerImpl(new
DummySchemaManagerImpl(schema));
+
+ final Row row = marshaller.marshal(keyTuple, tup.build());
+
+ assertEquals(row.numberValue(1), row.numberValue(2));
+ }
+
+ /**
+ *
+ */
+ @Test
+ public void testPrecisionRestrictionsForNumbers() {
+ schema = new SchemaDescriptor(
+ UUID.randomUUID(),
+ 42,
+ new Column[] {new Column("key", NativeTypes.INT64, false)},
+ new Column[] {new Column("number1", NativeTypes.numberOf(5),
false)}
+ );
+
+ final TupleBuilderImpl badTup = new TupleBuilderImpl(schema);
+
+ badTup.set("key", rnd.nextLong());
+ assertThrows(InvalidTypeException.class, () -> badTup.set("number1",
BigInteger.valueOf(999991L)), "Column's type mismatch");
+ assertThrows(InvalidTypeException.class, () -> badTup.set("number1",
new BigInteger("111111")), "Column's type mismatch");
+ assertThrows(InvalidTypeException.class, () -> badTup.set("number1",
BigInteger.valueOf(-999991L)), "Column's type mismatch");
+ assertThrows(InvalidTypeException.class, () -> badTup.set("number1",
new BigInteger("-111111")), "Column's type mismatch");
+ }
+
+ /**
+ *
+ */
+ @Test
+ public void testPrecisionRestrictionsForDecimal() {
+ schema = new SchemaDescriptor(
+ UUID.randomUUID(),
+ 42,
+ new Column[] {new Column("key", NativeTypes.INT64, false)},
+ new Column[] {
+ new Column("decimalCol", NativeTypes.decimalOf(9, 3), false),
+ }
+ );
+
+ final TupleBuilderImpl badTup = new TupleBuilderImpl(schema);
+
+ Tuple keyTuple = new TupleBuilderImpl(schema).set("key",
rnd.nextLong()).build();
+
+ TupleMarshaller marshaller = new TupleMarshallerImpl(new
DummySchemaManagerImpl(schema));
+
+ badTup.set("key", rnd.nextLong());
+ assertThrows(IllegalArgumentException.class,
+ () -> marshaller.marshal(keyTuple, badTup.set("decimalCol", new
BigDecimal("123456789.0123")).build()),
+ "Failed to set decimal value for column"
+ );
+ assertThrows(IllegalArgumentException.class,
+ () -> marshaller.marshal(keyTuple, badTup.set("decimalCol", new
BigDecimal("-1234567890123")).build()),
+ "Failed to set decimal value for column"
+ );
+ assertThrows(IllegalArgumentException.class,
+ () -> marshaller.marshal(keyTuple, badTup.set("decimalCol", new
BigDecimal("123456789.0123")).build()),
Review comment:
Seems, this was a duplicate of the first one.
##########
File path:
modules/schema/src/main/java/org/apache/ignite/internal/schema/DecimalNativeType.java
##########
@@ -57,13 +57,6 @@ public int scale() {
return scale;
}
- /** {@inheritDoc} */
- @Override public boolean mismatch(NativeType type) {
- return super.mismatch(type)
- || precision < ((NumericNativeType)type).precision
- || scale < ((NumericNativeType)type).scale;
Review comment:
Can we validate Decimal somehow?
E.g. check if `precision - scale >= type.precision - type.scale ` ?
I mean we always can convert types with different scales (truncate or
expand),
but must ensure if precision will be compatible after the conversion.
##########
File path:
modules/schema/src/main/java/org/apache/ignite/internal/schema/NumericTypeUtils.java
##########
@@ -0,0 +1,43 @@
+/*
+ * 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.schema;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import org.apache.ignite.schema.ColumnType;
+
+/**
+ * Numeric types utility class.
+ * */
+public class NumericTypeUtils {
+ /**
+ * check precision of BigInteger value.
+ */
+ public static boolean precisionDoesNotFit(BigInteger val, int precision) {
+ if (precision == ColumnType.NumberColumnType.UNLIMITED_PRECISION)
+ return false;
+ return calculatePrecision(val) > precision;
+ }
+
+ /**
+ * Calculates precision for BigInteger value.
+ */
+ public static int calculatePrecision(BigInteger val) {
Review comment:
```suggestion
* Calculates precision for BigInteger value.
* Note: BigInteger is converted to BigDecimal as it have no native
method for this.
*/
public static int precisionOf(BigInteger bigInt) {
```
--
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]
