[SPARK-23915][SQL] Add array_except function #21103

kiszk · 2018-04-18T18:06:43Z

What changes were proposed in this pull request?

The PR adds the SQL function array_except. The behavior of the function is based on Presto's one.

This function returns returns an array of the elements in array1 but not in array2.

Note: The order of elements in the result is not defined.

How was this patch tested?

Added UTs.

SparkQA · 2018-04-18T19:49:55Z

Test build #89524 has finished for PR 21103 at commit ce18ce0.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
abstract class ArraySetUtils extends BinaryExpression with ExpectsInputTypes
case class ArrayExcept(array1: Expression, array2: Expression) extends ArraySetUtils

SparkQA · 2018-04-19T07:05:01Z

Test build #89540 has finished for PR 21103 at commit 6813ba4.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

kiszk · 2018-04-19T08:34:55Z

retest this please

SparkQA · 2018-04-19T11:51:29Z

Test build #89561 has finished for PR 21103 at commit 6813ba4.

This patch fails PySpark unit tests.
This patch does not merge cleanly.
This patch adds no public classes.

SparkQA · 2018-04-19T19:04:43Z

Test build #89587 has finished for PR 21103 at commit 2d16556.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
abstract class ArraySetUtils extends BinaryExpression with ExpectsInputTypes

gatorsmile · 2018-04-19T22:08:12Z

cc @ueshin

SparkQA · 2018-04-19T22:51:19Z

Test build #89589 has finished for PR 21103 at commit 2551c09.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2018-04-20T06:24:34Z

Test build #89605 has finished for PR 21103 at commit cdb6257.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class ArrayExcept(left: Expression, right: Expression) extends ArraySetUtils

SparkQA · 2018-04-20T17:53:07Z

Test build #89629 has finished for PR 21103 at commit ad9f576.

This patch fails from timeout after a configured wait of `300m`.
This patch merges cleanly.
This patch adds no public classes.

kiszk · 2018-04-20T18:14:04Z

retest this please

SparkQA · 2018-04-20T22:02:59Z

Test build #89651 has finished for PR 21103 at commit ad9f576.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2018-06-26T05:37:16Z

Test build #92322 has finished for PR 21103 at commit ad9f576.

This patch fails Spark unit tests.
This patch does not merge cleanly.
This patch adds no public classes.

fix test failure

SparkQA · 2018-07-14T07:05:01Z

Test build #92997 has finished for PR 21103 at commit 5c06922.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class ArrayExcept(left: Expression, right: Expression) extends ArraySetLike

kiszk · 2018-07-14T13:50:58Z

retest this please

SparkQA · 2018-07-14T18:17:31Z

Test build #93001 has finished for PR 21103 at commit 5c06922.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class ArrayExcept(left: Expression, right: Expression) extends ArraySetLike

SparkQA · 2018-07-19T10:47:42Z

Test build #93270 has finished for PR 21103 at commit 26e3257.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class ArrayUnion(left: Expression, right: Expression) extends ArraySetLike

SparkQA · 2018-07-29T21:04:33Z

Test build #93750 has finished for PR 21103 at commit 3639b5b.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds no public classes.

kiszk · 2018-07-29T23:49:02Z

retest this please

cloud-fan · 2018-07-30T02:00:39Z

...catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala

+           |  {
+           |    $javaTypeName $value = $array2.$getter;
+           |    $hsJavaTypeName $hsValue = $genHsValue;
+           |    $hs.add$postFix($hsValue);


why wrap them inside {...}?

ah i see. Maybe a better way is

def withArray2NullCheck(body: String) = if (right.dataType.asInstanceOf[ArrayType].containsNull) { s""" |if ($array2.isNullAt($i)) { | $notFoundNullElement = false; |} else { | $body |}""".stripMargin } else { body }

This part will be part of else clause of if statement depends on $array2NullCheck (i.e. if (right.dataType.asInstanceOf[ArrayType].containsNull) is true).

cloud-fan · 2018-07-30T02:05:59Z

...catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala

+      }
+
+    nullSafeCodeGen(ctx, ev, (array1, array2) => {
+      if (openHashElementType != "") {


a better way to organize it

if (elementTypeSupportEquals) { ... nullSafeCodeGen(...) } else { nullSafeCodeGen(...) }

cloud-fan · 2018-07-30T02:08:32Z

...catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala

+           |$hs = new $openHashSet$postFix($classTag);
+           |$notFoundNullElement = true;
+           |int $pos = 0;
+           |for (int $i = 0; $i < $array2.numElements(); $i++) {


why add array2 to the hash set again?

We add some elements in array1to the hash set at L4192.
Since we want to create a hashset that have elements only of array2, we reallocate a hashset at L4198 and add elements of array2 again here.

then why we do the first iteration? only calculating the size?

Yes, only calculating array size for allocating a UnsafeArrayData or GenericArrayData.

is this worth? I feel using an ArrayBuffer like the interpreted path is faster.

To use 'ArrayBuffer' involves boxing. Let me consider 'ArrayBuffer.ofInt' or others with special null handling.

SparkQA · 2018-07-30T03:49:25Z

Test build #93759 has finished for PR 21103 at commit 3639b5b.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2018-07-31T05:22:39Z

...catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala

+      val (postFix, openHashElementType, hsJavaTypeName, genHsValue,
+           getter, setter, javaTypeName, primitiveTypeName, arrayDataBuilder) =
+        elementType match {
+          case BooleanType | ByteType | ShortType | IntegerType =>


technically we can have special handling for boolean type(at most 2 outputs), but I think it's very rare to see boolean type in array_except. Let's leave it unless users require it.

Yeah, I agree that it is very rare. Let us remove the special handling code for BooleanType.

SparkQA · 2018-07-31T07:05:01Z

Test build #93822 has finished for PR 21103 at commit 49b5ab3.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2018-07-31T07:05:01Z

Test build #93819 has finished for PR 21103 at commit 4d943c8.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2018-07-31T07:06:53Z

retest this please

SparkQA · 2018-07-31T11:09:03Z

Test build #93825 has finished for PR 21103 at commit 282445c.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2018-07-31T11:24:48Z

retest this please

SparkQA · 2018-07-31T15:33:24Z

Test build #93830 has finished for PR 21103 at commit 282445c.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2018-07-31T18:55:31Z

Test build #93831 has finished for PR 21103 at commit 72ef664.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2018-07-31T22:09:45Z

Test build #93836 has finished for PR 21103 at commit 93e7979.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

kiszk · 2018-08-01T00:30:10Z

retest this please

SparkQA · 2018-08-01T04:44:01Z

Test build #93851 has finished for PR 21103 at commit 93e7979.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

kiszk · 2018-08-01T05:41:05Z

retest this please

SparkQA · 2018-08-01T10:16:38Z

Test build #93870 has finished for PR 21103 at commit 93e7979.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

kiszk · 2018-08-01T14:24:22Z

cc @ueshin @cloud-fan

cloud-fan · 2018-08-01T18:52:53Z

thanks, merging to master!

## What changes were proposed in this pull request? This PR refactors `ArrayUnion` based on [this suggestion](#21103 (comment)). 1. Generate optimized code for all of the primitive types except `boolean` 1. Generate code using `ArrayBuilder` or `ArrayBuffer` 1. Leave only a generic path in the interpreted path ## How was this patch tested? Existing tests Author: Kazuaki Ishizaki <[email protected]> Closes #21937 from kiszk/SPARK-23914-follow.

kiszk force-pushed the SPARK-23915 branch from 6813ba4 to 2d16556 Compare April 19, 2018 18:58

kiszk force-pushed the SPARK-23915 branch from cdb6257 to ad9f576 Compare April 20, 2018 12:52

ueshin mentioned this pull request Apr 26, 2018

[SPARK-23922][SQL] Add arrays_overlap function #21028

Closed

kiszk added a commit to kiszk/spark that referenced this pull request Jun 27, 2018

merge changes in apache#21103

71b31f0

fix test failure

kiszk force-pushed the SPARK-23915 branch from ad9f576 to 5c06922 Compare July 14, 2018 06:10

cloud-fan reviewed Jul 30, 2018

View reviewed changes

kiszk added 2 commits July 31, 2018 04:23

address review comment

4d943c8

address review comment

49b5ab3

cloud-fan reviewed Jul 31, 2018

View reviewed changes

review address comment

282445c

fix pyspark test failure

72ef664

address array overflow

93e7979

kiszk mentioned this pull request Jul 31, 2018

[SPARK-23914][SQL][follow-up] refactor ArrayUnion #21937

Closed

asfgit closed this in 95a9d5e Aug 1, 2018

kiszk mentioned this pull request Sep 2, 2018

[SPARK-25308][SQL] ArrayContains function may return a error in the code generation phase. #22315

Closed

[SPARK-23915][SQL] Add array_except function #21103

[SPARK-23915][SQL] Add array_except function #21103

Uh oh!

Conversation

kiszk commented Apr 18, 2018

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

SparkQA commented Apr 18, 2018

Uh oh!

SparkQA commented Apr 19, 2018

Uh oh!

kiszk commented Apr 19, 2018

Uh oh!

SparkQA commented Apr 19, 2018

Uh oh!

SparkQA commented Apr 19, 2018

Uh oh!

gatorsmile commented Apr 19, 2018

Uh oh!

SparkQA commented Apr 19, 2018

Uh oh!

SparkQA commented Apr 20, 2018

Uh oh!

SparkQA commented Apr 20, 2018

Uh oh!

kiszk commented Apr 20, 2018

Uh oh!

SparkQA commented Apr 20, 2018

Uh oh!

SparkQA commented Jun 26, 2018

Uh oh!

SparkQA commented Jul 14, 2018

Uh oh!

kiszk commented Jul 14, 2018

Uh oh!

SparkQA commented Jul 14, 2018

Uh oh!

SparkQA commented Jul 19, 2018

Uh oh!

SparkQA commented Jul 29, 2018

Uh oh!

kiszk commented Jul 29, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kiszk Jul 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 30, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 31, 2018

Uh oh!

SparkQA commented Jul 31, 2018

Uh oh!

cloud-fan commented Jul 31, 2018

Uh oh!

SparkQA commented Jul 31, 2018

Uh oh!

kiszk Jul 30, 2018 •

edited

Loading