Skip to content

[SPARK-23921][SQL] Add array_sort function #21021

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kiszk wants to merge 14 commits into from
Closed

[SPARK-23921][SQL] Add array_sort function #21021

Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
72f31b3
initial commit
kiszk Apr 10, 2018
a9b6e3b
address review comments
kiszk Apr 10, 2018
d57c14a
adding an update in functions.py
kiszk Apr 10, 2018
9977f64
address review comment
kiszk Apr 10, 2018
172b2c5
address review comments
kiszk Apr 20, 2018
f2798f9
address review comment
kiszk Apr 26, 2018
175d981
address review comments
kiszk Apr 27, 2018
d1b0483
fix test failures in DataFrameFunctionSuite and others
kiszk Apr 29, 2018
04a3ae5
address review comments
kiszk May 1, 2018
9f63a76
address review comments
kiszk May 2, 2018
e3fcaaa
use ctx.addReferenceObj() for DataType in generated code
kiszk May 2, 2018
2ad6bb8
address review comments
kiszk May 4, 2018
2c4404c
Remove redundant cast
kiszk May 4, 2018
21521d8
avoid boxing as possible
kiszk May 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
address review comments
  • Loading branch information
@kiszk
kiszk committed May 1, 2018
commit 04a3ae57ae755df70c2ab2141e5b3297fe229463
37 changes: 17 additions & 20 deletions ...alyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,21 @@ trait ArraySortUtil extends ExpectsInputTypes {
val genericArrayData = classOf[GenericArrayData].getName
val array = ctx.freshName("array")
val c = ctx.freshName("c")
val sort = if (elementType == NullType) "" else {
val dataTypes = elementType match {
case DecimalType.Fixed(p, s) =>
s"org.apache.spark.sql.types.DataTypes.createDecimalType($p, $s)"
case ArrayType(et, cn) =>
val dt = s"org.apache.spark.sql.types.$et$$.MODULE$$"
s"org.apache.spark.sql.types.DataTypes.createArrayType($dt, $cn)"
case StructType(f) =>
"org.apache.spark.sql.types.StructType$.MODULE$." +
s"apply(new java.util.ArrayList(${f.length}))"
case _ =>
s"org.apache.spark.sql.types.$elementType$$.MODULE$$"
}
Copy link
Member

@ueshin ueshin May 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still wondering whether this will work or not. What if elementType is ArrayType(ArrayType(IntegerType))?
Can't we use a reference object of elementType?

Copy link
Member

@ueshin ueshin May 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also add some tests using ArrayType and StructType for elementType?

Copy link
Member Author

@kiszk kiszk May 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely, I added some complex test cases with nests.

if (elementType == NullType) {
s"${ev.value} = (($arrayData) $base).copy();"
Copy link
Member

@ueshin ueshin May 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: do we need cast base to ArrayData?

Copy link
Member Author

@kiszk kiszk May 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, done

} else {
val sortOrder = ctx.freshName("sortOrder")
val o1 = ctx.freshName("o1")
val o2 = ctx.freshName("o2")
Expand All @@ -204,6 +218,7 @@ trait ArraySortUtil extends ExpectsInputTypes {
s"int $c = ${ctx.genComp(elementType, s"(($jt) $o1)", s"(($jt) $o2)")};"
}
s"""
|Object[] $array = (Object[]) (($arrayData) $base).toObjectArray($dataTypes);
Copy link
Member

@ueshin ueshin May 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto.

|final int $sortOrder = $order ? 1 : -1;
|java.util.Arrays.sort($array, new java.util.Comparator() {
| @Override public int compare(Object $o1, Object $o2) {
Expand All @@ -218,27 +233,9 @@ trait ArraySortUtil extends ExpectsInputTypes {
| return $sortOrder * $c;
| }
|});
|${ev.value} = new $genericArrayData($array);
""".stripMargin
}
val dataTypes = elementType match {
case DecimalType.Fixed(p, s) =>
s"org.apache.spark.sql.types.DataTypes.createDecimalType($p, $s)"
case ArrayType(et, cn) =>
s"org.apache.spark.sql.types.DataTypes.createArrayType($et, $cn)"
case MapType(kt, vt, cn) =>
s"org.apache.spark.sql.types.DataTypes.createMapType($kt, $vt, $cn)"
case StructType(f) =>
"org.apache.spark.sql.types.StructType$.MODULE$." +
s"apply(new java.util.ArrayList(${f.length}))"
case _ =>
s"org.apache.spark.sql.types.DataTypes.$elementType"
}
s"""
|Object[] $array = (Object[]) (($arrayData) $base).toArray(
| $dataTypes, scala.reflect.ClassTag$$.MODULE$$.AnyRef());
|$sort
|${ev.value} = new $genericArrayData($array);
""".stripMargin
}

}
Expand Down