You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kyuubi.apache.org by GitBox <gi...@apache.org> on 2021/08/26 07:04:57 UTC

[GitHub] [incubator-kyuubi] qphien opened a new pull request #990: [WIP] [#KYUUBI 939] Add Z-Order extensions to support optimize SQL

qphien opened a new pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990


   <!--
   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?_
   This PR adds Z-Order extension to support `OPTIMIZE` SQL in https://github.com/apache/incubator-kyuubi/issues/939.
   
   Currently this PR only supports optimize `HiveTableRelation`.
   
   We need to set `hive.exec.dynamic.partition` to `true` and `hive.exec.dynamic.partition.mode` to `nonstrict` when a partitioned table is optimized.
   
   
   ### _How was this patch tested?_
   - [ ] 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
   


-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] ulysses-you commented on a change in pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
ulysses-you commented on a change in pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#discussion_r701545600



##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/kyuubi/sql/zorder/OptimizeBeforeWrite.scala
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.sql.zorder
+
+import org.apache.hadoop.hive.ql.ErrorMsg
+import org.apache.spark.SparkException
+import org.apache.spark.sql.SparkSession
+import org.apache.spark.sql.catalyst.catalog.HiveTableRelation
+import org.apache.spark.sql.catalyst.expressions.{And, AttributeSet, Expression, Or}
+import org.apache.spark.sql.catalyst.plans.logical.{Filter, InsertIntoStatement, LogicalPlan}
+import org.apache.spark.sql.catalyst.rules.Rule
+
+case class OptimizeBeforeWrite(session: SparkSession) extends Rule[LogicalPlan] {

Review comment:
       `OptimizeBeforeWrite ` -> `ZorderBeforeWrite`

##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/kyuubi/sql/zorder/OptimizeBeforeWrite.scala
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.sql.zorder
+
+import org.apache.hadoop.hive.ql.ErrorMsg
+import org.apache.spark.SparkException
+import org.apache.spark.sql.SparkSession
+import org.apache.spark.sql.catalyst.catalog.HiveTableRelation
+import org.apache.spark.sql.catalyst.expressions.{And, AttributeSet, Expression, Or}
+import org.apache.spark.sql.catalyst.plans.logical.{Filter, InsertIntoStatement, LogicalPlan}
+import org.apache.spark.sql.catalyst.rules.Rule
+
+case class OptimizeBeforeWrite(session: SparkSession) extends Rule[LogicalPlan] {
+  override def apply(plan: LogicalPlan): LogicalPlan = {
+    plan match {
+      case o@OptimizeZorderCommand(child) =>
+        var relation: Option[HiveTableRelation] = None
+        var partitionSpec: Map[String, Option[String]] = Map.empty
+        val hadoopConf = session.sessionState.newHadoopConf()
+        val newChild = child.resolveOperatorsUp {
+          case f@Filter(condition, _) if f.resolved =>
+            if (relation.isDefined) {
+              val tableRelation = relation.get
+              if (!tableRelation.isPartitioned) {
+                throw new ZorderException("Filters are only supported for partitioned table")
+              }
+
+              val partitionKeyIds = AttributeSet(tableRelation.partitionCols)
+              val predicates = splitConjunctivePredicates(condition)
+              val (_, otherPredicates) = predicates.partition { predicate =>
+                !predicate.references.isEmpty &&
+                  predicate.references.subsetOf(partitionKeyIds)
+              }
+
+              if (otherPredicates.nonEmpty) {
+                throw new ZorderException("Only partition column filters are allowed")
+              }
+
+              if (!hadoopConf.get("hive.exec.dynamic.partition", "true").toBoolean) {
+                throw new SparkException(ErrorMsg.DYNAMIC_PARTITION_DISABLED.getMsg)

Review comment:
       do we need the check ? Spark has already checked in `InsertIntoHiveTable`.

##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/kyuubi/sql/zorder/OptimizeBeforeWrite.scala
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.sql.zorder
+
+import org.apache.hadoop.hive.ql.ErrorMsg
+import org.apache.spark.SparkException
+import org.apache.spark.sql.SparkSession
+import org.apache.spark.sql.catalyst.catalog.HiveTableRelation
+import org.apache.spark.sql.catalyst.expressions.{And, AttributeSet, Expression, Or}
+import org.apache.spark.sql.catalyst.plans.logical.{Filter, InsertIntoStatement, LogicalPlan}
+import org.apache.spark.sql.catalyst.rules.Rule
+
+case class OptimizeBeforeWrite(session: SparkSession) extends Rule[LogicalPlan] {
+  override def apply(plan: LogicalPlan): LogicalPlan = {
+    plan match {
+      case o@OptimizeZorderCommand(child) =>
+        var relation: Option[HiveTableRelation] = None
+        var partitionSpec: Map[String, Option[String]] = Map.empty
+        val hadoopConf = session.sessionState.newHadoopConf()
+        val newChild = child.resolveOperatorsUp {
+          case f@Filter(condition, _) if f.resolved =>
+            if (relation.isDefined) {
+              val tableRelation = relation.get
+              if (!tableRelation.isPartitioned) {
+                throw new ZorderException("Filters are only supported for partitioned table")
+              }
+
+              val partitionKeyIds = AttributeSet(tableRelation.partitionCols)
+              val predicates = splitConjunctivePredicates(condition)

Review comment:
       why not check reference directly instead of split + check ?

##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/kyuubi/sql/zorder/Zorder.scala
##########
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.sql.zorder
+
+import scala.collection.mutable.ArrayBuffer
+
+import org.apache.spark.sql.catalyst.InternalRow
+import org.apache.spark.sql.catalyst.expressions.{BoundReference, Expression}
+import org.apache.spark.sql.catalyst.expressions.codegen.CodegenFallback
+import org.apache.spark.sql.types.{BinaryType, BooleanType, ByteType, DataType, DateType, Decimal, DecimalType, DoubleType, FloatType, IntegerType, LongType, ShortType, StringType, TimestampType}
+
+case class Zorder(children: Seq[Expression]) extends Expression with CodegenFallback {
+  private lazy val defaultNullValues = {
+    children.map {
+      case bf: BoundReference =>
+        bf.dataType match {
+          case BooleanType =>
+            false
+          case ByteType =>
+            Byte.MaxValue
+          case ShortType =>
+            Short.MaxValue
+          case IntegerType =>
+            Int.MaxValue
+          case LongType =>
+            Long.MaxValue
+          case FloatType =>
+            Float.MaxValue
+          case DoubleType =>
+            Double.MaxValue
+          case StringType =>
+            ""
+          case TimestampType =>
+            Long.MaxValue
+          case DateType =>
+            Int.MaxValue
+          case d: DecimalType =>
+            Long.MaxValue
+          case other: Any =>
+            throw new ZorderException("Unsupported z-order type: " + other.getClass)
+        }
+      case other: Any =>
+        throw new ZorderException("Unknown z-order column: " + other)
+    }
+  }
+
+  override def nullable: Boolean = true

Review comment:
       does zorder will produce null ?

##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/kyuubi/sql/zorder/OptimizeBeforeWrite.scala
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.sql.zorder
+
+import org.apache.hadoop.hive.ql.ErrorMsg
+import org.apache.spark.SparkException
+import org.apache.spark.sql.SparkSession
+import org.apache.spark.sql.catalyst.catalog.HiveTableRelation
+import org.apache.spark.sql.catalyst.expressions.{And, AttributeSet, Expression, Or}
+import org.apache.spark.sql.catalyst.plans.logical.{Filter, InsertIntoStatement, LogicalPlan}
+import org.apache.spark.sql.catalyst.rules.Rule
+
+case class OptimizeBeforeWrite(session: SparkSession) extends Rule[LogicalPlan] {
+  override def apply(plan: LogicalPlan): LogicalPlan = {
+    plan match {
+      case o@OptimizeZorderCommand(child) =>
+        var relation: Option[HiveTableRelation] = None
+        var partitionSpec: Map[String, Option[String]] = Map.empty
+        val hadoopConf = session.sessionState.newHadoopConf()
+        val newChild = child.resolveOperatorsUp {
+          case f@Filter(condition, _) if f.resolved =>

Review comment:
       We can make this code more clearer since we only support two pattern here:
   ```
   case f @ Filter(condition, r: HiveTableRelation) =>
     checkRelation()
   
   case r: HiveTableRelation =>
     checkRelation()
   
   ```

##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/kyuubi/sql/zorder/Zorder.scala
##########
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.sql.zorder
+
+import scala.collection.mutable.ArrayBuffer
+
+import org.apache.spark.sql.catalyst.InternalRow
+import org.apache.spark.sql.catalyst.expressions.{BoundReference, Expression}
+import org.apache.spark.sql.catalyst.expressions.codegen.CodegenFallback
+import org.apache.spark.sql.types.{BinaryType, BooleanType, ByteType, DataType, DateType, Decimal, DecimalType, DoubleType, FloatType, IntegerType, LongType, ShortType, StringType, TimestampType}
+
+case class Zorder(children: Seq[Expression]) extends Expression with CodegenFallback {
+  private lazy val defaultNullValues = {
+    children.map {
+      case bf: BoundReference =>
+        bf.dataType match {
+          case BooleanType =>
+            false
+          case ByteType =>
+            Byte.MaxValue
+          case ShortType =>
+            Short.MaxValue
+          case IntegerType =>
+            Int.MaxValue
+          case LongType =>
+            Long.MaxValue
+          case FloatType =>
+            Float.MaxValue
+          case DoubleType =>
+            Double.MaxValue
+          case StringType =>
+            ""
+          case TimestampType =>
+            Long.MaxValue
+          case DateType =>
+            Int.MaxValue
+          case d: DecimalType =>
+            Long.MaxValue
+          case other: Any =>
+            throw new ZorderException("Unsupported z-order type: " + other.getClass)
+        }
+      case other: Any =>
+        throw new ZorderException("Unknown z-order column: " + other)
+    }
+  }
+
+  override def nullable: Boolean = true
+
+  override def eval(input: InternalRow): Any = {
+    var i = 0
+    val evaluated = ArrayBuffer[Any]()

Review comment:
       ```scala
   val evaluated = children.zipWithIndex { case (child, index) =>
     val v = child.eval(input)
     if (v == null) {
       defaultNullValues(index)
     } else {
       v
     }
   }
   ```

##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/kyuubi/sql/zorder/OptimizeZorderCommand.scala
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.sql.zorder
+
+import org.apache.spark.sql.catalyst.analysis.{UnresolvedAttribute, UnresolvedRelation}
+import org.apache.spark.sql.catalyst.expressions.{Ascending, Attribute, Expression, NullsLast, SortOrder}
+import org.apache.spark.sql.catalyst.plans.logical.{Filter, LogicalPlan, Sort, UnaryNode}
+
+case class OptimizeZorderCommand(child: LogicalPlan) extends UnaryNode {
+  override def output: Seq[Attribute] = child.output
+}
+
+object OptimizeZorderCommand {
+
+  def apply(tableIdent: Seq[String],
+            whereExp: Option[Expression],
+            sortArr: Seq[UnresolvedAttribute]): OptimizeZorderCommand = {
+    val table = UnresolvedRelation(tableIdent)
+    val child = whereExp match {
+      case Some(x) => Filter(x, table)
+      case None => table
+    }
+
+    val sortOrder = SortOrder(Zorder(sortArr), Ascending, NullsLast, Seq.empty)
+    val zorderSort = Sort(Seq(sortOrder), false, child)

Review comment:
       I think we support the global zorder now

##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/kyuubi/sql/zorder/Zorder.scala
##########
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.sql.zorder
+
+import scala.collection.mutable.ArrayBuffer
+
+import org.apache.spark.sql.catalyst.InternalRow
+import org.apache.spark.sql.catalyst.expressions.{BoundReference, Expression}
+import org.apache.spark.sql.catalyst.expressions.codegen.CodegenFallback
+import org.apache.spark.sql.types.{BinaryType, BooleanType, ByteType, DataType, DateType, Decimal, DecimalType, DoubleType, FloatType, IntegerType, LongType, ShortType, StringType, TimestampType}
+
+case class Zorder(children: Seq[Expression]) extends Expression with CodegenFallback {
+  private lazy val defaultNullValues = {
+    children.map {
+      case bf: BoundReference =>
+        bf.dataType match {
+          case BooleanType =>
+            false
+          case ByteType =>
+            Byte.MaxValue
+          case ShortType =>
+            Short.MaxValue
+          case IntegerType =>
+            Int.MaxValue
+          case LongType =>
+            Long.MaxValue
+          case FloatType =>
+            Float.MaxValue
+          case DoubleType =>
+            Double.MaxValue
+          case StringType =>
+            ""
+          case TimestampType =>
+            Long.MaxValue
+          case DateType =>
+            Int.MaxValue
+          case d: DecimalType =>
+            Long.MaxValue
+          case other: Any =>
+            throw new ZorderException("Unsupported z-order type: " + other.getClass)
+        }
+      case other: Any =>
+        throw new ZorderException("Unknown z-order column: " + other)
+    }
+  }
+
+  override def nullable: Boolean = true
+
+  override def eval(input: InternalRow): Any = {
+    var i = 0
+    val evaluated = ArrayBuffer[Any]()
+    while (i < children.length) {
+      val ev = children(i).eval(input)
+      if (ev == null) {
+        evaluated += defaultNullValues(i)
+      } else {
+        evaluated += ev
+      }
+      i += 1
+    }
+
+    val binaryArr = evaluated.toArray.map {

Review comment:
       can we move this code into `ZorderBytesUtils` ?




-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#issuecomment-907985416


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#990](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (531ad50) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/f1ae664910d6cbcf327e126f8bc96f4b8dabb13b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f1ae664) will **decrease** coverage by `3.61%`.
   > The diff coverage is `56.11%`.
   
   > :exclamation: Current head 531ad50 differs from pull request most recent head 16971ec. Consider uploading reports for the commit 16971ec to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/graphs/tree.svg?width=650&height=150&src=pr&token=925D4tb9AH&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #990      +/-   ##
   ============================================
   - Coverage     80.77%   77.16%   -3.62%     
   - Complexity       11       88      +77     
   ============================================
     Files           146      161      +15     
     Lines          5389     6025     +636     
     Branches        636      732      +96     
   ============================================
   + Hits           4353     4649     +296     
   - Misses          660      943     +283     
   - Partials        376      433      +57     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ache/kyuubi/sql/zorder/KyuubiZorderExtension.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvS3l1dWJpWm9yZGVyRXh0ZW5zaW9uLnNjYWxh) | `0.00% <0.00%> (ø)` | |
   | [...he/kyuubi/events/AbstractEventLoggingService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9ldmVudHMvQWJzdHJhY3RFdmVudExvZ2dpbmdTZXJ2aWNlLnNjYWxh) | `0.00% <0.00%> (ø)` | |
   | [...ala/org/apache/kyuubi/events/EventLoggerType.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9ldmVudHMvRXZlbnRMb2dnZXJUeXBlLnNjYWxh) | `0.00% <ø> (ø)` | |
   | [...ala/org/apache/kyuubi/events/JsonEventLogger.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9ldmVudHMvSnNvbkV2ZW50TG9nZ2VyLnNjYWxh) | `0.00% <0.00%> (ø)` | |
   | [.../scala/org/apache/kyuubi/events/JsonProtocol.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9ldmVudHMvSnNvblByb3RvY29sLnNjYWxh) | `0.00% <0.00%> (ø)` | |
   | [...n/scala/org/apache/kyuubi/events/KyuubiEvent.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9ldmVudHMvS3l1dWJpRXZlbnQuc2NhbGE=) | `0.00% <0.00%> (ø)` | |
   | [...n/scala/org/apache/kyuubi/service/Serverable.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL1NlcnZlcmFibGUuc2NhbGE=) | `42.85% <ø> (-1.59%)` | :arrow_down: |
   | [.../org/apache/kyuubi/session/KyuubiSessionImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL0t5dXViaVNlc3Npb25JbXBsLnNjYWxh) | `85.29% <0.00%> (-2.59%)` | :arrow_down: |
   | [...rg/apache/spark/sql/execution/ZorderOrdering.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3Bhcmsvc3FsL2V4ZWN1dGlvbi9ab3JkZXJPcmRlcmluZy5zY2FsYQ==) | `28.57% <28.57%> (ø)` | |
   | [...uubi/engine/spark/events/SparkStatementEvent.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9ldmVudHMvU3BhcmtTdGF0ZW1lbnRFdmVudC5zY2FsYQ==) | `83.33% <33.33%> (ø)` | |
   | ... and [52 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [f1ae664...16971ec](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] cfmcgrady commented on a change in pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
cfmcgrady commented on a change in pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#discussion_r698942488



##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/kyuubi/sql/zorder/ZorderSparkSqlExtensionsParser.scala
##########
@@ -0,0 +1,134 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.sql.zorder
+
+import java.util.Locale
+
+import org.antlr.v4.runtime._
+import org.antlr.v4.runtime.atn.PredictionMode
+import org.antlr.v4.runtime.misc.{Interval, ParseCancellationException}
+import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.catalyst.{FunctionIdentifier, TableIdentifier}
+import org.apache.spark.sql.catalyst.expressions.Expression
+import org.apache.spark.sql.catalyst.parser.{ParseErrorListener, ParseException, ParserInterface, PostProcessor}
+import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
+import org.apache.spark.sql.catalyst.trees.Origin
+import org.apache.spark.sql.types.{DataType, StructType}
+
+class ZorderSparkSqlExtensionsParser(delegate: ParserInterface) extends ParserInterface {
+
+  private lazy val astBuilder = new ZorderSqlAstBuilder
+  override def parsePlan(sqlText: String): LogicalPlan = {
+    if (isZorderCommand(sqlText)) {
+      parse(sqlText) { parser =>
+        astBuilder.visit(parser.singleStatement()) }.asInstanceOf[LogicalPlan]
+    } else {
+      delegate.parsePlan(sqlText)
+    }
+  }
+
+  protected def parse[T](command: String)(toResult: ZorderSqlExtensionsParser => T): T = {
+    val lexer = new ZorderSqlExtensionsLexer(
+      new UpperCaseCharStream(CharStreams.fromString(command)))
+    lexer.removeErrorListeners()
+    lexer.addErrorListener(ParseErrorListener)
+
+    val tokenStream = new CommonTokenStream(lexer)
+    val parser = new ZorderSqlExtensionsParser(tokenStream)
+    parser.addParseListener(PostProcessor)
+    parser.removeErrorListeners()
+    parser.addErrorListener(ParseErrorListener)
+
+    try {
+      try {
+        // first, try parsing with potentially faster SLL mode
+        parser.getInterpreter.setPredictionMode(PredictionMode.SLL)
+        toResult(parser)
+      }
+      catch {
+        case _: ParseCancellationException =>
+          // if we fail, parse with LL mode
+          tokenStream.seek(0) // rewind input stream
+          parser.reset()
+
+          // Try Again.
+          parser.getInterpreter.setPredictionMode(PredictionMode.LL)
+          toResult(parser)
+      }
+    }
+    catch {
+      case e: ParseException if e.command.isDefined =>
+        throw e
+      case e: ParseException =>
+        throw e.withCommand(command)
+      case e: AnalysisException =>
+        val position = Origin(e.line, e.startPosition)
+        throw new ParseException(Option(command), e.message, position, position)
+    }
+  }
+
+  private def isZorderCommand(sqlText: String): Boolean = {
+    val normalized = sqlText.toLowerCase(Locale.ROOT).trim().replaceAll("\\s+", " ")

Review comment:
       I'm saying that you need to replace `"\\s+"` with `""`, not `" "`, we need `""` here, right?




-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] cfmcgrady commented on a change in pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
cfmcgrady commented on a change in pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#discussion_r698942488



##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/kyuubi/sql/zorder/ZorderSparkSqlExtensionsParser.scala
##########
@@ -0,0 +1,134 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.sql.zorder
+
+import java.util.Locale
+
+import org.antlr.v4.runtime._
+import org.antlr.v4.runtime.atn.PredictionMode
+import org.antlr.v4.runtime.misc.{Interval, ParseCancellationException}
+import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.catalyst.{FunctionIdentifier, TableIdentifier}
+import org.apache.spark.sql.catalyst.expressions.Expression
+import org.apache.spark.sql.catalyst.parser.{ParseErrorListener, ParseException, ParserInterface, PostProcessor}
+import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
+import org.apache.spark.sql.catalyst.trees.Origin
+import org.apache.spark.sql.types.{DataType, StructType}
+
+class ZorderSparkSqlExtensionsParser(delegate: ParserInterface) extends ParserInterface {
+
+  private lazy val astBuilder = new ZorderSqlAstBuilder
+  override def parsePlan(sqlText: String): LogicalPlan = {
+    if (isZorderCommand(sqlText)) {
+      parse(sqlText) { parser =>
+        astBuilder.visit(parser.singleStatement()) }.asInstanceOf[LogicalPlan]
+    } else {
+      delegate.parsePlan(sqlText)
+    }
+  }
+
+  protected def parse[T](command: String)(toResult: ZorderSqlExtensionsParser => T): T = {
+    val lexer = new ZorderSqlExtensionsLexer(
+      new UpperCaseCharStream(CharStreams.fromString(command)))
+    lexer.removeErrorListeners()
+    lexer.addErrorListener(ParseErrorListener)
+
+    val tokenStream = new CommonTokenStream(lexer)
+    val parser = new ZorderSqlExtensionsParser(tokenStream)
+    parser.addParseListener(PostProcessor)
+    parser.removeErrorListeners()
+    parser.addErrorListener(ParseErrorListener)
+
+    try {
+      try {
+        // first, try parsing with potentially faster SLL mode
+        parser.getInterpreter.setPredictionMode(PredictionMode.SLL)
+        toResult(parser)
+      }
+      catch {
+        case _: ParseCancellationException =>
+          // if we fail, parse with LL mode
+          tokenStream.seek(0) // rewind input stream
+          parser.reset()
+
+          // Try Again.
+          parser.getInterpreter.setPredictionMode(PredictionMode.LL)
+          toResult(parser)
+      }
+    }
+    catch {
+      case e: ParseException if e.command.isDefined =>
+        throw e
+      case e: ParseException =>
+        throw e.withCommand(command)
+      case e: AnalysisException =>
+        val position = Origin(e.line, e.startPosition)
+        throw new ParseException(Option(command), e.message, position, position)
+    }
+  }
+
+  private def isZorderCommand(sqlText: String): Boolean = {
+    val normalized = sqlText.toLowerCase(Locale.ROOT).trim().replaceAll("\\s+", " ")

Review comment:
       I'm saying that you need to replace `"\\s+"` with `""`, not `" "`, we need `""` here, right?




-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] qphien commented on pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
qphien commented on pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#issuecomment-913363248


   @ulysses-you Thanks for reviewing. All problems mentioned above are resolved.


-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#issuecomment-907985416


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#990](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3eb209d) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/01bebe36e81995ff38733a2e1b9dd432d2b60b3c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (01bebe3) will **decrease** coverage by `2.12%`.
   > The diff coverage is `48.94%`.
   
   > :exclamation: Current head 3eb209d differs from pull request most recent head b5573bb. Consider uploading reports for the commit b5573bb to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/graphs/tree.svg?width=650&height=150&src=pr&token=925D4tb9AH&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #990      +/-   ##
   ============================================
   - Coverage     80.35%   78.23%   -2.13%     
   - Complexity       14       71      +57     
   ============================================
     Files           154      162       +8     
     Lines          5626     6004     +378     
     Branches        647      722      +75     
   ============================================
   + Hits           4521     4697     +176     
   - Misses          719      879     +160     
   - Partials        386      428      +42     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ache/kyuubi/sql/zorder/KyuubiZorderExtension.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvS3l1dWJpWm9yZGVyRXh0ZW5zaW9uLnNjYWxh) | `0.00% <0.00%> (ø)` | |
   | [...apache/kyuubi/sql/zorder/ZorderSqlAstBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyU3FsQXN0QnVpbGRlci5zY2FsYQ==) | `31.57% <31.57%> (ø)` | |
   | [...in/scala/org/apache/kyuubi/sql/zorder/Zorder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyLnNjYWxh) | `38.09% <38.09%> (ø)` | |
   | [...rg/apache/kyuubi/sql/KyuubiSparkSQLExtension.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC9LeXV1YmlTcGFya1NRTEV4dGVuc2lvbi5zY2FsYQ==) | `88.88% <50.00%> (-11.12%)` | :arrow_down: |
   | [...rg/apache/kyuubi/sql/zorder/ZorderBytesUtils.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyQnl0ZXNVdGlscy5zY2FsYQ==) | `62.66% <62.66%> (ø)` | |
   | [...apache/kyuubi/sql/zorder/OptimizeBeforeWrite.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvT3B0aW1pemVCZWZvcmVXcml0ZS5zY2FsYQ==) | `65.90% <65.90%> (ø)` | |
   | [...bi/sql/zorder/ZorderSparkSqlExtensionsParser.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyU3BhcmtTcWxFeHRlbnNpb25zUGFyc2VyLnNjYWxh) | `68.08% <68.08%> (ø)` | |
   | [...ache/kyuubi/sql/zorder/OptimizeZorderCommand.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvT3B0aW1pemVab3JkZXJDb21tYW5kLnNjYWxh) | `81.81% <81.81%> (ø)` | |
   | [...org/apache/kyuubi/sql/zorder/ZorderException.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyRXhjZXB0aW9uLnNjYWxh) | `100.00% <100.00%> (ø)` | |
   | [.../org/apache/kyuubi/operation/KyuubiOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vS3l1dWJpT3BlcmF0aW9uLnNjYWxh) | `38.18% <0.00%> (-16.37%)` | :arrow_down: |
   | ... and [8 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [01bebe3...b5573bb](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] ulysses-you closed pull request #990: [KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
ulysses-you closed pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990


   


-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] qphien commented on a change in pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
qphien commented on a change in pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#discussion_r698924314



##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/spark/sql/execution/ZorderOrdering.scala
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution
+
+import org.apache.spark.sql.catalyst.InternalRow
+import org.apache.spark.sql.catalyst.expressions.{AttributeReference, BaseOrdering, SortOrder}
+import org.apache.spark.sql.types.{BinaryType, BooleanType, ByteType, DoubleType, FloatType, IntegerType, LongType, ShortType, StringType, StructType}
+
+class ZorderOrdering(ordering: Seq[SortOrder],
+                      schema: StructType) extends BaseOrdering {

Review comment:
       OK, It is more concise to use zorder UDF compared with current solution.




-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] cfmcgrady commented on a change in pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
cfmcgrady commented on a change in pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#discussion_r698409459



##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/kyuubi/sql/zorder/ZorderSparkSqlExtensionsParser.scala
##########
@@ -0,0 +1,134 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.sql.zorder
+
+import java.util.Locale
+
+import org.antlr.v4.runtime._
+import org.antlr.v4.runtime.atn.PredictionMode
+import org.antlr.v4.runtime.misc.{Interval, ParseCancellationException}
+import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.catalyst.{FunctionIdentifier, TableIdentifier}
+import org.apache.spark.sql.catalyst.expressions.Expression
+import org.apache.spark.sql.catalyst.parser.{ParseErrorListener, ParseException, ParserInterface, PostProcessor}
+import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
+import org.apache.spark.sql.catalyst.trees.Origin
+import org.apache.spark.sql.types.{DataType, StructType}
+
+class ZorderSparkSqlExtensionsParser(delegate: ParserInterface) extends ParserInterface {
+
+  private lazy val astBuilder = new ZorderSqlAstBuilder
+  override def parsePlan(sqlText: String): LogicalPlan = {
+    if (isZorderCommand(sqlText)) {
+      parse(sqlText) { parser =>
+        astBuilder.visit(parser.singleStatement()) }.asInstanceOf[LogicalPlan]
+    } else {
+      delegate.parsePlan(sqlText)
+    }
+  }
+
+  protected def parse[T](command: String)(toResult: ZorderSqlExtensionsParser => T): T = {
+    val lexer = new ZorderSqlExtensionsLexer(
+      new UpperCaseCharStream(CharStreams.fromString(command)))
+    lexer.removeErrorListeners()
+    lexer.addErrorListener(ParseErrorListener)
+
+    val tokenStream = new CommonTokenStream(lexer)
+    val parser = new ZorderSqlExtensionsParser(tokenStream)
+    parser.addParseListener(PostProcessor)
+    parser.removeErrorListeners()
+    parser.addErrorListener(ParseErrorListener)
+
+    try {
+      try {
+        // first, try parsing with potentially faster SLL mode
+        parser.getInterpreter.setPredictionMode(PredictionMode.SLL)
+        toResult(parser)
+      }
+      catch {
+        case _: ParseCancellationException =>
+          // if we fail, parse with LL mode
+          tokenStream.seek(0) // rewind input stream
+          parser.reset()
+
+          // Try Again.
+          parser.getInterpreter.setPredictionMode(PredictionMode.LL)
+          toResult(parser)
+      }
+    }
+    catch {
+      case e: ParseException if e.command.isDefined =>
+        throw e
+      case e: ParseException =>
+        throw e.withCommand(command)
+      case e: AnalysisException =>
+        val position = Origin(e.line, e.startPosition)
+        throw new ParseException(Option(command), e.message, position, position)
+    }
+  }
+
+  private def isZorderCommand(sqlText: String): Boolean = {
+    val normalized = sqlText.toLowerCase(Locale.ROOT).trim().replaceAll("\\s+", " ")

Review comment:
       Hm, Do you means `replaceAll("\\s+", "")`?




-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#issuecomment-907985416


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#990](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3eb209d) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/01bebe36e81995ff38733a2e1b9dd432d2b60b3c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (01bebe3) will **decrease** coverage by `2.12%`.
   > The diff coverage is `48.94%`.
   
   > :exclamation: Current head 3eb209d differs from pull request most recent head 8f3f4a6. Consider uploading reports for the commit 8f3f4a6 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/graphs/tree.svg?width=650&height=150&src=pr&token=925D4tb9AH&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #990      +/-   ##
   ============================================
   - Coverage     80.35%   78.23%   -2.13%     
   - Complexity       14       71      +57     
   ============================================
     Files           154      162       +8     
     Lines          5626     6004     +378     
     Branches        647      722      +75     
   ============================================
   + Hits           4521     4697     +176     
   - Misses          719      879     +160     
   - Partials        386      428      +42     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ache/kyuubi/sql/zorder/KyuubiZorderExtension.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvS3l1dWJpWm9yZGVyRXh0ZW5zaW9uLnNjYWxh) | `0.00% <0.00%> (ø)` | |
   | [...apache/kyuubi/sql/zorder/ZorderSqlAstBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyU3FsQXN0QnVpbGRlci5zY2FsYQ==) | `31.57% <31.57%> (ø)` | |
   | [...in/scala/org/apache/kyuubi/sql/zorder/Zorder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyLnNjYWxh) | `38.09% <38.09%> (ø)` | |
   | [...rg/apache/kyuubi/sql/KyuubiSparkSQLExtension.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC9LeXV1YmlTcGFya1NRTEV4dGVuc2lvbi5zY2FsYQ==) | `88.88% <50.00%> (-11.12%)` | :arrow_down: |
   | [...rg/apache/kyuubi/sql/zorder/ZorderBytesUtils.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyQnl0ZXNVdGlscy5zY2FsYQ==) | `62.66% <62.66%> (ø)` | |
   | [...apache/kyuubi/sql/zorder/OptimizeBeforeWrite.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvT3B0aW1pemVCZWZvcmVXcml0ZS5zY2FsYQ==) | `65.90% <65.90%> (ø)` | |
   | [...bi/sql/zorder/ZorderSparkSqlExtensionsParser.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyU3BhcmtTcWxFeHRlbnNpb25zUGFyc2VyLnNjYWxh) | `68.08% <68.08%> (ø)` | |
   | [...ache/kyuubi/sql/zorder/OptimizeZorderCommand.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvT3B0aW1pemVab3JkZXJDb21tYW5kLnNjYWxh) | `81.81% <81.81%> (ø)` | |
   | [...org/apache/kyuubi/sql/zorder/ZorderException.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyRXhjZXB0aW9uLnNjYWxh) | `100.00% <100.00%> (ø)` | |
   | [.../org/apache/kyuubi/operation/KyuubiOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vS3l1dWJpT3BlcmF0aW9uLnNjYWxh) | `38.18% <0.00%> (-16.37%)` | :arrow_down: |
   | ... and [8 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [01bebe3...8f3f4a6](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] qphien commented on a change in pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
qphien commented on a change in pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#discussion_r698919583



##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/spark/sql/execution/ZorderBytesUtils.scala
##########
@@ -0,0 +1,143 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution
+
+import java.lang.{Float => jFloat}
+import java.lang.{Double => jDouble}
+import java.nio.charset.Charset
+
+object ZorderBytesUtils {
+  def interleaveMultiByteArray(arrays: Array[Array[Byte]]): Array[Byte] = {
+    var totalLength = 0
+    var maxLength = 0
+    arrays.foreach(array => {
+      totalLength += array.length
+      maxLength = maxLength.max(array.length * 8)
+    })
+    val result = new Array[Byte](totalLength)
+    var resultBit = 0
+
+    for (bit <- 0 to maxLength) {
+      val bytePos = Math.floor(bit / 8).toInt
+      val bitPos = bit % 8
+
+      for (arr <- arrays) {
+        if (bytePos < arr.length) {
+          val resultBytePos = totalLength - 1 - Math.floor(resultBit / 8).toInt
+          val resultBitPos = resultBit % 8
+          result(resultBytePos) = updatePos(result(resultBytePos), resultBitPos,
+            arr(arr.length - 1 - bytePos), bitPos)
+          resultBit += 1
+        }
+      }
+    }
+    result
+  }
+
+  def updatePos(a: Byte, apos: Int, b: Byte, bpos: Int): Byte = {
+    var temp = (b & (1 << bpos)).toByte
+    if (apos > bpos) {
+      temp = (temp << (apos - bpos)).toByte
+    } else if (apos < bpos) {
+      temp = (temp >> (bpos - apos)).toByte
+    }
+    val atemp = (a & (1 << apos)).toByte
+    if (atemp == temp) {
+      return a
+    }
+    (a ^ (1 << apos)).toByte
+  }
+
+  def booleanToByte(a: Boolean): Array[Byte] = {
+    if (a) {
+      intToByte(1)
+    } else {
+      intToByte(0)
+    }
+  }
+
+  def byteToByte(a: Int): Array[Byte] = {
+    val tmp = (a ^ (1 << 7)).toByte
+    Array(tmp)
+  }
+
+  def shortToByte(a: Short): Array[Byte] = {
+    val tmp = a ^ (1 << 15)
+    Array(((tmp >> 8) & 0xff).toByte, (tmp & 0xff).toByte)
+  }
+
+  def intToByte(a: Int): Array[Byte] = {
+    val tmp = a ^ (1 << 31)
+    val result = new Array[Byte](4)
+    for (i <- 0 to 3) {
+      val offset = i * 8
+      result(3 - i) = ((tmp >> offset) & 0xff).toByte
+    }
+    result
+  }
+
+  def longToByte(a: Long): Array[Byte] = {
+    val tmp = a ^ (1 << 63)
+    val result = new Array[Byte](8)
+    for (i <- 0 to 7) {
+      val offset = i * 8
+      result(7 - i) = ((tmp >> offset) & 0xff).toByte
+    }
+    result
+  }
+
+  def floatToByte(a: Float): Array[Byte] = {
+    val fi = jFloat.floatToRawIntBits(a)
+    intToByte(fi)
+  }
+
+  def doubleToByte(a: Double): Array[Byte] = {
+    val dl = jDouble.doubleToRawLongBits(a)
+    longToByte(dl)
+  }
+
+  def stringToByte(str: String): Array[Byte] = {
+    // truncate or padding str to 8 byte

Review comment:
       @cfmcgrady Not yet, benchmark will be made when this PR is completed.




-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #990: [KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#issuecomment-907985416


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#990](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3983ee0) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/121de7fbb09725487343d64e4af497e86b7d5ad4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (121de7f) will **decrease** coverage by `2.21%`.
   > The diff coverage is `42.06%`.
   
   > :exclamation: Current head 3983ee0 differs from pull request most recent head 9190e31. Consider uploading reports for the commit 9190e31 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/graphs/tree.svg?width=650&height=150&src=pr&token=925D4tb9AH&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #990      +/-   ##
   ============================================
   - Coverage     80.64%   78.42%   -2.22%     
   - Complexity       14       61      +47     
   ============================================
     Files           159      167       +8     
     Lines          5837     6196     +359     
     Branches        657      723      +66     
   ============================================
   + Hits           4707     4859     +152     
   - Misses          735      914     +179     
   - Partials        395      423      +28     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ache/kyuubi/sql/zorder/KyuubiZorderExtension.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvS3l1dWJpWm9yZGVyRXh0ZW5zaW9uLnNjYWxh) | `0.00% <0.00%> (ø)` | |
   | [...in/scala/org/apache/kyuubi/sql/zorder/Zorder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyLnNjYWxh) | `15.00% <15.00%> (ø)` | |
   | [...apache/kyuubi/sql/zorder/ZorderSqlAstBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyU3FsQXN0QnVpbGRlci5zY2FsYQ==) | `31.57% <31.57%> (ø)` | |
   | [...rg/apache/kyuubi/sql/zorder/ZorderBytesUtils.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyQnl0ZXNVdGlscy5zY2FsYQ==) | `41.66% <41.66%> (ø)` | |
   | [...rg/apache/kyuubi/sql/KyuubiSparkSQLExtension.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC9LeXV1YmlTcGFya1NRTEV4dGVuc2lvbi5zY2FsYQ==) | `88.88% <50.00%> (-11.12%)` | :arrow_down: |
   | [...bi/sql/zorder/ZorderSparkSqlExtensionsParser.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyU3BhcmtTcWxFeHRlbnNpb25zUGFyc2VyLnNjYWxh) | `68.08% <68.08%> (ø)` | |
   | [...g/apache/kyuubi/sql/zorder/ZorderBeforeWrite.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyQmVmb3JlV3JpdGUuc2NhbGE=) | `74.07% <74.07%> (ø)` | |
   | [...ache/kyuubi/sql/zorder/OptimizeZorderCommand.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvT3B0aW1pemVab3JkZXJDb21tYW5kLnNjYWxh) | `81.81% <81.81%> (ø)` | |
   | [...org/apache/kyuubi/sql/zorder/ZorderException.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyRXhjZXB0aW9uLnNjYWxh) | `100.00% <100.00%> (ø)` | |
   | ... and [8 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [121de7f...9190e31](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] ulysses-you commented on pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
ulysses-you commented on pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#issuecomment-912180029


   @qphien can you fix the conflict ? thanks !


-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] ulysses-you commented on pull request #990: [KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
ulysses-you commented on pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#issuecomment-913521087


   @cfmcgrady @zhouyifan279 @timothy65535 @turboFei feel free to send PR if you have any thought about 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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #990: [KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#issuecomment-907985416


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#990](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3eb209d) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/01bebe36e81995ff38733a2e1b9dd432d2b60b3c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (01bebe3) will **decrease** coverage by `2.12%`.
   > The diff coverage is `48.94%`.
   
   > :exclamation: Current head 3eb209d differs from pull request most recent head 02edd77. Consider uploading reports for the commit 02edd77 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/graphs/tree.svg?width=650&height=150&src=pr&token=925D4tb9AH&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #990      +/-   ##
   ============================================
   - Coverage     80.35%   78.23%   -2.13%     
   - Complexity       14       71      +57     
   ============================================
     Files           154      162       +8     
     Lines          5626     6004     +378     
     Branches        647      722      +75     
   ============================================
   + Hits           4521     4697     +176     
   - Misses          719      879     +160     
   - Partials        386      428      +42     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ache/kyuubi/sql/zorder/KyuubiZorderExtension.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvS3l1dWJpWm9yZGVyRXh0ZW5zaW9uLnNjYWxh) | `0.00% <0.00%> (ø)` | |
   | [...apache/kyuubi/sql/zorder/ZorderSqlAstBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyU3FsQXN0QnVpbGRlci5zY2FsYQ==) | `31.57% <31.57%> (ø)` | |
   | [...in/scala/org/apache/kyuubi/sql/zorder/Zorder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyLnNjYWxh) | `38.09% <38.09%> (ø)` | |
   | [...rg/apache/kyuubi/sql/KyuubiSparkSQLExtension.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC9LeXV1YmlTcGFya1NRTEV4dGVuc2lvbi5zY2FsYQ==) | `88.88% <50.00%> (-11.12%)` | :arrow_down: |
   | [...rg/apache/kyuubi/sql/zorder/ZorderBytesUtils.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyQnl0ZXNVdGlscy5zY2FsYQ==) | `62.66% <62.66%> (ø)` | |
   | [...apache/kyuubi/sql/zorder/OptimizeBeforeWrite.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvT3B0aW1pemVCZWZvcmVXcml0ZS5zY2FsYQ==) | `65.90% <65.90%> (ø)` | |
   | [...bi/sql/zorder/ZorderSparkSqlExtensionsParser.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyU3BhcmtTcWxFeHRlbnNpb25zUGFyc2VyLnNjYWxh) | `68.08% <68.08%> (ø)` | |
   | [...ache/kyuubi/sql/zorder/OptimizeZorderCommand.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvT3B0aW1pemVab3JkZXJDb21tYW5kLnNjYWxh) | `81.81% <81.81%> (ø)` | |
   | [...org/apache/kyuubi/sql/zorder/ZorderException.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyRXhjZXB0aW9uLnNjYWxh) | `100.00% <100.00%> (ø)` | |
   | [.../org/apache/kyuubi/operation/KyuubiOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vS3l1dWJpT3BlcmF0aW9uLnNjYWxh) | `38.18% <0.00%> (-16.37%)` | :arrow_down: |
   | ... and [8 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [01bebe3...02edd77](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] codecov-commenter commented on pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#issuecomment-907985416


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#990](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (531ad50) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/f1ae664910d6cbcf327e126f8bc96f4b8dabb13b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f1ae664) will **decrease** coverage by `3.61%`.
   > The diff coverage is `56.11%`.
   
   > :exclamation: Current head 531ad50 differs from pull request most recent head be6f3b6. Consider uploading reports for the commit be6f3b6 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/graphs/tree.svg?width=650&height=150&src=pr&token=925D4tb9AH&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #990      +/-   ##
   ============================================
   - Coverage     80.77%   77.16%   -3.62%     
   - Complexity       11       88      +77     
   ============================================
     Files           146      161      +15     
     Lines          5389     6025     +636     
     Branches        636      732      +96     
   ============================================
   + Hits           4353     4649     +296     
   - Misses          660      943     +283     
   - Partials        376      433      +57     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ache/kyuubi/sql/zorder/KyuubiZorderExtension.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvS3l1dWJpWm9yZGVyRXh0ZW5zaW9uLnNjYWxh) | `0.00% <0.00%> (ø)` | |
   | [...he/kyuubi/events/AbstractEventLoggingService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9ldmVudHMvQWJzdHJhY3RFdmVudExvZ2dpbmdTZXJ2aWNlLnNjYWxh) | `0.00% <0.00%> (ø)` | |
   | [...ala/org/apache/kyuubi/events/EventLoggerType.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9ldmVudHMvRXZlbnRMb2dnZXJUeXBlLnNjYWxh) | `0.00% <ø> (ø)` | |
   | [...ala/org/apache/kyuubi/events/JsonEventLogger.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9ldmVudHMvSnNvbkV2ZW50TG9nZ2VyLnNjYWxh) | `0.00% <0.00%> (ø)` | |
   | [.../scala/org/apache/kyuubi/events/JsonProtocol.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9ldmVudHMvSnNvblByb3RvY29sLnNjYWxh) | `0.00% <0.00%> (ø)` | |
   | [...n/scala/org/apache/kyuubi/events/KyuubiEvent.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9ldmVudHMvS3l1dWJpRXZlbnQuc2NhbGE=) | `0.00% <0.00%> (ø)` | |
   | [...n/scala/org/apache/kyuubi/service/Serverable.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL1NlcnZlcmFibGUuc2NhbGE=) | `42.85% <ø> (-1.59%)` | :arrow_down: |
   | [.../org/apache/kyuubi/session/KyuubiSessionImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL0t5dXViaVNlc3Npb25JbXBsLnNjYWxh) | `85.29% <0.00%> (-2.59%)` | :arrow_down: |
   | [...rg/apache/spark/sql/execution/ZorderOrdering.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3Bhcmsvc3FsL2V4ZWN1dGlvbi9ab3JkZXJPcmRlcmluZy5zY2FsYQ==) | `28.57% <28.57%> (ø)` | |
   | [...uubi/engine/spark/events/SparkStatementEvent.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9ldmVudHMvU3BhcmtTdGF0ZW1lbnRFdmVudC5zY2FsYQ==) | `83.33% <33.33%> (ø)` | |
   | ... and [52 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [f1ae664...be6f3b6](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] qphien commented on pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
qphien commented on pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#issuecomment-912202129


   > @qphien can you fix the conflict ? thanks !
   
   OK, I have rebased all commit based on master branch.


-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] qphien commented on a change in pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
qphien commented on a change in pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#discussion_r698129898



##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/spark/sql/execution/ZorderBytesUtils.scala
##########
@@ -0,0 +1,143 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution
+
+import java.lang.{Float => jFloat}
+import java.lang.{Double => jDouble}
+import java.nio.charset.Charset
+
+object ZorderBytesUtils {
+  def interleaveMultiByteArray(arrays: Array[Array[Byte]]): Array[Byte] = {
+    var totalLength = 0
+    var maxLength = 0
+    arrays.foreach(array => {
+      totalLength += array.length
+      maxLength = maxLength.max(array.length * 8)
+    })
+    val result = new Array[Byte](totalLength)
+    var resultBit = 0
+
+    for (bit <- 0 to maxLength) {
+      val bytePos = Math.floor(bit / 8).toInt
+      val bitPos = bit % 8
+
+      for (arr <- arrays) {
+        if (bytePos < arr.length) {
+          val resultBytePos = totalLength - 1 - Math.floor(resultBit / 8).toInt
+          val resultBitPos = resultBit % 8
+          result(resultBytePos) = updatePos(result(resultBytePos), resultBitPos,
+            arr(arr.length - 1 - bytePos), bitPos)
+          resultBit += 1
+        }
+      }
+    }
+    result
+  }
+
+  def updatePos(a: Byte, apos: Int, b: Byte, bpos: Int): Byte = {
+    var temp = (b & (1 << bpos)).toByte
+    if (apos > bpos) {
+      temp = (temp << (apos - bpos)).toByte
+    } else if (apos < bpos) {
+      temp = (temp >> (bpos - apos)).toByte
+    }
+    val atemp = (a & (1 << apos)).toByte
+    if (atemp == temp) {
+      return a
+    }
+    (a ^ (1 << apos)).toByte
+  }
+
+  def booleanToByte(a: Boolean): Array[Byte] = {
+    if (a) {
+      intToByte(1)
+    } else {
+      intToByte(0)
+    }
+  }
+
+  def byteToByte(a: Int): Array[Byte] = {
+    val tmp = (a ^ (1 << 7)).toByte
+    Array(tmp)
+  }
+
+  def shortToByte(a: Short): Array[Byte] = {
+    val tmp = a ^ (1 << 15)
+    Array(((tmp >> 8) & 0xff).toByte, (tmp & 0xff).toByte)
+  }
+
+  def intToByte(a: Int): Array[Byte] = {
+    val tmp = a ^ (1 << 31)
+    val result = new Array[Byte](4)
+    for (i <- 0 to 3) {
+      val offset = i * 8
+      result(3 - i) = ((tmp >> offset) & 0xff).toByte
+    }
+    result
+  }
+
+  def longToByte(a: Long): Array[Byte] = {
+    val tmp = a ^ (1 << 63)
+    val result = new Array[Byte](8)
+    for (i <- 0 to 7) {
+      val offset = i * 8
+      result(7 - i) = ((tmp >> offset) & 0xff).toByte
+    }
+    result
+  }
+
+  def floatToByte(a: Float): Array[Byte] = {
+    val fi = jFloat.floatToRawIntBits(a)
+    intToByte(fi)
+  }
+
+  def doubleToByte(a: Double): Array[Byte] = {
+    val dl = jDouble.doubleToRawLongBits(a)
+    longToByte(dl)
+  }
+
+  def stringToByte(str: String): Array[Byte] = {
+    // truncate or padding str to 8 byte

Review comment:
       @cfmcgrady Yep, current implementation can not resolve this problem. I think a way to solve this problem is to map string to rank and then generate z-value with rank value. However, this solution needs to sort all string values globally. In consideration of globally sort cost and z-order data skipping effectiveness, a comprise solution is to take sample of rows and get some ordered bounds, then map each row to bound index. For example:
   ```
   Rows:
   ["http://www.example.com/a",
   "http://www.example.com/d",
   "http://www.example.com/g",
   "http://www.example.com/h",
   "http://www.example.com/i",
   "http://www.example.com/n",
   "http://www.example.com/o",
   "http://www.example.com/q",
   "http://www.example.com/t"]
   
   Sample Bounds:
   ["http://www.example.com/a", "http://www.example.com/g"]
   ["http://www.example.com/h", "http://www.example.com/n"]
   ["http://www.example.com/o", "http://www.example.com/t"]
   
   Map row to bound index:
   { ("http://www.example.com/a" -> 0),
   ("http://www.example.com/d" -> 0),
   ("http://www.example.com/g" -> 0),
   ("http://www.example.com/h" -> 1),
   ("http://www.example.com/i" -> 1),
   ("http://www.example.com/n" -> 1),
   ("http://www.example.com/o" ->2),
   ("http://www.example.com/q" -> 2),
   ("http://www.example.com/t" -> 2)]
   ```
   
   Bound index is used to generate z-value




-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] ulysses-you commented on pull request #990: [KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
ulysses-you commented on pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#issuecomment-913513264


   thanks @qphien , merging to master for 1.4.0


-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] qphien commented on a change in pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
qphien commented on a change in pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#discussion_r698129898



##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/spark/sql/execution/ZorderBytesUtils.scala
##########
@@ -0,0 +1,143 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution
+
+import java.lang.{Float => jFloat}
+import java.lang.{Double => jDouble}
+import java.nio.charset.Charset
+
+object ZorderBytesUtils {
+  def interleaveMultiByteArray(arrays: Array[Array[Byte]]): Array[Byte] = {
+    var totalLength = 0
+    var maxLength = 0
+    arrays.foreach(array => {
+      totalLength += array.length
+      maxLength = maxLength.max(array.length * 8)
+    })
+    val result = new Array[Byte](totalLength)
+    var resultBit = 0
+
+    for (bit <- 0 to maxLength) {
+      val bytePos = Math.floor(bit / 8).toInt
+      val bitPos = bit % 8
+
+      for (arr <- arrays) {
+        if (bytePos < arr.length) {
+          val resultBytePos = totalLength - 1 - Math.floor(resultBit / 8).toInt
+          val resultBitPos = resultBit % 8
+          result(resultBytePos) = updatePos(result(resultBytePos), resultBitPos,
+            arr(arr.length - 1 - bytePos), bitPos)
+          resultBit += 1
+        }
+      }
+    }
+    result
+  }
+
+  def updatePos(a: Byte, apos: Int, b: Byte, bpos: Int): Byte = {
+    var temp = (b & (1 << bpos)).toByte
+    if (apos > bpos) {
+      temp = (temp << (apos - bpos)).toByte
+    } else if (apos < bpos) {
+      temp = (temp >> (bpos - apos)).toByte
+    }
+    val atemp = (a & (1 << apos)).toByte
+    if (atemp == temp) {
+      return a
+    }
+    (a ^ (1 << apos)).toByte
+  }
+
+  def booleanToByte(a: Boolean): Array[Byte] = {
+    if (a) {
+      intToByte(1)
+    } else {
+      intToByte(0)
+    }
+  }
+
+  def byteToByte(a: Int): Array[Byte] = {
+    val tmp = (a ^ (1 << 7)).toByte
+    Array(tmp)
+  }
+
+  def shortToByte(a: Short): Array[Byte] = {
+    val tmp = a ^ (1 << 15)
+    Array(((tmp >> 8) & 0xff).toByte, (tmp & 0xff).toByte)
+  }
+
+  def intToByte(a: Int): Array[Byte] = {
+    val tmp = a ^ (1 << 31)
+    val result = new Array[Byte](4)
+    for (i <- 0 to 3) {
+      val offset = i * 8
+      result(3 - i) = ((tmp >> offset) & 0xff).toByte
+    }
+    result
+  }
+
+  def longToByte(a: Long): Array[Byte] = {
+    val tmp = a ^ (1 << 63)
+    val result = new Array[Byte](8)
+    for (i <- 0 to 7) {
+      val offset = i * 8
+      result(7 - i) = ((tmp >> offset) & 0xff).toByte
+    }
+    result
+  }
+
+  def floatToByte(a: Float): Array[Byte] = {
+    val fi = jFloat.floatToRawIntBits(a)
+    intToByte(fi)
+  }
+
+  def doubleToByte(a: Double): Array[Byte] = {
+    val dl = jDouble.doubleToRawLongBits(a)
+    longToByte(dl)
+  }
+
+  def stringToByte(str: String): Array[Byte] = {
+    // truncate or padding str to 8 byte

Review comment:
       @cfmcgrady Yep, current implementation can not resolve this problem. I think a way to solve this problem is to map string to rank and then generate z-value with rank value. However, this solution needs to sort all string values globally. In consideration of globally sort cost and z-order data skipping effectiveness, a comprise solution is to take sample of rows and get some ordered bounds, then map each row to bound index. For example:
   ```
   Rows:
   ["http://www.example.com/a",
   "http://www.example.com/d",
   "http://www.example.com/g",
   "http://www.example.com/h",
   "http://www.example.com/i",
   "http://www.example.com/n",
   "http://www.example.com/o",
   "http://www.example.com/q",
   "http://www.example.com/t"]
   
   Sample Bounds:
   ["http://www.example.com/a", "http://www.example.com/g"]
   ["http://www.example.com/h", "http://www.example.com/n"]
   ["http://www.example.com/o", "http://www.example.com/t"]
   
   Map row to bound index:
   { 
   ("http://www.example.com/a" -> 0),
   ("http://www.example.com/d" -> 0),
   ("http://www.example.com/g" -> 0),
   ("http://www.example.com/h" -> 1),
   ("http://www.example.com/i" -> 1),
   ("http://www.example.com/n" -> 1),
   ("http://www.example.com/o" ->2),
   ("http://www.example.com/q" -> 2),
   ("http://www.example.com/t" -> 2)
   }
   ```
   
   Bound index is used to generate z-value




-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] cfmcgrady commented on a change in pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
cfmcgrady commented on a change in pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#discussion_r698141724



##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/spark/sql/execution/ZorderBytesUtils.scala
##########
@@ -0,0 +1,143 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution
+
+import java.lang.{Float => jFloat}
+import java.lang.{Double => jDouble}
+import java.nio.charset.Charset
+
+object ZorderBytesUtils {
+  def interleaveMultiByteArray(arrays: Array[Array[Byte]]): Array[Byte] = {
+    var totalLength = 0
+    var maxLength = 0
+    arrays.foreach(array => {
+      totalLength += array.length
+      maxLength = maxLength.max(array.length * 8)
+    })
+    val result = new Array[Byte](totalLength)
+    var resultBit = 0
+
+    for (bit <- 0 to maxLength) {
+      val bytePos = Math.floor(bit / 8).toInt
+      val bitPos = bit % 8
+
+      for (arr <- arrays) {
+        if (bytePos < arr.length) {
+          val resultBytePos = totalLength - 1 - Math.floor(resultBit / 8).toInt
+          val resultBitPos = resultBit % 8
+          result(resultBytePos) = updatePos(result(resultBytePos), resultBitPos,
+            arr(arr.length - 1 - bytePos), bitPos)
+          resultBit += 1
+        }
+      }
+    }
+    result
+  }
+
+  def updatePos(a: Byte, apos: Int, b: Byte, bpos: Int): Byte = {
+    var temp = (b & (1 << bpos)).toByte
+    if (apos > bpos) {
+      temp = (temp << (apos - bpos)).toByte
+    } else if (apos < bpos) {
+      temp = (temp >> (bpos - apos)).toByte
+    }
+    val atemp = (a & (1 << apos)).toByte
+    if (atemp == temp) {
+      return a
+    }
+    (a ^ (1 << apos)).toByte
+  }
+
+  def booleanToByte(a: Boolean): Array[Byte] = {
+    if (a) {
+      intToByte(1)
+    } else {
+      intToByte(0)
+    }
+  }
+
+  def byteToByte(a: Int): Array[Byte] = {
+    val tmp = (a ^ (1 << 7)).toByte
+    Array(tmp)
+  }
+
+  def shortToByte(a: Short): Array[Byte] = {
+    val tmp = a ^ (1 << 15)
+    Array(((tmp >> 8) & 0xff).toByte, (tmp & 0xff).toByte)
+  }
+
+  def intToByte(a: Int): Array[Byte] = {
+    val tmp = a ^ (1 << 31)
+    val result = new Array[Byte](4)
+    for (i <- 0 to 3) {
+      val offset = i * 8
+      result(3 - i) = ((tmp >> offset) & 0xff).toByte
+    }
+    result
+  }
+
+  def longToByte(a: Long): Array[Byte] = {
+    val tmp = a ^ (1 << 63)
+    val result = new Array[Byte](8)
+    for (i <- 0 to 7) {
+      val offset = i * 8
+      result(7 - i) = ((tmp >> offset) & 0xff).toByte
+    }
+    result
+  }
+
+  def floatToByte(a: Float): Array[Byte] = {
+    val fi = jFloat.floatToRawIntBits(a)
+    intToByte(fi)
+  }
+
+  def doubleToByte(a: Double): Array[Byte] = {
+    val dl = jDouble.doubleToRawLongBits(a)
+    longToByte(dl)
+  }
+
+  def stringToByte(str: String): Array[Byte] = {
+    // truncate or padding str to 8 byte

Review comment:
       This is a more general solution as I know. Since this is an approximate solution, do we have a benchmark?




-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#issuecomment-907985416


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#990](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3eb209d) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/01bebe36e81995ff38733a2e1b9dd432d2b60b3c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (01bebe3) will **decrease** coverage by `2.12%`.
   > The diff coverage is `48.94%`.
   
   > :exclamation: Current head 3eb209d differs from pull request most recent head 6c9d176. Consider uploading reports for the commit 6c9d176 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/graphs/tree.svg?width=650&height=150&src=pr&token=925D4tb9AH&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #990      +/-   ##
   ============================================
   - Coverage     80.35%   78.23%   -2.13%     
   - Complexity       14       71      +57     
   ============================================
     Files           154      162       +8     
     Lines          5626     6004     +378     
     Branches        647      722      +75     
   ============================================
   + Hits           4521     4697     +176     
   - Misses          719      879     +160     
   - Partials        386      428      +42     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ache/kyuubi/sql/zorder/KyuubiZorderExtension.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvS3l1dWJpWm9yZGVyRXh0ZW5zaW9uLnNjYWxh) | `0.00% <0.00%> (ø)` | |
   | [...apache/kyuubi/sql/zorder/ZorderSqlAstBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyU3FsQXN0QnVpbGRlci5zY2FsYQ==) | `31.57% <31.57%> (ø)` | |
   | [...in/scala/org/apache/kyuubi/sql/zorder/Zorder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyLnNjYWxh) | `38.09% <38.09%> (ø)` | |
   | [...rg/apache/kyuubi/sql/KyuubiSparkSQLExtension.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC9LeXV1YmlTcGFya1NRTEV4dGVuc2lvbi5zY2FsYQ==) | `88.88% <50.00%> (-11.12%)` | :arrow_down: |
   | [...rg/apache/kyuubi/sql/zorder/ZorderBytesUtils.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyQnl0ZXNVdGlscy5zY2FsYQ==) | `62.66% <62.66%> (ø)` | |
   | [...apache/kyuubi/sql/zorder/OptimizeBeforeWrite.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvT3B0aW1pemVCZWZvcmVXcml0ZS5zY2FsYQ==) | `65.90% <65.90%> (ø)` | |
   | [...bi/sql/zorder/ZorderSparkSqlExtensionsParser.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyU3BhcmtTcWxFeHRlbnNpb25zUGFyc2VyLnNjYWxh) | `68.08% <68.08%> (ø)` | |
   | [...ache/kyuubi/sql/zorder/OptimizeZorderCommand.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvT3B0aW1pemVab3JkZXJDb21tYW5kLnNjYWxh) | `81.81% <81.81%> (ø)` | |
   | [...org/apache/kyuubi/sql/zorder/ZorderException.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyRXhjZXB0aW9uLnNjYWxh) | `100.00% <100.00%> (ø)` | |
   | [.../org/apache/kyuubi/operation/KyuubiOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vS3l1dWJpT3BlcmF0aW9uLnNjYWxh) | `38.18% <0.00%> (-16.37%)` | :arrow_down: |
   | ... and [8 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [01bebe3...6c9d176](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] cfmcgrady commented on a change in pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
cfmcgrady commented on a change in pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#discussion_r696507532



##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/spark/sql/execution/ZorderBytesUtils.scala
##########
@@ -0,0 +1,143 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution
+
+import java.lang.{Float => jFloat}
+import java.lang.{Double => jDouble}
+import java.nio.charset.Charset
+
+object ZorderBytesUtils {
+  def interleaveMultiByteArray(arrays: Array[Array[Byte]]): Array[Byte] = {
+    var totalLength = 0
+    var maxLength = 0
+    arrays.foreach(array => {
+      totalLength += array.length
+      maxLength = maxLength.max(array.length * 8)
+    })
+    val result = new Array[Byte](totalLength)
+    var resultBit = 0
+
+    for (bit <- 0 to maxLength) {
+      val bytePos = Math.floor(bit / 8).toInt
+      val bitPos = bit % 8
+
+      for (arr <- arrays) {
+        if (bytePos < arr.length) {
+          val resultBytePos = totalLength - 1 - Math.floor(resultBit / 8).toInt
+          val resultBitPos = resultBit % 8
+          result(resultBytePos) = updatePos(result(resultBytePos), resultBitPos,
+            arr(arr.length - 1 - bytePos), bitPos)
+          resultBit += 1
+        }
+      }
+    }
+    result
+  }
+
+  def updatePos(a: Byte, apos: Int, b: Byte, bpos: Int): Byte = {
+    var temp = (b & (1 << bpos)).toByte
+    if (apos > bpos) {
+      temp = (temp << (apos - bpos)).toByte
+    } else if (apos < bpos) {
+      temp = (temp >> (bpos - apos)).toByte
+    }
+    val atemp = (a & (1 << apos)).toByte
+    if (atemp == temp) {
+      return a
+    }
+    (a ^ (1 << apos)).toByte
+  }
+
+  def booleanToByte(a: Boolean): Array[Byte] = {
+    if (a) {
+      intToByte(1)
+    } else {
+      intToByte(0)
+    }
+  }
+
+  def byteToByte(a: Int): Array[Byte] = {
+    val tmp = (a ^ (1 << 7)).toByte
+    Array(tmp)
+  }
+
+  def shortToByte(a: Short): Array[Byte] = {
+    val tmp = a ^ (1 << 15)
+    Array(((tmp >> 8) & 0xff).toByte, (tmp & 0xff).toByte)
+  }
+
+  def intToByte(a: Int): Array[Byte] = {
+    val tmp = a ^ (1 << 31)
+    val result = new Array[Byte](4)
+    for (i <- 0 to 3) {
+      val offset = i * 8
+      result(3 - i) = ((tmp >> offset) & 0xff).toByte
+    }
+    result
+  }
+
+  def longToByte(a: Long): Array[Byte] = {
+    val tmp = a ^ (1 << 63)
+    val result = new Array[Byte](8)
+    for (i <- 0 to 7) {
+      val offset = i * 8
+      result(7 - i) = ((tmp >> offset) & 0xff).toByte
+    }
+    result
+  }
+
+  def floatToByte(a: Float): Array[Byte] = {
+    val fi = jFloat.floatToRawIntBits(a)
+    intToByte(fi)
+  }
+
+  def doubleToByte(a: Double): Array[Byte] = {
+    val dl = jDouble.doubleToRawLongBits(a)
+    longToByte(dl)
+  }
+
+  def stringToByte(str: String): Array[Byte] = {
+    // truncate or padding str to 8 byte

Review comment:
       In the current implementation, only the first 8 bytes of string will be used to generate zvalue, It's limited when the input string is greater than 8 bytes.
   Suppose we have the following table:
   
   |col_a|
   | --- |
   |http://www.example.com/a|
   |http://www.example.com/b|
   |http://www.example.com/c|
   |http://www.example.com/d|
   
   the zvalue of these rows:
   
   ```scala
   Seq(
     "http://www.example.com/a",
     "http://www.example.com/b",
     "http://www.example.com/c",
     "http://www.example.com/d",
   ).foreach(s => {
     println(s"input string: ${s}, zvalue: ${ZorderBytesUtils.stringToByte(s).mkString(",")}")
   })
   ```
   ```
   input string: http://www.example.com/a, zvalue: 104,116,116,112,58,47,47,119
   input string: http://www.example.com/b, zvalue: 104,116,116,112,58,47,47,119
   input string: http://www.example.com/c, zvalue: 104,116,116,112,58,47,47,119
   input string: http://www.example.com/d, zvalue: 104,116,116,112,58,47,47,119
   ```
   
   All of these rows have the same zvalue, and we can't do effective data skipping here.
   




-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#issuecomment-907985416


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#990](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (16971ec) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/f1ae664910d6cbcf327e126f8bc96f4b8dabb13b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f1ae664) will **decrease** coverage by `3.56%`.
   > The diff coverage is `47.10%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/graphs/tree.svg?width=650&height=150&src=pr&token=925D4tb9AH&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #990      +/-   ##
   ============================================
   - Coverage     80.77%   77.20%   -3.57%     
   - Complexity       11       88      +77     
   ============================================
     Files           146      161      +15     
     Lines          5389     6015     +626     
     Branches        636      728      +92     
   ============================================
   + Hits           4353     4644     +291     
   - Misses          660      943     +283     
   - Partials        376      428      +52     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ache/kyuubi/sql/zorder/KyuubiZorderExtension.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvS3l1dWJpWm9yZGVyRXh0ZW5zaW9uLnNjYWxh) | `0.00% <0.00%> (ø)` | |
   | [...rg/apache/spark/sql/execution/ZorderOrdering.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3Bhcmsvc3FsL2V4ZWN1dGlvbi9ab3JkZXJPcmRlcmluZy5zY2FsYQ==) | `28.57% <28.57%> (ø)` | |
   | [...apache/kyuubi/sql/zorder/ZorderSqlAstBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyU3FsQXN0QnVpbGRlci5zY2FsYQ==) | `40.29% <40.29%> (ø)` | |
   | [...rg/apache/spark/sql/execution/ZorderSortExec.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3Bhcmsvc3FsL2V4ZWN1dGlvbi9ab3JkZXJTb3J0RXhlYy5zY2FsYQ==) | `44.70% <44.70%> (ø)` | |
   | [...cala/org/apache/kyuubi/sql/zorder/ZorderSort.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyU29ydC5zY2FsYQ==) | `50.00% <50.00%> (ø)` | |
   | [...ache/spark/sql/execution/OptimizeBeforeWrite.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3Bhcmsvc3FsL2V4ZWN1dGlvbi9PcHRpbWl6ZUJlZm9yZVdyaXRlLnNjYWxh) | `50.57% <50.57%> (ø)` | |
   | [.../apache/spark/sql/execution/ZorderBytesUtils.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3Bhcmsvc3FsL2V4ZWN1dGlvbi9ab3JkZXJCeXRlc1V0aWxzLnNjYWxh) | `50.66% <50.66%> (ø)` | |
   | [...rg/apache/kyuubi/sql/KyuubiSparkSQLExtension.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC9LeXV1YmlTcGFya1NRTEV4dGVuc2lvbi5zY2FsYQ==) | `88.88% <66.66%> (-11.12%)` | :arrow_down: |
   | [...bi/sql/zorder/ZorderSparkSqlExtensionsParser.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyU3BhcmtTcWxFeHRlbnNpb25zUGFyc2VyLnNjYWxh) | `68.08% <68.08%> (ø)` | |
   | [...ache/kyuubi/sql/zorder/OptimizeZorderCommand.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvT3B0aW1pemVab3JkZXJDb21tYW5kLnNjYWxh) | `80.00% <80.00%> (ø)` | |
   | ... and [41 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [f1ae664...16971ec](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] qphien commented on a change in pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
qphien commented on a change in pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#discussion_r698920731



##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/kyuubi/sql/zorder/ZorderSparkSqlExtensionsParser.scala
##########
@@ -0,0 +1,134 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.sql.zorder
+
+import java.util.Locale
+
+import org.antlr.v4.runtime._
+import org.antlr.v4.runtime.atn.PredictionMode
+import org.antlr.v4.runtime.misc.{Interval, ParseCancellationException}
+import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.catalyst.{FunctionIdentifier, TableIdentifier}
+import org.apache.spark.sql.catalyst.expressions.Expression
+import org.apache.spark.sql.catalyst.parser.{ParseErrorListener, ParseException, ParserInterface, PostProcessor}
+import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
+import org.apache.spark.sql.catalyst.trees.Origin
+import org.apache.spark.sql.types.{DataType, StructType}
+
+class ZorderSparkSqlExtensionsParser(delegate: ParserInterface) extends ParserInterface {
+
+  private lazy val astBuilder = new ZorderSqlAstBuilder
+  override def parsePlan(sqlText: String): LogicalPlan = {
+    if (isZorderCommand(sqlText)) {
+      parse(sqlText) { parser =>
+        astBuilder.visit(parser.singleStatement()) }.asInstanceOf[LogicalPlan]
+    } else {
+      delegate.parsePlan(sqlText)
+    }
+  }
+
+  protected def parse[T](command: String)(toResult: ZorderSqlExtensionsParser => T): T = {
+    val lexer = new ZorderSqlExtensionsLexer(
+      new UpperCaseCharStream(CharStreams.fromString(command)))
+    lexer.removeErrorListeners()
+    lexer.addErrorListener(ParseErrorListener)
+
+    val tokenStream = new CommonTokenStream(lexer)
+    val parser = new ZorderSqlExtensionsParser(tokenStream)
+    parser.addParseListener(PostProcessor)
+    parser.removeErrorListeners()
+    parser.addErrorListener(ParseErrorListener)
+
+    try {
+      try {
+        // first, try parsing with potentially faster SLL mode
+        parser.getInterpreter.setPredictionMode(PredictionMode.SLL)
+        toResult(parser)
+      }
+      catch {
+        case _: ParseCancellationException =>
+          // if we fail, parse with LL mode
+          tokenStream.seek(0) // rewind input stream
+          parser.reset()
+
+          // Try Again.
+          parser.getInterpreter.setPredictionMode(PredictionMode.LL)
+          toResult(parser)
+      }
+    }
+    catch {
+      case e: ParseException if e.command.isDefined =>
+        throw e
+      case e: ParseException =>
+        throw e.withCommand(command)
+      case e: AnalysisException =>
+        val position = Origin(e.line, e.startPosition)
+        throw new ParseException(Option(command), e.message, position, position)
+    }
+  }
+
+  private def isZorderCommand(sqlText: String): Boolean = {
+    val normalized = sqlText.toLowerCase(Locale.ROOT).trim().replaceAll("\\s+", " ")

Review comment:
       @cfmcgrady In this case, just `OPTIMIZE` SQL is supported, I think both are okay.




-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#issuecomment-907985416


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#990](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (16971ec) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/01bebe36e81995ff38733a2e1b9dd432d2b60b3c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (01bebe3) will **decrease** coverage by `3.15%`.
   > The diff coverage is `47.10%`.
   
   > :exclamation: Current head 16971ec differs from pull request most recent head 8f3f4a6. Consider uploading reports for the commit 8f3f4a6 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/graphs/tree.svg?width=650&height=150&src=pr&token=925D4tb9AH&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #990      +/-   ##
   ============================================
   - Coverage     80.35%   77.20%   -3.16%     
   - Complexity       14       88      +74     
   ============================================
     Files           154      161       +7     
     Lines          5626     6015     +389     
     Branches        647      728      +81     
   ============================================
   + Hits           4521     4644     +123     
   - Misses          719      943     +224     
   - Partials        386      428      +42     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ache/kyuubi/sql/zorder/KyuubiZorderExtension.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvS3l1dWJpWm9yZGVyRXh0ZW5zaW9uLnNjYWxh) | `0.00% <0.00%> (ø)` | |
   | [...rg/apache/spark/sql/execution/ZorderOrdering.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3Bhcmsvc3FsL2V4ZWN1dGlvbi9ab3JkZXJPcmRlcmluZy5zY2FsYQ==) | `28.57% <28.57%> (ø)` | |
   | [...apache/kyuubi/sql/zorder/ZorderSqlAstBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyU3FsQXN0QnVpbGRlci5zY2FsYQ==) | `40.29% <40.29%> (ø)` | |
   | [...rg/apache/spark/sql/execution/ZorderSortExec.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3Bhcmsvc3FsL2V4ZWN1dGlvbi9ab3JkZXJTb3J0RXhlYy5zY2FsYQ==) | `44.70% <44.70%> (ø)` | |
   | [...cala/org/apache/kyuubi/sql/zorder/ZorderSort.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyU29ydC5zY2FsYQ==) | `50.00% <50.00%> (ø)` | |
   | [...ache/spark/sql/execution/OptimizeBeforeWrite.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3Bhcmsvc3FsL2V4ZWN1dGlvbi9PcHRpbWl6ZUJlZm9yZVdyaXRlLnNjYWxh) | `50.57% <50.57%> (ø)` | |
   | [.../apache/spark/sql/execution/ZorderBytesUtils.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3Bhcmsvc3FsL2V4ZWN1dGlvbi9ab3JkZXJCeXRlc1V0aWxzLnNjYWxh) | `50.66% <50.66%> (ø)` | |
   | [...rg/apache/kyuubi/sql/KyuubiSparkSQLExtension.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC9LeXV1YmlTcGFya1NRTEV4dGVuc2lvbi5zY2FsYQ==) | `88.88% <66.66%> (-11.12%)` | :arrow_down: |
   | [...bi/sql/zorder/ZorderSparkSqlExtensionsParser.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyU3BhcmtTcWxFeHRlbnNpb25zUGFyc2VyLnNjYWxh) | `68.08% <68.08%> (ø)` | |
   | [...ache/kyuubi/sql/zorder/OptimizeZorderCommand.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvT3B0aW1pemVab3JkZXJDb21tYW5kLnNjYWxh) | `80.00% <80.00%> (ø)` | |
   | ... and [21 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [01bebe3...8f3f4a6](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] qphien commented on a change in pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
qphien commented on a change in pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#discussion_r698919583



##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/spark/sql/execution/ZorderBytesUtils.scala
##########
@@ -0,0 +1,143 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution
+
+import java.lang.{Float => jFloat}
+import java.lang.{Double => jDouble}
+import java.nio.charset.Charset
+
+object ZorderBytesUtils {
+  def interleaveMultiByteArray(arrays: Array[Array[Byte]]): Array[Byte] = {
+    var totalLength = 0
+    var maxLength = 0
+    arrays.foreach(array => {
+      totalLength += array.length
+      maxLength = maxLength.max(array.length * 8)
+    })
+    val result = new Array[Byte](totalLength)
+    var resultBit = 0
+
+    for (bit <- 0 to maxLength) {
+      val bytePos = Math.floor(bit / 8).toInt
+      val bitPos = bit % 8
+
+      for (arr <- arrays) {
+        if (bytePos < arr.length) {
+          val resultBytePos = totalLength - 1 - Math.floor(resultBit / 8).toInt
+          val resultBitPos = resultBit % 8
+          result(resultBytePos) = updatePos(result(resultBytePos), resultBitPos,
+            arr(arr.length - 1 - bytePos), bitPos)
+          resultBit += 1
+        }
+      }
+    }
+    result
+  }
+
+  def updatePos(a: Byte, apos: Int, b: Byte, bpos: Int): Byte = {
+    var temp = (b & (1 << bpos)).toByte
+    if (apos > bpos) {
+      temp = (temp << (apos - bpos)).toByte
+    } else if (apos < bpos) {
+      temp = (temp >> (bpos - apos)).toByte
+    }
+    val atemp = (a & (1 << apos)).toByte
+    if (atemp == temp) {
+      return a
+    }
+    (a ^ (1 << apos)).toByte
+  }
+
+  def booleanToByte(a: Boolean): Array[Byte] = {
+    if (a) {
+      intToByte(1)
+    } else {
+      intToByte(0)
+    }
+  }
+
+  def byteToByte(a: Int): Array[Byte] = {
+    val tmp = (a ^ (1 << 7)).toByte
+    Array(tmp)
+  }
+
+  def shortToByte(a: Short): Array[Byte] = {
+    val tmp = a ^ (1 << 15)
+    Array(((tmp >> 8) & 0xff).toByte, (tmp & 0xff).toByte)
+  }
+
+  def intToByte(a: Int): Array[Byte] = {
+    val tmp = a ^ (1 << 31)
+    val result = new Array[Byte](4)
+    for (i <- 0 to 3) {
+      val offset = i * 8
+      result(3 - i) = ((tmp >> offset) & 0xff).toByte
+    }
+    result
+  }
+
+  def longToByte(a: Long): Array[Byte] = {
+    val tmp = a ^ (1 << 63)
+    val result = new Array[Byte](8)
+    for (i <- 0 to 7) {
+      val offset = i * 8
+      result(7 - i) = ((tmp >> offset) & 0xff).toByte
+    }
+    result
+  }
+
+  def floatToByte(a: Float): Array[Byte] = {
+    val fi = jFloat.floatToRawIntBits(a)
+    intToByte(fi)
+  }
+
+  def doubleToByte(a: Double): Array[Byte] = {
+    val dl = jDouble.doubleToRawLongBits(a)
+    longToByte(dl)
+  }
+
+  def stringToByte(str: String): Array[Byte] = {
+    // truncate or padding str to 8 byte

Review comment:
       @cfmcgrady Not yet, benchmark will be made when this PR is completed.

##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/kyuubi/sql/zorder/ZorderSparkSqlExtensionsParser.scala
##########
@@ -0,0 +1,134 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.sql.zorder
+
+import java.util.Locale
+
+import org.antlr.v4.runtime._
+import org.antlr.v4.runtime.atn.PredictionMode
+import org.antlr.v4.runtime.misc.{Interval, ParseCancellationException}
+import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.catalyst.{FunctionIdentifier, TableIdentifier}
+import org.apache.spark.sql.catalyst.expressions.Expression
+import org.apache.spark.sql.catalyst.parser.{ParseErrorListener, ParseException, ParserInterface, PostProcessor}
+import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
+import org.apache.spark.sql.catalyst.trees.Origin
+import org.apache.spark.sql.types.{DataType, StructType}
+
+class ZorderSparkSqlExtensionsParser(delegate: ParserInterface) extends ParserInterface {
+
+  private lazy val astBuilder = new ZorderSqlAstBuilder
+  override def parsePlan(sqlText: String): LogicalPlan = {
+    if (isZorderCommand(sqlText)) {
+      parse(sqlText) { parser =>
+        astBuilder.visit(parser.singleStatement()) }.asInstanceOf[LogicalPlan]
+    } else {
+      delegate.parsePlan(sqlText)
+    }
+  }
+
+  protected def parse[T](command: String)(toResult: ZorderSqlExtensionsParser => T): T = {
+    val lexer = new ZorderSqlExtensionsLexer(
+      new UpperCaseCharStream(CharStreams.fromString(command)))
+    lexer.removeErrorListeners()
+    lexer.addErrorListener(ParseErrorListener)
+
+    val tokenStream = new CommonTokenStream(lexer)
+    val parser = new ZorderSqlExtensionsParser(tokenStream)
+    parser.addParseListener(PostProcessor)
+    parser.removeErrorListeners()
+    parser.addErrorListener(ParseErrorListener)
+
+    try {
+      try {
+        // first, try parsing with potentially faster SLL mode
+        parser.getInterpreter.setPredictionMode(PredictionMode.SLL)
+        toResult(parser)
+      }
+      catch {
+        case _: ParseCancellationException =>
+          // if we fail, parse with LL mode
+          tokenStream.seek(0) // rewind input stream
+          parser.reset()
+
+          // Try Again.
+          parser.getInterpreter.setPredictionMode(PredictionMode.LL)
+          toResult(parser)
+      }
+    }
+    catch {
+      case e: ParseException if e.command.isDefined =>
+        throw e
+      case e: ParseException =>
+        throw e.withCommand(command)
+      case e: AnalysisException =>
+        val position = Origin(e.line, e.startPosition)
+        throw new ParseException(Option(command), e.message, position, position)
+    }
+  }
+
+  private def isZorderCommand(sqlText: String): Boolean = {
+    val normalized = sqlText.toLowerCase(Locale.ROOT).trim().replaceAll("\\s+", " ")

Review comment:
       @cfmcgrady In this case, just `OPTIMIZE` SQL is supported, I think both are okay.

##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/spark/sql/execution/ZorderOrdering.scala
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution
+
+import org.apache.spark.sql.catalyst.InternalRow
+import org.apache.spark.sql.catalyst.expressions.{AttributeReference, BaseOrdering, SortOrder}
+import org.apache.spark.sql.types.{BinaryType, BooleanType, ByteType, DoubleType, FloatType, IntegerType, LongType, ShortType, StringType, StructType}
+
+class ZorderOrdering(ordering: Seq[SortOrder],
+                      schema: StructType) extends BaseOrdering {

Review comment:
       OK, It is more concise to use zorder UDF compared with current solution.




-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] cfmcgrady commented on a change in pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
cfmcgrady commented on a change in pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#discussion_r696490927



##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/spark/sql/execution/ZorderBytesUtils.scala
##########
@@ -0,0 +1,143 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution
+
+import java.lang.{Float => jFloat}
+import java.lang.{Double => jDouble}
+import java.nio.charset.Charset
+
+object ZorderBytesUtils {
+  def interleaveMultiByteArray(arrays: Array[Array[Byte]]): Array[Byte] = {
+    var totalLength = 0
+    var maxLength = 0
+    arrays.foreach(array => {
+      totalLength += array.length
+      maxLength = maxLength.max(array.length * 8)
+    })
+    val result = new Array[Byte](totalLength)
+    var resultBit = 0
+
+    for (bit <- 0 to maxLength) {

Review comment:
       As per https://github.com/databricks/scala-style-guide#traversal-and-zipwithindex, we should use while loop for the performance-sensitive code.




-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] ulysses-you commented on a change in pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
ulysses-you commented on a change in pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#discussion_r698272949



##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/kyuubi/sql/zorder/OptimizeZorderCommand.scala
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.sql.zorder
+
+import org.apache.spark.sql.catalyst.analysis.{UnresolvedRelation, UnresolvedStar}
+import org.apache.spark.sql.catalyst.expressions.{Attribute, Expression, SortOrder}
+import org.apache.spark.sql.catalyst.plans.logical.{Filter, LogicalPlan, Project, UnaryNode}
+
+case class OptimizeZorderCommand(child: LogicalPlan) extends UnaryNode {
+  override def output: Seq[Attribute] = child.output
+}
+
+object OptimizeZorderCommand {
+  def apply(tableIdent: Seq[String],
+            whereExp: Option[Expression],
+            sortOrder: Seq[SortOrder]): OptimizeZorderCommand = {
+    val table = UnresolvedRelation(tableIdent)
+    val child = whereExp match {
+      case Some(x) => Project(Seq(UnresolvedStar(None)), Filter(x, table))
+      case None => Project(Seq(UnresolvedStar(None)), table)

Review comment:
       Do we really need `Project` ?

##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/spark/sql/execution/ZorderOrdering.scala
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution
+
+import org.apache.spark.sql.catalyst.InternalRow
+import org.apache.spark.sql.catalyst.expressions.{AttributeReference, BaseOrdering, SortOrder}
+import org.apache.spark.sql.types.{BinaryType, BooleanType, ByteType, DoubleType, FloatType, IntegerType, LongType, ShortType, StringType, StructType}
+
+class ZorderOrdering(ordering: Seq[SortOrder],
+                      schema: StructType) extends BaseOrdering {
+  private lazy val projectList = ordering.map {
+    case o if o.child.isInstanceOf[AttributeReference] =>
+      o.child.asInstanceOf[AttributeReference]
+  }
+
+  private lazy val indexFields = {
+    val columnsMap = this.schema.fields.map(item => (item.name, item)).toMap
+    projectList.map(item => {
+      val newItem = columnsMap(item.name)
+      (this.schema.fields.indexOf(newItem), newItem)
+    })
+  }
+
+  private def getZvalue(row: InternalRow): Array[Byte] = {
+    val values = indexFields.map {
+      case (index, field) =>
+        field.dataType match {
+          case BooleanType =>
+            ZorderBytesUtils.booleanToByte(
+              if (row.isNullAt(index)) false else row.getBoolean(index)
+            )
+          case ByteType =>
+            ZorderBytesUtils.byteToByte(
+              if (row.isNullAt(index)) Byte.MaxValue else row.getByte(index)
+            )
+          case ShortType =>
+            ZorderBytesUtils.shortToByte(
+              if (row.isNullAt(index)) Short.MaxValue else row.getShort(index)
+            )
+          case IntegerType =>
+            ZorderBytesUtils.intToByte(
+              if (row.isNullAt(index)) Int.MaxValue else row.getInt(index)
+            )
+          case LongType =>
+            ZorderBytesUtils.longToByte(
+              if (row.isNullAt(index)) Long.MaxValue else row.getLong(index)
+            )
+          case FloatType =>
+            ZorderBytesUtils.floatToByte(
+              if (row.isNullAt(index)) Float.MaxValue else row.getFloat(index)
+            )
+          case DoubleType =>
+            ZorderBytesUtils.doubleToByte(
+              if (row.isNullAt(index)) Double.MaxValue else row.getDouble(index)
+            )
+          case StringType =>
+            ZorderBytesUtils.stringToByte(
+              if (row.isNullAt(index)) "" else row.getString(index)
+            )
+          case _ => null
+//          case DateType
+//          case TimestampType
+//          case DecimalType

Review comment:
       if we don't support them now, we can add exception during analysis

##########
File path: dev/kyuubi-extension-spark-3-1/src/main/scala/org/apache/spark/sql/execution/ZorderOrdering.scala
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution
+
+import org.apache.spark.sql.catalyst.InternalRow
+import org.apache.spark.sql.catalyst.expressions.{AttributeReference, BaseOrdering, SortOrder}
+import org.apache.spark.sql.types.{BinaryType, BooleanType, ByteType, DoubleType, FloatType, IntegerType, LongType, ShortType, StringType, StructType}
+
+class ZorderOrdering(ordering: Seq[SortOrder],
+                      schema: StructType) extends BaseOrdering {

Review comment:
       Can we make a new `Expression` which return the binary type value and produced by Z-order ?
   
   IMO we can apply Z-order with Spark `Ordering` simply, like `Sort(zorder(c1, c2, c3), ...)` instead. We can still name it as `zorder by c1, c2, c3` in `.g4`. The benifits of this approach is:
   
   * more cleaner since less code copy
   * support global z-order, otherwise we need to add a new partitioner




-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #990: [WIP][KYUUBI #939] Add Z-Order extensions to support optimize SQL

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #990:
URL: https://github.com/apache/incubator-kyuubi/pull/990#issuecomment-907985416


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#990](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (16971ec) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/f1ae664910d6cbcf327e126f8bc96f4b8dabb13b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f1ae664) will **decrease** coverage by `3.56%`.
   > The diff coverage is `47.10%`.
   
   > :exclamation: Current head 16971ec differs from pull request most recent head 8db909c. Consider uploading reports for the commit 8db909c to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/graphs/tree.svg?width=650&height=150&src=pr&token=925D4tb9AH&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #990      +/-   ##
   ============================================
   - Coverage     80.77%   77.20%   -3.57%     
   - Complexity       11       88      +77     
   ============================================
     Files           146      161      +15     
     Lines          5389     6015     +626     
     Branches        636      728      +92     
   ============================================
   + Hits           4353     4644     +291     
   - Misses          660      943     +283     
   - Partials        376      428      +52     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ache/kyuubi/sql/zorder/KyuubiZorderExtension.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvS3l1dWJpWm9yZGVyRXh0ZW5zaW9uLnNjYWxh) | `0.00% <0.00%> (ø)` | |
   | [...rg/apache/spark/sql/execution/ZorderOrdering.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3Bhcmsvc3FsL2V4ZWN1dGlvbi9ab3JkZXJPcmRlcmluZy5zY2FsYQ==) | `28.57% <28.57%> (ø)` | |
   | [...apache/kyuubi/sql/zorder/ZorderSqlAstBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyU3FsQXN0QnVpbGRlci5zY2FsYQ==) | `40.29% <40.29%> (ø)` | |
   | [...rg/apache/spark/sql/execution/ZorderSortExec.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3Bhcmsvc3FsL2V4ZWN1dGlvbi9ab3JkZXJTb3J0RXhlYy5zY2FsYQ==) | `44.70% <44.70%> (ø)` | |
   | [...cala/org/apache/kyuubi/sql/zorder/ZorderSort.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyU29ydC5zY2FsYQ==) | `50.00% <50.00%> (ø)` | |
   | [...ache/spark/sql/execution/OptimizeBeforeWrite.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3Bhcmsvc3FsL2V4ZWN1dGlvbi9PcHRpbWl6ZUJlZm9yZVdyaXRlLnNjYWxh) | `50.57% <50.57%> (ø)` | |
   | [.../apache/spark/sql/execution/ZorderBytesUtils.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3Bhcmsvc3FsL2V4ZWN1dGlvbi9ab3JkZXJCeXRlc1V0aWxzLnNjYWxh) | `50.66% <50.66%> (ø)` | |
   | [...rg/apache/kyuubi/sql/KyuubiSparkSQLExtension.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC9LeXV1YmlTcGFya1NRTEV4dGVuc2lvbi5zY2FsYQ==) | `88.88% <66.66%> (-11.12%)` | :arrow_down: |
   | [...bi/sql/zorder/ZorderSparkSqlExtensionsParser.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvWm9yZGVyU3BhcmtTcWxFeHRlbnNpb25zUGFyc2VyLnNjYWxh) | `68.08% <68.08%> (ø)` | |
   | [...ache/kyuubi/sql/zorder/OptimizeZorderCommand.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGV2L2t5dXViaS1leHRlbnNpb24tc3BhcmstMy0xL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NxbC96b3JkZXIvT3B0aW1pemVab3JkZXJDb21tYW5kLnNjYWxh) | `80.00% <80.00%> (ø)` | |
   | ... and [41 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/990/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [f1ae664...8db909c](https://codecov.io/gh/apache/incubator-kyuubi/pull/990?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org