lidavidm commented on code in PR #37748:
URL: https://github.com/apache/arrow/pull/37748#discussion_r1327770755


##########
java/algorithm/src/main/java/org/apache/arrow/algorithm/sort/DefaultVectorComparators.java:
##########
@@ -345,6 +387,293 @@ public VectorValueComparator<Float8Vector> createNew() {
     }
   }
 
+  /**
+   * Default comparator for bit type.
+   * The comparison is based on values, with null comes first.
+   */
+  public static class BitComparator extends VectorValueComparator<BitVector> {
+
+    public BitComparator() {
+      super(-1);
+    }
+
+    @Override
+    public int compareNotNull(int index1, int index2) {
+      boolean value1 = vector1.get(index1) != 0;
+      boolean value2 = vector2.get(index2) != 0;
+
+      return Boolean.compare(value1, value2);
+    }
+
+    @Override
+    public VectorValueComparator<BitVector> createNew() {
+      return new BitComparator();
+    }
+  }
+
+  /**
+   * Default comparator for DateDay type.
+   * The comparison is based on values, with null comes first.
+   */
+  public static class DateDayComparator extends 
VectorValueComparator<DateDayVector> {
+
+    public DateDayComparator() {
+      super(DateDayVector.TYPE_WIDTH);
+    }
+
+    @Override
+    public int compareNotNull(int index1, int index2) {
+      int value1 = vector1.get(index1);
+      int value2 = vector2.get(index2);
+      return Integer.compare(value1, value2);
+    }
+
+    @Override
+    public VectorValueComparator<DateDayVector> createNew() {
+      return new DateDayComparator();
+    }
+  }
+
+  /**
+   * Default comparator for DateMilli type.
+   * The comparison is based on values, with null comes first.
+   */
+  public static class DateMilliComparator extends 
VectorValueComparator<DateMilliVector> {
+
+    public DateMilliComparator() {
+      super(DateMilliVector.TYPE_WIDTH);
+    }
+
+    @Override
+    public int compareNotNull(int index1, int index2) {
+      long value1 = vector1.get(index1);
+      long value2 = vector2.get(index2);
+
+      return Long.compare(value1, value2);
+    }
+
+    @Override
+    public VectorValueComparator<DateMilliVector> createNew() {
+      return new DateMilliComparator();
+    }
+  }
+
+  /**
+   * Default comparator for Decimal256 type.
+   * The comparison is based on values, with null comes first.
+   */
+  public static class Decimal256Comparator extends 
VectorValueComparator<Decimal256Vector> {
+
+    public Decimal256Comparator() {
+      super(Decimal256Vector.TYPE_WIDTH);
+    }
+
+    @Override
+    public int compareNotNull(int index1, int index2) {
+      BigDecimal value1 = vector1.getObjectNotNull(index1);

Review Comment:
   It looks like VectorValueComparator ensures the slot is not null before 
calling this, so is adding the new getObjectNotNull method necessary?



##########
java/algorithm/src/main/java/org/apache/arrow/algorithm/sort/DefaultVectorComparators.java:
##########
@@ -345,6 +387,293 @@ public VectorValueComparator<Float8Vector> createNew() {
     }
   }
 
+  /**
+   * Default comparator for bit type.
+   * The comparison is based on values, with null comes first.
+   */
+  public static class BitComparator extends VectorValueComparator<BitVector> {
+
+    public BitComparator() {
+      super(-1);
+    }
+
+    @Override
+    public int compareNotNull(int index1, int index2) {
+      boolean value1 = vector1.get(index1) != 0;
+      boolean value2 = vector2.get(index2) != 0;
+
+      return Boolean.compare(value1, value2);
+    }
+
+    @Override
+    public VectorValueComparator<BitVector> createNew() {
+      return new BitComparator();
+    }
+  }
+
+  /**
+   * Default comparator for DateDay type.
+   * The comparison is based on values, with null comes first.
+   */
+  public static class DateDayComparator extends 
VectorValueComparator<DateDayVector> {
+
+    public DateDayComparator() {
+      super(DateDayVector.TYPE_WIDTH);
+    }
+
+    @Override
+    public int compareNotNull(int index1, int index2) {
+      int value1 = vector1.get(index1);
+      int value2 = vector2.get(index2);
+      return Integer.compare(value1, value2);
+    }
+
+    @Override
+    public VectorValueComparator<DateDayVector> createNew() {
+      return new DateDayComparator();
+    }
+  }
+
+  /**
+   * Default comparator for DateMilli type.
+   * The comparison is based on values, with null comes first.
+   */
+  public static class DateMilliComparator extends 
VectorValueComparator<DateMilliVector> {
+
+    public DateMilliComparator() {
+      super(DateMilliVector.TYPE_WIDTH);
+    }
+
+    @Override
+    public int compareNotNull(int index1, int index2) {
+      long value1 = vector1.get(index1);
+      long value2 = vector2.get(index2);
+
+      return Long.compare(value1, value2);
+    }
+
+    @Override
+    public VectorValueComparator<DateMilliVector> createNew() {
+      return new DateMilliComparator();
+    }
+  }
+
+  /**
+   * Default comparator for Decimal256 type.
+   * The comparison is based on values, with null comes first.
+   */
+  public static class Decimal256Comparator extends 
VectorValueComparator<Decimal256Vector> {
+
+    public Decimal256Comparator() {
+      super(Decimal256Vector.TYPE_WIDTH);
+    }
+
+    @Override
+    public int compareNotNull(int index1, int index2) {
+      BigDecimal value1 = vector1.getObjectNotNull(index1);

Review Comment:
   (And if it is necessary, it might be preferable to use the regular getObject 
and replace any nulls with a sentinel value, since otherwise there is no 
guarantee on the value that is actually in the null slot - but it seems that 
shouldn't happen in the first place.)



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