You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kyuubi.apache.org by ul...@apache.org on 2021/10/12 11:25:22 UTC

[incubator-kyuubi] branch master updated: [KYUUBI #1218] Z-order int and long should respect negative number

This is an automated email from the ASF dual-hosted git repository.

ulyssesyou pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-kyuubi.git


The following commit(s) were added to refs/heads/master by this push:
     new 36aab5b  [KYUUBI #1218] Z-order int and long should respect negative number
36aab5b is described below

commit 36aab5b95ff4d598248d9a80385783ac68a70b0f
Author: ulysses-you <ul...@gmail.com>
AuthorDate: Tue Oct 12 19:25:11 2021 +0800

    [KYUUBI #1218] Z-order int and long should respect negative number
    
    <!--
    Thanks for sending a pull request!
    
    Here are some tips for you:
      1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/contributions.html
      2. If the PR is related to an issue in https://github.com/apache/incubator-kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
      3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
    -->
    
    ### _Why are the changes needed?_
    <!--
    Please clarify why the changes are needed. For instance,
      1. If you add a feature, you can talk about the use case of it.
      2. If you fix a bug, you can clarify why it is a bug.
    -->
    Made a mistake at https://github.com/apache/incubator-kyuubi/pull/1192 that we can't reverse the short highest bit for int and int highest bit for long. Otherwise the negative number will get the wrong order.
    
    ### _How was this patch tested?_
    - [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible
    
    - [ ] Add screenshots for manual tests if appropriate
    
    - [ ] [Run test](https://kyuubi.readthedocs.io/en/latest/develop_tools/testing.html#running-tests) locally before make a pull request
    
    Closes #1218 from ulysses-you/fix-zorder.
    
    Closes #1218
    
    3f7f22d3 [ulysses-you] fix
    
    Authored-by: ulysses-you <ul...@gmail.com>
    Signed-off-by: ulysses-you <ul...@apache.org>
---
 .../kyuubi/sql/zorder/ZorderBytesUtils.scala       | 45 +++++-----------------
 .../scala/org/apache/spark/sql/ZorderSuite.scala   | 24 +++++++++---
 2 files changed, 28 insertions(+), 41 deletions(-)

diff --git a/dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/kyuubi/sql/zorder/ZorderBytesUtils.scala b/dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/kyuubi/sql/zorder/ZorderBytesUtils.scala
index 1e60916..fd27036 100644
--- a/dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/kyuubi/sql/zorder/ZorderBytesUtils.scala
+++ b/dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/kyuubi/sql/zorder/ZorderBytesUtils.scala
@@ -120,20 +120,11 @@ object ZorderBytesUtils {
   def intToByte(a: Int): Array[Byte] = {
     val result = new Array[Byte](4)
     var i = 0
-    if (a <= Short.MaxValue) {
-      val tmp = a ^ BIT_16_MASK
-      while (i <= 1) {
-        val offset = i * 8
-        result(3 - i) = ((tmp >> offset) & 0xff).toByte
-        i += 1
-      }
-    } else {
-      val tmp = a ^ BIT_32_MASK
-      while (i <= 3) {
-        val offset = i * 8
-        result(3 - i) = ((tmp >> offset) & 0xff).toByte
-        i += 1
-      }
+    val tmp = a ^ BIT_32_MASK
+    while (i <= 3) {
+      val offset = i * 8
+      result(3 - i) = ((tmp >> offset) & 0xff).toByte
+      i += 1
     }
     result
   }
@@ -141,27 +132,11 @@ object ZorderBytesUtils {
   def longToByte(a: Long): Array[Byte] = {
     val result = new Array[Byte](8)
     var i = 0
-    if (a <= Short.MaxValue) {
-      val tmp = a ^ BIT_16_MASK
-      while (i <= 1) {
-        val offset = i * 8
-        result(7 - i) = ((tmp >> offset) & 0xff).toByte
-        i += 1
-      }
-    } else if (a <= Int.MaxValue) {
-      val tmp = a ^ BIT_32_MASK
-      while (i <= 3) {
-        val offset = i * 8
-        result(7 - i) = ((tmp >> offset) & 0xff).toByte
-        i += 1
-      }
-    } else {
-      val tmp = a ^ BIT_64_MASK
-      while (i <= 7) {
-        val offset = i * 8
-        result(7 - i) = ((tmp >> offset) & 0xff).toByte
-        i += 1
-      }
+    val tmp = a ^ BIT_64_MASK
+    while (i <= 7) {
+      val offset = i * 8
+      result(7 - i) = ((tmp >> offset) & 0xff).toByte
+      i += 1
     }
     result
   }
diff --git a/dev/kyuubi-extension-spark-3-1/src/test/scala/org/apache/spark/sql/ZorderSuite.scala b/dev/kyuubi-extension-spark-3-1/src/test/scala/org/apache/spark/sql/ZorderSuite.scala
index ad74984..dae66fb 100644
--- a/dev/kyuubi-extension-spark-3-1/src/test/scala/org/apache/spark/sql/ZorderSuite.scala
+++ b/dev/kyuubi-extension-spark-3-1/src/test/scala/org/apache/spark/sql/ZorderSuite.scala
@@ -358,14 +358,14 @@ trait ZorderSuite extends KyuubiSparkSQLExtensionTest with ExpressionEvalHelper
 //    // scalastyle:on
 
     val expected = Array(
-      0xAB, 0xAA, 0xAA, 0xBA, 0xAE, 0xAB, 0xAA, 0xEA, 0xBA, 0xAE,
+      0xFB, 0xEA, 0xAA, 0xBA, 0xAE, 0xAB, 0xAA, 0xEA, 0xBA, 0xAE,
       0xAB, 0xAA, 0xEA, 0xBA, 0xA6, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA,
       0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA,
       0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA,
-      0xAA, 0xBA, 0xAA, 0xAA, 0xAA, 0xBA, 0xAA, 0xBA, 0xAA, 0xBA,
+      0xBA, 0xBB, 0xAA, 0xAA, 0xAA, 0xBA, 0xAA, 0xBA, 0xAA, 0xBA,
       0xAA, 0xBA, 0xAA, 0xBA, 0xAA, 0xBA, 0xAA, 0x9A, 0xAA, 0xAA,
       0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA,
-      0xAA, 0xAA, 0xFE, 0xAF, 0xEA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA,
+      0xAA, 0xAA, 0xAA, 0xAA, 0xEA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA,
       0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA,
       0xAA, 0xAA, 0xBE, 0xAA, 0xAA, 0x8A, 0xBA, 0xAA, 0x2A, 0xEA,
       0xA8, 0xAA, 0xAA, 0xA2, 0xAA, 0xAA, 0x8A, 0xAA, 0xAA, 0x2F,
@@ -448,13 +448,17 @@ trait ZorderSuite extends KyuubiSparkSQLExtensionTest with ExpressionEvalHelper
 
   test("test special value of short int long type") {
     val df1 = spark.createDataFrame(Seq(
+      (-1, -1L),
+      (Int.MinValue, Int.MinValue.toLong),
       (1, 1L),
       (Int.MaxValue - 1, Int.MaxValue.toLong),
       (Int.MaxValue - 1, Int.MaxValue.toLong - 1),
       (Int.MaxValue, Int.MaxValue.toLong + 1),
       (Int.MaxValue, Int.MaxValue.toLong))).toDF("c1", "c2")
     val expected1 =
-      Row(1, 1L) ::
+      Row(Int.MinValue, Int.MinValue.toLong) ::
+        Row(-1, -1L) ::
+        Row(1, 1L) ::
         Row(Int.MaxValue - 1, Int.MaxValue.toLong - 1) ::
         Row(Int.MaxValue - 1, Int.MaxValue.toLong) ::
         Row(Int.MaxValue, Int.MaxValue.toLong) ::
@@ -462,13 +466,17 @@ trait ZorderSuite extends KyuubiSparkSQLExtensionTest with ExpressionEvalHelper
     checkSort(df1, expected1, Array(IntegerType, LongType))
 
     val df2 = spark.createDataFrame(Seq(
+      (-1, -1.toShort),
+      (Short.MinValue.toInt, Short.MinValue),
       (1, 1.toShort),
       (Short.MaxValue.toInt, (Short.MaxValue - 1).toShort),
       (Short.MaxValue.toInt + 1, (Short.MaxValue - 1).toShort),
       (Short.MaxValue.toInt, Short.MaxValue),
       (Short.MaxValue.toInt + 1, Short.MaxValue))).toDF("c1", "c2")
     val expected2 =
-      Row(1, 1.toShort) ::
+      Row(Short.MinValue.toInt, Short.MinValue) ::
+        Row(-1, -1.toShort) ::
+        Row(1, 1.toShort) ::
         Row(Short.MaxValue.toInt, Short.MaxValue - 1) ::
         Row(Short.MaxValue.toInt, Short.MaxValue) ::
         Row(Short.MaxValue.toInt + 1, Short.MaxValue - 1) ::
@@ -476,13 +484,17 @@ trait ZorderSuite extends KyuubiSparkSQLExtensionTest with ExpressionEvalHelper
     checkSort(df2, expected2, Array(IntegerType, ShortType))
 
     val df3 = spark.createDataFrame(Seq(
+      (-1L, -1.toShort),
+      (Short.MinValue.toLong, Short.MinValue),
       (1L, 1.toShort),
       (Short.MaxValue.toLong, (Short.MaxValue - 1).toShort),
       (Short.MaxValue.toLong + 1, (Short.MaxValue - 1).toShort),
       (Short.MaxValue.toLong, Short.MaxValue),
       (Short.MaxValue.toLong + 1, Short.MaxValue))).toDF("c1", "c2")
     val expected3 =
-      Row(1L, 1.toShort) ::
+      Row(Short.MinValue.toLong, Short.MinValue) ::
+        Row(-1L, -1.toShort) ::
+        Row(1L, 1.toShort) ::
         Row(Short.MaxValue.toLong, Short.MaxValue - 1) ::
         Row(Short.MaxValue.toLong, Short.MaxValue) ::
         Row(Short.MaxValue.toLong + 1, Short.MaxValue - 1) ::