yihua commented on code in PR #9894: URL: https://github.com/apache/hudi/pull/9894#discussion_r1632553603
########## hudi-common/src/main/java/org/apache/hudi/common/table/read/HoodieBaseFileGroupRecordBuffer.java: ########## @@ -147,6 +153,37 @@ public void close() { records.clear(); } + /** + * Compares two {@link Comparable}s. If both are numbers, converts them to {@link Long} for comparison. + * If one of the {@link Comparable}s is a String, assumes that both are String values for comparison. + * + * @param o1 {@link Comparable} object. + * @param o2 other {@link Comparable} object to compare to. + * @return comparison result. + */ + @VisibleForTesting + static int compareTo(Comparable o1, Comparable o2) { + // TODO(HUDI-7848): fix the delete records to contain the correct ordering value type + // so this util with the number comparison is not necessary. + try { + return o1.compareTo(o2); + } catch (ClassCastException e) { + if (o1 instanceof Number && o2 instanceof Number) { + Long o1LongValue = ((Number) o1).longValue(); + Long o2LongValue = ((Number) o2).longValue(); + return o1LongValue.compareTo(o2LongValue); Review Comment: We can constrain the comparison to Long and Integer only to limit the possibility of wrong results. I'll create a follow-up PR to fix 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: commits-unsubscr...@hudi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org