ptupitsyn commented on code in PR #7400:
URL: https://github.com/apache/ignite-3/pull/7400#discussion_r2689633751


##########
modules/api/src/main/java/org/apache/ignite/table/TupleImpl.java:
##########
@@ -459,4 +483,89 @@ private <T> T valueNotNull(String columnName) {
 
         return value;
     }
+
+    private static byte castToByte(Number number) {
+        Class<? extends Number> cls = number.getClass();
+
+        if (cls == Long.class || cls == Integer.class || cls == Short.class) {

Review Comment:
   Should we use `instanceof` instead of comparing classes?



##########
modules/api/src/main/java/org/apache/ignite/table/TupleImpl.java:
##########
@@ -459,4 +483,89 @@ private <T> T valueNotNull(String columnName) {
 
         return value;
     }
+
+    private static byte castToByte(Number number) {
+        Class<? extends Number> cls = number.getClass();
+
+        if (cls == Long.class || cls == Integer.class || cls == Short.class) {
+            long longVal = number.longValue();
+            byte byteVal = number.byteValue();
+
+            if (longVal == byteVal) {
+                return byteVal;
+            }
+
+            throw new ArithmeticException("Byte value overflow");
+        }
+
+        return (byte) number;
+    }
+
+    private static short castToShort(Number number) {
+        Class<? extends Number> cls = number.getClass();
+
+        if (cls == Long.class || cls == Integer.class || cls == Byte.class) {
+            long longVal = number.longValue();
+            short shortVal = number.shortValue();
+
+            if (longVal == shortVal) {
+                return shortVal;
+            }
+
+            throw new ArithmeticException("Short value overflow");
+        }
+
+        return (short) number;
+    }
+
+    private static int castToInt(Number number) {

Review Comment:
   There is `Math.toIntExact` which might be simpler and faster.



##########
modules/api/src/main/java/org/apache/ignite/table/TupleImpl.java:
##########
@@ -459,4 +483,89 @@ private <T> T valueNotNull(String columnName) {
 
         return value;
     }
+
+    private static byte castToByte(Number number) {
+        Class<? extends Number> cls = number.getClass();
+
+        if (cls == Long.class || cls == Integer.class || cls == Short.class) {
+            long longVal = number.longValue();
+            byte byteVal = number.byteValue();
+
+            if (longVal == byteVal) {
+                return byteVal;
+            }
+
+            throw new ArithmeticException("Byte value overflow");

Review Comment:
   Let's add the value to the exception text:
   
   ```suggestion
               throw new ArithmeticException("Byte value overflow: " + longVal);
   ```



##########
modules/api/src/main/java/org/apache/ignite/table/TupleImpl.java:
##########
@@ -459,4 +483,89 @@ private <T> T valueNotNull(String columnName) {
 
         return value;
     }
+
+    private static byte castToByte(Number number) {
+        Class<? extends Number> cls = number.getClass();
+
+        if (cls == Long.class || cls == Integer.class || cls == Short.class) {
+            long longVal = number.longValue();
+            byte byteVal = number.byteValue();
+
+            if (longVal == byteVal) {
+                return byteVal;
+            }
+
+            throw new ArithmeticException("Byte value overflow");
+        }
+
+        return (byte) number;
+    }
+
+    private static short castToShort(Number number) {
+        Class<? extends Number> cls = number.getClass();
+
+        if (cls == Long.class || cls == Integer.class || cls == Byte.class) {
+            long longVal = number.longValue();
+            short shortVal = number.shortValue();
+
+            if (longVal == shortVal) {
+                return shortVal;
+            }
+
+            throw new ArithmeticException("Short value overflow");
+        }
+
+        return (short) number;
+    }
+
+    private static int castToInt(Number number) {
+        Class<? extends Number> cls = number.getClass();
+
+        if (cls == Long.class || cls == Short.class || cls == Byte.class) {
+            long longVal = number.longValue();
+            int intVal = number.intValue();
+
+            if (longVal == intVal) {
+                return intVal;
+            }
+
+            throw new ArithmeticException("Int value overflow");
+        }
+
+        return (int) number;
+    }
+
+    private static long castToLong(Number number) {
+        Class<? extends Number> cls = number.getClass();
+
+        if (cls == Integer.class || cls == Short.class || cls == Byte.class) {
+            return number.longValue();
+        }
+
+        return (long) number;

Review Comment:
   ```suggestion
           return number.longValue();
   ```
   
   Isn't this enough?



##########
modules/api/src/main/java/org/apache/ignite/table/TupleImpl.java:
##########
@@ -459,4 +483,89 @@ private <T> T valueNotNull(String columnName) {
 
         return value;
     }
+
+    private static byte castToByte(Number number) {
+        Class<? extends Number> cls = number.getClass();

Review Comment:
   Let's add a fast path for Byte to the very top of the method - likely the 
most common case
   
   ```
       if (number instanceof Byte b) {
           return b;
       }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to