You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "panbingkun (via GitHub)" <gi...@apache.org> on 2024/09/04 06:28:45 UTC
[PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
panbingkun opened a new pull request, #47984:
URL: https://github.com/apache/spark/pull/47984
<!--
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://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
8. If you want to add or modify an error type or message, please read the guideline first in
'common/utils/src/main/resources/error/README.md'.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
-->
### Was this patch authored or co-authored using generative AI tooling?
<!--
If generative AI tooling has been used in the process of authoring this patch, please include the
phrase: 'Generated-by: ' followed by the name of the tool and its version.
If no, write 'No'.
Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
-->
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1749897841
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -1618,23 +1619,198 @@ case class ArrayBinarySearch(array: Expression, value: Expression)
}
}
+ // byte
+ // unbox byte
+ @transient private lazy val foldableByteArrayData: Array[Byte] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toByteArray()
+ }
+ @transient private lazy val foldableByteArrayDataObjectType =
+ ObjectType(classOf[Array[Byte]])
+ // boxed byte
+ @transient private lazy val foldableBoxedByteArrayData: Array[java.lang.Byte] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toArray[java.lang.Byte](ByteType)
+ }
+ @transient private lazy val foldableBoxedByteArrayDataObjectType =
+ ObjectType(classOf[Array[java.lang.Byte]])
+
+ // boolean
+ // boxed boolean
+ @transient private lazy val foldableBoxedBooleanArrayData: Array[java.lang.Boolean] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toArray[java.lang.Boolean](BooleanType)
+ }
+ @transient private lazy val foldableBoxedBooleanArrayDataObjectType =
+ ObjectType(classOf[Array[java.lang.Boolean]])
+
+ // short
+ // unbox short
+ @transient private lazy val foldableShortArrayData: Array[Short] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toShortArray()
+ }
+ @transient private lazy val foldableShortArrayDataObjectType =
+ ObjectType(classOf[Array[Short]])
+ // boxed short
+ @transient private lazy val foldableBoxedShortArrayData: Array[java.lang.Short] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toArray[java.lang.Short](ShortType)
+ }
+ @transient private lazy val foldableBoxedShortArrayDataObjectType =
+ ObjectType(classOf[Array[java.lang.Short]])
+
+ // int
+ // unbox int
+ @transient private lazy val foldableIntArrayData: Array[Int] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toIntArray()
+ }
+ @transient private lazy val foldableIntArrayDataObjectType =
+ ObjectType(classOf[Array[Int]])
+ // boxed int
+ @transient private lazy val foldableBoxedIntArrayData: Array[java.lang.Integer] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toArray[java.lang.Integer](IntegerType)
+ }
+ @transient private lazy val foldableBoxedIntArrayDataObjectType =
+ ObjectType(classOf[Array[java.lang.Integer]])
+
+ // long
+ // unbox long
+ @transient private lazy val foldableLongArrayData: Array[Long] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toLongArray()
+ }
+ @transient private lazy val foldableLongArrayDataObjectType =
+ ObjectType(classOf[Array[Long]])
+ // boxed long
+ @transient private lazy val foldableBoxedLongArrayData: Array[java.lang.Long] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toArray[java.lang.Long](LongType)
+ }
+ @transient private lazy val foldableBoxedLongArrayDataObjectType =
+ ObjectType(classOf[Array[java.lang.Long]])
+
+ // float
+ // unbox float
+ @transient private lazy val foldableFloatArrayData: Array[Float] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toFloatArray()
+ }
+ @transient private lazy val foldableFloatArrayDataObjectType =
+ ObjectType(classOf[Array[Float]])
+ // boxed float
+ @transient private lazy val foldableBoxedFloatArrayData: Array[java.lang.Float] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toArray[java.lang.Float](FloatType)
+ }
+ @transient private lazy val foldableBoxedFloatArrayDataObjectType =
+ ObjectType(classOf[Array[java.lang.Float]])
+
+ // double
+ // unbox double
+ @transient private lazy val foldableDoubleArrayData: Array[Double] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toDoubleArray()
+ }
+ @transient private lazy val foldableDoubleArrayDataObjectType =
+ ObjectType(classOf[Array[Double]])
+ // boxed double
+ @transient private lazy val foldableBoxedDoubleArrayData: Array[java.lang.Double] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toArray[java.lang.Double](DoubleType)
+ }
+ @transient private lazy val foldableBoxedDoubleArrayDataObjectType =
+ ObjectType(classOf[Array[java.lang.Double]])
+
@transient private lazy val elementObjectType = ObjectType(classOf[DataType])
- @transient private lazy val comparatorObjectType = ObjectType(classOf[Comparator[Object]])
- override def replacement: Expression =
+ @transient private lazy val comparatorObjectType = ObjectType(classOf[Comparator[Object]])
+
+ private def unboxArgumentsAndInputTypes(): (Seq[Expression], Seq[AbstractDataType]) =
+ elementType match {
+ case ByteType =>
+ (Seq(Literal(foldableByteArrayData, foldableByteArrayDataObjectType), value),
+ Seq(foldableByteArrayDataObjectType, elementType))
+ case ShortType =>
+ (Seq(Literal(foldableShortArrayData, foldableShortArrayDataObjectType), value),
+ Seq(foldableShortArrayDataObjectType, elementType))
+ case IntegerType =>
+ (Seq(Literal(foldableIntArrayData, foldableIntArrayDataObjectType), value),
+ Seq(foldableIntArrayDataObjectType, elementType))
+ case LongType =>
+ (Seq(Literal(foldableLongArrayData, foldableLongArrayDataObjectType), value),
+ Seq(foldableLongArrayDataObjectType, elementType))
+ case FloatType =>
+ (Seq(Literal(foldableFloatArrayData, foldableFloatArrayDataObjectType), value),
+ Seq(foldableFloatArrayDataObjectType, elementType))
+ case DoubleType =>
+ (Seq(Literal(foldableDoubleArrayData, foldableDoubleArrayDataObjectType), value),
+ Seq(foldableDoubleArrayDataObjectType, elementType))
+ }
+
+ private def boxedArgumentsAndInputTypes(): (Seq[Expression], Seq[AbstractDataType]) =
+ elementType match {
+ case BooleanType =>
+ (Seq(Literal(foldableBoxedBooleanArrayData, foldableBoxedBooleanArrayDataObjectType),
+ value),
+ Seq(foldableBoxedBooleanArrayDataObjectType, elementType))
+ case ByteType =>
+ (Seq(Literal(foldableBoxedByteArrayData, foldableBoxedByteArrayDataObjectType), value),
+ Seq(foldableBoxedByteArrayDataObjectType, elementType))
+ case ShortType =>
+ (Seq(Literal(foldableBoxedShortArrayData, foldableBoxedShortArrayDataObjectType), value),
+ Seq(foldableBoxedShortArrayDataObjectType, elementType))
+ case IntegerType =>
+ (Seq(Literal(foldableBoxedIntArrayData, foldableBoxedIntArrayDataObjectType), value),
+ Seq(foldableBoxedIntArrayDataObjectType, elementType))
+ case LongType =>
+ (Seq(Literal(foldableBoxedLongArrayData, foldableBoxedLongArrayDataObjectType), value),
+ Seq(foldableBoxedLongArrayDataObjectType, elementType))
+ case FloatType =>
+ (Seq(Literal(foldableBoxedFloatArrayData, foldableBoxedFloatArrayDataObjectType), value),
+ Seq(foldableBoxedFloatArrayDataObjectType, elementType))
+ case DoubleType =>
+ (Seq(Literal(foldableBoxedDoubleArrayData, foldableBoxedDoubleArrayDataObjectType), value),
+ Seq(foldableBoxedDoubleArrayDataObjectType, elementType))
+ }
+
+ override def replacement: Expression = {
if (canPerformFastBinarySearch) {
- StaticInvoke(
- classOf[ArrayExpressionUtils],
- IntegerType,
- "binarySearch",
- Seq(array, value),
- inputTypes)
+ if (arrayIsFoldable) {
Review Comment:
I'm a bit confused. What's the difference between foldable and non-foldable arrays regarding this optimization?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1756224398
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -1612,39 +1726,28 @@ case class ArrayBinarySearch(array: Expression, value: Expression)
override def compare(o1: Any, o2: Any): Int =
(o1, o2) match {
case (null, null) => 0
- case (null, _) => 1
- case (_, null) => -1
+ case (null, _) => -1
Review Comment:
fix bug
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1772487830
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -1525,6 +1525,103 @@ case class ArrayContains(left: Expression, right: Expression)
copy(left = newLeft, right = newRight)
}
+/**
+ * This expression converts data of `ArrayData` to an array of java type.
+ *
+ * NOTE: When the data type of expression is `ArrayType`, and the expression is foldable,
+ * the `ConstantFolding` can do constant folding optimization automatically,
+ * (avoiding frequent calls to `ArrayData.to{XXX}Array()`).
+ */
+case class ToJavaArray(array: Expression)
Review Comment:
Done. I have also extracted the Invoke logic related to ToJavaArray into a new file: `ToJavaArrayUtils.java`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #47984:
URL: https://github.com/apache/spark/pull/47984#issuecomment-2360647414
cc @cloud-fan
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1771364577
##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ArrayExpressionUtils.java:
##########
@@ -19,158 +19,288 @@
import java.util.Arrays;
import java.util.Comparator;
+import scala.reflect.ClassTag$;
+
import org.apache.spark.sql.catalyst.util.ArrayData;
import org.apache.spark.sql.catalyst.util.SQLOrderingUtil;
-import org.apache.spark.sql.types.ByteType$;
-import org.apache.spark.sql.types.BooleanType$;
import org.apache.spark.sql.types.DataType;
-import org.apache.spark.sql.types.DoubleType$;
-import org.apache.spark.sql.types.FloatType$;
-import org.apache.spark.sql.types.IntegerType$;
-import org.apache.spark.sql.types.LongType$;
-import org.apache.spark.sql.types.ShortType$;
+
+import static org.apache.spark.sql.types.DataTypes.BooleanType;
+import static org.apache.spark.sql.types.DataTypes.ByteType;
+import static org.apache.spark.sql.types.DataTypes.DoubleType;
+import static org.apache.spark.sql.types.DataTypes.FloatType;
+import static org.apache.spark.sql.types.DataTypes.IntegerType;
+import static org.apache.spark.sql.types.DataTypes.LongType;
+import static org.apache.spark.sql.types.DataTypes.ShortType;
public class ArrayExpressionUtils {
- private static final Comparator<Object> booleanComp = (o1, o2) -> {
+ // comparator
+ // Boolean ascending nullable comparator
+ private static final Comparator<Boolean> booleanComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- boolean c1 = (Boolean) o1, c2 = (Boolean) o2;
- return c1 == c2 ? 0 : (c1 ? 1 : -1);
+ return o1.equals(o2) ? 0 : (o1 ? 1 : -1);
};
- private static final Comparator<Object> byteComp = (o1, o2) -> {
+ // Byte ascending nullable comparator
+ private static final Comparator<Byte> byteComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- byte c1 = (Byte) o1, c2 = (Byte) o2;
- return Byte.compare(c1, c2);
+ return Byte.compare(o1, o2);
};
- private static final Comparator<Object> shortComp = (o1, o2) -> {
+ // Short ascending nullable comparator
+ private static final Comparator<Short> shortComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- short c1 = (Short) o1, c2 = (Short) o2;
- return Short.compare(c1, c2);
+ return Short.compare(o1, o2);
};
- private static final Comparator<Object> integerComp = (o1, o2) -> {
+ // Integer ascending nullable comparator
+ private static final Comparator<Integer> integerComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- int c1 = (Integer) o1, c2 = (Integer) o2;
- return Integer.compare(c1, c2);
+ return Integer.compare(o1, o2);
};
- private static final Comparator<Object> longComp = (o1, o2) -> {
+ // Long ascending nullable comparator
+ private static final Comparator<Long> longComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- long c1 = (Long) o1, c2 = (Long) o2;
- return Long.compare(c1, c2);
+ return Long.compare(o1, o2);
};
- private static final Comparator<Object> floatComp = (o1, o2) -> {
+ // Float ascending nullable comparator
+ private static final Comparator<Float> floatComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- float c1 = (Float) o1, c2 = (Float) o2;
- return SQLOrderingUtil.compareFloats(c1, c2);
+ return SQLOrderingUtil.compareFloats(o1, o2);
};
- private static final Comparator<Object> doubleComp = (o1, o2) -> {
+ // Double ascending nullable comparator
+ private static final Comparator<Double> doubleComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- double c1 = (Double) o1, c2 = (Double) o2;
- return SQLOrderingUtil.compareDoubles(c1, c2);
+ return SQLOrderingUtil.compareDoubles(o1, o2);
};
- public static int binarySearchNullSafe(ArrayData data, Boolean value) {
- return Arrays.binarySearch(data.toObjectArray(BooleanType$.MODULE$), value, booleanComp);
+ // boolean
+ // boolean non-nullable
+ public static int binarySearch(boolean[] data, boolean value) {
Review Comment:
since the array is sorted, I think it should be pretty easy to find a boolean value from it, by just checking the first or the last element.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1770932660
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -1525,6 +1526,126 @@ case class ArrayContains(left: Expression, right: Expression)
copy(left = newLeft, right = newRight)
}
+/**
+ * This expression converts data of `ArrayData` to an array of java type.
+ *
+ * NOTE: When the data type of expression is `ArrayType`, and the expression is foldable,
+ * the `ConstantFolding` can do constant folding optimization automatically,
+ * (avoiding frequent calls to `ArrayData.to{XXX}Array()`).
+ */
+case class ToJavaArray(array: Expression)
+ extends UnaryExpression
+ with ImplicitCastInputTypes
+ with NullIntolerant
+ with QueryErrorsBase {
+
+ override def checkInputDataTypes(): TypeCheckResult = array.dataType match {
+ case ArrayType(_, _) =>
+ TypeCheckResult.TypeCheckSuccess
+ case _ =>
+ DataTypeMismatch(
+ errorSubClass = "UNEXPECTED_INPUT_TYPE",
+ messageParameters = Map(
+ "paramIndex" -> ordinalNumber(0),
+ "requiredType" -> toSQLType(ArrayType),
+ "inputSql" -> toSQLExpr(array),
+ "inputType" -> toSQLType(array.dataType))
+ )
+ }
+
+ override def inputTypes: Seq[AbstractDataType] = Seq(array.dataType)
+ override def dataType: DataType = {
+ if (canPerformFast) {
+ elementType match {
+ case ByteType => ObjectType(classOf[Array[Byte]])
+ case ShortType => ObjectType(classOf[Array[Short]])
+ case IntegerType => ObjectType(classOf[Array[Int]])
+ case LongType => ObjectType(classOf[Array[Long]])
+ case FloatType => ObjectType(classOf[Array[Float]])
+ case DoubleType => ObjectType(classOf[Array[Double]])
+ }
+ } else if (isPrimitiveType) {
+ elementType match {
+ case BooleanType => ObjectType(classOf[Array[java.lang.Boolean]])
+ case ByteType => ObjectType(classOf[Array[java.lang.Byte]])
+ case ShortType => ObjectType(classOf[Array[java.lang.Short]])
+ case IntegerType => ObjectType(classOf[Array[java.lang.Integer]])
+ case LongType => ObjectType(classOf[Array[java.lang.Long]])
+ case FloatType => ObjectType(classOf[Array[java.lang.Float]])
+ case DoubleType => ObjectType(classOf[Array[java.lang.Double]])
+ }
+ } else {
+ ObjectType(classOf[Array[Object]])
+ }
+ }
+
+ override def child: Expression = array
+ override def prettyName: String = "to_java_array"
+
+ @transient lazy val elementType: DataType =
+ array.dataType.asInstanceOf[ArrayType].elementType
+ private def resultArrayElementNullable: Boolean =
+ array.dataType.asInstanceOf[ArrayType].containsNull
+ private def isPrimitiveType: Boolean = CodeGenerator.isPrimitiveType(elementType)
+ private def canPerformFast: Boolean =
+ isPrimitiveType && elementType != BooleanType && !resultArrayElementNullable
+
+ private def toJavaArray(array: Any): Any = {
+ val arrayData = array.asInstanceOf[ArrayData]
+ if (canPerformFast) {
+ elementType match {
+ case ByteType => arrayData.toByteArray()
+ case ShortType => arrayData.toShortArray()
+ case IntegerType => arrayData.toIntArray()
+ case LongType => arrayData.toLongArray()
+ case FloatType => arrayData.toFloatArray()
+ case DoubleType => arrayData.toDoubleArray()
+ }
+ } else if (isPrimitiveType) {
+ elementType match {
+ case BooleanType => arrayData.toArray[java.lang.Boolean](BooleanType)
+ case ByteType => arrayData.toArray[java.lang.Byte](ByteType)
+ case ShortType => arrayData.toArray[java.lang.Short](ShortType)
+ case IntegerType => arrayData.toArray[java.lang.Integer](IntegerType)
+ case LongType => arrayData.toArray[java.lang.Long](LongType)
+ case FloatType => arrayData.toArray[java.lang.Float](FloatType)
+ case DoubleType => arrayData.toArray[java.lang.Double](DoubleType)
Review Comment:
If we use `Invoke` to implement `ToJavaArray`, we must wrap it with `java.lang.xxx`, otherwise `ConstantFolding` cannot optimize it because:
https://github.com/apache/spark/blob/d2e8c1cb60e34a1c7e92374c07d682aa5ca79145/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala#L74-L75
https://github.com/apache/spark/blob/d2e8c1cb60e34a1c7e92374c07d682aa5ca79145/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala#L88-L94
Cannot optimize `ObjectType(java.lang.Object)`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #47984:
URL: https://github.com/apache/spark/pull/47984#issuecomment-2342733804
> @zhengruifeng This is a good idea. I think `ArrayBinarySearch` should be replaced by `invoke_binary_search_function(ToJavaArray(array_expr))`, and `ConstantFolding` should do the optimization automatically. For the new expression `ToJavaArray`, it can create primitive java array if the element type is primitive type and not nullable.
@zhengruifeng @cloud-fan
The latest logic based on `ToJavaType` has been submitted.
Please help review it when you have free time, 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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1768747975
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -1525,6 +1526,126 @@ case class ArrayContains(left: Expression, right: Expression)
copy(left = newLeft, right = newRight)
}
+/**
+ * This expression converts data of `ArrayData` to an array of java type.
+ *
+ * NOTE: When the data type of expression is `ArrayType`, and the expression is foldable,
+ * the `ConstantFolding` can do constant folding optimization automatically,
+ * (avoiding frequent calls to `ArrayData.to{XXX}Array()`).
+ */
+case class ToJavaArray(array: Expression)
+ extends UnaryExpression
+ with ImplicitCastInputTypes
Review Comment:
since we override `checkInputDataTypes` directly, we do not need to extend `ImplicitCastInputTypes`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun closed pull request #47984: [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array
URL: https://github.com/apache/spark/pull/47984
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1754110387
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -1519,6 +1520,124 @@ case class ArrayContains(left: Expression, right: Expression)
copy(left = newLeft, right = newRight)
}
+/**
+ * This expression converts data of `ArrayData` to an array of java type.
+ *
+ * NOTE: When the data type of expression is `ArrayType`, and the expression is foldable,
+ * the `ConstantFolding` can do constant folding optimization automatically,
+ * (avoiding frequent calls to `ArrayData.to{XXX}Array()`).
+ */
+case class ToJavaArray(array: Expression)
+ extends UnaryExpression
+ with ImplicitCastInputTypes
+ with NullIntolerant
+ with QueryErrorsBase {
+
+ override def checkInputDataTypes(): TypeCheckResult = array.dataType match {
+ case ArrayType(_, _) =>
+ TypeCheckResult.TypeCheckSuccess
+ case _ =>
+ DataTypeMismatch(
+ errorSubClass = "UNEXPECTED_INPUT_TYPE",
+ messageParameters = Map(
+ "paramIndex" -> ordinalNumber(0),
+ "requiredType" -> toSQLType(ArrayType),
+ "inputSql" -> toSQLExpr(array),
+ "inputType" -> toSQLType(array.dataType))
+ )
+ }
+
+ override def inputTypes: Seq[AbstractDataType] = Seq(array.dataType)
+ override def dataType: DataType = {
+ if (isPrimitiveType && elementType != BooleanType && !resultArrayElementNullable) {
+ elementType match {
+ case ByteType => ObjectType(classOf[Array[Byte]])
+ case ShortType => ObjectType(classOf[Array[Short]])
+ case IntegerType => ObjectType(classOf[Array[Int]])
+ case LongType => ObjectType(classOf[Array[Long]])
+ case FloatType => ObjectType(classOf[Array[Float]])
+ case DoubleType => ObjectType(classOf[Array[Double]])
+ }
+ } else if (isPrimitiveType) {
+ elementType match {
+ case BooleanType => ObjectType(classOf[Array[java.lang.Boolean]])
+ case ByteType => ObjectType(classOf[Array[java.lang.Byte]])
+ case ShortType => ObjectType(classOf[Array[java.lang.Short]])
+ case IntegerType => ObjectType(classOf[Array[java.lang.Integer]])
+ case LongType => ObjectType(classOf[Array[java.lang.Long]])
+ case FloatType => ObjectType(classOf[Array[java.lang.Float]])
+ case DoubleType => ObjectType(classOf[Array[java.lang.Double]])
+ }
+ } else {
+ ObjectType(classOf[Array[Object]])
+ }
+ }
+
+ override def child: Expression = array
+ override def prettyName: String = "to_java_array"
+
+ @transient lazy val elementType: DataType =
+ array.dataType.asInstanceOf[ArrayType].elementType
+ private def resultArrayElementNullable: Boolean =
+ array.dataType.asInstanceOf[ArrayType].containsNull
+ @transient private lazy val isPrimitiveType: Boolean = CodeGenerator.isPrimitiveType(elementType)
+
+ private def toJavaArray(array: Any): Any = {
+ val arrayData = array.asInstanceOf[ArrayData]
Review Comment:
`GenericArrayData.toXXXArray` still needs a deep copy
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #47984:
URL: https://github.com/apache/spark/pull/47984#issuecomment-2328158996
```
object ArrayBinarySearchBenchmark extends SqlBasedBenchmark {
private val N = 10000000
private val M = 100
private val arrayData = (0 until M).mkString("array(", ",", ")")
private val exprs = s"array_binary_search($arrayData, value % $M)"
private val df = spark.range(N).toDF("value")
private def doBenchmark(): Unit = {
df.selectExpr(exprs).noop()
}
override def runBenchmarkSuite(mainArgs: Array[String]): Unit = {
runBenchmark("array binary search") {
val benchmark = new Benchmark("array binary search", N, output = output)
benchmark.addCase("no foldable optimize", M) { _ =>
doBenchmark()
}
benchmark.run()
}
}
}
```
Before:
```
```
After:
```
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1753942612
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala:
##########
@@ -920,6 +920,7 @@ object FunctionRegistry {
registerInternalExpression[EWM]("ewm")
registerInternalExpression[NullIndex]("null_index")
registerInternalExpression[CastTimestampNTZToLong]("timestamp_ntz_to_long")
+ registerInternalExpression[ToJavaArray]("to_java_array")
Review Comment:
`registerInternalExpression` is for functions API, I think we don't need to register it 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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1756233364
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala:
##########
@@ -137,14 +137,32 @@ class CollectionExpressionsSuite
test("ArrayBinarySearch") {
// primitive type: boolean、byte、short、int、long、float、double
+ // boolean
Review Comment:
Yes, there are below
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1754262601
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala:
##########
@@ -920,6 +920,7 @@ object FunctionRegistry {
registerInternalExpression[EWM]("ewm")
registerInternalExpression[NullIndex]("null_index")
registerInternalExpression[CastTimestampNTZToLong]("timestamp_ntz_to_long")
+ registerInternalExpression[ToJavaArray]("to_java_array")
Review Comment:
Updated.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1768745114
##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ArrayExpressionUtils.java:
##########
@@ -19,158 +19,168 @@
import java.util.Arrays;
import java.util.Comparator;
-import org.apache.spark.sql.catalyst.util.ArrayData;
import org.apache.spark.sql.catalyst.util.SQLOrderingUtil;
-import org.apache.spark.sql.types.ByteType$;
-import org.apache.spark.sql.types.BooleanType$;
-import org.apache.spark.sql.types.DataType;
-import org.apache.spark.sql.types.DoubleType$;
-import org.apache.spark.sql.types.FloatType$;
-import org.apache.spark.sql.types.IntegerType$;
-import org.apache.spark.sql.types.LongType$;
-import org.apache.spark.sql.types.ShortType$;
public class ArrayExpressionUtils {
- private static final Comparator<Object> booleanComp = (o1, o2) -> {
+ // comparator
+ // Boolean ascending nullable comparator
+ private static final Comparator<Boolean> booleanComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- boolean c1 = (Boolean) o1, c2 = (Boolean) o2;
- return c1 == c2 ? 0 : (c1 ? 1 : -1);
+ return o1.equals(o2) ? 0 : (o1 ? 1 : -1);
};
- private static final Comparator<Object> byteComp = (o1, o2) -> {
+ // Byte ascending nullable comparator
+ private static final Comparator<Byte> byteComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- byte c1 = (Byte) o1, c2 = (Byte) o2;
- return Byte.compare(c1, c2);
+ return Byte.compare(o1, o2);
};
- private static final Comparator<Object> shortComp = (o1, o2) -> {
+ // Short ascending nullable comparator
+ private static final Comparator<Short> shortComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- short c1 = (Short) o1, c2 = (Short) o2;
- return Short.compare(c1, c2);
+ return Short.compare(o1, o2);
};
- private static final Comparator<Object> integerComp = (o1, o2) -> {
+ // Integer ascending nullable comparator
+ private static final Comparator<Integer> integerComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- int c1 = (Integer) o1, c2 = (Integer) o2;
- return Integer.compare(c1, c2);
+ return Integer.compare(o1, o2);
};
- private static final Comparator<Object> longComp = (o1, o2) -> {
+ // Long ascending nullable comparator
+ private static final Comparator<Long> longComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- long c1 = (Long) o1, c2 = (Long) o2;
- return Long.compare(c1, c2);
+ return Long.compare(o1, o2);
};
- private static final Comparator<Object> floatComp = (o1, o2) -> {
+ // Float ascending nullable comparator
+ private static final Comparator<Float> floatComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- float c1 = (Float) o1, c2 = (Float) o2;
- return SQLOrderingUtil.compareFloats(c1, c2);
+ return SQLOrderingUtil.compareFloats(o1, o2);
};
- private static final Comparator<Object> doubleComp = (o1, o2) -> {
+ // Double ascending nullable comparator
+ private static final Comparator<Double> doubleComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- double c1 = (Double) o1, c2 = (Double) o2;
- return SQLOrderingUtil.compareDoubles(c1, c2);
+ return SQLOrderingUtil.compareDoubles(o1, o2);
};
- public static int binarySearchNullSafe(ArrayData data, Boolean value) {
- return Arrays.binarySearch(data.toObjectArray(BooleanType$.MODULE$), value, booleanComp);
+ // Boolean nullable
Review Comment:
should we have a version for primitive boolean?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1772503057
##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ArrayExpressionUtils.java:
##########
@@ -19,158 +19,288 @@
import java.util.Arrays;
import java.util.Comparator;
+import scala.reflect.ClassTag$;
+
import org.apache.spark.sql.catalyst.util.ArrayData;
import org.apache.spark.sql.catalyst.util.SQLOrderingUtil;
-import org.apache.spark.sql.types.ByteType$;
-import org.apache.spark.sql.types.BooleanType$;
import org.apache.spark.sql.types.DataType;
-import org.apache.spark.sql.types.DoubleType$;
-import org.apache.spark.sql.types.FloatType$;
-import org.apache.spark.sql.types.IntegerType$;
-import org.apache.spark.sql.types.LongType$;
-import org.apache.spark.sql.types.ShortType$;
+
+import static org.apache.spark.sql.types.DataTypes.BooleanType;
+import static org.apache.spark.sql.types.DataTypes.ByteType;
+import static org.apache.spark.sql.types.DataTypes.DoubleType;
+import static org.apache.spark.sql.types.DataTypes.FloatType;
+import static org.apache.spark.sql.types.DataTypes.IntegerType;
+import static org.apache.spark.sql.types.DataTypes.LongType;
+import static org.apache.spark.sql.types.DataTypes.ShortType;
public class ArrayExpressionUtils {
- private static final Comparator<Object> booleanComp = (o1, o2) -> {
+ // comparator
+ // Boolean ascending nullable comparator
+ private static final Comparator<Boolean> booleanComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- boolean c1 = (Boolean) o1, c2 = (Boolean) o2;
- return c1 == c2 ? 0 : (c1 ? 1 : -1);
+ return o1.equals(o2) ? 0 : (o1 ? 1 : -1);
};
- private static final Comparator<Object> byteComp = (o1, o2) -> {
+ // Byte ascending nullable comparator
+ private static final Comparator<Byte> byteComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- byte c1 = (Byte) o1, c2 = (Byte) o2;
- return Byte.compare(c1, c2);
+ return Byte.compare(o1, o2);
};
- private static final Comparator<Object> shortComp = (o1, o2) -> {
+ // Short ascending nullable comparator
+ private static final Comparator<Short> shortComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- short c1 = (Short) o1, c2 = (Short) o2;
- return Short.compare(c1, c2);
+ return Short.compare(o1, o2);
};
- private static final Comparator<Object> integerComp = (o1, o2) -> {
+ // Integer ascending nullable comparator
+ private static final Comparator<Integer> integerComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- int c1 = (Integer) o1, c2 = (Integer) o2;
- return Integer.compare(c1, c2);
+ return Integer.compare(o1, o2);
};
- private static final Comparator<Object> longComp = (o1, o2) -> {
+ // Long ascending nullable comparator
+ private static final Comparator<Long> longComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- long c1 = (Long) o1, c2 = (Long) o2;
- return Long.compare(c1, c2);
+ return Long.compare(o1, o2);
};
- private static final Comparator<Object> floatComp = (o1, o2) -> {
+ // Float ascending nullable comparator
+ private static final Comparator<Float> floatComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- float c1 = (Float) o1, c2 = (Float) o2;
- return SQLOrderingUtil.compareFloats(c1, c2);
+ return SQLOrderingUtil.compareFloats(o1, o2);
};
- private static final Comparator<Object> doubleComp = (o1, o2) -> {
+ // Double ascending nullable comparator
+ private static final Comparator<Double> doubleComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- double c1 = (Double) o1, c2 = (Double) o2;
- return SQLOrderingUtil.compareDoubles(c1, c2);
+ return SQLOrderingUtil.compareDoubles(o1, o2);
};
- public static int binarySearchNullSafe(ArrayData data, Boolean value) {
- return Arrays.binarySearch(data.toObjectArray(BooleanType$.MODULE$), value, booleanComp);
+ // boolean
+ // boolean non-nullable
+ public static int binarySearch(boolean[] data, boolean value) {
Review Comment:
I see, we should have consistent result between the primitive and boxed versions
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1750043326
##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ArrayExpressionUtils.java:
##########
@@ -116,58 +116,131 @@ public class ArrayExpressionUtils {
return SQLOrderingUtil.compareDoubles(c1, c2);
};
+ // boolean
+ // foldable optimize
+ public static int binarySearchNullSafe(Boolean[] data, Boolean value) {
Review Comment:
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1772487830
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -1525,6 +1525,103 @@ case class ArrayContains(left: Expression, right: Expression)
copy(left = newLeft, right = newRight)
}
+/**
+ * This expression converts data of `ArrayData` to an array of java type.
+ *
+ * NOTE: When the data type of expression is `ArrayType`, and the expression is foldable,
+ * the `ConstantFolding` can do constant folding optimization automatically,
+ * (avoiding frequent calls to `ArrayData.to{XXX}Array()`).
+ */
+case class ToJavaArray(array: Expression)
Review Comment:
Done. I have also extracted `the Invoke logic` related to `ToJavaArray` into a new file: `ToJavaArrayUtils.java`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1754107399
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -1519,6 +1520,124 @@ case class ArrayContains(left: Expression, right: Expression)
copy(left = newLeft, right = newRight)
}
+/**
+ * This expression converts data of `ArrayData` to an array of java type.
+ *
+ * NOTE: When the data type of expression is `ArrayType`, and the expression is foldable,
+ * the `ConstantFolding` can do constant folding optimization automatically,
+ * (avoiding frequent calls to `ArrayData.to{XXX}Array()`).
+ */
+case class ToJavaArray(array: Expression)
+ extends UnaryExpression
+ with ImplicitCastInputTypes
+ with NullIntolerant
+ with QueryErrorsBase {
+
+ override def checkInputDataTypes(): TypeCheckResult = array.dataType match {
+ case ArrayType(_, _) =>
+ TypeCheckResult.TypeCheckSuccess
+ case _ =>
+ DataTypeMismatch(
+ errorSubClass = "UNEXPECTED_INPUT_TYPE",
+ messageParameters = Map(
+ "paramIndex" -> ordinalNumber(0),
+ "requiredType" -> toSQLType(ArrayType),
+ "inputSql" -> toSQLExpr(array),
+ "inputType" -> toSQLType(array.dataType))
+ )
+ }
+
+ override def inputTypes: Seq[AbstractDataType] = Seq(array.dataType)
+ override def dataType: DataType = {
+ if (isPrimitiveType && elementType != BooleanType && !resultArrayElementNullable) {
+ elementType match {
+ case ByteType => ObjectType(classOf[Array[Byte]])
+ case ShortType => ObjectType(classOf[Array[Short]])
+ case IntegerType => ObjectType(classOf[Array[Int]])
+ case LongType => ObjectType(classOf[Array[Long]])
+ case FloatType => ObjectType(classOf[Array[Float]])
+ case DoubleType => ObjectType(classOf[Array[Double]])
+ }
+ } else if (isPrimitiveType) {
+ elementType match {
+ case BooleanType => ObjectType(classOf[Array[java.lang.Boolean]])
+ case ByteType => ObjectType(classOf[Array[java.lang.Byte]])
+ case ShortType => ObjectType(classOf[Array[java.lang.Short]])
+ case IntegerType => ObjectType(classOf[Array[java.lang.Integer]])
+ case LongType => ObjectType(classOf[Array[java.lang.Long]])
+ case FloatType => ObjectType(classOf[Array[java.lang.Float]])
+ case DoubleType => ObjectType(classOf[Array[java.lang.Double]])
+ }
+ } else {
+ ObjectType(classOf[Array[Object]])
+ }
+ }
+
+ override def child: Expression = array
+ override def prettyName: String = "to_java_array"
+
+ @transient lazy val elementType: DataType =
+ array.dataType.asInstanceOf[ArrayType].elementType
+ private def resultArrayElementNullable: Boolean =
+ array.dataType.asInstanceOf[ArrayType].containsNull
+ @transient private lazy val isPrimitiveType: Boolean = CodeGenerator.isPrimitiveType(elementType)
+
+ private def toJavaArray(array: Any): Any = {
+ val arrayData = array.asInstanceOf[ArrayData]
Review Comment:
I am wondering how the array value is cached, do we need to match `GenericArrayData` in this method?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1754278893
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala:
##########
@@ -137,14 +137,32 @@ class CollectionExpressionsSuite
test("ArrayBinarySearch") {
// primitive type: boolean、byte、short、int、long、float、double
+ // boolean
+ // boolean foldable
Review Comment:
foldable test case
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala:
##########
@@ -137,14 +137,32 @@ class CollectionExpressionsSuite
test("ArrayBinarySearch") {
// primitive type: boolean、byte、short、int、long、float、double
+ // boolean
+ // boolean foldable
val a0_0 = Literal.create(Seq(false, true),
ArrayType(BooleanType, containsNull = false))
checkEvaluation(ArrayBinarySearch(a0_0, Literal(true)), 1)
val a0_1 = Literal.create(Seq(null, false, true), ArrayType(BooleanType))
checkEvaluation(ArrayBinarySearch(a0_1, Literal(false)), 1)
val a0_2 = Literal.create(Seq(null, false, true), ArrayType(BooleanType))
checkEvaluation(ArrayBinarySearch(a0_2, Literal(null, BooleanType)), null)
-
+ val a0_3 = CreateArray(Seq(Literal(false), Literal(true)))
+ checkEvaluation(ArrayBinarySearch(a0_3, Literal(true)), 1)
+ val a0_4 = CreateArray(Seq(Literal(null, BooleanType), Literal(false), Literal(true)))
+ checkEvaluation(ArrayBinarySearch(a0_4, Literal(false)), 1)
+ val a0_5 = CreateArray(Seq(Literal(null, BooleanType), Literal(false), Literal(true)))
+ checkEvaluation(ArrayBinarySearch(a0_5, Literal(null, BooleanType)), null)
+ // boolean non-foldable
Review Comment:
non-foldable test case
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #47984:
URL: https://github.com/apache/spark/pull/47984#issuecomment-2342780364
- Before:
```
Running benchmark: array binary search
Running case: has foldable optimize
Stopped after 100 iterations, 371269 ms
OpenJDK 64-Bit Server VM 17.0.10+7-LTS on Mac OS X 14.6.1
Apple M2
array binary search: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
has foldable optimize 3585 3713 100 2.8 358.5 1.0X
```
- After:
```
Running benchmark: array binary search
Running case: has foldable optimize
Stopped after 100 iterations, 21097 ms
OpenJDK 64-Bit Server VM 17.0.10+7-LTS on Mac OS X 14.6.1
Apple M2
array binary search: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
has foldable optimize 201 211 23 49.9 20.1 1.0X
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1753244653
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/benchmark/ArrayBinarySearchBenchmark.scala:
##########
@@ -0,0 +1,59 @@
+/*
+ * 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.benchmark
+
+import org.apache.spark.benchmark.Benchmark
+import org.apache.spark.sql.Column
+import org.apache.spark.sql.functions.{col, lit}
+import org.apache.spark.sql.types.StructType
+
+/**
+ * Synthetic benchmark for array_binary_search functions.
+ * To run this benchmark:
+ * {{{
+ * 1. without sbt:
+ * bin/spark-submit --class <this class>
+ * --jars <spark core test jar>,<spark catalyst test jar> <sql core test jar>
+ * 2. build/sbt "sql/Test/runMain <this class>"
+ * 3. generate result:
+ * SPARK_GENERATE_BENCHMARK_FILES=1 build/sbt "sql/Test/runMain <this class>"
+ * Results will be written to "benchmarks/ArrayBinarySearchBenchmark-results.txt".
+ * }}}
+ */
+object ArrayBinarySearchBenchmark extends SqlBasedBenchmark {
Review Comment:
If we don't need `ArrayBinarySearchBenchmark`, I can delete it.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1766680923
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/benchmark/ArrayBinarySearchBenchmark.scala:
##########
@@ -0,0 +1,59 @@
+/*
+ * 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.benchmark
+
+import org.apache.spark.benchmark.Benchmark
+import org.apache.spark.sql.Column
+import org.apache.spark.sql.functions.{col, lit}
+import org.apache.spark.sql.types.StructType
+
+/**
+ * Synthetic benchmark for array_binary_search functions.
+ * To run this benchmark:
+ * {{{
+ * 1. without sbt:
+ * bin/spark-submit --class <this class>
+ * --jars <spark core test jar>,<spark catalyst test jar> <sql core test jar>
+ * 2. build/sbt "sql/Test/runMain <this class>"
+ * 3. generate result:
+ * SPARK_GENERATE_BENCHMARK_FILES=1 build/sbt "sql/Test/runMain <this class>"
+ * Results will be written to "benchmarks/ArrayBinarySearchBenchmark-results.txt".
+ * }}}
+ */
+object ArrayBinarySearchBenchmark extends SqlBasedBenchmark {
Review Comment:
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1767826212
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/benchmark/ArrayBinarySearchBenchmark.scala:
##########
@@ -0,0 +1,59 @@
+/*
+ * 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.benchmark
+
+import org.apache.spark.benchmark.Benchmark
+import org.apache.spark.sql.Column
+import org.apache.spark.sql.functions.{col, lit}
+import org.apache.spark.sql.types.StructType
+
+/**
+ * Synthetic benchmark for array_binary_search functions.
+ * To run this benchmark:
+ * {{{
+ * 1. without sbt:
+ * bin/spark-submit --class <this class>
+ * --jars <spark core test jar>,<spark catalyst test jar> <sql core test jar>
+ * 2. build/sbt "sql/Test/runMain <this class>"
+ * 3. generate result:
+ * SPARK_GENERATE_BENCHMARK_FILES=1 build/sbt "sql/Test/runMain <this class>"
+ * Results will be written to "benchmarks/ArrayBinarySearchBenchmark-results.txt".
+ * }}}
+ */
+object ArrayBinarySearchBenchmark extends SqlBasedBenchmark {
Review Comment:
I have already deleted it.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1770954515
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala:
##########
@@ -90,7 +90,24 @@ trait InvokeLike extends Expression with NonSQLExpression with ImplicitCastInput
// serializability, because the type-level info with java.io.Serializable and
// java.io.Externalizable marker interfaces are not strong guarantees.
// This restriction can be relaxed in the future to expose more optimizations.
- !dt.existsRecursively(_.isInstanceOf[ObjectType])
+ !dt.existsRecursively {
+ case ObjectType(cls) if cls == classOf[Array[java.lang.Boolean]] => false
Review Comment:
- `case ObjectType(cls) if cls == classOf[Array[java.lang....]]` must be added here.
- `case ObjectType(cls) if cls == classOf[java.lang....]` was added by me casually (unrelated to this PR, but I think it should be added)
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #47984:
URL: https://github.com/apache/spark/pull/47984#issuecomment-2371041115
@zhengruifeng @cloud-fan
I'm very sorry that I broke this PR and couldn't restore it, so I opened a new one
https://github.com/apache/spark/pull/48225
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1770932660
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -1525,6 +1526,126 @@ case class ArrayContains(left: Expression, right: Expression)
copy(left = newLeft, right = newRight)
}
+/**
+ * This expression converts data of `ArrayData` to an array of java type.
+ *
+ * NOTE: When the data type of expression is `ArrayType`, and the expression is foldable,
+ * the `ConstantFolding` can do constant folding optimization automatically,
+ * (avoiding frequent calls to `ArrayData.to{XXX}Array()`).
+ */
+case class ToJavaArray(array: Expression)
+ extends UnaryExpression
+ with ImplicitCastInputTypes
+ with NullIntolerant
+ with QueryErrorsBase {
+
+ override def checkInputDataTypes(): TypeCheckResult = array.dataType match {
+ case ArrayType(_, _) =>
+ TypeCheckResult.TypeCheckSuccess
+ case _ =>
+ DataTypeMismatch(
+ errorSubClass = "UNEXPECTED_INPUT_TYPE",
+ messageParameters = Map(
+ "paramIndex" -> ordinalNumber(0),
+ "requiredType" -> toSQLType(ArrayType),
+ "inputSql" -> toSQLExpr(array),
+ "inputType" -> toSQLType(array.dataType))
+ )
+ }
+
+ override def inputTypes: Seq[AbstractDataType] = Seq(array.dataType)
+ override def dataType: DataType = {
+ if (canPerformFast) {
+ elementType match {
+ case ByteType => ObjectType(classOf[Array[Byte]])
+ case ShortType => ObjectType(classOf[Array[Short]])
+ case IntegerType => ObjectType(classOf[Array[Int]])
+ case LongType => ObjectType(classOf[Array[Long]])
+ case FloatType => ObjectType(classOf[Array[Float]])
+ case DoubleType => ObjectType(classOf[Array[Double]])
+ }
+ } else if (isPrimitiveType) {
+ elementType match {
+ case BooleanType => ObjectType(classOf[Array[java.lang.Boolean]])
+ case ByteType => ObjectType(classOf[Array[java.lang.Byte]])
+ case ShortType => ObjectType(classOf[Array[java.lang.Short]])
+ case IntegerType => ObjectType(classOf[Array[java.lang.Integer]])
+ case LongType => ObjectType(classOf[Array[java.lang.Long]])
+ case FloatType => ObjectType(classOf[Array[java.lang.Float]])
+ case DoubleType => ObjectType(classOf[Array[java.lang.Double]])
+ }
+ } else {
+ ObjectType(classOf[Array[Object]])
+ }
+ }
+
+ override def child: Expression = array
+ override def prettyName: String = "to_java_array"
+
+ @transient lazy val elementType: DataType =
+ array.dataType.asInstanceOf[ArrayType].elementType
+ private def resultArrayElementNullable: Boolean =
+ array.dataType.asInstanceOf[ArrayType].containsNull
+ private def isPrimitiveType: Boolean = CodeGenerator.isPrimitiveType(elementType)
+ private def canPerformFast: Boolean =
+ isPrimitiveType && elementType != BooleanType && !resultArrayElementNullable
+
+ private def toJavaArray(array: Any): Any = {
+ val arrayData = array.asInstanceOf[ArrayData]
+ if (canPerformFast) {
+ elementType match {
+ case ByteType => arrayData.toByteArray()
+ case ShortType => arrayData.toShortArray()
+ case IntegerType => arrayData.toIntArray()
+ case LongType => arrayData.toLongArray()
+ case FloatType => arrayData.toFloatArray()
+ case DoubleType => arrayData.toDoubleArray()
+ }
+ } else if (isPrimitiveType) {
+ elementType match {
+ case BooleanType => arrayData.toArray[java.lang.Boolean](BooleanType)
+ case ByteType => arrayData.toArray[java.lang.Byte](ByteType)
+ case ShortType => arrayData.toArray[java.lang.Short](ShortType)
+ case IntegerType => arrayData.toArray[java.lang.Integer](IntegerType)
+ case LongType => arrayData.toArray[java.lang.Long](LongType)
+ case FloatType => arrayData.toArray[java.lang.Float](FloatType)
+ case DoubleType => arrayData.toArray[java.lang.Double](DoubleType)
Review Comment:
If we use `Invoke` to implement `ToJavaArray`, we must wrap it with `java.lang.[Short/Integer/...]`, otherwise `ConstantFolding` cannot optimize it because:
https://github.com/apache/spark/blob/d2e8c1cb60e34a1c7e92374c07d682aa5ca79145/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala#L74-L75
https://github.com/apache/spark/blob/d2e8c1cb60e34a1c7e92374c07d682aa5ca79145/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala#L88-L94
Cannot optimize `ObjectType(java.lang.Object)`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #47984:
URL: https://github.com/apache/spark/pull/47984#issuecomment-2328726761
Thanks @panbingkun for working on this!
Existing usages (pyspark and ml) both apply binary search with a literal double array, this optimization will improve the performance of them.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1771358154
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -1525,6 +1525,103 @@ case class ArrayContains(left: Expression, right: Expression)
copy(left = newLeft, right = newRight)
}
+/**
+ * This expression converts data of `ArrayData` to an array of java type.
+ *
+ * NOTE: When the data type of expression is `ArrayType`, and the expression is foldable,
+ * the `ConstantFolding` can do constant folding optimization automatically,
+ * (avoiding frequent calls to `ArrayData.to{XXX}Array()`).
+ */
+case class ToJavaArray(array: Expression)
Review Comment:
Can we have a new file for it? This file is already large enough.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1772510920
##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ArrayExpressionUtils.java:
##########
@@ -19,158 +19,288 @@
import java.util.Arrays;
import java.util.Comparator;
+import scala.reflect.ClassTag$;
+
import org.apache.spark.sql.catalyst.util.ArrayData;
import org.apache.spark.sql.catalyst.util.SQLOrderingUtil;
-import org.apache.spark.sql.types.ByteType$;
-import org.apache.spark.sql.types.BooleanType$;
import org.apache.spark.sql.types.DataType;
-import org.apache.spark.sql.types.DoubleType$;
-import org.apache.spark.sql.types.FloatType$;
-import org.apache.spark.sql.types.IntegerType$;
-import org.apache.spark.sql.types.LongType$;
-import org.apache.spark.sql.types.ShortType$;
+
+import static org.apache.spark.sql.types.DataTypes.BooleanType;
+import static org.apache.spark.sql.types.DataTypes.ByteType;
+import static org.apache.spark.sql.types.DataTypes.DoubleType;
+import static org.apache.spark.sql.types.DataTypes.FloatType;
+import static org.apache.spark.sql.types.DataTypes.IntegerType;
+import static org.apache.spark.sql.types.DataTypes.LongType;
+import static org.apache.spark.sql.types.DataTypes.ShortType;
public class ArrayExpressionUtils {
- private static final Comparator<Object> booleanComp = (o1, o2) -> {
+ // comparator
+ // Boolean ascending nullable comparator
+ private static final Comparator<Boolean> booleanComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- boolean c1 = (Boolean) o1, c2 = (Boolean) o2;
- return c1 == c2 ? 0 : (c1 ? 1 : -1);
+ return o1.equals(o2) ? 0 : (o1 ? 1 : -1);
};
- private static final Comparator<Object> byteComp = (o1, o2) -> {
+ // Byte ascending nullable comparator
+ private static final Comparator<Byte> byteComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- byte c1 = (Byte) o1, c2 = (Byte) o2;
- return Byte.compare(c1, c2);
+ return Byte.compare(o1, o2);
};
- private static final Comparator<Object> shortComp = (o1, o2) -> {
+ // Short ascending nullable comparator
+ private static final Comparator<Short> shortComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- short c1 = (Short) o1, c2 = (Short) o2;
- return Short.compare(c1, c2);
+ return Short.compare(o1, o2);
};
- private static final Comparator<Object> integerComp = (o1, o2) -> {
+ // Integer ascending nullable comparator
+ private static final Comparator<Integer> integerComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- int c1 = (Integer) o1, c2 = (Integer) o2;
- return Integer.compare(c1, c2);
+ return Integer.compare(o1, o2);
};
- private static final Comparator<Object> longComp = (o1, o2) -> {
+ // Long ascending nullable comparator
+ private static final Comparator<Long> longComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- long c1 = (Long) o1, c2 = (Long) o2;
- return Long.compare(c1, c2);
+ return Long.compare(o1, o2);
};
- private static final Comparator<Object> floatComp = (o1, o2) -> {
+ // Float ascending nullable comparator
+ private static final Comparator<Float> floatComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- float c1 = (Float) o1, c2 = (Float) o2;
- return SQLOrderingUtil.compareFloats(c1, c2);
+ return SQLOrderingUtil.compareFloats(o1, o2);
};
- private static final Comparator<Object> doubleComp = (o1, o2) -> {
+ // Double ascending nullable comparator
+ private static final Comparator<Double> doubleComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- double c1 = (Double) o1, c2 = (Double) o2;
- return SQLOrderingUtil.compareDoubles(c1, c2);
+ return SQLOrderingUtil.compareDoubles(o1, o2);
};
- public static int binarySearchNullSafe(ArrayData data, Boolean value) {
- return Arrays.binarySearch(data.toObjectArray(BooleanType$.MODULE$), value, booleanComp);
+ // boolean
+ // boolean non-nullable
+ public static int binarySearch(boolean[] data, boolean value) {
Review Comment:
Yea! ❤️
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #47984:
URL: https://github.com/apache/spark/pull/47984#issuecomment-2367148671
I will update another version today.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1770945052
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -1525,6 +1526,126 @@ case class ArrayContains(left: Expression, right: Expression)
copy(left = newLeft, right = newRight)
}
+/**
+ * This expression converts data of `ArrayData` to an array of java type.
+ *
+ * NOTE: When the data type of expression is `ArrayType`, and the expression is foldable,
+ * the `ConstantFolding` can do constant folding optimization automatically,
+ * (avoiding frequent calls to `ArrayData.to{XXX}Array()`).
+ */
+case class ToJavaArray(array: Expression)
+ extends UnaryExpression
+ with ImplicitCastInputTypes
+ with NullIntolerant
+ with QueryErrorsBase {
+
+ override def checkInputDataTypes(): TypeCheckResult = array.dataType match {
+ case ArrayType(_, _) =>
+ TypeCheckResult.TypeCheckSuccess
+ case _ =>
+ DataTypeMismatch(
+ errorSubClass = "UNEXPECTED_INPUT_TYPE",
+ messageParameters = Map(
+ "paramIndex" -> ordinalNumber(0),
+ "requiredType" -> toSQLType(ArrayType),
+ "inputSql" -> toSQLExpr(array),
+ "inputType" -> toSQLType(array.dataType))
+ )
+ }
+
+ override def inputTypes: Seq[AbstractDataType] = Seq(array.dataType)
+ override def dataType: DataType = {
+ if (canPerformFast) {
+ elementType match {
+ case ByteType => ObjectType(classOf[Array[Byte]])
+ case ShortType => ObjectType(classOf[Array[Short]])
+ case IntegerType => ObjectType(classOf[Array[Int]])
+ case LongType => ObjectType(classOf[Array[Long]])
+ case FloatType => ObjectType(classOf[Array[Float]])
+ case DoubleType => ObjectType(classOf[Array[Double]])
+ }
+ } else if (isPrimitiveType) {
+ elementType match {
+ case BooleanType => ObjectType(classOf[Array[java.lang.Boolean]])
+ case ByteType => ObjectType(classOf[Array[java.lang.Byte]])
+ case ShortType => ObjectType(classOf[Array[java.lang.Short]])
+ case IntegerType => ObjectType(classOf[Array[java.lang.Integer]])
+ case LongType => ObjectType(classOf[Array[java.lang.Long]])
+ case FloatType => ObjectType(classOf[Array[java.lang.Float]])
+ case DoubleType => ObjectType(classOf[Array[java.lang.Double]])
+ }
+ } else {
+ ObjectType(classOf[Array[Object]])
+ }
+ }
+
+ override def child: Expression = array
+ override def prettyName: String = "to_java_array"
+
+ @transient lazy val elementType: DataType =
+ array.dataType.asInstanceOf[ArrayType].elementType
+ private def resultArrayElementNullable: Boolean =
+ array.dataType.asInstanceOf[ArrayType].containsNull
+ private def isPrimitiveType: Boolean = CodeGenerator.isPrimitiveType(elementType)
+ private def canPerformFast: Boolean =
+ isPrimitiveType && elementType != BooleanType && !resultArrayElementNullable
+
+ private def toJavaArray(array: Any): Any = {
+ val arrayData = array.asInstanceOf[ArrayData]
+ if (canPerformFast) {
+ elementType match {
+ case ByteType => arrayData.toByteArray()
+ case ShortType => arrayData.toShortArray()
+ case IntegerType => arrayData.toIntArray()
+ case LongType => arrayData.toLongArray()
+ case FloatType => arrayData.toFloatArray()
+ case DoubleType => arrayData.toDoubleArray()
+ }
+ } else if (isPrimitiveType) {
+ elementType match {
+ case BooleanType => arrayData.toArray[java.lang.Boolean](BooleanType)
+ case ByteType => arrayData.toArray[java.lang.Byte](ByteType)
+ case ShortType => arrayData.toArray[java.lang.Short](ShortType)
+ case IntegerType => arrayData.toArray[java.lang.Integer](IntegerType)
+ case LongType => arrayData.toArray[java.lang.Long](LongType)
+ case FloatType => arrayData.toArray[java.lang.Float](FloatType)
+ case DoubleType => arrayData.toArray[java.lang.Double](DoubleType)
+ }
+ } else {
+ arrayData.toObjectArray(elementType)
+ }
+ }
+
+ private def toJavaArrayCodegen(
+ ctx: CodegenContext,
+ ev: ExprCode,
+ array: String): String = {
+ val elementTypeTerm = ctx.addReferenceObj("elementTypeTerm", elementType)
+ if (canPerformFast) {
+ val primitiveTypeName = CodeGenerator.primitiveTypeName(elementType)
+ s"""${ev.value} = $array.to${primitiveTypeName}Array();"""
+ } else if (isPrimitiveType) {
+ val boxedJavaType = CodeGenerator.boxedType(elementType)
+ val classTagTerm = ctx.addReferenceObj("classTagTerm",
+ ClassTag(SparkClassUtils.classForName(s"java.lang.$boxedJavaType")))
+ s"""${ev.value} = ($boxedJavaType[]) $array.toArray($elementTypeTerm, $classTagTerm);"""
+ } else {
+ s"""${ev.value} = $array.toObjectArray($elementTypeTerm);"""
+ }
+ }
+
+ override def nullSafeEval(array: Any): Any = {
Review Comment:
Based on the currently submitted version (use `Invoke` to implement `ToJavaArray`), I run benchmark 3 times and the performance is as follows:
```shell
Running benchmark: array binary search
Running case: has foldable optimize
Stopped after 100 iterations, 23205 ms
OpenJDK 64-Bit Server VM 17.0.10+7-LTS on Mac OS X 15.0
Apple M2
array binary search: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
has foldable optimize 223 232 18 44.8 22.3 1.0X
Running benchmark: array binary search
Running case: has foldable optimize
Stopped after 100 iterations, 20181 ms
OpenJDK 64-Bit Server VM 17.0.10+7-LTS on Mac OS X 15.0
Apple M2
array binary search: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
has foldable optimize 195 202 8 51.2 19.5 1.0X
Running benchmark: array binary search
Running case: has foldable optimize
Stopped after 100 iterations, 20819 ms
OpenJDK 64-Bit Server VM 17.0.10+7-LTS on Mac OS X 15.0
Apple M2
array binary search: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
has foldable optimize 202 208 8 49.4 20.2 1.0X
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1771386596
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala:
##########
@@ -90,7 +90,24 @@ trait InvokeLike extends Expression with NonSQLExpression with ImplicitCastInput
// serializability, because the type-level info with java.io.Serializable and
// java.io.Externalizable marker interfaces are not strong guarantees.
// This restriction can be relaxed in the future to expose more optimizations.
Review Comment:
We should update the comment here. We do not block all ObjectType now.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1754196197
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala:
##########
@@ -137,14 +137,32 @@ class CollectionExpressionsSuite
test("ArrayBinarySearch") {
// primitive type: boolean、byte、short、int、long、float、double
+ // boolean
Review Comment:
do we have non-foldable test cases in it?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1749883062
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -1602,6 +1602,7 @@ case class ArrayBinarySearch(array: Expression, value: Expression)
@transient private lazy val isPrimitiveType: Boolean = CodeGenerator.isPrimitiveType(elementType)
@transient private lazy val canPerformFastBinarySearch: Boolean = isPrimitiveType &&
elementType != BooleanType && !resultArrayElementNullable
+ @transient private lazy val arrayIsFoldable: Boolean = array.foldable
Review Comment:
This isn't worth a lazy val. Can a simple `def` work?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1750046424
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -1618,23 +1619,198 @@ case class ArrayBinarySearch(array: Expression, value: Expression)
}
}
+ // byte
+ // unbox byte
+ @transient private lazy val foldableByteArrayData: Array[Byte] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toByteArray()
+ }
+ @transient private lazy val foldableByteArrayDataObjectType =
+ ObjectType(classOf[Array[Byte]])
+ // boxed byte
+ @transient private lazy val foldableBoxedByteArrayData: Array[java.lang.Byte] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toArray[java.lang.Byte](ByteType)
+ }
+ @transient private lazy val foldableBoxedByteArrayDataObjectType =
+ ObjectType(classOf[Array[java.lang.Byte]])
+
+ // boolean
+ // boxed boolean
+ @transient private lazy val foldableBoxedBooleanArrayData: Array[java.lang.Boolean] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toArray[java.lang.Boolean](BooleanType)
+ }
+ @transient private lazy val foldableBoxedBooleanArrayDataObjectType =
+ ObjectType(classOf[Array[java.lang.Boolean]])
+
+ // short
+ // unbox short
+ @transient private lazy val foldableShortArrayData: Array[Short] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toShortArray()
+ }
+ @transient private lazy val foldableShortArrayDataObjectType =
+ ObjectType(classOf[Array[Short]])
+ // boxed short
+ @transient private lazy val foldableBoxedShortArrayData: Array[java.lang.Short] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toArray[java.lang.Short](ShortType)
+ }
+ @transient private lazy val foldableBoxedShortArrayDataObjectType =
+ ObjectType(classOf[Array[java.lang.Short]])
+
+ // int
+ // unbox int
+ @transient private lazy val foldableIntArrayData: Array[Int] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toIntArray()
+ }
+ @transient private lazy val foldableIntArrayDataObjectType =
+ ObjectType(classOf[Array[Int]])
+ // boxed int
+ @transient private lazy val foldableBoxedIntArrayData: Array[java.lang.Integer] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toArray[java.lang.Integer](IntegerType)
+ }
+ @transient private lazy val foldableBoxedIntArrayDataObjectType =
+ ObjectType(classOf[Array[java.lang.Integer]])
+
+ // long
+ // unbox long
+ @transient private lazy val foldableLongArrayData: Array[Long] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toLongArray()
+ }
+ @transient private lazy val foldableLongArrayDataObjectType =
+ ObjectType(classOf[Array[Long]])
+ // boxed long
+ @transient private lazy val foldableBoxedLongArrayData: Array[java.lang.Long] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toArray[java.lang.Long](LongType)
+ }
+ @transient private lazy val foldableBoxedLongArrayDataObjectType =
+ ObjectType(classOf[Array[java.lang.Long]])
+
+ // float
+ // unbox float
+ @transient private lazy val foldableFloatArrayData: Array[Float] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toFloatArray()
+ }
+ @transient private lazy val foldableFloatArrayDataObjectType =
+ ObjectType(classOf[Array[Float]])
+ // boxed float
+ @transient private lazy val foldableBoxedFloatArrayData: Array[java.lang.Float] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toArray[java.lang.Float](FloatType)
+ }
+ @transient private lazy val foldableBoxedFloatArrayDataObjectType =
+ ObjectType(classOf[Array[java.lang.Float]])
+
+ // double
+ // unbox double
+ @transient private lazy val foldableDoubleArrayData: Array[Double] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toDoubleArray()
+ }
+ @transient private lazy val foldableDoubleArrayDataObjectType =
+ ObjectType(classOf[Array[Double]])
+ // boxed double
+ @transient private lazy val foldableBoxedDoubleArrayData: Array[java.lang.Double] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toArray[java.lang.Double](DoubleType)
+ }
+ @transient private lazy val foldableBoxedDoubleArrayDataObjectType =
+ ObjectType(classOf[Array[java.lang.Double]])
+
@transient private lazy val elementObjectType = ObjectType(classOf[DataType])
- @transient private lazy val comparatorObjectType = ObjectType(classOf[Comparator[Object]])
- override def replacement: Expression =
+ @transient private lazy val comparatorObjectType = ObjectType(classOf[Comparator[Object]])
+
+ private def unboxArgumentsAndInputTypes(): (Seq[Expression], Seq[AbstractDataType]) =
+ elementType match {
+ case ByteType =>
+ (Seq(Literal(foldableByteArrayData, foldableByteArrayDataObjectType), value),
+ Seq(foldableByteArrayDataObjectType, elementType))
+ case ShortType =>
+ (Seq(Literal(foldableShortArrayData, foldableShortArrayDataObjectType), value),
+ Seq(foldableShortArrayDataObjectType, elementType))
+ case IntegerType =>
+ (Seq(Literal(foldableIntArrayData, foldableIntArrayDataObjectType), value),
+ Seq(foldableIntArrayDataObjectType, elementType))
+ case LongType =>
+ (Seq(Literal(foldableLongArrayData, foldableLongArrayDataObjectType), value),
+ Seq(foldableLongArrayDataObjectType, elementType))
+ case FloatType =>
+ (Seq(Literal(foldableFloatArrayData, foldableFloatArrayDataObjectType), value),
+ Seq(foldableFloatArrayDataObjectType, elementType))
+ case DoubleType =>
+ (Seq(Literal(foldableDoubleArrayData, foldableDoubleArrayDataObjectType), value),
+ Seq(foldableDoubleArrayDataObjectType, elementType))
+ }
+
+ private def boxedArgumentsAndInputTypes(): (Seq[Expression], Seq[AbstractDataType]) =
+ elementType match {
+ case BooleanType =>
+ (Seq(Literal(foldableBoxedBooleanArrayData, foldableBoxedBooleanArrayDataObjectType),
+ value),
+ Seq(foldableBoxedBooleanArrayDataObjectType, elementType))
+ case ByteType =>
+ (Seq(Literal(foldableBoxedByteArrayData, foldableBoxedByteArrayDataObjectType), value),
+ Seq(foldableBoxedByteArrayDataObjectType, elementType))
+ case ShortType =>
+ (Seq(Literal(foldableBoxedShortArrayData, foldableBoxedShortArrayDataObjectType), value),
+ Seq(foldableBoxedShortArrayDataObjectType, elementType))
+ case IntegerType =>
+ (Seq(Literal(foldableBoxedIntArrayData, foldableBoxedIntArrayDataObjectType), value),
+ Seq(foldableBoxedIntArrayDataObjectType, elementType))
+ case LongType =>
+ (Seq(Literal(foldableBoxedLongArrayData, foldableBoxedLongArrayDataObjectType), value),
+ Seq(foldableBoxedLongArrayDataObjectType, elementType))
+ case FloatType =>
+ (Seq(Literal(foldableBoxedFloatArrayData, foldableBoxedFloatArrayDataObjectType), value),
+ Seq(foldableBoxedFloatArrayDataObjectType, elementType))
+ case DoubleType =>
+ (Seq(Literal(foldableBoxedDoubleArrayData, foldableBoxedDoubleArrayDataObjectType), value),
+ Seq(foldableBoxedDoubleArrayDataObjectType, elementType))
+ }
+
+ override def replacement: Expression = {
if (canPerformFastBinarySearch) {
- StaticInvoke(
- classOf[ArrayExpressionUtils],
- IntegerType,
- "binarySearch",
- Seq(array, value),
- inputTypes)
+ if (arrayIsFoldable) {
Review Comment:
Take the following case as an example:
```scala
val a6_0 = Literal.create(Seq(1.0d, 2.0d, 3.0d), ArrayType(DoubleType, containsNull = false))
checkEvaluation(ArrayBinarySearch(a6_0, Literal(1.0d)), 0)
```
Before:
<img width="555" alt="image" src="https://github.com/user-attachments/assets/f7de438e-1bb0-4e14-9015-4a7f8dfab35a">
After:
<img width="607" alt="image" src="https://github.com/user-attachments/assets/a7d2cf49-b3ef-48ee-bcbe-a053b669dd9d">
<img width="379" alt="image" src="https://github.com/user-attachments/assets/60dc3507-e0df-4d7f-89fa-fd8c201c1bbb">
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #47984:
URL: https://github.com/apache/spark/pull/47984#issuecomment-2337938335
has an offline discussion with @panbingkun , another approach maybe:
the bottleneck is `ArrayData.toXXXArray` that requires a deep copy of the whole array, if `ConstantFolding` can convert all foldable arrays to literals (with `GenericArrayData` type value), then maybe we can optimize it by override `toXXXArray` methods to directly return the `val array` in some way
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #47984:
URL: https://github.com/apache/spark/pull/47984#issuecomment-2338060858
@zhengruifeng This is a good idea. I think `ArrayBinarySearch` should be replaced by `invoke_binary_search_function(ToJavaArray(array_expr))`, and `ConstantFolding` should do the optimization automatically. For the new expression `ToJavaArray`, it can create primitive java array if the element type is primitive type and not nullable.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1771345978
##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ArrayExpressionUtils.java:
##########
@@ -19,158 +19,168 @@
import java.util.Arrays;
import java.util.Comparator;
-import org.apache.spark.sql.catalyst.util.ArrayData;
import org.apache.spark.sql.catalyst.util.SQLOrderingUtil;
-import org.apache.spark.sql.types.ByteType$;
-import org.apache.spark.sql.types.BooleanType$;
-import org.apache.spark.sql.types.DataType;
-import org.apache.spark.sql.types.DoubleType$;
-import org.apache.spark.sql.types.FloatType$;
-import org.apache.spark.sql.types.IntegerType$;
-import org.apache.spark.sql.types.LongType$;
-import org.apache.spark.sql.types.ShortType$;
public class ArrayExpressionUtils {
- private static final Comparator<Object> booleanComp = (o1, o2) -> {
+ // comparator
+ // Boolean ascending nullable comparator
+ private static final Comparator<Boolean> booleanComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- boolean c1 = (Boolean) o1, c2 = (Boolean) o2;
- return c1 == c2 ? 0 : (c1 ? 1 : -1);
+ return o1.equals(o2) ? 0 : (o1 ? 1 : -1);
};
- private static final Comparator<Object> byteComp = (o1, o2) -> {
+ // Byte ascending nullable comparator
+ private static final Comparator<Byte> byteComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- byte c1 = (Byte) o1, c2 = (Byte) o2;
- return Byte.compare(c1, c2);
+ return Byte.compare(o1, o2);
};
- private static final Comparator<Object> shortComp = (o1, o2) -> {
+ // Short ascending nullable comparator
+ private static final Comparator<Short> shortComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- short c1 = (Short) o1, c2 = (Short) o2;
- return Short.compare(c1, c2);
+ return Short.compare(o1, o2);
};
- private static final Comparator<Object> integerComp = (o1, o2) -> {
+ // Integer ascending nullable comparator
+ private static final Comparator<Integer> integerComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- int c1 = (Integer) o1, c2 = (Integer) o2;
- return Integer.compare(c1, c2);
+ return Integer.compare(o1, o2);
};
- private static final Comparator<Object> longComp = (o1, o2) -> {
+ // Long ascending nullable comparator
+ private static final Comparator<Long> longComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- long c1 = (Long) o1, c2 = (Long) o2;
- return Long.compare(c1, c2);
+ return Long.compare(o1, o2);
};
- private static final Comparator<Object> floatComp = (o1, o2) -> {
+ // Float ascending nullable comparator
+ private static final Comparator<Float> floatComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- float c1 = (Float) o1, c2 = (Float) o2;
- return SQLOrderingUtil.compareFloats(c1, c2);
+ return SQLOrderingUtil.compareFloats(o1, o2);
};
- private static final Comparator<Object> doubleComp = (o1, o2) -> {
+ // Double ascending nullable comparator
+ private static final Comparator<Double> doubleComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- double c1 = (Double) o1, c2 = (Double) o2;
- return SQLOrderingUtil.compareDoubles(c1, c2);
+ return SQLOrderingUtil.compareDoubles(o1, o2);
};
- public static int binarySearchNullSafe(ArrayData data, Boolean value) {
- return Arrays.binarySearch(data.toObjectArray(BooleanType$.MODULE$), value, booleanComp);
+ // Boolean nullable
Review Comment:
I don't think type parameter works for primitive types...
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1766615854
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/benchmark/ArrayBinarySearchBenchmark.scala:
##########
@@ -0,0 +1,59 @@
+/*
+ * 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.benchmark
+
+import org.apache.spark.benchmark.Benchmark
+import org.apache.spark.sql.Column
+import org.apache.spark.sql.functions.{col, lit}
+import org.apache.spark.sql.types.StructType
+
+/**
+ * Synthetic benchmark for array_binary_search functions.
+ * To run this benchmark:
+ * {{{
+ * 1. without sbt:
+ * bin/spark-submit --class <this class>
+ * --jars <spark core test jar>,<spark catalyst test jar> <sql core test jar>
+ * 2. build/sbt "sql/Test/runMain <this class>"
+ * 3. generate result:
+ * SPARK_GENERATE_BENCHMARK_FILES=1 build/sbt "sql/Test/runMain <this class>"
+ * Results will be written to "benchmarks/ArrayBinarySearchBenchmark-results.txt".
+ * }}}
+ */
+object ArrayBinarySearchBenchmark extends SqlBasedBenchmark {
Review Comment:
I think we don't need it, we can delete it before merge
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1770738439
##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ArrayExpressionUtils.java:
##########
@@ -19,158 +19,168 @@
import java.util.Arrays;
import java.util.Comparator;
-import org.apache.spark.sql.catalyst.util.ArrayData;
import org.apache.spark.sql.catalyst.util.SQLOrderingUtil;
-import org.apache.spark.sql.types.ByteType$;
-import org.apache.spark.sql.types.BooleanType$;
-import org.apache.spark.sql.types.DataType;
-import org.apache.spark.sql.types.DoubleType$;
-import org.apache.spark.sql.types.FloatType$;
-import org.apache.spark.sql.types.IntegerType$;
-import org.apache.spark.sql.types.LongType$;
-import org.apache.spark.sql.types.ShortType$;
public class ArrayExpressionUtils {
- private static final Comparator<Object> booleanComp = (o1, o2) -> {
+ // comparator
+ // Boolean ascending nullable comparator
+ private static final Comparator<Boolean> booleanComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- boolean c1 = (Boolean) o1, c2 = (Boolean) o2;
- return c1 == c2 ? 0 : (c1 ? 1 : -1);
+ return o1.equals(o2) ? 0 : (o1 ? 1 : -1);
};
- private static final Comparator<Object> byteComp = (o1, o2) -> {
+ // Byte ascending nullable comparator
+ private static final Comparator<Byte> byteComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- byte c1 = (Byte) o1, c2 = (Byte) o2;
- return Byte.compare(c1, c2);
+ return Byte.compare(o1, o2);
};
- private static final Comparator<Object> shortComp = (o1, o2) -> {
+ // Short ascending nullable comparator
+ private static final Comparator<Short> shortComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- short c1 = (Short) o1, c2 = (Short) o2;
- return Short.compare(c1, c2);
+ return Short.compare(o1, o2);
};
- private static final Comparator<Object> integerComp = (o1, o2) -> {
+ // Integer ascending nullable comparator
+ private static final Comparator<Integer> integerComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- int c1 = (Integer) o1, c2 = (Integer) o2;
- return Integer.compare(c1, c2);
+ return Integer.compare(o1, o2);
};
- private static final Comparator<Object> longComp = (o1, o2) -> {
+ // Long ascending nullable comparator
+ private static final Comparator<Long> longComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- long c1 = (Long) o1, c2 = (Long) o2;
- return Long.compare(c1, c2);
+ return Long.compare(o1, o2);
};
- private static final Comparator<Object> floatComp = (o1, o2) -> {
+ // Float ascending nullable comparator
+ private static final Comparator<Float> floatComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- float c1 = (Float) o1, c2 = (Float) o2;
- return SQLOrderingUtil.compareFloats(c1, c2);
+ return SQLOrderingUtil.compareFloats(o1, o2);
};
- private static final Comparator<Object> doubleComp = (o1, o2) -> {
+ // Double ascending nullable comparator
+ private static final Comparator<Double> doubleComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- double c1 = (Double) o1, c2 = (Double) o2;
- return SQLOrderingUtil.compareDoubles(c1, c2);
+ return SQLOrderingUtil.compareDoubles(o1, o2);
};
- public static int binarySearchNullSafe(ArrayData data, Boolean value) {
- return Arrays.binarySearch(data.toObjectArray(BooleanType$.MODULE$), value, booleanComp);
+ // Boolean nullable
Review Comment:
Perhaps we can simplify our logic in this way.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1754107399
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -1519,6 +1520,124 @@ case class ArrayContains(left: Expression, right: Expression)
copy(left = newLeft, right = newRight)
}
+/**
+ * This expression converts data of `ArrayData` to an array of java type.
+ *
+ * NOTE: When the data type of expression is `ArrayType`, and the expression is foldable,
+ * the `ConstantFolding` can do constant folding optimization automatically,
+ * (avoiding frequent calls to `ArrayData.to{XXX}Array()`).
+ */
+case class ToJavaArray(array: Expression)
+ extends UnaryExpression
+ with ImplicitCastInputTypes
+ with NullIntolerant
+ with QueryErrorsBase {
+
+ override def checkInputDataTypes(): TypeCheckResult = array.dataType match {
+ case ArrayType(_, _) =>
+ TypeCheckResult.TypeCheckSuccess
+ case _ =>
+ DataTypeMismatch(
+ errorSubClass = "UNEXPECTED_INPUT_TYPE",
+ messageParameters = Map(
+ "paramIndex" -> ordinalNumber(0),
+ "requiredType" -> toSQLType(ArrayType),
+ "inputSql" -> toSQLExpr(array),
+ "inputType" -> toSQLType(array.dataType))
+ )
+ }
+
+ override def inputTypes: Seq[AbstractDataType] = Seq(array.dataType)
+ override def dataType: DataType = {
+ if (isPrimitiveType && elementType != BooleanType && !resultArrayElementNullable) {
+ elementType match {
+ case ByteType => ObjectType(classOf[Array[Byte]])
+ case ShortType => ObjectType(classOf[Array[Short]])
+ case IntegerType => ObjectType(classOf[Array[Int]])
+ case LongType => ObjectType(classOf[Array[Long]])
+ case FloatType => ObjectType(classOf[Array[Float]])
+ case DoubleType => ObjectType(classOf[Array[Double]])
+ }
+ } else if (isPrimitiveType) {
+ elementType match {
+ case BooleanType => ObjectType(classOf[Array[java.lang.Boolean]])
+ case ByteType => ObjectType(classOf[Array[java.lang.Byte]])
+ case ShortType => ObjectType(classOf[Array[java.lang.Short]])
+ case IntegerType => ObjectType(classOf[Array[java.lang.Integer]])
+ case LongType => ObjectType(classOf[Array[java.lang.Long]])
+ case FloatType => ObjectType(classOf[Array[java.lang.Float]])
+ case DoubleType => ObjectType(classOf[Array[java.lang.Double]])
+ }
+ } else {
+ ObjectType(classOf[Array[Object]])
+ }
+ }
+
+ override def child: Expression = array
+ override def prettyName: String = "to_java_array"
+
+ @transient lazy val elementType: DataType =
+ array.dataType.asInstanceOf[ArrayType].elementType
+ private def resultArrayElementNullable: Boolean =
+ array.dataType.asInstanceOf[ArrayType].containsNull
+ @transient private lazy val isPrimitiveType: Boolean = CodeGenerator.isPrimitiveType(elementType)
+
+ private def toJavaArray(array: Any): Any = {
+ val arrayData = array.asInstanceOf[ArrayData]
Review Comment:
I am wondering how the array value is cached, do we need to match `GenericArrayData(val array: Array[Any])` in this method?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1756233364
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala:
##########
@@ -137,14 +137,32 @@ class CollectionExpressionsSuite
test("ArrayBinarySearch") {
// primitive type: boolean、byte、short、int、long、float、double
+ // boolean
Review Comment:
Yes, as follows
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1754276516
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -1519,6 +1520,124 @@ case class ArrayContains(left: Expression, right: Expression)
copy(left = newLeft, right = newRight)
}
+/**
+ * This expression converts data of `ArrayData` to an array of java type.
+ *
+ * NOTE: When the data type of expression is `ArrayType`, and the expression is foldable,
+ * the `ConstantFolding` can do constant folding optimization automatically,
+ * (avoiding frequent calls to `ArrayData.to{XXX}Array()`).
+ */
+case class ToJavaArray(array: Expression)
+ extends UnaryExpression
+ with ImplicitCastInputTypes
+ with NullIntolerant
+ with QueryErrorsBase {
+
+ override def checkInputDataTypes(): TypeCheckResult = array.dataType match {
+ case ArrayType(_, _) =>
+ TypeCheckResult.TypeCheckSuccess
+ case _ =>
+ DataTypeMismatch(
+ errorSubClass = "UNEXPECTED_INPUT_TYPE",
+ messageParameters = Map(
+ "paramIndex" -> ordinalNumber(0),
+ "requiredType" -> toSQLType(ArrayType),
+ "inputSql" -> toSQLExpr(array),
+ "inputType" -> toSQLType(array.dataType))
+ )
+ }
+
+ override def inputTypes: Seq[AbstractDataType] = Seq(array.dataType)
+ override def dataType: DataType = {
+ if (isPrimitiveType && elementType != BooleanType && !resultArrayElementNullable) {
+ elementType match {
+ case ByteType => ObjectType(classOf[Array[Byte]])
+ case ShortType => ObjectType(classOf[Array[Short]])
+ case IntegerType => ObjectType(classOf[Array[Int]])
+ case LongType => ObjectType(classOf[Array[Long]])
+ case FloatType => ObjectType(classOf[Array[Float]])
+ case DoubleType => ObjectType(classOf[Array[Double]])
+ }
+ } else if (isPrimitiveType) {
+ elementType match {
+ case BooleanType => ObjectType(classOf[Array[java.lang.Boolean]])
+ case ByteType => ObjectType(classOf[Array[java.lang.Byte]])
+ case ShortType => ObjectType(classOf[Array[java.lang.Short]])
+ case IntegerType => ObjectType(classOf[Array[java.lang.Integer]])
+ case LongType => ObjectType(classOf[Array[java.lang.Long]])
+ case FloatType => ObjectType(classOf[Array[java.lang.Float]])
+ case DoubleType => ObjectType(classOf[Array[java.lang.Double]])
+ }
+ } else {
+ ObjectType(classOf[Array[Object]])
+ }
+ }
+
+ override def child: Expression = array
+ override def prettyName: String = "to_java_array"
+
+ @transient lazy val elementType: DataType =
+ array.dataType.asInstanceOf[ArrayType].elementType
+ private def resultArrayElementNullable: Boolean =
+ array.dataType.asInstanceOf[ArrayType].containsNull
+ @transient private lazy val isPrimitiveType: Boolean = CodeGenerator.isPrimitiveType(elementType)
+
+ private def toJavaArray(array: Any): Any = {
+ val arrayData = array.asInstanceOf[ArrayData]
Review Comment:
I think `ConstantFolding` did the following optimization
https://github.com/apache/spark/blob/44d08cb97eff82550cf07297f4266f34e4a9146d/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala#L80-L83
and when we calculate the value of `Literal`, we don't care about each `input`, so it only did it once
https://github.com/apache/spark/blob/44d08cb97eff82550cf07297f4266f34e4a9146d/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/literals.scala#L441
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1768751608
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -1525,6 +1526,126 @@ case class ArrayContains(left: Expression, right: Expression)
copy(left = newLeft, right = newRight)
}
+/**
+ * This expression converts data of `ArrayData` to an array of java type.
+ *
+ * NOTE: When the data type of expression is `ArrayType`, and the expression is foldable,
+ * the `ConstantFolding` can do constant folding optimization automatically,
+ * (avoiding frequent calls to `ArrayData.to{XXX}Array()`).
+ */
+case class ToJavaArray(array: Expression)
+ extends UnaryExpression
+ with ImplicitCastInputTypes
+ with NullIntolerant
+ with QueryErrorsBase {
+
+ override def checkInputDataTypes(): TypeCheckResult = array.dataType match {
+ case ArrayType(_, _) =>
+ TypeCheckResult.TypeCheckSuccess
+ case _ =>
+ DataTypeMismatch(
+ errorSubClass = "UNEXPECTED_INPUT_TYPE",
+ messageParameters = Map(
+ "paramIndex" -> ordinalNumber(0),
+ "requiredType" -> toSQLType(ArrayType),
+ "inputSql" -> toSQLExpr(array),
+ "inputType" -> toSQLType(array.dataType))
+ )
+ }
+
+ override def inputTypes: Seq[AbstractDataType] = Seq(array.dataType)
+ override def dataType: DataType = {
+ if (canPerformFast) {
+ elementType match {
+ case ByteType => ObjectType(classOf[Array[Byte]])
+ case ShortType => ObjectType(classOf[Array[Short]])
+ case IntegerType => ObjectType(classOf[Array[Int]])
+ case LongType => ObjectType(classOf[Array[Long]])
+ case FloatType => ObjectType(classOf[Array[Float]])
+ case DoubleType => ObjectType(classOf[Array[Double]])
+ }
+ } else if (isPrimitiveType) {
+ elementType match {
+ case BooleanType => ObjectType(classOf[Array[java.lang.Boolean]])
+ case ByteType => ObjectType(classOf[Array[java.lang.Byte]])
+ case ShortType => ObjectType(classOf[Array[java.lang.Short]])
+ case IntegerType => ObjectType(classOf[Array[java.lang.Integer]])
+ case LongType => ObjectType(classOf[Array[java.lang.Long]])
+ case FloatType => ObjectType(classOf[Array[java.lang.Float]])
+ case DoubleType => ObjectType(classOf[Array[java.lang.Double]])
+ }
+ } else {
+ ObjectType(classOf[Array[Object]])
+ }
+ }
+
+ override def child: Expression = array
+ override def prettyName: String = "to_java_array"
+
+ @transient lazy val elementType: DataType =
+ array.dataType.asInstanceOf[ArrayType].elementType
+ private def resultArrayElementNullable: Boolean =
+ array.dataType.asInstanceOf[ArrayType].containsNull
+ private def isPrimitiveType: Boolean = CodeGenerator.isPrimitiveType(elementType)
+ private def canPerformFast: Boolean =
+ isPrimitiveType && elementType != BooleanType && !resultArrayElementNullable
+
+ private def toJavaArray(array: Any): Any = {
+ val arrayData = array.asInstanceOf[ArrayData]
+ if (canPerformFast) {
+ elementType match {
+ case ByteType => arrayData.toByteArray()
+ case ShortType => arrayData.toShortArray()
+ case IntegerType => arrayData.toIntArray()
+ case LongType => arrayData.toLongArray()
+ case FloatType => arrayData.toFloatArray()
+ case DoubleType => arrayData.toDoubleArray()
+ }
+ } else if (isPrimitiveType) {
+ elementType match {
+ case BooleanType => arrayData.toArray[java.lang.Boolean](BooleanType)
+ case ByteType => arrayData.toArray[java.lang.Byte](ByteType)
+ case ShortType => arrayData.toArray[java.lang.Short](ShortType)
+ case IntegerType => arrayData.toArray[java.lang.Integer](IntegerType)
+ case LongType => arrayData.toArray[java.lang.Long](LongType)
+ case FloatType => arrayData.toArray[java.lang.Float](FloatType)
+ case DoubleType => arrayData.toArray[java.lang.Double](DoubleType)
+ }
+ } else {
+ arrayData.toObjectArray(elementType)
+ }
+ }
+
+ private def toJavaArrayCodegen(
+ ctx: CodegenContext,
+ ev: ExprCode,
+ array: String): String = {
+ val elementTypeTerm = ctx.addReferenceObj("elementTypeTerm", elementType)
+ if (canPerformFast) {
+ val primitiveTypeName = CodeGenerator.primitiveTypeName(elementType)
+ s"""${ev.value} = $array.to${primitiveTypeName}Array();"""
+ } else if (isPrimitiveType) {
+ val boxedJavaType = CodeGenerator.boxedType(elementType)
+ val classTagTerm = ctx.addReferenceObj("classTagTerm",
+ ClassTag(SparkClassUtils.classForName(s"java.lang.$boxedJavaType")))
+ s"""${ev.value} = ($boxedJavaType[]) $array.toArray($elementTypeTerm, $classTagTerm);"""
+ } else {
+ s"""${ev.value} = $array.toObjectArray($elementTypeTerm);"""
+ }
+ }
+
+ override def nullSafeEval(array: Any): Any = {
Review Comment:
looks like this expression itself can also be implemented with `Invoke`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #47984:
URL: https://github.com/apache/spark/pull/47984#issuecomment-2371052388
I will close it.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1770946781
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -1525,6 +1526,126 @@ case class ArrayContains(left: Expression, right: Expression)
copy(left = newLeft, right = newRight)
}
+/**
+ * This expression converts data of `ArrayData` to an array of java type.
+ *
+ * NOTE: When the data type of expression is `ArrayType`, and the expression is foldable,
+ * the `ConstantFolding` can do constant folding optimization automatically,
+ * (avoiding frequent calls to `ArrayData.to{XXX}Array()`).
+ */
+case class ToJavaArray(array: Expression)
+ extends UnaryExpression
+ with ImplicitCastInputTypes
+ with NullIntolerant
+ with QueryErrorsBase {
+
+ override def checkInputDataTypes(): TypeCheckResult = array.dataType match {
+ case ArrayType(_, _) =>
+ TypeCheckResult.TypeCheckSuccess
+ case _ =>
+ DataTypeMismatch(
+ errorSubClass = "UNEXPECTED_INPUT_TYPE",
+ messageParameters = Map(
+ "paramIndex" -> ordinalNumber(0),
+ "requiredType" -> toSQLType(ArrayType),
+ "inputSql" -> toSQLExpr(array),
+ "inputType" -> toSQLType(array.dataType))
+ )
+ }
+
+ override def inputTypes: Seq[AbstractDataType] = Seq(array.dataType)
+ override def dataType: DataType = {
+ if (canPerformFast) {
+ elementType match {
+ case ByteType => ObjectType(classOf[Array[Byte]])
+ case ShortType => ObjectType(classOf[Array[Short]])
+ case IntegerType => ObjectType(classOf[Array[Int]])
+ case LongType => ObjectType(classOf[Array[Long]])
+ case FloatType => ObjectType(classOf[Array[Float]])
+ case DoubleType => ObjectType(classOf[Array[Double]])
+ }
+ } else if (isPrimitiveType) {
+ elementType match {
+ case BooleanType => ObjectType(classOf[Array[java.lang.Boolean]])
+ case ByteType => ObjectType(classOf[Array[java.lang.Byte]])
+ case ShortType => ObjectType(classOf[Array[java.lang.Short]])
+ case IntegerType => ObjectType(classOf[Array[java.lang.Integer]])
+ case LongType => ObjectType(classOf[Array[java.lang.Long]])
+ case FloatType => ObjectType(classOf[Array[java.lang.Float]])
+ case DoubleType => ObjectType(classOf[Array[java.lang.Double]])
+ }
+ } else {
+ ObjectType(classOf[Array[Object]])
+ }
+ }
+
+ override def child: Expression = array
+ override def prettyName: String = "to_java_array"
+
+ @transient lazy val elementType: DataType =
+ array.dataType.asInstanceOf[ArrayType].elementType
+ private def resultArrayElementNullable: Boolean =
+ array.dataType.asInstanceOf[ArrayType].containsNull
+ private def isPrimitiveType: Boolean = CodeGenerator.isPrimitiveType(elementType)
+ private def canPerformFast: Boolean =
+ isPrimitiveType && elementType != BooleanType && !resultArrayElementNullable
+
+ private def toJavaArray(array: Any): Any = {
+ val arrayData = array.asInstanceOf[ArrayData]
+ if (canPerformFast) {
+ elementType match {
+ case ByteType => arrayData.toByteArray()
+ case ShortType => arrayData.toShortArray()
+ case IntegerType => arrayData.toIntArray()
+ case LongType => arrayData.toLongArray()
+ case FloatType => arrayData.toFloatArray()
+ case DoubleType => arrayData.toDoubleArray()
+ }
+ } else if (isPrimitiveType) {
+ elementType match {
+ case BooleanType => arrayData.toArray[java.lang.Boolean](BooleanType)
+ case ByteType => arrayData.toArray[java.lang.Byte](ByteType)
+ case ShortType => arrayData.toArray[java.lang.Short](ShortType)
+ case IntegerType => arrayData.toArray[java.lang.Integer](IntegerType)
+ case LongType => arrayData.toArray[java.lang.Long](LongType)
+ case FloatType => arrayData.toArray[java.lang.Float](FloatType)
+ case DoubleType => arrayData.toArray[java.lang.Double](DoubleType)
+ }
+ } else {
+ arrayData.toObjectArray(elementType)
+ }
+ }
+
+ private def toJavaArrayCodegen(
+ ctx: CodegenContext,
+ ev: ExprCode,
+ array: String): String = {
+ val elementTypeTerm = ctx.addReferenceObj("elementTypeTerm", elementType)
+ if (canPerformFast) {
+ val primitiveTypeName = CodeGenerator.primitiveTypeName(elementType)
+ s"""${ev.value} = $array.to${primitiveTypeName}Array();"""
+ } else if (isPrimitiveType) {
+ val boxedJavaType = CodeGenerator.boxedType(elementType)
+ val classTagTerm = ctx.addReferenceObj("classTagTerm",
+ ClassTag(SparkClassUtils.classForName(s"java.lang.$boxedJavaType")))
+ s"""${ev.value} = ($boxedJavaType[]) $array.toArray($elementTypeTerm, $classTagTerm);"""
+ } else {
+ s"""${ev.value} = $array.toObjectArray($elementTypeTerm);"""
+ }
+ }
+
+ override def nullSafeEval(array: Any): Any = {
Review Comment:
Base data hits `java.lang.Integer` 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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1750060654
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -1618,23 +1619,198 @@ case class ArrayBinarySearch(array: Expression, value: Expression)
}
}
+ // byte
+ // unbox byte
+ @transient private lazy val foldableByteArrayData: Array[Byte] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toByteArray()
+ }
+ @transient private lazy val foldableByteArrayDataObjectType =
+ ObjectType(classOf[Array[Byte]])
+ // boxed byte
+ @transient private lazy val foldableBoxedByteArrayData: Array[java.lang.Byte] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toArray[java.lang.Byte](ByteType)
+ }
+ @transient private lazy val foldableBoxedByteArrayDataObjectType =
+ ObjectType(classOf[Array[java.lang.Byte]])
+
+ // boolean
+ // boxed boolean
+ @transient private lazy val foldableBoxedBooleanArrayData: Array[java.lang.Boolean] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toArray[java.lang.Boolean](BooleanType)
+ }
+ @transient private lazy val foldableBoxedBooleanArrayDataObjectType =
+ ObjectType(classOf[Array[java.lang.Boolean]])
+
+ // short
+ // unbox short
+ @transient private lazy val foldableShortArrayData: Array[Short] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toShortArray()
+ }
+ @transient private lazy val foldableShortArrayDataObjectType =
+ ObjectType(classOf[Array[Short]])
+ // boxed short
+ @transient private lazy val foldableBoxedShortArrayData: Array[java.lang.Short] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toArray[java.lang.Short](ShortType)
+ }
+ @transient private lazy val foldableBoxedShortArrayDataObjectType =
+ ObjectType(classOf[Array[java.lang.Short]])
+
+ // int
+ // unbox int
+ @transient private lazy val foldableIntArrayData: Array[Int] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toIntArray()
+ }
+ @transient private lazy val foldableIntArrayDataObjectType =
+ ObjectType(classOf[Array[Int]])
+ // boxed int
+ @transient private lazy val foldableBoxedIntArrayData: Array[java.lang.Integer] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toArray[java.lang.Integer](IntegerType)
+ }
+ @transient private lazy val foldableBoxedIntArrayDataObjectType =
+ ObjectType(classOf[Array[java.lang.Integer]])
+
+ // long
+ // unbox long
+ @transient private lazy val foldableLongArrayData: Array[Long] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toLongArray()
+ }
+ @transient private lazy val foldableLongArrayDataObjectType =
+ ObjectType(classOf[Array[Long]])
+ // boxed long
+ @transient private lazy val foldableBoxedLongArrayData: Array[java.lang.Long] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toArray[java.lang.Long](LongType)
+ }
+ @transient private lazy val foldableBoxedLongArrayDataObjectType =
+ ObjectType(classOf[Array[java.lang.Long]])
+
+ // float
+ // unbox float
+ @transient private lazy val foldableFloatArrayData: Array[Float] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toFloatArray()
+ }
+ @transient private lazy val foldableFloatArrayDataObjectType =
+ ObjectType(classOf[Array[Float]])
+ // boxed float
+ @transient private lazy val foldableBoxedFloatArrayData: Array[java.lang.Float] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toArray[java.lang.Float](FloatType)
+ }
+ @transient private lazy val foldableBoxedFloatArrayDataObjectType =
+ ObjectType(classOf[Array[java.lang.Float]])
+
+ // double
+ // unbox double
+ @transient private lazy val foldableDoubleArrayData: Array[Double] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toDoubleArray()
+ }
+ @transient private lazy val foldableDoubleArrayDataObjectType =
+ ObjectType(classOf[Array[Double]])
+ // boxed double
+ @transient private lazy val foldableBoxedDoubleArrayData: Array[java.lang.Double] = {
+ assert(array.foldable)
+ array.eval().asInstanceOf[ArrayData].toArray[java.lang.Double](DoubleType)
+ }
+ @transient private lazy val foldableBoxedDoubleArrayDataObjectType =
+ ObjectType(classOf[Array[java.lang.Double]])
+
@transient private lazy val elementObjectType = ObjectType(classOf[DataType])
- @transient private lazy val comparatorObjectType = ObjectType(classOf[Comparator[Object]])
- override def replacement: Expression =
+ @transient private lazy val comparatorObjectType = ObjectType(classOf[Comparator[Object]])
+
+ private def unboxArgumentsAndInputTypes(): (Seq[Expression], Seq[AbstractDataType]) =
+ elementType match {
+ case ByteType =>
+ (Seq(Literal(foldableByteArrayData, foldableByteArrayDataObjectType), value),
+ Seq(foldableByteArrayDataObjectType, elementType))
+ case ShortType =>
+ (Seq(Literal(foldableShortArrayData, foldableShortArrayDataObjectType), value),
+ Seq(foldableShortArrayDataObjectType, elementType))
+ case IntegerType =>
+ (Seq(Literal(foldableIntArrayData, foldableIntArrayDataObjectType), value),
+ Seq(foldableIntArrayDataObjectType, elementType))
+ case LongType =>
+ (Seq(Literal(foldableLongArrayData, foldableLongArrayDataObjectType), value),
+ Seq(foldableLongArrayDataObjectType, elementType))
+ case FloatType =>
+ (Seq(Literal(foldableFloatArrayData, foldableFloatArrayDataObjectType), value),
+ Seq(foldableFloatArrayDataObjectType, elementType))
+ case DoubleType =>
+ (Seq(Literal(foldableDoubleArrayData, foldableDoubleArrayDataObjectType), value),
+ Seq(foldableDoubleArrayDataObjectType, elementType))
+ }
+
+ private def boxedArgumentsAndInputTypes(): (Seq[Expression], Seq[AbstractDataType]) =
+ elementType match {
+ case BooleanType =>
+ (Seq(Literal(foldableBoxedBooleanArrayData, foldableBoxedBooleanArrayDataObjectType),
+ value),
+ Seq(foldableBoxedBooleanArrayDataObjectType, elementType))
+ case ByteType =>
+ (Seq(Literal(foldableBoxedByteArrayData, foldableBoxedByteArrayDataObjectType), value),
+ Seq(foldableBoxedByteArrayDataObjectType, elementType))
+ case ShortType =>
+ (Seq(Literal(foldableBoxedShortArrayData, foldableBoxedShortArrayDataObjectType), value),
+ Seq(foldableBoxedShortArrayDataObjectType, elementType))
+ case IntegerType =>
+ (Seq(Literal(foldableBoxedIntArrayData, foldableBoxedIntArrayDataObjectType), value),
+ Seq(foldableBoxedIntArrayDataObjectType, elementType))
+ case LongType =>
+ (Seq(Literal(foldableBoxedLongArrayData, foldableBoxedLongArrayDataObjectType), value),
+ Seq(foldableBoxedLongArrayDataObjectType, elementType))
+ case FloatType =>
+ (Seq(Literal(foldableBoxedFloatArrayData, foldableBoxedFloatArrayDataObjectType), value),
+ Seq(foldableBoxedFloatArrayDataObjectType, elementType))
+ case DoubleType =>
+ (Seq(Literal(foldableBoxedDoubleArrayData, foldableBoxedDoubleArrayDataObjectType), value),
+ Seq(foldableBoxedDoubleArrayDataObjectType, elementType))
+ }
+
+ override def replacement: Expression = {
if (canPerformFastBinarySearch) {
- StaticInvoke(
- classOf[ArrayExpressionUtils],
- IntegerType,
- "binarySearch",
- Seq(array, value),
- inputTypes)
+ if (arrayIsFoldable) {
Review Comment:
In the case where the `array` is `foldable`, after optimization, the array `only` needs to be `toArray` `once` instead of toArray `every time`.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #47984:
URL: https://github.com/apache/spark/pull/47984#issuecomment-2338186873
Great suggestion, let me give it a try, 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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1749888056
##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ArrayExpressionUtils.java:
##########
@@ -116,58 +116,131 @@ public class ArrayExpressionUtils {
return SQLOrderingUtil.compareDoubles(c1, c2);
};
+ // boolean
+ // foldable optimize
+ public static int binarySearchNullSafe(Boolean[] data, Boolean value) {
Review Comment:
For nullable arrays, I don't think concrete types can help the performance. `Object[]` should be OK.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1749886956
##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ArrayExpressionUtils.java:
##########
@@ -116,58 +116,131 @@ public class ArrayExpressionUtils {
return SQLOrderingUtil.compareDoubles(c1, c2);
};
+ // boolean
+ // foldable optimize
+ public static int binarySearchNullSafe(Boolean[] data, Boolean value) {
Review Comment:
I think it's better to take `ArrayData` here to simplify the expression implementation. We can call `arrayData.toBooleanArray` or `.toObjectArray` for non-nullable and nullable arrays.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1753942612
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala:
##########
@@ -920,6 +920,7 @@ object FunctionRegistry {
registerInternalExpression[EWM]("ewm")
registerInternalExpression[NullIndex]("null_index")
registerInternalExpression[CastTimestampNTZToLong]("timestamp_ntz_to_long")
+ registerInternalExpression[ToJavaArray]("to_java_array")
Review Comment:
`registerInternalExpression` if for functions API, I think we don't need to register it 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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1768750460
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -1525,6 +1526,126 @@ case class ArrayContains(left: Expression, right: Expression)
copy(left = newLeft, right = newRight)
}
+/**
+ * This expression converts data of `ArrayData` to an array of java type.
+ *
+ * NOTE: When the data type of expression is `ArrayType`, and the expression is foldable,
+ * the `ConstantFolding` can do constant folding optimization automatically,
+ * (avoiding frequent calls to `ArrayData.to{XXX}Array()`).
+ */
+case class ToJavaArray(array: Expression)
+ extends UnaryExpression
+ with ImplicitCastInputTypes
+ with NullIntolerant
+ with QueryErrorsBase {
+
+ override def checkInputDataTypes(): TypeCheckResult = array.dataType match {
+ case ArrayType(_, _) =>
+ TypeCheckResult.TypeCheckSuccess
+ case _ =>
+ DataTypeMismatch(
+ errorSubClass = "UNEXPECTED_INPUT_TYPE",
+ messageParameters = Map(
+ "paramIndex" -> ordinalNumber(0),
+ "requiredType" -> toSQLType(ArrayType),
+ "inputSql" -> toSQLExpr(array),
+ "inputType" -> toSQLType(array.dataType))
+ )
+ }
+
+ override def inputTypes: Seq[AbstractDataType] = Seq(array.dataType)
+ override def dataType: DataType = {
+ if (canPerformFast) {
+ elementType match {
+ case ByteType => ObjectType(classOf[Array[Byte]])
+ case ShortType => ObjectType(classOf[Array[Short]])
+ case IntegerType => ObjectType(classOf[Array[Int]])
+ case LongType => ObjectType(classOf[Array[Long]])
+ case FloatType => ObjectType(classOf[Array[Float]])
+ case DoubleType => ObjectType(classOf[Array[Double]])
+ }
+ } else if (isPrimitiveType) {
+ elementType match {
+ case BooleanType => ObjectType(classOf[Array[java.lang.Boolean]])
+ case ByteType => ObjectType(classOf[Array[java.lang.Byte]])
+ case ShortType => ObjectType(classOf[Array[java.lang.Short]])
+ case IntegerType => ObjectType(classOf[Array[java.lang.Integer]])
+ case LongType => ObjectType(classOf[Array[java.lang.Long]])
+ case FloatType => ObjectType(classOf[Array[java.lang.Float]])
+ case DoubleType => ObjectType(classOf[Array[java.lang.Double]])
+ }
+ } else {
+ ObjectType(classOf[Array[Object]])
+ }
+ }
+
+ override def child: Expression = array
+ override def prettyName: String = "to_java_array"
+
+ @transient lazy val elementType: DataType =
+ array.dataType.asInstanceOf[ArrayType].elementType
+ private def resultArrayElementNullable: Boolean =
+ array.dataType.asInstanceOf[ArrayType].containsNull
+ private def isPrimitiveType: Boolean = CodeGenerator.isPrimitiveType(elementType)
+ private def canPerformFast: Boolean =
+ isPrimitiveType && elementType != BooleanType && !resultArrayElementNullable
+
+ private def toJavaArray(array: Any): Any = {
+ val arrayData = array.asInstanceOf[ArrayData]
+ if (canPerformFast) {
+ elementType match {
+ case ByteType => arrayData.toByteArray()
+ case ShortType => arrayData.toShortArray()
+ case IntegerType => arrayData.toIntArray()
+ case LongType => arrayData.toLongArray()
+ case FloatType => arrayData.toFloatArray()
+ case DoubleType => arrayData.toDoubleArray()
+ }
+ } else if (isPrimitiveType) {
+ elementType match {
+ case BooleanType => arrayData.toArray[java.lang.Boolean](BooleanType)
+ case ByteType => arrayData.toArray[java.lang.Byte](ByteType)
+ case ShortType => arrayData.toArray[java.lang.Short](ShortType)
+ case IntegerType => arrayData.toArray[java.lang.Integer](IntegerType)
+ case LongType => arrayData.toArray[java.lang.Long](LongType)
+ case FloatType => arrayData.toArray[java.lang.Float](FloatType)
+ case DoubleType => arrayData.toArray[java.lang.Double](DoubleType)
Review Comment:
are there performance benefits to use concrete boxed types over Object?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1768753327
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -1525,6 +1526,126 @@ case class ArrayContains(left: Expression, right: Expression)
copy(left = newLeft, right = newRight)
}
+/**
+ * This expression converts data of `ArrayData` to an array of java type.
+ *
+ * NOTE: When the data type of expression is `ArrayType`, and the expression is foldable,
+ * the `ConstantFolding` can do constant folding optimization automatically,
+ * (avoiding frequent calls to `ArrayData.to{XXX}Array()`).
+ */
+case class ToJavaArray(array: Expression)
+ extends UnaryExpression
+ with ImplicitCastInputTypes
+ with NullIntolerant
+ with QueryErrorsBase {
+
+ override def checkInputDataTypes(): TypeCheckResult = array.dataType match {
+ case ArrayType(_, _) =>
+ TypeCheckResult.TypeCheckSuccess
+ case _ =>
+ DataTypeMismatch(
+ errorSubClass = "UNEXPECTED_INPUT_TYPE",
+ messageParameters = Map(
+ "paramIndex" -> ordinalNumber(0),
+ "requiredType" -> toSQLType(ArrayType),
+ "inputSql" -> toSQLExpr(array),
+ "inputType" -> toSQLType(array.dataType))
+ )
+ }
+
+ override def inputTypes: Seq[AbstractDataType] = Seq(array.dataType)
+ override def dataType: DataType = {
+ if (canPerformFast) {
+ elementType match {
+ case ByteType => ObjectType(classOf[Array[Byte]])
+ case ShortType => ObjectType(classOf[Array[Short]])
+ case IntegerType => ObjectType(classOf[Array[Int]])
+ case LongType => ObjectType(classOf[Array[Long]])
+ case FloatType => ObjectType(classOf[Array[Float]])
+ case DoubleType => ObjectType(classOf[Array[Double]])
+ }
+ } else if (isPrimitiveType) {
+ elementType match {
+ case BooleanType => ObjectType(classOf[Array[java.lang.Boolean]])
+ case ByteType => ObjectType(classOf[Array[java.lang.Byte]])
+ case ShortType => ObjectType(classOf[Array[java.lang.Short]])
+ case IntegerType => ObjectType(classOf[Array[java.lang.Integer]])
+ case LongType => ObjectType(classOf[Array[java.lang.Long]])
+ case FloatType => ObjectType(classOf[Array[java.lang.Float]])
+ case DoubleType => ObjectType(classOf[Array[java.lang.Double]])
+ }
+ } else {
+ ObjectType(classOf[Array[Object]])
+ }
+ }
+
+ override def child: Expression = array
+ override def prettyName: String = "to_java_array"
+
+ @transient lazy val elementType: DataType =
+ array.dataType.asInstanceOf[ArrayType].elementType
+ private def resultArrayElementNullable: Boolean =
+ array.dataType.asInstanceOf[ArrayType].containsNull
+ private def isPrimitiveType: Boolean = CodeGenerator.isPrimitiveType(elementType)
+ private def canPerformFast: Boolean =
+ isPrimitiveType && elementType != BooleanType && !resultArrayElementNullable
+
+ private def toJavaArray(array: Any): Any = {
+ val arrayData = array.asInstanceOf[ArrayData]
+ if (canPerformFast) {
+ elementType match {
+ case ByteType => arrayData.toByteArray()
+ case ShortType => arrayData.toShortArray()
+ case IntegerType => arrayData.toIntArray()
+ case LongType => arrayData.toLongArray()
+ case FloatType => arrayData.toFloatArray()
+ case DoubleType => arrayData.toDoubleArray()
+ }
+ } else if (isPrimitiveType) {
+ elementType match {
+ case BooleanType => arrayData.toArray[java.lang.Boolean](BooleanType)
+ case ByteType => arrayData.toArray[java.lang.Byte](ByteType)
+ case ShortType => arrayData.toArray[java.lang.Short](ShortType)
+ case IntegerType => arrayData.toArray[java.lang.Integer](IntegerType)
+ case LongType => arrayData.toArray[java.lang.Long](LongType)
+ case FloatType => arrayData.toArray[java.lang.Float](FloatType)
+ case DoubleType => arrayData.toArray[java.lang.Double](DoubleType)
Review Comment:
ah we don't need to pass the comparator
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1770738277
##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ArrayExpressionUtils.java:
##########
@@ -19,158 +19,168 @@
import java.util.Arrays;
import java.util.Comparator;
-import org.apache.spark.sql.catalyst.util.ArrayData;
import org.apache.spark.sql.catalyst.util.SQLOrderingUtil;
-import org.apache.spark.sql.types.ByteType$;
-import org.apache.spark.sql.types.BooleanType$;
-import org.apache.spark.sql.types.DataType;
-import org.apache.spark.sql.types.DoubleType$;
-import org.apache.spark.sql.types.FloatType$;
-import org.apache.spark.sql.types.IntegerType$;
-import org.apache.spark.sql.types.LongType$;
-import org.apache.spark.sql.types.ShortType$;
public class ArrayExpressionUtils {
- private static final Comparator<Object> booleanComp = (o1, o2) -> {
+ // comparator
+ // Boolean ascending nullable comparator
+ private static final Comparator<Boolean> booleanComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- boolean c1 = (Boolean) o1, c2 = (Boolean) o2;
- return c1 == c2 ? 0 : (c1 ? 1 : -1);
+ return o1.equals(o2) ? 0 : (o1 ? 1 : -1);
};
- private static final Comparator<Object> byteComp = (o1, o2) -> {
+ // Byte ascending nullable comparator
+ private static final Comparator<Byte> byteComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- byte c1 = (Byte) o1, c2 = (Byte) o2;
- return Byte.compare(c1, c2);
+ return Byte.compare(o1, o2);
};
- private static final Comparator<Object> shortComp = (o1, o2) -> {
+ // Short ascending nullable comparator
+ private static final Comparator<Short> shortComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- short c1 = (Short) o1, c2 = (Short) o2;
- return Short.compare(c1, c2);
+ return Short.compare(o1, o2);
};
- private static final Comparator<Object> integerComp = (o1, o2) -> {
+ // Integer ascending nullable comparator
+ private static final Comparator<Integer> integerComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- int c1 = (Integer) o1, c2 = (Integer) o2;
- return Integer.compare(c1, c2);
+ return Integer.compare(o1, o2);
};
- private static final Comparator<Object> longComp = (o1, o2) -> {
+ // Long ascending nullable comparator
+ private static final Comparator<Long> longComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- long c1 = (Long) o1, c2 = (Long) o2;
- return Long.compare(c1, c2);
+ return Long.compare(o1, o2);
};
- private static final Comparator<Object> floatComp = (o1, o2) -> {
+ // Float ascending nullable comparator
+ private static final Comparator<Float> floatComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- float c1 = (Float) o1, c2 = (Float) o2;
- return SQLOrderingUtil.compareFloats(c1, c2);
+ return SQLOrderingUtil.compareFloats(o1, o2);
};
- private static final Comparator<Object> doubleComp = (o1, o2) -> {
+ // Double ascending nullable comparator
+ private static final Comparator<Double> doubleComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- double c1 = (Double) o1, c2 = (Double) o2;
- return SQLOrderingUtil.compareDoubles(c1, c2);
+ return SQLOrderingUtil.compareDoubles(o1, o2);
};
- public static int binarySearchNullSafe(ArrayData data, Boolean value) {
- return Arrays.binarySearch(data.toObjectArray(BooleanType$.MODULE$), value, booleanComp);
+ // Boolean nullable
Review Comment:
This is a great suggestion, although `Arrays` did not provide a method `public static int binarySearch(boolean[] a, boolean key)`, we can implement it ourselves
<img width="557" alt="image" src="https://github.com/user-attachments/assets/9c4630b8-258a-46f9-9e6f-d30b1605b780">
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1772486571
##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ArrayExpressionUtils.java:
##########
@@ -19,158 +19,288 @@
import java.util.Arrays;
import java.util.Comparator;
+import scala.reflect.ClassTag$;
+
import org.apache.spark.sql.catalyst.util.ArrayData;
import org.apache.spark.sql.catalyst.util.SQLOrderingUtil;
-import org.apache.spark.sql.types.ByteType$;
-import org.apache.spark.sql.types.BooleanType$;
import org.apache.spark.sql.types.DataType;
-import org.apache.spark.sql.types.DoubleType$;
-import org.apache.spark.sql.types.FloatType$;
-import org.apache.spark.sql.types.IntegerType$;
-import org.apache.spark.sql.types.LongType$;
-import org.apache.spark.sql.types.ShortType$;
+
+import static org.apache.spark.sql.types.DataTypes.BooleanType;
+import static org.apache.spark.sql.types.DataTypes.ByteType;
+import static org.apache.spark.sql.types.DataTypes.DoubleType;
+import static org.apache.spark.sql.types.DataTypes.FloatType;
+import static org.apache.spark.sql.types.DataTypes.IntegerType;
+import static org.apache.spark.sql.types.DataTypes.LongType;
+import static org.apache.spark.sql.types.DataTypes.ShortType;
public class ArrayExpressionUtils {
- private static final Comparator<Object> booleanComp = (o1, o2) -> {
+ // comparator
+ // Boolean ascending nullable comparator
+ private static final Comparator<Boolean> booleanComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- boolean c1 = (Boolean) o1, c2 = (Boolean) o2;
- return c1 == c2 ? 0 : (c1 ? 1 : -1);
+ return o1.equals(o2) ? 0 : (o1 ? 1 : -1);
};
- private static final Comparator<Object> byteComp = (o1, o2) -> {
+ // Byte ascending nullable comparator
+ private static final Comparator<Byte> byteComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- byte c1 = (Byte) o1, c2 = (Byte) o2;
- return Byte.compare(c1, c2);
+ return Byte.compare(o1, o2);
};
- private static final Comparator<Object> shortComp = (o1, o2) -> {
+ // Short ascending nullable comparator
+ private static final Comparator<Short> shortComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- short c1 = (Short) o1, c2 = (Short) o2;
- return Short.compare(c1, c2);
+ return Short.compare(o1, o2);
};
- private static final Comparator<Object> integerComp = (o1, o2) -> {
+ // Integer ascending nullable comparator
+ private static final Comparator<Integer> integerComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- int c1 = (Integer) o1, c2 = (Integer) o2;
- return Integer.compare(c1, c2);
+ return Integer.compare(o1, o2);
};
- private static final Comparator<Object> longComp = (o1, o2) -> {
+ // Long ascending nullable comparator
+ private static final Comparator<Long> longComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- long c1 = (Long) o1, c2 = (Long) o2;
- return Long.compare(c1, c2);
+ return Long.compare(o1, o2);
};
- private static final Comparator<Object> floatComp = (o1, o2) -> {
+ // Float ascending nullable comparator
+ private static final Comparator<Float> floatComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- float c1 = (Float) o1, c2 = (Float) o2;
- return SQLOrderingUtil.compareFloats(c1, c2);
+ return SQLOrderingUtil.compareFloats(o1, o2);
};
- private static final Comparator<Object> doubleComp = (o1, o2) -> {
+ // Double ascending nullable comparator
+ private static final Comparator<Double> doubleComp = (o1, o2) -> {
if (o1 == null && o2 == null) {
return 0;
} else if (o1 == null) {
return -1;
} else if (o2 == null) {
return 1;
}
- double c1 = (Double) o1, c2 = (Double) o2;
- return SQLOrderingUtil.compareDoubles(c1, c2);
+ return SQLOrderingUtil.compareDoubles(o1, o2);
};
- public static int binarySearchNullSafe(ArrayData data, Boolean value) {
- return Arrays.binarySearch(data.toObjectArray(BooleanType$.MODULE$), value, booleanComp);
+ // boolean
+ // boolean non-nullable
+ public static int binarySearch(boolean[] data, boolean value) {
Review Comment:
@cloud-fan
Do we need to consider the following scenarios?
<img width="633" alt="image" src="https://github.com/user-attachments/assets/73e6348b-9aea-4831-8132-d30808501634">
Because like `Arrays.binarySearch(int[] a, int key)` also support `multiple elements` in order.
<img width="621" alt="image" src="https://github.com/user-attachments/assets/fe5e19b3-2a63-41a8-8326-d63f00b22d60">
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1772487963
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala:
##########
@@ -90,7 +90,24 @@ trait InvokeLike extends Expression with NonSQLExpression with ImplicitCastInput
// serializability, because the type-level info with java.io.Serializable and
// java.io.Externalizable marker interfaces are not strong guarantees.
// This restriction can be relaxed in the future to expose more optimizations.
Review Comment:
Done.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #47984:
URL: https://github.com/apache/spark/pull/47984#discussion_r1770949125
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala:
##########
@@ -90,7 +90,24 @@ trait InvokeLike extends Expression with NonSQLExpression with ImplicitCastInput
// serializability, because the type-level info with java.io.Serializable and
// java.io.Externalizable marker interfaces are not strong guarantees.
// This restriction can be relaxed in the future to expose more optimizations.
- !dt.existsRecursively(_.isInstanceOf[ObjectType])
+ !dt.existsRecursively {
+ case ObjectType(cls) if cls == classOf[Array[java.lang.Boolean]] => false
Review Comment:
When we use `Invoke` to implement `ToJavaArray`, we must update here, otherwise we cannot hit `ConstantFolding` optimization
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49506][SQL] Optimize ArrayBinarySearch for foldable array [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #47984:
URL: https://github.com/apache/spark/pull/47984#issuecomment-2367578912
If we `don't` use `Invoke` to implement `ToJavaArray`, the benchmark data is as follows (run 3 times):
- box as `java.lang.Integer`
- `java.lang.Object`
```shell
Running benchmark: array binary search
Running case: has foldable optimize
Stopped after 100 iterations, 23226 ms
OpenJDK 64-Bit Server VM 17.0.10+7-LTS on Mac OS X 15.0
Apple M2
array binary search: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
has foldable optimize 225 232 7 44.4 22.5 1.0X
Running benchmark: array binary search
Running case: has foldable optimize
Stopped after 100 iterations, 23022 ms
OpenJDK 64-Bit Server VM 17.0.10+7-LTS on Mac OS X 15.0
Apple M2
array binary search: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
has foldable optimize 223 230 8 44.8 22.3 1.0X
Running benchmark: array binary search
Running case: has foldable optimize
Stopped after 100 iterations, 24869 ms
OpenJDK 64-Bit Server VM 17.0.10+7-LTS on Mac OS X 15.0
Apple M2
array binary search: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
has foldable optimize 242 249 10 41.4 24.2 1.0X
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org