PR comments.

enso-org · Feb 14, 2025 · ac4672e · ac4672e
1 parent f5ddd97
commit ac4672e
Show file tree

Hide file tree

Showing 6 changed files with 98 additions and 69 deletions.
diff --git a/std-bits/table/src/main/java/org/enso/table/data/column/operation/StorageIterators.java b/std-bits/table/src/main/java/org/enso/table/data/column/operation/StorageIterators.java
@@ -191,7 +191,7 @@ private static <B extends BuilderForType<T>, T> ColumnStorage<T> buildOverLongAr
       LongBuildOperation<B> operation) {
     var data = source.getArray();
     Context context = Context.getCurrent();
-    assert source.getSize() < Integer.MAX_VALUE;
+    assert source.getSize() <= Integer.MAX_VALUE;
     for (int i = 0; i < source.getSize(); i++) {
       boolean isNothing = source.isNothing(i);
       if (preserveNothing && isNothing) {
@@ -265,7 +265,7 @@ private static <B extends BuilderForType<T>, T> ColumnStorage<T> buildOverDouble
       DoubleBuildOperation<B> operation) {
     var data = source.getArray();
     Context context = Context.getCurrent();
-    assert source.getSize() < Integer.MAX_VALUE;
+    assert source.getSize() <= Integer.MAX_VALUE;
     for (int i = 0; i < source.getSize(); i++) {
       boolean isNothing = source.isNothing(i);
       if (preserveNothing && isNothing) {
@@ -455,7 +455,7 @@ private static <T> ColumnStorage<T> mapOverLongArrayStorage(
       LongMapOperation<T> operation) {
     var data = source.getArray();
     Context context = Context.getCurrent();
-    assert source.getSize() < Integer.MAX_VALUE;
+    assert source.getSize() <= Integer.MAX_VALUE;
     for (int index = 0; index < source.getSize(); index++) {
       if (preserveNothing && source.isNothing(index)) {
         builder.appendNulls(1);
@@ -526,7 +526,7 @@ private static <T> ColumnStorage<T> mapOverDoubleArrayStorage(
       DoubleMapOperation<T> operation) {
     var data = source.getArray();
     Context context = Context.getCurrent();
-    assert source.getSize() < Integer.MAX_VALUE;
+    assert source.getSize() <= Integer.MAX_VALUE;
     for (int index = 0; index < source.getSize(); index++) {
       if (preserveNothing && source.isNothing(index)) {
         builder.appendNulls(1);
@@ -713,7 +713,7 @@ private static <T> ColumnStorage<T> zipOverLongArrayStorages(
     long size2 = source2.getSize();
 
     long size = Math.max(size1, size2);
-    assert size < Integer.MAX_VALUE;
+    assert size <= Integer.MAX_VALUE;
     var builder = builderConstructor.apply(size);
 
     Context context = Context.getCurrent();
@@ -800,7 +800,7 @@ private static <T> ColumnStorage<T> zipOverDoubleArrayStorages(
     long size2 = source2.getSize();
 
     long size = Math.max(size1, size2);
-    assert size < Integer.MAX_VALUE;
+    assert size <= Integer.MAX_VALUE;
     var builder = builderConstructor.apply(size);
 
     Context context = Context.getCurrent();

diff --git a/...nso/table/data/column/operation/map/numeric/arithmetic/NumericBinaryOpImplementation.java b/...nso/table/data/column/operation/map/numeric/arithmetic/NumericBinaryOpImplementation.java
@@ -7,7 +7,11 @@
 import org.enso.table.data.column.operation.StorageIterators;
 import org.enso.table.data.column.operation.map.BinaryMapOperation;
 import org.enso.table.data.column.operation.map.MapOperationProblemAggregator;
-import org.enso.table.data.column.storage.*;
+import org.enso.table.data.column.storage.ColumnDoubleStorage;
+import org.enso.table.data.column.storage.ColumnLongStorage;
+import org.enso.table.data.column.storage.ColumnStorage;
+import org.enso.table.data.column.storage.ColumnStorageFacade;
+import org.enso.table.data.column.storage.Storage;
 import org.enso.table.data.column.storage.numeric.BigDecimalStorage;
 import org.enso.table.data.column.storage.numeric.BigIntegerStorage;
 import org.enso.table.data.column.storage.numeric.DoubleStorage;
@@ -20,6 +24,21 @@
 /** An operation expecting a numeric argument and returning a numeric column. */
 public abstract class NumericBinaryOpImplementation<T extends Number, I extends Storage<? super T>>
     extends BinaryMapOperation<T, I> {
+  public static ColumnStorage<BigDecimal> asBigDecimal(ColumnStorage<BigInteger> storage) {
+    return new ColumnStorageFacade<>(storage, BigDecimal::new);
+  }
+
+  public static ColumnStorage<BigDecimal> asBigDecimal(ColumnDoubleStorage storage) {
+    return new ColumnStorageFacade<>(storage, BigDecimal::valueOf);
+  }
+
+  public static ColumnStorage<BigDecimal> asBigDecimal(ColumnLongStorage storage) {
+    return new ColumnStorageFacade<>(storage, BigDecimal::valueOf);
+  }
+
+  public static ColumnStorage<BigInteger> asBigInteger(ColumnLongStorage storage) {
+    return new ColumnStorageFacade<>(storage, BigInteger::valueOf);
+  }
 
   protected abstract double doDouble(
       double a, double b, long ix, MapOperationProblemAggregator problemAggregator);
@@ -76,20 +95,17 @@ public Storage<?> runBinaryMap(
         case BigDecimalStorage s -> runBigDecimalMap(
             s, BigDecimal.valueOf(argAsDouble), problemAggregator);
         case BigIntegerStorage s -> runDoubleMap(
-            new DoubleStorageFacade<>(s, BigInteger::doubleValue), argAsDouble, problemAggregator);
+            DoubleStorageFacade.forBigInteger(s), argAsDouble, problemAggregator);
         case ColumnDoubleStorage s -> runDoubleMap(s, argAsDouble, problemAggregator);
         case ColumnLongStorage s -> runDoubleLongMap(s, argAsDouble, problemAggregator);
         default -> throw newUnsupported(storage);
       };
     } else if (arg instanceof BigDecimal bd) {
       return switch (storage) {
         case BigDecimalStorage s -> runBigDecimalMap(s, bd, problemAggregator);
-        case BigIntegerStorage s -> runBigDecimalMap(
-            new ColumnStorageFacade<>(s, BigDecimal::new), bd, problemAggregator);
-        case ColumnDoubleStorage s -> runBigDecimalMap(
-            new ColumnStorageFacade<>(s, BigDecimal::valueOf), bd, problemAggregator);
-        case ColumnLongStorage s -> runBigDecimalMap(
-            new ColumnStorageFacade<>(s, BigDecimal::valueOf), bd, problemAggregator);
+        case BigIntegerStorage s -> runBigDecimalMap(asBigDecimal(s), bd, problemAggregator);
+        case ColumnDoubleStorage s -> runBigDecimalMap(asBigDecimal(s), bd, problemAggregator);
+        case ColumnLongStorage s -> runBigDecimalMap(asBigDecimal(s), bd, problemAggregator);
         default -> throw newUnsupported(storage);
       };
     } else {
@@ -102,46 +118,38 @@ public Storage<? extends Number> runZip(
       I storage, Storage<?> arg, MapOperationProblemAggregator problemAggregator) {
     if (storage instanceof ColumnDoubleStorage lhs) {
       return switch (arg) {
-        case BigDecimalStorage rhs -> runBigDecimalZip(
-            new ColumnStorageFacade<>(lhs, BigDecimal::valueOf), rhs, problemAggregator);
+        case BigDecimalStorage rhs -> runBigDecimalZip(asBigDecimal(lhs), rhs, problemAggregator);
         case BigIntegerStorage rhs -> runDoubleZip(
-            lhs, new DoubleStorageFacade<>(rhs, BigInteger::doubleValue), problemAggregator);
+            lhs, DoubleStorageFacade.forBigInteger(rhs), problemAggregator);
         case ColumnDoubleStorage rhs -> runDoubleZip(lhs, rhs, problemAggregator);
         case ColumnLongStorage rhs -> runDoubleZip(
-            lhs, new DoubleStorageFacade<>(rhs, Long::doubleValue), problemAggregator);
+            lhs, DoubleStorageFacade.forLong(rhs), problemAggregator);
         default -> throw newUnsupported(arg);
       };
     } else if (storage instanceof ColumnLongStorage lhs) {
       return switch (arg) {
-        case BigDecimalStorage rhs -> runBigDecimalZip(
-            new ColumnStorageFacade<>(lhs, BigDecimal::valueOf), rhs, problemAggregator);
-        case BigIntegerStorage rhs -> runBigIntegerZip(
-            new ColumnStorageFacade<>(lhs, BigInteger::valueOf), rhs, problemAggregator);
+        case BigDecimalStorage rhs -> runBigDecimalZip(asBigDecimal(lhs), rhs, problemAggregator);
+        case BigIntegerStorage rhs -> runBigIntegerZip(asBigInteger(lhs), rhs, problemAggregator);
         case ColumnDoubleStorage rhs -> runDoubleZip(
-            new DoubleStorageFacade<>(lhs, Long::doubleValue), rhs, problemAggregator);
+            DoubleStorageFacade.forLong(lhs), rhs, problemAggregator);
         case ColumnLongStorage rhs -> runLongZip(lhs, rhs, problemAggregator);
         default -> throw newUnsupported(arg);
       };
     } else if (storage instanceof BigIntegerStorage lhs) {
       return switch (arg) {
-        case BigDecimalStorage rhs -> runBigDecimalZip(
-            new ColumnStorageFacade<>(lhs, BigDecimal::new), rhs, problemAggregator);
+        case BigDecimalStorage rhs -> runBigDecimalZip(asBigDecimal(lhs), rhs, problemAggregator);
         case BigIntegerStorage rhs -> runBigIntegerZip(lhs, rhs, problemAggregator);
         case ColumnDoubleStorage rhs -> runDoubleZip(
-            new DoubleStorageFacade<>(lhs, BigInteger::doubleValue), rhs, problemAggregator);
-        case ColumnLongStorage rhs -> runBigIntegerZip(
-            lhs, new ColumnStorageFacade<>(rhs, BigInteger::valueOf), problemAggregator);
+            DoubleStorageFacade.forBigInteger(lhs), rhs, problemAggregator);
+        case ColumnLongStorage rhs -> runBigIntegerZip(lhs, asBigInteger(rhs), problemAggregator);
         default -> throw newUnsupported(arg);
       };
     } else if (storage instanceof BigDecimalStorage lhs) {
       return switch (arg) {
         case BigDecimalStorage rhs -> runBigDecimalZip(lhs, rhs, problemAggregator);
-        case BigIntegerStorage rhs -> runBigDecimalZip(
-            lhs, new ColumnStorageFacade<>(rhs, BigDecimal::new), problemAggregator);
-        case ColumnDoubleStorage rhs -> runBigDecimalZip(
-            lhs, new ColumnStorageFacade<>(rhs, BigDecimal::valueOf), problemAggregator);
-        case ColumnLongStorage rhs -> runBigDecimalZip(
-            lhs, new ColumnStorageFacade<>(rhs, BigDecimal::valueOf), problemAggregator);
+        case BigIntegerStorage rhs -> runBigDecimalZip(lhs, asBigDecimal(rhs), problemAggregator);
+        case ColumnDoubleStorage rhs -> runBigDecimalZip(lhs, asBigDecimal(rhs), problemAggregator);
+        case ColumnLongStorage rhs -> runBigDecimalZip(lhs, asBigDecimal(rhs), problemAggregator);
         default -> throw newUnsupported(arg);
       };
     } else {
@@ -155,8 +163,7 @@ private static Storage<? extends Number> allNullStorageOfSameType(Storage<?> sto
       case BigIntegerStorage s -> BigIntegerStorage.makeEmpty(storage.getSize());
       case BigDecimalStorage s -> BigDecimalStorage.makeEmpty(storage.getSize());
       case ColumnDoubleStorage s -> DoubleStorage.makeEmpty(storage.getSize());
-      default -> throw new IllegalStateException(
-          "Unsupported storage: " + storage.getClass().getCanonicalName());
+      default -> throw newUnsupported(storage);
     };
   }
 

diff --git a/...able/data/column/operation/map/numeric/arithmetic/NumericBinaryOpReturningBigDecimal.java b/...able/data/column/operation/map/numeric/arithmetic/NumericBinaryOpReturningBigDecimal.java
@@ -10,6 +10,8 @@
 import org.enso.table.data.column.storage.numeric.BigDecimalStorage;
 import org.enso.table.data.column.storage.numeric.BigIntegerStorage;
 
+import static org.enso.table.data.column.operation.map.numeric.arithmetic.NumericBinaryOpImplementation.asBigDecimal;
+
 public abstract class NumericBinaryOpReturningBigDecimal<
         T extends Number, I extends Storage<? super T>>
     extends BinaryMapOperation<T, I> {
@@ -19,10 +21,10 @@ public NumericBinaryOpReturningBigDecimal(String name) {
 
   private static ColumnStorage<BigDecimal> asBigDecimalStorage(Storage<?> storage) {
     return switch (storage) {
-      case ColumnDoubleStorage s -> new ColumnStorageFacade<>(s, BigDecimal::valueOf);
-      case ColumnLongStorage s -> new ColumnStorageFacade<>(s, BigDecimal::valueOf);
+      case ColumnDoubleStorage s -> asBigDecimal(s);
+      case ColumnLongStorage s -> asBigDecimal(s);
       case BigDecimalStorage s -> s;
-      case BigIntegerStorage s -> new ColumnStorageFacade<>(s, BigDecimal::new);
+      case BigIntegerStorage s -> asBigDecimal(s);
       default -> throw NumericBinaryOpImplementation.newUnsupported(storage);
     };
   }

diff --git a/...so/table/data/column/operation/map/numeric/arithmetic/NumericBinaryOpReturningDouble.java b/...so/table/data/column/operation/map/numeric/arithmetic/NumericBinaryOpReturningDouble.java
@@ -27,9 +27,9 @@ public NumericBinaryOpReturningDouble(String name) {
   private static ColumnDoubleStorage asDoubleStorage(Storage<?> storage) {
     return switch (storage) {
       case ColumnDoubleStorage s -> s;
-      case ColumnLongStorage s -> new DoubleStorageFacade<>(s, Long::doubleValue);
-      case BigDecimalStorage s -> new DoubleStorageFacade<>(s, BigDecimal::doubleValue);
-      case BigIntegerStorage s -> new DoubleStorageFacade<>(s, BigInteger::doubleValue);
+      case ColumnLongStorage s -> DoubleStorageFacade.forLong(s);
+      case BigDecimalStorage s -> DoubleStorageFacade.forBigDecimal(s);
+      case BigIntegerStorage s -> DoubleStorageFacade.forBigInteger(s);
       default -> throw NumericBinaryOpImplementation.newUnsupported(storage);
     };
   }

diff --git a/.../java/org/enso/table/data/column/operation/map/numeric/comparisons/NumericComparison.java b/.../java/org/enso/table/data/column/operation/map/numeric/comparisons/NumericComparison.java
@@ -18,6 +18,9 @@
 import org.enso.table.data.column.storage.numeric.BigIntegerStorage;
 import org.enso.table.data.column.storage.numeric.DoubleStorageFacade;
 
+import static org.enso.table.data.column.operation.map.numeric.arithmetic.NumericBinaryOpImplementation.asBigDecimal;
+import static org.enso.table.data.column.operation.map.numeric.arithmetic.NumericBinaryOpImplementation.asBigInteger;
+
 public abstract class NumericComparison<T extends Number, I extends Storage<? super T>>
     extends BinaryMapOperation<T, I> {
 
@@ -60,12 +63,9 @@ public Storage<Boolean> runBinaryMap(
     } else if (arg instanceof BigDecimal bigDecimal) {
       return switch (storage) {
         case BigDecimalStorage s -> runBigDecimalMap(s, bigDecimal, problemAggregator);
-        case BigIntegerStorage s -> runBigDecimalMap(
-            new ColumnStorageFacade<>(s, BigDecimal::new), bigDecimal, problemAggregator);
-        case ColumnDoubleStorage s -> runBigDecimalMap(
-            new ColumnStorageFacade<>(s, BigDecimal::valueOf), bigDecimal, problemAggregator);
-        case ColumnLongStorage s -> runBigDecimalMap(
-            new ColumnStorageFacade<>(s, BigDecimal::valueOf), bigDecimal, problemAggregator);
+        case BigIntegerStorage s -> runBigDecimalMap(asBigDecimal(s), bigDecimal, problemAggregator);
+        case ColumnDoubleStorage s -> runBigDecimalMap(asBigDecimal(s), bigDecimal, problemAggregator);
+        case ColumnLongStorage s -> runBigDecimalMap(asBigDecimal(s), bigDecimal, problemAggregator);
         default -> throw newUnsupported(storage);
       };
     } else if (NumericConverter.isCoercibleToLong(arg)) {
@@ -85,7 +85,7 @@ public Storage<Boolean> runBinaryMap(
         case BigDecimalStorage s -> runBigDecimalMap(
             s, BigDecimal.valueOf(argAsDouble), problemAggregator);
         case BigIntegerStorage s -> runDoubleMap(
-            new DoubleStorageFacade<>(s, BigInteger::doubleValue), argAsDouble, problemAggregator);
+            DoubleStorageFacade.forBigInteger(s), argAsDouble, problemAggregator);
         case ColumnDoubleStorage s -> runDoubleMap(s, argAsDouble, problemAggregator);
         case ColumnLongStorage s -> runDoubleLongMap(s, argAsDouble, problemAggregator);
         default -> throw newUnsupported(storage);
@@ -172,46 +172,38 @@ public Storage<Boolean> runZip(
       I storage, Storage<?> arg, MapOperationProblemAggregator problemAggregator) {
     if (storage instanceof ColumnDoubleStorage lhs) {
       return switch (arg) {
-        case BigDecimalStorage rhs -> runBigDecimalZip(
-            new ColumnStorageFacade<>(lhs, BigDecimal::valueOf), rhs, problemAggregator);
+        case BigDecimalStorage rhs -> runBigDecimalZip(asBigDecimal(lhs), rhs, problemAggregator);
         case BigIntegerStorage rhs -> runDoubleZip(
-            lhs, new DoubleStorageFacade<>(rhs, BigInteger::doubleValue), problemAggregator);
+            lhs, DoubleStorageFacade.forBigInteger(rhs), problemAggregator);
         case ColumnDoubleStorage rhs -> runDoubleZip(lhs, rhs, problemAggregator);
         case ColumnLongStorage rhs -> runDoubleZip(
-            lhs, new DoubleStorageFacade<>(rhs, Long::doubleValue), problemAggregator);
+            lhs, DoubleStorageFacade.forLong(rhs), problemAggregator);
         default -> runMixedZip(storage, arg, problemAggregator);
       };
     } else if (storage instanceof ColumnLongStorage lhs) {
       return switch (arg) {
-        case BigDecimalStorage rhs -> runBigDecimalZip(
-            new ColumnStorageFacade<>(lhs, BigDecimal::valueOf), rhs, problemAggregator);
-        case BigIntegerStorage rhs -> runBigIntegerZip(
-            new ColumnStorageFacade<>(lhs, BigInteger::valueOf), rhs, problemAggregator);
+        case BigDecimalStorage rhs -> runBigDecimalZip(asBigDecimal(lhs), rhs, problemAggregator);
+        case BigIntegerStorage rhs -> runBigIntegerZip(asBigInteger(lhs), rhs, problemAggregator);
         case ColumnDoubleStorage rhs -> runDoubleZip(
-            new DoubleStorageFacade<>(lhs, Long::doubleValue), rhs, problemAggregator);
+            DoubleStorageFacade.forLong(lhs), rhs, problemAggregator);
         case ColumnLongStorage rhs -> runLongZip(lhs, rhs, problemAggregator);
         default -> runMixedZip(storage, arg, problemAggregator);
       };
     } else if (storage instanceof BigIntegerStorage lhs) {
       return switch (arg) {
-        case BigDecimalStorage rhs -> runBigDecimalZip(
-            new ColumnStorageFacade<>(lhs, BigDecimal::new), rhs, problemAggregator);
+        case BigDecimalStorage rhs -> runBigDecimalZip(asBigDecimal(lhs), rhs, problemAggregator);
         case BigIntegerStorage rhs -> runBigIntegerZip(lhs, rhs, problemAggregator);
         case ColumnDoubleStorage rhs -> runDoubleZip(
-            new DoubleStorageFacade<>(lhs, BigInteger::doubleValue), rhs, problemAggregator);
-        case ColumnLongStorage rhs -> runBigIntegerZip(
-            lhs, new ColumnStorageFacade<>(rhs, BigInteger::valueOf), problemAggregator);
+            DoubleStorageFacade.forBigInteger(lhs), rhs, problemAggregator);
+        case ColumnLongStorage rhs -> runBigIntegerZip(lhs, asBigInteger(rhs), problemAggregator);
         default -> runMixedZip(storage, arg, problemAggregator);
       };
     } else if (storage instanceof BigDecimalStorage lhs) {
       return switch (arg) {
         case BigDecimalStorage rhs -> runBigDecimalZip(lhs, rhs, problemAggregator);
-        case BigIntegerStorage rhs -> runBigDecimalZip(
-            lhs, new ColumnStorageFacade<>(rhs, BigDecimal::new), problemAggregator);
-        case ColumnDoubleStorage rhs -> runBigDecimalZip(
-            lhs, new ColumnStorageFacade<>(rhs, BigDecimal::valueOf), problemAggregator);
-        case ColumnLongStorage rhs -> runBigDecimalZip(
-            lhs, new ColumnStorageFacade<>(rhs, BigDecimal::valueOf), problemAggregator);
+        case BigIntegerStorage rhs -> runBigDecimalZip(lhs, asBigDecimal(rhs), problemAggregator);
+        case ColumnDoubleStorage rhs -> runBigDecimalZip(lhs, asBigDecimal(rhs), problemAggregator);
+        case ColumnLongStorage rhs -> runBigDecimalZip(lhs, asBigDecimal(rhs), problemAggregator);
         default -> runMixedZip(storage, arg, problemAggregator);
       };
     } else {

diff --git a/...s/table/src/main/java/org/enso/table/data/column/storage/numeric/DoubleStorageFacade.java b/...s/table/src/main/java/org/enso/table/data/column/storage/numeric/DoubleStorageFacade.java
@@ -1,7 +1,10 @@
 package org.enso.table.data.column.storage.numeric;
 
+import java.math.BigDecimal;
+import java.math.BigInteger;
 import java.util.function.ToDoubleFunction;
 import org.enso.table.data.column.storage.ColumnDoubleStorage;
+import org.enso.table.data.column.storage.ColumnLongStorage;
 import org.enso.table.data.column.storage.ColumnStorage;
 import org.enso.table.data.column.storage.ValueIsNothingException;
 import org.enso.table.data.column.storage.type.FloatType;
@@ -17,6 +20,31 @@ public DoubleStorageFacade(ColumnStorage<T> parent, ToDoubleFunction<T> converte
     this.converter = converter;
   }
 
+  public static ColumnDoubleStorage forLong(ColumnLongStorage parent) {
+    return new DoubleStorageFacade<>(parent, Long::doubleValue) {
+      @Override
+      public double getItemAsDouble(long index) throws ValueIsNothingException {
+        return (double)parent.getItemAsLong(index);
+      }
+
+      @Override
+      public Double getItemBoxed(long index) {
+        if (isNothing(index)) {
+          return null;
+        }
+        return getItemAsDouble(index);
+      }
+    };
+  }
+
+  public static ColumnDoubleStorage forBigInteger(ColumnStorage<BigInteger> parent) {
+    return new DoubleStorageFacade<>(parent, BigInteger::doubleValue);
+  }
+
+  public static ColumnDoubleStorage forBigDecimal(ColumnStorage<BigDecimal> parent) {
+    return new DoubleStorageFacade<>(parent, BigDecimal::doubleValue);
+  }
+
   @Override
   public double getItemAsDouble(long index) throws ValueIsNothingException {
     if (isNothing(index)) {