You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by bersprockets <gi...@git.apache.org> on 2018/05/03 19:05:09 UTC
[GitHub] spark pull request #21231: [SPARK-24119][SQL]Add interpreted execution to So...
GitHub user bersprockets opened a pull request:
https://github.com/apache/spark/pull/21231
[SPARK-24119][SQL]Add interpreted execution to SortPrefix expression
## What changes were proposed in this pull request?
Implemented eval in SortPrefix expression.
## How was this patch tested?
- ran existing sbt SQL tests
- added unit test
- ran existing Python SQL tests
- manual tests: disabling codegen -- patching code to disable beyond what spark.sql.codegen.wholeStage=false can do -- and running sbt SQL tests
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/bersprockets/spark sortprefixeval
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/21231.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #21231
----
commit 280b941a1adc2ffcd82810a69f3d9e475607b70a
Author: Bruce Robbins <be...@...>
Date: 2018-04-28T23:26:48Z
Checkpoint changes
commit 4a810ae8776893ba1d12e620930473fdd61d1f49
Author: Bruce Robbins <be...@...>
Date: 2018-04-29T18:25:26Z
Checkpoint commit
commit d1a7e220040ba1cfe4facb7d6eef9ae1251768aa
Author: Bruce Robbins <be...@...>
Date: 2018-04-29T19:19:37Z
Checkpoint commit
commit 4dbb0b7ae959a6a4f85122a887bab4e1563255f0
Author: Bruce Robbins <be...@...>
Date: 2018-04-29T22:33:21Z
Comment on testing oddity
commit 227b6ac71bcfe6a54051f47dd16aec047b0a98d9
Author: Bruce Robbins <be...@...>
Date: 2018-04-30T21:22:09Z
Add boolean test for Sortprefix
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21231: [SPARK-24119][SQL]Add interpreted execution to So...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/21231#discussion_r185974124
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/SortOrder.scala ---
@@ -147,7 +148,40 @@ case class SortPrefix(child: SortOrder) extends UnaryExpression {
(!child.isAscending && child.nullOrdering == NullsLast)
}
- override def eval(input: InternalRow): Any = throw new UnsupportedOperationException
+ override def eval(input: InternalRow): Any = {
+ val value = child.child.eval(input)
+ if (value == null) {
+ return null
+ }
+ val prefix = child.child.dataType match {
+ case BooleanType =>
+ if (value.asInstanceOf[Boolean]) 1L else 0L
+ case _: IntegralType =>
+ value.asInstanceOf[java.lang.Number].longValue()
+ case DateType | TimestampType =>
+ value.asInstanceOf[java.lang.Number].longValue()
+ case FloatType | DoubleType =>
+ val dVal = value.asInstanceOf[java.lang.Number].doubleValue()
+ DoublePrefixComparator.computePrefix(dVal)
+ case StringType =>
+ StringPrefixComparator.computePrefix(value.asInstanceOf[UTF8String])
+ case BinaryType =>
+ BinaryPrefixComparator.computePrefix(value.asInstanceOf[Array[Byte]])
+ case dt: DecimalType if dt.precision - dt.scale <= Decimal.MAX_LONG_DIGITS =>
+ val dtValue = value.asInstanceOf[Decimal]
+ if (dt.precision <= Decimal.MAX_LONG_DIGITS) {
+ dtValue.toUnscaledLong
+ } else {
+ val p = Decimal.MAX_LONG_DIGITS
+ val s = p - (dt.precision - dt.scale)
+ if (dtValue.changePrecision(p, s)) dtValue.toUnscaledLong else Long.MinValue
+ }
+ case dt: DecimalType =>
+ val dtValue = value.asInstanceOf[Decimal].toDouble
+ DoublePrefixComparator.computePrefix(dtValue)
+ }
--- End diff --
Is it ok to not have a default case?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21231
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/21231
retest this please.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21231
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90288/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21231
**[Test build #90253 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90253/testReport)** for PR 21231 at commit [`1060a66`](https://github.com/apache/spark/commit/1060a666463426f2f70f2fe0a4fa7e2b66f22c67).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21231
**[Test build #90253 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90253/testReport)** for PR 21231 at commit [`1060a66`](https://github.com/apache/spark/commit/1060a666463426f2f70f2fe0a4fa7e2b66f22c67).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21231: [SPARK-24119][SQL]Add interpreted execution to So...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21231#discussion_r186673964
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/SortOrder.scala ---
@@ -147,7 +148,44 @@ case class SortPrefix(child: SortOrder) extends UnaryExpression {
(!child.isAscending && child.nullOrdering == NullsLast)
}
- override def eval(input: InternalRow): Any = throw new UnsupportedOperationException
+ private lazy val calcPrefix: Any => Long = child.child.dataType match {
+ case BooleanType => (raw) =>
+ if (raw.asInstanceOf[Boolean]) 1 else 0
+ case _: IntegralType => (raw) =>
+ raw.asInstanceOf[java.lang.Number].longValue()
+ case DateType | TimestampType =>
+ _.asInstanceOf[java.lang.Number].longValue()
--- End diff --
They are, we could try to specialize this a bit more by using `java.lang.Integer` and `java.lang.Long`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on the issue:
https://github.com/apache/spark/pull/21231
@maropu @hvanhovell @viirya Are all pending issues resolved?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21231
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21231
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21231: [SPARK-24119][SQL]Add interpreted execution to So...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/21231#discussion_r186307162
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/SortOrderExpressionsSuite.scala ---
@@ -0,0 +1,90 @@
+/*
+ * 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.catalyst.expressions
+
+import java.sql.{Date, Timestamp}
+import java.util.TimeZone
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.types._
+import org.apache.spark.unsafe.types.UTF8String
+import org.apache.spark.util.collection.unsafe.sort.PrefixComparators._
+
+class SortOrderExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
+
+ test("SortPrefix") {
+ // Explicitly choose a time zone, since Date objects can create different values depending on
+ // local time zone of the machine on which the test is running
+ TimeZone.setDefault(TimeZone.getTimeZone("America/Los_Angeles"))
--- End diff --
nit: do we need to restore the original time zone?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/21231
LGTM cc: @hvanhovell
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/21231
LGTM
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21231
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90433/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21231
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90242/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on the issue:
https://github.com/apache/spark/pull/21231
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21231: [SPARK-24119][SQL]Add interpreted execution to So...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/21231#discussion_r186668643
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/SortOrder.scala ---
@@ -147,7 +148,44 @@ case class SortPrefix(child: SortOrder) extends UnaryExpression {
(!child.isAscending && child.nullOrdering == NullsLast)
}
- override def eval(input: InternalRow): Any = throw new UnsupportedOperationException
+ private lazy val calcPrefix: Any => Long = child.child.dataType match {
+ case BooleanType => (raw) =>
+ if (raw.asInstanceOf[Boolean]) 1 else 0
+ case _: IntegralType => (raw) =>
+ raw.asInstanceOf[java.lang.Number].longValue()
+ case DateType | TimestampType =>
+ _.asInstanceOf[java.lang.Number].longValue()
+ case FloatType | DoubleType => (raw) => {
+ val dVal = raw.asInstanceOf[java.lang.Number].doubleValue()
+ DoublePrefixComparator.computePrefix(dVal)
+ }
+ case StringType => (raw) =>
+ StringPrefixComparator.computePrefix(raw.asInstanceOf[UTF8String])
+ case BinaryType => (raw) =>
+ BinaryPrefixComparator.computePrefix(raw.asInstanceOf[Array[Byte]])
+ case dt: DecimalType if dt.precision - dt.scale <= Decimal.MAX_LONG_DIGITS => (raw) => {
+ val value = raw.asInstanceOf[Decimal]
+ if (dt.precision <= Decimal.MAX_LONG_DIGITS) {
--- End diff --
Can we split this into two function closures? No need to do this at runtime.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21231
**[Test build #90156 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90156/testReport)** for PR 21231 at commit [`227b6ac`](https://github.com/apache/spark/commit/227b6ac71bcfe6a54051f47dd16aec047b0a98d9).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21231
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21231
**[Test build #90391 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90391/testReport)** for PR 21231 at commit [`28f1b70`](https://github.com/apache/spark/commit/28f1b7041ef6b4a233e523476e2a7ab3e9a17a71).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21231
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21231
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/21231
@hvanhovell @viirya @kiszk
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21231
**[Test build #90159 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90159/testReport)** for PR 21231 at commit [`227b6ac`](https://github.com/apache/spark/commit/227b6ac71bcfe6a54051f47dd16aec047b0a98d9).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21231
**[Test build #90156 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90156/testReport)** for PR 21231 at commit [`227b6ac`](https://github.com/apache/spark/commit/227b6ac71bcfe6a54051f47dd16aec047b0a98d9).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21231
**[Test build #90159 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90159/testReport)** for PR 21231 at commit [`227b6ac`](https://github.com/apache/spark/commit/227b6ac71bcfe6a54051f47dd16aec047b0a98d9).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21231
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90391/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21231
**[Test build #90288 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90288/testReport)** for PR 21231 at commit [`dc9a070`](https://github.com/apache/spark/commit/dc9a0702584c9fd16a1d25c675aaf2b424cd7623).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21231
**[Test build #90288 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90288/testReport)** for PR 21231 at commit [`dc9a070`](https://github.com/apache/spark/commit/dc9a0702584c9fd16a1d25c675aaf2b424cd7623).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21231
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21231: [SPARK-24119][SQL]Add interpreted execution to So...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/21231#discussion_r186021674
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/SortOrderExpressionsSuite.scala ---
@@ -0,0 +1,82 @@
+/*
+ * 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.catalyst.expressions
+
+import java.sql.{Date, Timestamp}
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.types._
+import org.apache.spark.unsafe.types.UTF8String
+import org.apache.spark.util.collection.unsafe.sort.PrefixComparators.{BinaryPrefixComparator, DoublePrefixComparator, StringPrefixComparator}
--- End diff --
Is it better to use `org.apache.spark.util.collection.unsafe.sort.PrefixComparators._`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21231
**[Test build #90242 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90242/testReport)** for PR 21231 at commit [`1060a66`](https://github.com/apache/spark/commit/1060a666463426f2f70f2fe0a4fa7e2b66f22c67).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21231: [SPARK-24119][SQL]Add interpreted execution to So...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on a diff in the pull request:
https://github.com/apache/spark/pull/21231#discussion_r186307490
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/SortOrderExpressionsSuite.scala ---
@@ -0,0 +1,90 @@
+/*
+ * 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.catalyst.expressions
+
+import java.sql.{Date, Timestamp}
+import java.util.TimeZone
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.types._
+import org.apache.spark.unsafe.types.UTF8String
+import org.apache.spark.util.collection.unsafe.sort.PrefixComparators._
+
+class SortOrderExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
+
+ test("SortPrefix") {
+ // Explicitly choose a time zone, since Date objects can create different values depending on
+ // local time zone of the machine on which the test is running
+ TimeZone.setDefault(TimeZone.getTimeZone("America/Los_Angeles"))
--- End diff --
@kiszk Maybe not a nit. I should fix that.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21231
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90156/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21231: [SPARK-24119][SQL]Add interpreted execution to So...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/21231#discussion_r186022301
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/SortOrder.scala ---
@@ -147,7 +148,40 @@ case class SortPrefix(child: SortOrder) extends UnaryExpression {
(!child.isAscending && child.nullOrdering == NullsLast)
}
- override def eval(input: InternalRow): Any = throw new UnsupportedOperationException
+ override def eval(input: InternalRow): Any = {
+ val value = child.child.eval(input)
+ if (value == null) {
+ return null
+ }
+ val prefix = child.child.dataType match {
+ case BooleanType =>
+ if (value.asInstanceOf[Boolean]) 1L else 0L
+ case _: IntegralType =>
+ value.asInstanceOf[java.lang.Number].longValue()
+ case DateType | TimestampType =>
+ value.asInstanceOf[java.lang.Number].longValue()
+ case FloatType | DoubleType =>
+ val dVal = value.asInstanceOf[java.lang.Number].doubleValue()
+ DoublePrefixComparator.computePrefix(dVal)
+ case StringType =>
+ StringPrefixComparator.computePrefix(value.asInstanceOf[UTF8String])
+ case BinaryType =>
+ BinaryPrefixComparator.computePrefix(value.asInstanceOf[Array[Byte]])
+ case dt: DecimalType if dt.precision - dt.scale <= Decimal.MAX_LONG_DIGITS =>
+ val dtValue = value.asInstanceOf[Decimal]
+ if (dt.precision <= Decimal.MAX_LONG_DIGITS) {
+ dtValue.toUnscaledLong
+ } else {
+ val p = Decimal.MAX_LONG_DIGITS
+ val s = p - (dt.precision - dt.scale)
+ if (dtValue.changePrecision(p, s)) dtValue.toUnscaledLong else Long.MinValue
+ }
+ case dt: DecimalType =>
+ val dtValue = value.asInstanceOf[Decimal].toDouble
+ DoublePrefixComparator.computePrefix(dtValue)
+ }
+ prefix
--- End diff --
Would it be possible to remove `prefix` and `val prefix = ` to directly return the value of the block?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21231: [SPARK-24119][SQL]Add interpreted execution to So...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/21231
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on the issue:
https://github.com/apache/spark/pull/21231
@hvanhovell @maropu @viirya @kiszk Thanks for all the help!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21231
**[Test build #90433 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90433/testReport)** for PR 21231 at commit [`590ba26`](https://github.com/apache/spark/commit/590ba26c54b22de670cc699dcd0e1e48aaf71ab2).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21231
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on the issue:
https://github.com/apache/spark/pull/21231
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/21231
retest this please.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21231
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91554/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21231
**[Test build #91554 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91554/testReport)** for PR 21231 at commit [`590ba26`](https://github.com/apache/spark/commit/590ba26c54b22de670cc699dcd0e1e48aaf71ab2).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21231
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90253/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21231
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90159/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21231
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91557/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21231
**[Test build #90391 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90391/testReport)** for PR 21231 at commit [`28f1b70`](https://github.com/apache/spark/commit/28f1b7041ef6b4a233e523476e2a7ab3e9a17a71).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21231: [SPARK-24119][SQL]Add interpreted execution to So...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/21231#discussion_r185973696
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/SortOrder.scala ---
@@ -147,7 +148,40 @@ case class SortPrefix(child: SortOrder) extends UnaryExpression {
(!child.isAscending && child.nullOrdering == NullsLast)
}
- override def eval(input: InternalRow): Any = throw new UnsupportedOperationException
+ override def eval(input: InternalRow): Any = {
+ val value = child.child.eval(input)
+ if (value == null) {
+ return null
+ }
+ val prefix = child.child.dataType match {
+ case BooleanType =>
+ if (value.asInstanceOf[Boolean]) 1L else 0L
--- End diff --
`value.isInstanceOf[Boolean]`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21231: [SPARK-24119][SQL]Add interpreted execution to So...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/21231#discussion_r185973474
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/SortOrder.scala ---
@@ -147,7 +148,40 @@ case class SortPrefix(child: SortOrder) extends UnaryExpression {
(!child.isAscending && child.nullOrdering == NullsLast)
}
- override def eval(input: InternalRow): Any = throw new UnsupportedOperationException
+ override def eval(input: InternalRow): Any = {
+ val value = child.child.eval(input)
+ if (value == null) {
+ return null
+ }
+ val prefix = child.child.dataType match {
--- End diff --
It is time-consuming to do this pattern-matching, so can you make a function outside? e.g.,
https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala#L720
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21231: [SPARK-24119][SQL]Add interpreted execution to So...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/21231#discussion_r186913964
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/SortOrder.scala ---
@@ -147,7 +148,40 @@ case class SortPrefix(child: SortOrder) extends UnaryExpression {
(!child.isAscending && child.nullOrdering == NullsLast)
}
- override def eval(input: InternalRow): Any = throw new UnsupportedOperationException
+ private lazy val calcPrefix: Any => Long = child.child.dataType match {
+ case BooleanType => (raw) =>
+ if (raw.asInstanceOf[Boolean]) 1 else 0
+ case DateType | TimestampType | _: IntegralType => (raw) =>
--- End diff --
We could do the same patten matching in `doGenCode`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21231
**[Test build #90433 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90433/testReport)** for PR 21231 at commit [`590ba26`](https://github.com/apache/spark/commit/590ba26c54b22de670cc699dcd0e1e48aaf71ab2).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21231
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21231: [SPARK-24119][SQL]Add interpreted execution to So...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/21231#discussion_r186925349
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/SortOrder.scala ---
@@ -147,7 +148,40 @@ case class SortPrefix(child: SortOrder) extends UnaryExpression {
(!child.isAscending && child.nullOrdering == NullsLast)
}
- override def eval(input: InternalRow): Any = throw new UnsupportedOperationException
+ private lazy val calcPrefix: Any => Long = child.child.dataType match {
+ case BooleanType => (raw) =>
+ if (raw.asInstanceOf[Boolean]) 1 else 0
+ case DateType | TimestampType | _: IntegralType => (raw) =>
+ raw.asInstanceOf[java.lang.Number].longValue()
+ case FloatType | DoubleType => (raw) => {
+ val dVal = raw.asInstanceOf[java.lang.Number].doubleValue()
+ DoublePrefixComparator.computePrefix(dVal)
+ }
+ case StringType => (raw) =>
+ StringPrefixComparator.computePrefix(raw.asInstanceOf[UTF8String])
+ case BinaryType => (raw) =>
+ BinaryPrefixComparator.computePrefix(raw.asInstanceOf[Array[Byte]])
+ case dt: DecimalType if dt.precision <= Decimal.MAX_LONG_DIGITS =>
+ _.asInstanceOf[Decimal].toUnscaledLong
+ case dt: DecimalType if dt.precision - dt.scale <= Decimal.MAX_LONG_DIGITS => (raw) => {
+ val value = raw.asInstanceOf[Decimal]
+ val p = Decimal.MAX_LONG_DIGITS
+ val s = p - (dt.precision - dt.scale)
--- End diff --
This is a nit but we can move L168 & L169 out of closure too. Like:
```scala
case dt: DecimalType if dt.precision - dt.scale <= Decimal.MAX_LONG_DIGITS =>
val p = Decimal.MAX_LONG_DIGITS
val s = p - (dt.precision - dt.scale)
(raw) => {
val value = raw.asInstanceOf[Decimal]
if (value.changePrecision(p, s)) value.toUnscaledLong else Long.MinValue
}
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21231
**[Test build #91554 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91554/testReport)** for PR 21231 at commit [`590ba26`](https://github.com/apache/spark/commit/590ba26c54b22de670cc699dcd0e1e48aaf71ab2).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21231: [SPARK-24119][SQL]Add interpreted execution to So...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on a diff in the pull request:
https://github.com/apache/spark/pull/21231#discussion_r187192485
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/SortOrder.scala ---
@@ -147,7 +148,40 @@ case class SortPrefix(child: SortOrder) extends UnaryExpression {
(!child.isAscending && child.nullOrdering == NullsLast)
}
- override def eval(input: InternalRow): Any = throw new UnsupportedOperationException
+ private lazy val calcPrefix: Any => Long = child.child.dataType match {
+ case BooleanType => (raw) =>
+ if (raw.asInstanceOf[Boolean]) 1 else 0
+ case DateType | TimestampType | _: IntegralType => (raw) =>
--- End diff --
I'm a little apprehensive about changing existing and properly functioning code (in doGenCode), even though it would make it slightly more readable. If you think I should, though, I will.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/21231
LGTM
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21231
**[Test build #91557 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91557/testReport)** for PR 21231 at commit [`590ba26`](https://github.com/apache/spark/commit/590ba26c54b22de670cc699dcd0e1e48aaf71ab2).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21231
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on the issue:
https://github.com/apache/spark/pull/21231
ping @hvanhovell
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21231
**[Test build #90242 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90242/testReport)** for PR 21231 at commit [`1060a66`](https://github.com/apache/spark/commit/1060a666463426f2f70f2fe0a4fa7e2b66f22c67).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21231: [SPARK-24119][SQL]Add interpreted execution to So...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/21231#discussion_r186672482
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/SortOrder.scala ---
@@ -147,7 +148,44 @@ case class SortPrefix(child: SortOrder) extends UnaryExpression {
(!child.isAscending && child.nullOrdering == NullsLast)
}
- override def eval(input: InternalRow): Any = throw new UnsupportedOperationException
+ private lazy val calcPrefix: Any => Long = child.child.dataType match {
+ case BooleanType => (raw) =>
+ if (raw.asInstanceOf[Boolean]) 1 else 0
+ case _: IntegralType => (raw) =>
+ raw.asInstanceOf[java.lang.Number].longValue()
+ case DateType | TimestampType =>
+ _.asInstanceOf[java.lang.Number].longValue()
--- End diff --
Aren't the above two cases being the same thing?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21231
**[Test build #91557 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91557/testReport)** for PR 21231 at commit [`590ba26`](https://github.com/apache/spark/commit/590ba26c54b22de670cc699dcd0e1e48aaf71ab2).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21231: [SPARK-24119][SQL]Add interpreted execution to SortPrefi...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on the issue:
https://github.com/apache/spark/pull/21231
@maropu @kiszk Hopefully I've addressed all comments. Please take a look.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21231: [SPARK-24119][SQL]Add interpreted execution to So...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/21231#discussion_r186667062
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/SortOrderExpressionsSuite.scala ---
@@ -0,0 +1,96 @@
+/*
+ * 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.catalyst.expressions
+
+import java.sql.{Date, Timestamp}
+import java.util.TimeZone
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.types._
+import org.apache.spark.unsafe.types.UTF8String
+import org.apache.spark.util.collection.unsafe.sort.PrefixComparators._
+
+class SortOrderExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
+
+ test("SortPrefix") {
+
--- End diff --
nit: remove the blank line.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org