From 55b1d1232914a7a10b8c4a4aab527ec103ea26d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Ili=C4=87?= Date: Wed, 28 Aug 2024 17:44:03 +0200 Subject: [PATCH] [Kernel] Fix binary comparator to use the unsigned comparison (#3617) ## Description Fixed binary comparator. Previously, bytes were compared as signed, which was incorrect. ## How was this patch tested? Tests added to `DefaultExpressionEvaluatorSuite.scala` --- .../expressions/DefaultExpressionUtils.java | 2 +- .../DefaultExpressionEvaluatorSuite.scala | 48 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/kernel/kernel-defaults/src/main/java/io/delta/kernel/defaults/internal/expressions/DefaultExpressionUtils.java b/kernel/kernel-defaults/src/main/java/io/delta/kernel/defaults/internal/expressions/DefaultExpressionUtils.java index 86e00d453e0..bf7c98fdc84 100644 --- a/kernel/kernel-defaults/src/main/java/io/delta/kernel/defaults/internal/expressions/DefaultExpressionUtils.java +++ b/kernel/kernel-defaults/src/main/java/io/delta/kernel/defaults/internal/expressions/DefaultExpressionUtils.java @@ -53,7 +53,7 @@ class DefaultExpressionUtils { int i = 0; while (i < leftOp.length && i < rightOp.length) { if (leftOp[i] != rightOp[i]) { - return Byte.compare(leftOp[i], rightOp[i]); + return Byte.toUnsignedInt(leftOp[i]) - Byte.toUnsignedInt(rightOp[i]); } i++; } diff --git a/kernel/kernel-defaults/src/test/scala/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluatorSuite.scala b/kernel/kernel-defaults/src/test/scala/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluatorSuite.scala index 44ef0aca9a8..bb1cc6b6809 100644 --- a/kernel/kernel-defaults/src/test/scala/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluatorSuite.scala +++ b/kernel/kernel-defaults/src/test/scala/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluatorSuite.scala @@ -676,6 +676,54 @@ class DefaultExpressionEvaluatorSuite extends AnyFunSuite with ExpressionSuiteBa ofBinary("apples".getBytes()), ofNull(BinaryType.BINARY) ), + ( + ofBinary(Array[Byte]()), + ofBinary(Array[Byte](5.toByte)), + ofBinary(Array[Byte]()), + ofNull(BinaryType.BINARY) + ), + ( + ofBinary(Array[Byte](0.toByte)), // 00000000 + ofBinary(Array[Byte](-1.toByte)), // 11111111 + ofBinary(Array[Byte](0.toByte)), + ofNull(BinaryType.BINARY) + ), + ( + ofBinary(Array[Byte](127.toByte)), // 01111111 + ofBinary(Array[Byte](-1.toByte)), // 11111111 + ofBinary(Array[Byte](127.toByte)), + ofNull(BinaryType.BINARY) + ), + ( + ofBinary(Array[Byte](5.toByte, 10.toByte)), + ofBinary(Array[Byte](6.toByte)), + ofBinary(Array[Byte](5.toByte, 10.toByte)), + ofNull(BinaryType.BINARY) + ), + ( + ofBinary(Array[Byte](5.toByte, 10.toByte)), + ofBinary(Array[Byte](5.toByte, 100.toByte)), + ofBinary(Array[Byte](5.toByte, 10.toByte)), + ofNull(BinaryType.BINARY) + ), + ( + ofBinary(Array[Byte](5.toByte, 10.toByte, 5.toByte)), // 00000101 00001010 00000101 + ofBinary(Array[Byte](5.toByte, -3.toByte)), // 00000101 11111101 + ofBinary(Array[Byte](5.toByte, 10.toByte, 5.toByte)), + ofNull(BinaryType.BINARY) + ), + ( + ofBinary(Array[Byte](5.toByte, -25.toByte, 5.toByte)), // 00000101 11100111 00000101 + ofBinary(Array[Byte](5.toByte, -9.toByte)), // 00000101 11110111 + ofBinary(Array[Byte](5.toByte, -25.toByte, 5.toByte)), + ofNull(BinaryType.BINARY) + ), + ( + ofBinary(Array[Byte](5.toByte, 10.toByte)), + ofBinary(Array[Byte](5.toByte, 10.toByte, 0.toByte)), + ofBinary(Array[Byte](5.toByte, 10.toByte)), + ofNull(BinaryType.BINARY) + ), ( ofDecimal(BigDecimalJ.valueOf(1.12), 7, 3), ofDecimal(BigDecimalJ.valueOf(5233.232), 7, 3),