You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/07/23 14:12:22 UTC

[GitHub] [ignite-3] vladErmakov07 opened a new pull request #243: IGNITE-13668 first impl

vladErmakov07 opened a new pull request #243:
URL: https://github.com/apache/ignite-3/pull/243


   


-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679908636



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/NumericTypeUtils.java
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.
+ * */

Review comment:
       ```suggestion
    */
   ```




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679905488



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/DecimalNativeType.java
##########
@@ -60,8 +60,7 @@ public int scale() {
     /** {@inheritDoc} */
     @Override public boolean mismatch(NativeType type) {
         return super.mismatch(type)
-            || precision < ((NumericNativeType)type).precision
-            || scale < ((NumericNativeType)type).scale;
+            || precision < ((DecimalNativeType)type).precision;

Review comment:
       Agree, we are ok to round up numbers that has higher scale.
   But what if user pass a number of higher precision due to it high scale?
   
   E.g. table column type is Decimal(8,2), but user pass 123.123456789.
   We detect user number as of Decimal(12,9), but actually, 
   it perfectly fits to the column type after rounding value to 123.12.
   
   Have I missed smth?
   




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

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

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

[GitHub] [ignite-3] vladErmakov07 commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

vladErmakov07 commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679918657



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/NumericTypeUtils.java
##########
@@ -0,0 +1,74 @@
+/*
+ * 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 {
+    /**
+     * Calculates byte size for BigInteger value.
+     * */
+    public static int sizeInBytes(BigInteger val) {
+        return val.bitLength() / 8 + 1;
+    }
+
+    /**
+     * Calculates byte size for BigDecimal value.
+     * */
+    public static int sizeInBytes(BigDecimal val, DecimalNativeType type) {
+        int shift;
+        if (type.scale() == 0)
+            shift = 0;
+        else if (type.scale() <= Byte.MAX_VALUE)
+            shift = 1;
+        else if (type.scale() <= Short.MAX_VALUE)
+            shift = 2;
+        else
+            shift = 4;
+
+        return sizeInBytes(val.unscaledValue()) + shift;
+    }
+
+    /**
+     * check precision of BigInteger value.
+     * */
+    public static boolean precisionDoesNotFit(BigInteger val, int precision) {
+        if (precision == ColumnType.NumberColumnType.UNDEFINED)
+            return false;
+        return calculatePrecision(val) > precision;
+    }
+
+    /**
+     * Calculates precision for BigInteger value.
+     * */
+    public static int calculatePrecision(BigInteger val) {
+        return new BigDecimal(val).precision();

Review comment:
       Unfortunately, only BigDecimal class has the precision function. I can copy this function, but I don't think it would make sense.




-- 
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

[GitHub] [ignite-3] vladErmakov07 commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

vladErmakov07 commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679950546



##########
File path: modules/api/src/main/java/org/apache/ignite/schema/ColumnType.java
##########
@@ -105,15 +105,55 @@ public static VarLenColumnType blobOf(int length) {
         return new VarLenColumnType(ColumnTypeSpec.BLOB, length);
     }
 
+    /**
+     * Number type factory method.
+     *
+     * @param precision Precision of value.
+     * @return Number type.
+     */
+    public static NumberColumnType numberOf(int precision) {
+        if (precision <= 0)
+            throw new IllegalArgumentException("Precision [" + precision + "] must be positive integer value.");
+
+        return new NumberColumnType(ColumnTypeSpec.NUMBER, precision);
+    }
+
+    /**
+     * Number type factory method.
+     *
+     * @return Number type.
+     */
+    public static NumberColumnType numberOf() {
+        return new NumberColumnType(ColumnTypeSpec.NUMBER, NumberColumnType.UNDEFINED);
+    }
+
     /**
      * Decimal type factory method.
      *
      * @param precision Precision.
      * @param scale Scale.
-     * @return Numeric type.
+     * @return Decimal type.
+     */
+    public static DecimalColumnType decimalOf(int precision, int scale) {
+        if (precision <= 0)
+            throw new IllegalArgumentException("Precision [" + precision + "] must be positive integer value.");
+        if (scale < 0)
+            throw new IllegalArgumentException("Scale [" + scale + "] must be non-negative integer value.");

Review comment:
       Precision should be Strictly positive value. It has no sense to store value with zero length. 
   Scale should be non-negative. I think it is ok and means that we will store only Integer part of value.

##########
File path: modules/api/src/main/java/org/apache/ignite/schema/ColumnType.java
##########
@@ -105,15 +105,55 @@ public static VarLenColumnType blobOf(int length) {
         return new VarLenColumnType(ColumnTypeSpec.BLOB, length);
     }
 
+    /**
+     * Number type factory method.
+     *
+     * @param precision Precision of value.
+     * @return Number type.
+     */
+    public static NumberColumnType numberOf(int precision) {
+        if (precision <= 0)
+            throw new IllegalArgumentException("Precision [" + precision + "] must be positive integer value.");
+
+        return new NumberColumnType(ColumnTypeSpec.NUMBER, precision);
+    }
+
+    /**
+     * Number type factory method.
+     *
+     * @return Number type.
+     */
+    public static NumberColumnType numberOf() {
+        return new NumberColumnType(ColumnTypeSpec.NUMBER, NumberColumnType.UNDEFINED);
+    }
+
     /**
      * Decimal type factory method.
      *
      * @param precision Precision.
      * @param scale Scale.
-     * @return Numeric type.
+     * @return Decimal type.
+     */
+    public static DecimalColumnType decimalOf(int precision, int scale) {
+        if (precision <= 0)
+            throw new IllegalArgumentException("Precision [" + precision + "] must be positive integer value.");

Review comment:
       Precision should be Strictly positive value. It has no sense to store value with zero length. 
   Scale should be non-negative. I think it is ok and means that we will store only Integer part of value.




-- 
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

[GitHub] [ignite-3] vladErmakov07 commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

vladErmakov07 commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679963039



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/NumericTypeUtils.java
##########
@@ -0,0 +1,74 @@
+/*
+ * 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 {
+    /**
+     * Calculates byte size for BigInteger value.
+     * */
+    public static int sizeInBytes(BigInteger val) {
+        return val.bitLength() / 8 + 1;

Review comment:
       BigInteger returns the bits size without regard to the sign. So, 8 bits is already a number exceeding Byte.MAX_VALUE. Here is an example of a number that the suggested function does not work on.
   
   BigInteger integer = BigInteger.valueOf(Byte.MAX_VALUE + 1);
   System.out.println(integer.toByteArray().length); // 2
   System.out.println(integer.bitLength()); // 8
   System.out.println(integer.bitLength() / 8 + 1); // 2
   System.out.println((integer.bitLength() + 7) / 8); // 1 (!)
   
   




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679899587



##########
File path: modules/api/src/main/java/org/apache/ignite/schema/ColumnType.java
##########
@@ -206,6 +252,48 @@ public int scale() {
         }
     }
 
+    /**
+     * Number column type.
+     */
+    public static class NumberColumnType extends ColumnType {
+        /** Undefined precision. */
+        public static final int UNDEFINED = -1;

Review comment:
       ```suggestion
           private static final int UNDEFINED_PRECISON = -1;
   ```




-- 
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

[GitHub] [ignite-3] vladErmakov07 commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

vladErmakov07 commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r681814607



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/DecimalNativeType.java
##########
@@ -60,8 +60,7 @@ public int scale() {
     /** {@inheritDoc} */
     @Override public boolean mismatch(NativeType type) {
         return super.mismatch(type)
-            || precision < ((NumericNativeType)type).precision
-            || scale < ((NumericNativeType)type).scale;
+            || precision < ((DecimalNativeType)type).precision;

Review comment:
       I decided to check precision after scaling in RowAssembler.




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679943026



##########
File path: modules/schema/src/test/java/org/apache/ignite/internal/schema/RowTest.java
##########
@@ -421,6 +433,26 @@ else if (type == NativeTypeSpec.STRING) {
                         nonNullVarLenValSize += RowAssembler.utf8EncodedLength((CharSequence)vals[i]);
                     }
                 }
+                else if (type == NativeTypeSpec.NUMBER) {
+                    if (schema.isKeyColumn(i)) {
+                        nonNullVarLenKeyCols++;
+                        nonNullVarLenKeySize += NumericTypeUtils.sizeInBytes((BigInteger)vals[i]);

Review comment:
       Let's move sizeInBytes method to RowAssembler.




-- 
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

[GitHub] [ignite-3] vladErmakov07 commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

vladErmakov07 commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r681814885



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/NumericTypeUtils.java
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.
+ * */

Review comment:
       Done, thanks.




-- 
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

[GitHub] [ignite-3] vladErmakov07 commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

vladErmakov07 commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r684087234



##########
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:
       If we validate here, we will discard even those values that could be stored. 
   For example, there is a special case with zero value string representations:
   
   System.out.println(new BigDecimal("0.00").scale()); //2
   System.out.println(new BigDecimal("0E-2").scale()); //2
   System.out.println(new BigDecimal("0E+2").scale()); //-2
   
   All of these expressions are "zero" and we can definitely store them. But we cannot understand this here.




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679922408



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/NumericTypeUtils.java
##########
@@ -0,0 +1,74 @@
+/*
+ * 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 {
+    /**
+     * Calculates byte size for BigInteger value.
+     * */
+    public static int sizeInBytes(BigInteger val) {
+        return val.bitLength() / 8 + 1;

Review comment:
       We use the same expression in Row/Columns classes for null-map size calculation.
   I don't think this method is useful, maybe smth like next worth to be added to the common IgniteUtils:
   `int bytesForBits(int numberOfBits)`




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679940432



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/Row.java
##########
@@ -233,8 +235,55 @@ public Double doubleValueBoxed(int col) throws InvalidTypeException {
      * @throws InvalidTypeException If actual column type does not match the requested column type.
      */
     public BigDecimal decimalValue(int col) throws InvalidTypeException {
-        // TODO: IGNITE-13668 decimal support
-        return null;
+        long offLen = findColumn(col, NativeTypeSpec.DECIMAL);
+
+        if (offLen < 0)
+            return offLen == -1 ? null : (BigDecimal)rowSchema().column(col).defaultValue();
+
+        int off = offset(offLen);
+        int len = length(offLen);
+
+        DecimalNativeType type = (DecimalNativeType)schema.column(col).type();
+
+        int shift;
+        int scale;
+
+        if (type.scale() == 0) {
+            shift = 0;
+            scale = 0;
+        } else if (type.scale() < Byte.MAX_VALUE) {
+            shift = 1;
+            scale = readByte(off);
+        } else if (type.scale() < Short.MAX_VALUE) {
+            shift = 2;
+            scale = readShort(off);
+        } else {
+            shift = 4;
+            scale = readInteger(off);
+        }

Review comment:
       You can convert byte/shot value to unsigned int and got twice more range.
   
   Also it is possible to extract a method for writing varInt values, where
   sign-bit indicates where the value fits the byte or not.
   ```
   scale = readByte(off);
   
   if (scale < 0)
       scale = ((-scale) << 8) | readByte(off+1);
   ```




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679933887



##########
File path: modules/api/src/main/java/org/apache/ignite/schema/ColumnType.java
##########
@@ -105,15 +105,55 @@ public static VarLenColumnType blobOf(int length) {
         return new VarLenColumnType(ColumnTypeSpec.BLOB, length);
     }
 
+    /**
+     * Number type factory method.
+     *
+     * @param precision Precision of value.
+     * @return Number type.
+     */
+    public static NumberColumnType numberOf(int precision) {
+        if (precision <= 0)

Review comment:
       What does zero precision value mean?




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679959650



##########
File path: modules/table/src/test/java/org/apache/ignite/internal/table/type/NumericTypesSerializerTest.java
##########
@@ -0,0 +1,203 @@
+/*
+ * 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.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.registry.SchemaRegistryImpl;
+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.util.Pair;
+import org.apache.ignite.schema.ColumnType;
+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();
+
+    /** Tuple marshaller. */
+    private TupleMarshaller marshaller;
+
+    /** 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))
+        );
+    }
+
+    @BeforeEach
+    public void setup() {
+        long seed = System.currentTimeMillis();
+
+        rnd = new Random(seed);
+
+        marshaller = new TupleMarshallerImpl(new SchemaRegistryImpl(v -> null) {
+            @Override public SchemaDescriptor schema() {
+                return schema;
+            }
+
+            @Override public SchemaDescriptor schema(int ver) {
+                return schema;
+            }
+
+            @Override public int lastSchemaVersion() {
+                return schema.version();
+            }
+        });
+
+    }
+
+    /**
+     *
+     */
+    @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();
+
+        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(12, 3), false),
+            }
+        );
+
+        final TupleBuilderImpl badTup = new TupleBuilderImpl(schema);
+
+        badTup.set("key", rnd.nextLong());
+        assertThrows(InvalidTypeException.class, () -> badTup.set("decimalCol", new BigDecimal("123456789.0123")), "Column's type mismatch");
+        assertThrows(InvalidTypeException.class, () -> badTup.set("decimalCol", new BigDecimal("-1234567890123")), "Column's type mismatch");
+        assertThrows(InvalidTypeException.class, () -> badTup.set("decimalCol", new BigDecimal(123456789.0123d)), "Column's type mismatch");

Review comment:
       Let's add a test for 123.123456789 and 123456789.3210




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 first impl

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r675644415



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/NativeTypes.java
##########
@@ -202,6 +219,12 @@ public static NativeType from(ColumnType type) {
                         ((ColumnType.VarLenColumnType)type).length() : Integer.MAX_VALUE
                 );
 
+            case NUMBER:
+                ColumnType.NumberColumnType numberType = (ColumnType.NumberColumnType)type;
+                if (numberType.precision() > 0)
+                    return new FixLenNumberNativeType(numberType.precision());
+                return new FixLenNumberNativeType(numberType.precision());

Review comment:
       Why do we need 'if' conditino here?




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679905488



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/DecimalNativeType.java
##########
@@ -60,8 +60,7 @@ public int scale() {
     /** {@inheritDoc} */
     @Override public boolean mismatch(NativeType type) {
         return super.mismatch(type)
-            || precision < ((NumericNativeType)type).precision
-            || scale < ((NumericNativeType)type).scale;
+            || precision < ((DecimalNativeType)type).precision;

Review comment:
       Agree, we are ok to round up numbers that has higher scale.
   But what if user pass a number of higher precision due to it high scale?
   
   E.g. table column type is Decimal(8,2), but user pass 123.123456789.
   We detect user number as of Decimal(12,9), but actually, it perfectly fits to the column type after round up.
   
   Have I missed smth?
   




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679913208



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/NumericTypeUtils.java
##########
@@ -0,0 +1,74 @@
+/*
+ * 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 {
+    /**
+     * Calculates byte size for BigInteger value.
+     * */
+    public static int sizeInBytes(BigInteger val) {
+        return val.bitLength() / 8 + 1;
+    }
+
+    /**
+     * Calculates byte size for BigDecimal value.
+     * */
+    public static int sizeInBytes(BigDecimal val, DecimalNativeType type) {
+        int shift;
+        if (type.scale() == 0)
+            shift = 0;
+        else if (type.scale() <= Byte.MAX_VALUE)
+            shift = 1;
+        else if (type.scale() <= Short.MAX_VALUE)
+            shift = 2;
+        else
+            shift = 4;
+
+        return sizeInBytes(val.unscaledValue()) + shift;
+    }
+
+    /**
+     * check precision of BigInteger value.
+     * */
+    public static boolean precisionDoesNotFit(BigInteger val, int precision) {
+        if (precision == ColumnType.NumberColumnType.UNDEFINED)
+            return false;
+        return calculatePrecision(val) > precision;
+    }
+
+    /**
+     * Calculates precision for BigInteger value.
+     * */
+    public static int calculatePrecision(BigInteger val) {
+        return new BigDecimal(val).precision();

Review comment:
       Why is BigInteger converted to BigDecimal?




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679958187



##########
File path: modules/table/src/test/java/org/apache/ignite/internal/table/type/NumericTypesSerializerTest.java
##########
@@ -0,0 +1,203 @@
+/*
+ * 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.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.registry.SchemaRegistryImpl;
+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.util.Pair;
+import org.apache.ignite.schema.ColumnType;
+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();
+
+    /** Tuple marshaller. */
+    private TupleMarshaller marshaller;
+
+    /** 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))
+        );
+    }
+
+    @BeforeEach
+    public void setup() {
+        long seed = System.currentTimeMillis();
+
+        rnd = new Random(seed);
+
+        marshaller = new TupleMarshallerImpl(new SchemaRegistryImpl(v -> null) {
+            @Override public SchemaDescriptor schema() {
+                return schema;
+            }
+
+            @Override public SchemaDescriptor schema(int ver) {
+                return schema;
+            }
+
+            @Override public int lastSchemaVersion() {
+                return schema.version();
+            }
+        });
+
+    }
+
+    /**
+     *
+     */
+    @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();
+
+        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),
+            }

Review comment:
       ```suggestion
               new Column[] {new Column("number1", NativeTypes.numberOf(5), false)}
   ```




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

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

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

[GitHub] [ignite-3] vladErmakov07 commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

vladErmakov07 commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r684087234



##########
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:
       If we validate here, we will discard even those values that we could have stored. 
   For example, there is a special case with zero value string representations:
   
   System.out.println(new BigDecimal("0.00").scale()); //2
   System.out.println(new BigDecimal("0E-2").scale()); //2
   System.out.println(new BigDecimal("0E+2").scale()); //-2
   
   All of these expressions are "zero" and we can definitely store them. But we cannot understand this here.




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679910738



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/NumericTypeUtils.java
##########
@@ -0,0 +1,74 @@
+/*
+ * 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 {
+    /**
+     * Calculates byte size for BigInteger value.
+     * */
+    public static int sizeInBytes(BigInteger val) {
+        return val.bitLength() / 8 + 1;

Review comment:
       8-bit length value will return 2. Is it ok?
   ```suggestion
           return (val.bitLength() + 7) / 8;
   ```




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679931183



##########
File path: modules/api/src/main/java/org/apache/ignite/schema/ColumnType.java
##########
@@ -105,15 +105,55 @@ public static VarLenColumnType blobOf(int length) {
         return new VarLenColumnType(ColumnTypeSpec.BLOB, length);
     }
 
+    /**
+     * Number type factory method.
+     *
+     * @param precision Precision of value.
+     * @return Number type.
+     */
+    public static NumberColumnType numberOf(int precision) {
+        if (precision <= 0)
+            throw new IllegalArgumentException("Precision [" + precision + "] must be positive integer value.");
+
+        return new NumberColumnType(ColumnTypeSpec.NUMBER, precision);
+    }
+
+    /**
+     * Number type factory method.
+     *
+     * @return Number type.
+     */
+    public static NumberColumnType numberOf() {
+        return new NumberColumnType(ColumnTypeSpec.NUMBER, NumberColumnType.UNDEFINED);
+    }
+
     /**
      * Decimal type factory method.
      *
      * @param precision Precision.
      * @param scale Scale.
-     * @return Numeric type.
+     * @return Decimal type.
+     */
+    public static DecimalColumnType decimalOf(int precision, int scale) {
+        if (precision <= 0)
+            throw new IllegalArgumentException("Precision [" + precision + "] must be positive integer value.");
+        if (scale < 0)
+            throw new IllegalArgumentException("Scale [" + scale + "] must be non-negative integer value.");

Review comment:
       Just non-negative or strictly positive ?




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679910738



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/NumericTypeUtils.java
##########
@@ -0,0 +1,74 @@
+/*
+ * 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 {
+    /**
+     * Calculates byte size for BigInteger value.
+     * */
+    public static int sizeInBytes(BigInteger val) {
+        return val.bitLength() / 8 + 1;

Review comment:
       Size of 2 will be returned for 8-bit length value. Is it ok?
   ```suggestion
           return (val.bitLength() + 7) / 8;
   ```




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679931461



##########
File path: modules/api/src/main/java/org/apache/ignite/schema/ColumnType.java
##########
@@ -105,15 +105,55 @@ public static VarLenColumnType blobOf(int length) {
         return new VarLenColumnType(ColumnTypeSpec.BLOB, length);
     }
 
+    /**
+     * Number type factory method.
+     *
+     * @param precision Precision of value.
+     * @return Number type.
+     */
+    public static NumberColumnType numberOf(int precision) {
+        if (precision <= 0)
+            throw new IllegalArgumentException("Precision [" + precision + "] must be positive integer value.");
+
+        return new NumberColumnType(ColumnTypeSpec.NUMBER, precision);
+    }
+
+    /**
+     * Number type factory method.
+     *
+     * @return Number type.
+     */
+    public static NumberColumnType numberOf() {
+        return new NumberColumnType(ColumnTypeSpec.NUMBER, NumberColumnType.UNDEFINED);
+    }
+
     /**
      * Decimal type factory method.
      *
      * @param precision Precision.
      * @param scale Scale.
-     * @return Numeric type.
+     * @return Decimal type.
+     */
+    public static DecimalColumnType decimalOf(int precision, int scale) {
+        if (precision <= 0)
+            throw new IllegalArgumentException("Precision [" + precision + "] must be positive integer value.");

Review comment:
       Strictly positive or non-negative?




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679908891



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/NumericTypeUtils.java
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.
+ * */

Review comment:
       Fix javadocs for methods as well.




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r680792484



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/SchemaDescriptorConverter.java
##########
@@ -151,11 +160,13 @@ private static Serializable convertDefault(NativeType type, String dflt) {
             case DOUBLE:
                 return Double.parseDouble(dflt);
             case DECIMAL:
-                return new BigDecimal(dflt);
+                return new BigDecimal(dflt).setScale(((DecimalNativeType)type).scale(), RoundingMode.HALF_UP);

Review comment:
       I've created a ticket IGNITE-15217 to improve this.




-- 
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

[GitHub] [ignite-3] AMashenkov merged pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov merged pull request #243:
URL: https://github.com/apache/ignite-3/pull/243


   


-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679930593



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/SchemaDescriptorConverter.java
##########
@@ -151,11 +160,13 @@ private static Serializable convertDefault(NativeType type, String dflt) {
             case DOUBLE:
                 return Double.parseDouble(dflt);
             case DECIMAL:
-                return new BigDecimal(dflt);
+                return new BigDecimal(dflt).setScale(((DecimalNativeType)type).scale(), RoundingMode.HALF_UP);

Review comment:
       SQL standard does not specify whether number should be rounded or truncated.
   But we must mention this in documentation and ColumnType javadoc.




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679899587



##########
File path: modules/api/src/main/java/org/apache/ignite/schema/ColumnType.java
##########
@@ -206,6 +252,48 @@ public int scale() {
         }
     }
 
+    /**
+     * Number column type.
+     */
+    public static class NumberColumnType extends ColumnType {
+        /** Undefined precision. */
+        public static final int UNDEFINED = -1;

Review comment:
       ```suggestion
           private static final int UNDEFINED_PRECISION = -1;
   ```




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679920572



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/NumericTypeUtils.java
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.
+ * */

Review comment:
       I don't think method is ever useful.
   We use same size calculation in Row/Columns to calculate null-map size.
   May be it worth to add smth like next to the common IgniteUtils.
   `int bytesForBits(int bits)`




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r684220039



##########
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());

Review comment:
       Let's check if rows will have same binary representation.




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679935159



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/NumericTypeUtils.java
##########
@@ -0,0 +1,74 @@
+/*
+ * 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 {
+    /**
+     * Calculates byte size for BigInteger value.
+     * */
+    public static int sizeInBytes(BigInteger val) {
+        return val.bitLength() / 8 + 1;
+    }
+
+    /**
+     * Calculates byte size for BigDecimal value.
+     * */
+    public static int sizeInBytes(BigDecimal val, DecimalNativeType type) {

Review comment:
       Seems, this depends on protocol.




-- 
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

[GitHub] [ignite-3] vladErmakov07 commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

vladErmakov07 commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679966261



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/DecimalNativeType.java
##########
@@ -60,8 +60,7 @@ public int scale() {
     /** {@inheritDoc} */
     @Override public boolean mismatch(NativeType type) {
         return super.mismatch(type)
-            || precision < ((NumericNativeType)type).precision
-            || scale < ((NumericNativeType)type).scale;
+            || precision < ((DecimalNativeType)type).precision;

Review comment:
       No, I missed this case. Thanks, will think, how to fix it.




-- 
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

[GitHub] [ignite-3] AMashenkov commented on pull request #243: IGNITE-13668 first impl

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#issuecomment-885717215


   Let's add a test for TupleMarshallerImpl and check values of all the types can be serialized/deserialized.


-- 
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

[GitHub] [ignite-3] vladErmakov07 commented on a change in pull request #243: IGNITE-13668 first impl

Posted by GitBox <gi...@apache.org>.

vladErmakov07 commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r676756055



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/NativeTypes.java
##########
@@ -202,6 +219,12 @@ public static NativeType from(ColumnType type) {
                         ((ColumnType.VarLenColumnType)type).length() : Integer.MAX_VALUE
                 );
 
+            case NUMBER:
+                ColumnType.NumberColumnType numberType = (ColumnType.NumberColumnType)type;
+                if (numberType.precision() > 0)
+                    return new FixLenNumberNativeType(numberType.precision());
+                return new FixLenNumberNativeType(numberType.precision());

Review comment:
       I am sorry, request was not ready for review. Now it`s ready: I made many changes including fix for this 'if' condition.




-- 
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

[GitHub] [ignite-3] vladErmakov07 commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

vladErmakov07 commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r684088778



##########
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:
       Yes, we can use zero. I finally removed Utils class and inlined suggested expression.




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679932782



##########
File path: modules/api/src/main/java/org/apache/ignite/schema/ColumnType.java
##########
@@ -105,15 +105,55 @@ public static VarLenColumnType blobOf(int length) {
         return new VarLenColumnType(ColumnTypeSpec.BLOB, length);
     }
 
+    /**
+     * Number type factory method.
+     *
+     * @param precision Precision of value.
+     * @return Number type.
+     */
+    public static NumberColumnType numberOf(int precision) {
+        if (precision <= 0)
+            throw new IllegalArgumentException("Precision [" + precision + "] must be positive integer value.");
+
+        return new NumberColumnType(ColumnTypeSpec.NUMBER, precision);
+    }
+
+    /**
+     * Number type factory method.
+     *
+     * @return Number type.
+     */
+    public static NumberColumnType numberOf() {
+        return new NumberColumnType(ColumnTypeSpec.NUMBER, NumberColumnType.UNDEFINED);
+    }
+
     /**
      * Decimal type factory method.
      *
      * @param precision Precision.
      * @param scale Scale.
-     * @return Numeric type.
+     * @return Decimal type.
+     */
+    public static DecimalColumnType decimalOf(int precision, int scale) {
+        if (precision <= 0)
+            throw new IllegalArgumentException("Precision [" + precision + "] must be positive integer value.");

Review comment:
       SQL 92 says
   ```
            An exact numeric value has a precision and a scale. The precision
            is a positive integer .... The scale is a
            non-negative integer. A scale of 0 indicates that the number is an
            integer....
   ```




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679957790



##########
File path: modules/table/src/test/java/org/apache/ignite/internal/table/type/NumericTypesSerializerTest.java
##########
@@ -0,0 +1,203 @@
+/*
+ * 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.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.registry.SchemaRegistryImpl;
+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.util.Pair;
+import org.apache.ignite.schema.ColumnType;
+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();
+
+    /** Tuple marshaller. */
+    private TupleMarshaller marshaller;
+
+    /** 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))
+        );
+    }
+
+    @BeforeEach
+    public void setup() {
+        long seed = System.currentTimeMillis();
+
+        rnd = new Random(seed);
+
+        marshaller = new TupleMarshallerImpl(new SchemaRegistryImpl(v -> null) {

Review comment:
       We already have DummySchemaManagerImpl that can be reused.
   Maybe it make sense to rename it DummySchemaRegistry




-- 
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

[GitHub] [ignite-3] vladErmakov07 commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

vladErmakov07 commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r681816561



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/Row.java
##########
@@ -233,8 +235,55 @@ public Double doubleValueBoxed(int col) throws InvalidTypeException {
      * @throws InvalidTypeException If actual column type does not match the requested column type.
      */
     public BigDecimal decimalValue(int col) throws InvalidTypeException {
-        // TODO: IGNITE-13668 decimal support
-        return null;
+        long offLen = findColumn(col, NativeTypeSpec.DECIMAL);
+
+        if (offLen < 0)
+            return offLen == -1 ? null : (BigDecimal)rowSchema().column(col).defaultValue();
+
+        int off = offset(offLen);
+        int len = length(offLen);
+
+        DecimalNativeType type = (DecimalNativeType)schema.column(col).type();
+
+        int shift;
+        int scale;
+
+        if (type.scale() == 0) {
+            shift = 0;
+            scale = 0;
+        } else if (type.scale() < Byte.MAX_VALUE) {
+            shift = 1;
+            scale = readByte(off);
+        } else if (type.scale() < Short.MAX_VALUE) {
+            shift = 2;
+            scale = readShort(off);
+        } else {
+            shift = 4;
+            scale = readInteger(off);
+        }

Review comment:
       Since the values will be cast to user-defined scale, I decided not to store the original scale at all.

##########
File path: modules/schema/src/test/java/org/apache/ignite/internal/schema/RowTest.java
##########
@@ -421,6 +433,26 @@ else if (type == NativeTypeSpec.STRING) {
                         nonNullVarLenValSize += RowAssembler.utf8EncodedLength((CharSequence)vals[i]);
                     }
                 }
+                else if (type == NativeTypeSpec.NUMBER) {
+                    if (schema.isKeyColumn(i)) {
+                        nonNullVarLenKeyCols++;
+                        nonNullVarLenKeySize += NumericTypeUtils.sizeInBytes((BigInteger)vals[i]);

Review comment:
       Done




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r680791500



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/NumericTypeUtils.java
##########
@@ -0,0 +1,74 @@
+/*
+ * 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 {
+    /**
+     * Calculates byte size for BigInteger value.
+     * */
+    public static int sizeInBytes(BigInteger val) {
+        return val.bitLength() / 8 + 1;

Review comment:
       Got it!




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679899587



##########
File path: modules/api/src/main/java/org/apache/ignite/schema/ColumnType.java
##########
@@ -206,6 +252,48 @@ public int scale() {
         }
     }
 
+    /**
+     * Number column type.
+     */
+    public static class NumberColumnType extends ColumnType {
+        /** Undefined precision. */
+        public static final int UNDEFINED = -1;

Review comment:
       ```suggestion
           private static final int UNLIMITED_PRECISION = -1;
   ```




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679933887



##########
File path: modules/api/src/main/java/org/apache/ignite/schema/ColumnType.java
##########
@@ -105,15 +105,55 @@ public static VarLenColumnType blobOf(int length) {
         return new VarLenColumnType(ColumnTypeSpec.BLOB, length);
     }
 
+    /**
+     * Number type factory method.
+     *
+     * @param precision Precision of value.
+     * @return Number type.
+     */
+    public static NumberColumnType numberOf(int precision) {
+        if (precision <= 0)

Review comment:
       What does zero precision value mean?




-- 
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

[GitHub] [ignite-3] vladErmakov07 commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

vladErmakov07 commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r681818746



##########
File path: modules/table/src/test/java/org/apache/ignite/internal/table/type/NumericTypesSerializerTest.java
##########
@@ -0,0 +1,203 @@
+/*
+ * 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.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.registry.SchemaRegistryImpl;
+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.util.Pair;
+import org.apache.ignite.schema.ColumnType;
+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();
+
+    /** Tuple marshaller. */
+    private TupleMarshaller marshaller;
+
+    /** 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))
+        );
+    }
+
+    @BeforeEach
+    public void setup() {
+        long seed = System.currentTimeMillis();
+
+        rnd = new Random(seed);
+
+        marshaller = new TupleMarshallerImpl(new SchemaRegistryImpl(v -> null) {
+            @Override public SchemaDescriptor schema() {
+                return schema;
+            }
+
+            @Override public SchemaDescriptor schema(int ver) {
+                return schema;
+            }
+
+            @Override public int lastSchemaVersion() {
+                return schema.version();
+            }
+        });
+
+    }
+
+    /**
+     *
+     */
+    @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();
+
+        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(12, 3), false),
+            }
+        );
+
+        final TupleBuilderImpl badTup = new TupleBuilderImpl(schema);
+
+        badTup.set("key", rnd.nextLong());
+        assertThrows(InvalidTypeException.class, () -> badTup.set("decimalCol", new BigDecimal("123456789.0123")), "Column's type mismatch");
+        assertThrows(InvalidTypeException.class, () -> badTup.set("decimalCol", new BigDecimal("-1234567890123")), "Column's type mismatch");
+        assertThrows(InvalidTypeException.class, () -> badTup.set("decimalCol", new BigDecimal(123456789.0123d)), "Column's type mismatch");

Review comment:
       I added lot`s of cases with different bigDecimal representation, as well as yours.




-- 
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

[GitHub] [ignite-3] AMashenkov commented on a change in pull request #243: IGNITE-13668 Number native type support

Posted by GitBox <gi...@apache.org>.

AMashenkov commented on a change in pull request #243:
URL: https://github.com/apache/ignite-3/pull/243#discussion_r679899587



##########
File path: modules/api/src/main/java/org/apache/ignite/schema/ColumnType.java
##########
@@ -206,6 +252,48 @@ public int scale() {
         }
     }
 
+    /**
+     * Number column type.
+     */
+    public static class NumberColumnType extends ColumnType {
+        /** Undefined precision. */
+        public static final int UNDEFINED = -1;

Review comment:
       ```suggestion
           private static final int UNDEFINED = -1;
   ```




-- 
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