remove wrappers

LakshSingla · Dec 14, 2023 · c13551e · c13551e
1 parent 0c5b719
commit c13551e
Show file tree

Hide file tree

Showing 20 changed files with 256 additions and 784 deletions.
diff --git a/processing/src/main/java/org/apache/druid/frame/write/FrameWriterUtils.java b/processing/src/main/java/org/apache/druid/frame/write/FrameWriterUtils.java
@@ -34,8 +34,6 @@
 import org.apache.druid.segment.column.ColumnType;
 import org.apache.druid.segment.column.RowSignature;
 import org.apache.druid.segment.data.ComparableIntArray;
-import org.apache.druid.segment.data.ComparableList;
-import org.apache.druid.segment.data.ComparableStringArray;
 import org.apache.druid.segment.data.IndexedInts;
 
 import javax.annotation.Nullable;
@@ -164,10 +162,6 @@ public static List<ByteBuffer> getUtf8ByteBuffersFromStringArraySelector(
       for (Object value : (Object[]) row) {
         retVal.add(getUtf8ByteBufferFromString((String) value));
       }
-    } else if (row instanceof ComparableStringArray) {
-      for (String value : ((ComparableStringArray) row).getDelegate()) {
-        retVal.add(getUtf8ByteBufferFromString(value));
-      }
     } else {
       throw new ISE("Unexpected type %s found", row.getClass().getName());
     }
@@ -201,10 +195,6 @@ public static List<? extends Number> getNumericArrayFromObject(Object row)
       for (Object value : (Object[]) row) {
         retVal.add((Number) value);
       }
-    } else if (row instanceof ComparableList) {
-      for (Object value : ((ComparableList) row).getDelegate()) {
-        retVal.add((Number) value);
-      }
     } else if (row instanceof ComparableIntArray) {
       for (int value : ((ComparableIntArray) row).getDelegate()) {
         retVal.add(value);

diff --git a/...ache/druid/query/groupby/epinephelinae/column/ArrayLongGroupByColumnSelectorStrategy.java b/...ache/druid/query/groupby/epinephelinae/column/ArrayLongGroupByColumnSelectorStrategy.java
@@ -24,10 +24,12 @@
 import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap;
 import org.apache.druid.java.util.common.ISE;
 import org.apache.druid.segment.ColumnValueSelector;
+import org.apache.druid.segment.DimensionHandlerUtils;
 import org.apache.druid.segment.column.ValueType;
 
 import java.util.Arrays;
 import java.util.List;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 
 public class ArrayLongGroupByColumnSelectorStrategy extends ArrayNumericGroupByColumnSelectorStrategy<Long>

diff --git a/...e/druid/query/groupby/epinephelinae/column/ArrayNumericGroupByColumnSelectorStrategy.java b/...e/druid/query/groupby/epinephelinae/column/ArrayNumericGroupByColumnSelectorStrategy.java
@@ -28,7 +28,6 @@
 import org.apache.druid.query.ordering.StringComparator;
 import org.apache.druid.query.ordering.StringComparators;
 import org.apache.druid.segment.ColumnValueSelector;
-import org.apache.druid.segment.data.ComparableList;
 
 import javax.annotation.Nullable;
 import java.nio.ByteBuffer;
@@ -39,6 +38,7 @@ public abstract class ArrayNumericGroupByColumnSelectorStrategy<T>
 {
   protected static final int GROUP_BY_MISSING_VALUE = -1;
 
+  // TODO(laksh): Keep the dictionary types as List<T> instead of Object[] to allow for equality comparisons
   protected final List<List<T>> dictionary;
   protected final Object2IntMap<List<T>> reverseDictionary;
   protected long estimatedFootprint = 0L;
@@ -83,7 +83,7 @@ public void processValueFromGroupingKey(
     // GROUP_BY_MISSING_VALUE is used to indicate empty rows, which are omitted from the result map.
     if (id != GROUP_BY_MISSING_VALUE) {
       final List<T> value = dictionary.get(id);
-      resultRow.set(selectorPlus.getResultRowPosition(), new ComparableList(value));
+      resultRow.set(selectorPlus.getResultRowPosition(), value);
     } else {
       resultRow.set(selectorPlus.getResultRowPosition(), null);
     }

diff --git a/...he/druid/query/groupby/epinephelinae/column/ArrayStringGroupByColumnSelectorStrategy.java b/...he/druid/query/groupby/epinephelinae/column/ArrayStringGroupByColumnSelectorStrategy.java
@@ -31,7 +31,6 @@
 import org.apache.druid.segment.ColumnValueSelector;
 import org.apache.druid.segment.column.ValueType;
 import org.apache.druid.segment.data.ComparableIntArray;
-import org.apache.druid.segment.data.ComparableStringArray;
 
 import javax.annotation.Nullable;
 import java.nio.ByteBuffer;
@@ -90,11 +89,11 @@ public void processValueFromGroupingKey(
       final int[] intRepresentation = intListToInt.inverse()
                                                   .get(id)
                                                   .getDelegate();
-      final String[] stringRepresentaion = new String[intRepresentation.length];
+      final Object[] stringRepresentaion = new Object[intRepresentation.length];
       for (int i = 0; i < intRepresentation.length; i++) {
         stringRepresentaion[i] = dictionaryToInt.inverse().get(intRepresentation[i]);
       }
-      resultRow.set(selectorPlus.getResultRowPosition(), ComparableStringArray.of(stringRepresentaion));
+      resultRow.set(selectorPlus.getResultRowPosition(), stringRepresentaion);
     } else {
       resultRow.set(selectorPlus.getResultRowPosition(), null);
     }

diff --git a/processing/src/main/java/org/apache/druid/segment/DimensionHandlerUtils.java b/processing/src/main/java/org/apache/druid/segment/DimensionHandlerUtils.java
@@ -40,12 +40,11 @@
 import org.apache.druid.segment.column.ColumnType;
 import org.apache.druid.segment.column.TypeSignature;
 import org.apache.druid.segment.column.ValueType;
-import org.apache.druid.segment.data.ComparableList;
-import org.apache.druid.segment.data.ComparableStringArray;
 
 import javax.annotation.Nullable;
 import java.math.BigDecimal;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Objects;
 import java.util.concurrent.ConcurrentHashMap;
@@ -300,6 +299,9 @@ public static String convertObjectToString(@Nullable Object valObj)
   {
     if (valObj == null) {
       return null;
+    } else if (valObj instanceof Object[]) {
+      // TODO(laksh): Get this change vetted
+      return Arrays.toString((Object[]) valObj);
     }
     return valObj.toString();
   }
@@ -407,11 +409,11 @@ public static Object convertObjectToType(
           case STRING:
             return coerceToStringArray(obj);
           case LONG:
-            return coerceToObjectWithCoercionFunction(obj, DimensionHandlerUtils::convertObjectToLong);
+            return coerceToObjectArrayWithElementCoercionFunction(obj, DimensionHandlerUtils::convertObjectToLong);
           case FLOAT:
-            return coerceToObjectWithCoercionFunction(obj, DimensionHandlerUtils::convertObjectToFloat);
+            return coerceToObjectArrayWithElementCoercionFunction(obj, DimensionHandlerUtils::convertObjectToFloat);
           case DOUBLE:
-            return coerceToObjectWithCoercionFunction(obj, DimensionHandlerUtils::convertObjectToDouble);
+            return coerceToObjectArrayWithElementCoercionFunction(obj, DimensionHandlerUtils::convertObjectToDouble);
         }
 
       default:
@@ -424,21 +426,23 @@ public static Object[] convertToList(Object obj, ValueType elementType)
   {
     switch (elementType) {
       case LONG:
-        return coerceToObjectWithCoercionFunction(obj, DimensionHandlerUtils::convertObjectToLong);
+        return coerceToObjectArrayWithElementCoercionFunction(obj, DimensionHandlerUtils::convertObjectToLong);
       case FLOAT:
-        return coerceToObjectWithCoercionFunction(obj, DimensionHandlerUtils::convertObjectToFloat);
+        return coerceToObjectArrayWithElementCoercionFunction(obj, DimensionHandlerUtils::convertObjectToFloat);
       case DOUBLE:
-        return coerceToObjectWithCoercionFunction(obj, DimensionHandlerUtils::convertObjectToDouble);
+        return coerceToObjectArrayWithElementCoercionFunction(obj, DimensionHandlerUtils::convertObjectToDouble);
     }
     throw new ISE(
-        "Unable to convert object of type[%s] to [%s]",
-        obj.getClass().getName(),
-        ComparableList.class.getName()
+        "Unable to convert object of type[%s] to Object[]",
+        obj.getClass().getName()
     );
   }
 
 
-  private static Object[] coerceToObjectWithCoercionFunction(Object obj, Function<Object, Object> coercionFunction)
+  public static Object[] coerceToObjectArrayWithElementCoercionFunction(
+      Object obj,
+      Function<Object, Object> coercionFunction
+  )
   {
     if (obj == null) {
       return null;
@@ -455,22 +459,21 @@ private static Object[] coerceToObjectWithCoercionFunction(Object obj, Function<
       Object[] objects = (Object[]) obj;
       Object[] retVal = new Object[objects.length];
       for (int i = 0; i < objects.length; i++) {
-        retVal[i] = convertObjectToString(objects[i]);
+        retVal[i] = coercionFunction.apply(objects[i]);
       }
       return retVal;
     }
     throw new ISE(
-        "Unable to convert object of type[%s] to [%s]",
-        obj.getClass().getName(),
-        ComparableStringArray.class.getName()
+        "Unable to convert object of type[%s] to Object[]",
+        obj.getClass().getName()
     );
 
   }
 
   @Nullable
   public static Object[] coerceToStringArray(Object obj)
   {
-    return coerceToObjectWithCoercionFunction(obj, DimensionHandlerUtils::convertObjectToString);
+    return coerceToObjectArrayWithElementCoercionFunction(obj, DimensionHandlerUtils::convertObjectToString);
   }
 
   public static int compareObjectsAsType(

diff --git a/processing/src/main/java/org/apache/druid/segment/data/ComparableList.java b/processing/src/main/java/org/apache/druid/segment/data/ComparableList.java