diff --git a/docs/source/contributor-guide/expression-audits/hash_funcs.md b/docs/source/contributor-guide/expression-audits/hash_funcs.md index a294069df14..c999e2b8bbe 100644 --- a/docs/source/contributor-guide/expression-audits/hash_funcs.md +++ b/docs/source/contributor-guide/expression-audits/hash_funcs.md @@ -61,7 +61,7 @@ ## sha2 - Spark 3.4.3 (audited 2026-05-27): identical to 3.5.8. -- Spark 3.5.8 (audited 2026-05-27): baseline. `Sha2(left, right) extends BinaryExpression`; `inputTypes = Seq(BinaryType, IntegerType) -> StringType`. The `numBits` argument selects SHA-224/256/384/512 (0 is treated as 256); other values return NULL. Comet routes via `CometSha2` to the native `sha2` UDF; non-foldable `numBits` falls back to Spark. +- Spark 3.5.8 (audited 2026-05-27): baseline. `Sha2(left, right) extends BinaryExpression`; `inputTypes = Seq(BinaryType, IntegerType) -> StringType`. The `numBits` argument selects SHA-224/256/384/512 (0 is treated as 256); other values return NULL. Comet routes via `CometSha2` to the native `SparkSha2` UDF for both foldable and column `numBits`. Tests cover STRING and BINARY payload columns with row-varying bit lengths, including invalid values and NULL. - Spark 4.0.1 (audited 2026-05-27): trait set gains `DefaultStringProducingExpression` and the `nullIntolerant: Boolean` refactor. Runtime unchanged. - Spark 4.1.1 (audited 2026-05-27): identical to 4.0.1. diff --git a/docs/source/user-guide/latest/expressions.md b/docs/source/user-guide/latest/expressions.md index cf8baac011d..6b40e4da8c2 100644 --- a/docs/source/user-guide/latest/expressions.md +++ b/docs/source/user-guide/latest/expressions.md @@ -362,7 +362,7 @@ to Spark ([#2837](https://github.com/apache/datafusion-comet/issues/2837)). Enab | `md5` | ✅ | Native | | | `sha` | ✅ | Native | | | `sha1` | ✅ | Native | | -| `sha2` | ✅ | Native | | +| `sha2` | ✅ | Native | Supports literal and column `numBits` ([audit](../../contributor-guide/expression-audits/hash_funcs.md#sha2)) | | `xxhash64` | ✅ | Native | Includes decimals with precision 19–38 and nested decimals | --- diff --git a/spark/src/main/scala/org/apache/comet/serde/hash.scala b/spark/src/main/scala/org/apache/comet/serde/hash.scala index 04cd184fe41..a0227a8a241 100644 --- a/spark/src/main/scala/org/apache/comet/serde/hash.scala +++ b/spark/src/main/scala/org/apache/comet/serde/hash.scala @@ -22,7 +22,7 @@ package org.apache.comet.serde import org.apache.spark.sql.catalyst.expressions.{Attribute, Expression, Murmur3Hash, Sha1, Sha2, XxHash64} import org.apache.spark.sql.types.{ArrayType, DataType, IntegerType, LongType, MapType, StringType, StructType} -import org.apache.comet.serde.QueryPlanSerde.{exprToProtoInternal, isTimeType, scalarFunctionExprToProtoWithReturnType, serializeDataType, supportedDataType} +import org.apache.comet.serde.QueryPlanSerde.{exprToProtoInternal, isTimeType, scalarFunctionExprToProtoWithReturnType, serializeDataType, supportedDataType, withNullShortCircuit} object CometXxHash64 extends CometExpressionSerde[XxHash64] { @@ -74,19 +74,10 @@ object CometMurmur3Hash extends CometExpressionSerde[Murmur3Hash] { object CometSha2 extends CometExpressionSerde[Sha2] { - private val nonFoldableNumBitsReason = - "The `numBits` argument must be a foldable literal value" - - override def getUnsupportedReasons(): Seq[String] = - HashUtils.unsupportedReasons :+ nonFoldableNumBitsReason + override def getUnsupportedReasons(): Seq[String] = HashUtils.unsupportedReasons - override def getSupportLevel(expr: Sha2): SupportLevel = { - if (!expr.right.foldable) { - Unsupported(Some(nonFoldableNumBitsReason)) - } else { - HashUtils.supportLevelForChildren(expr) - } - } + override def getSupportLevel(expr: Sha2): SupportLevel = + HashUtils.supportLevelForChildren(expr) override def convert( expr: Sha2, @@ -94,7 +85,9 @@ object CometSha2 extends CometExpressionSerde[Sha2] { binding: Boolean): Option[ExprOuterClass.Expr] = { val leftExpr = exprToProtoInternal(expr.left, inputs, binding) val numBitsExpr = exprToProtoInternal(expr.right, inputs, binding) - scalarFunctionExprToProtoWithReturnType("sha2", StringType, false, leftExpr, numBitsExpr) + withNullShortCircuit( + expr, + scalarFunctionExprToProtoWithReturnType("sha2", StringType, false, leftExpr, numBitsExpr)) } } diff --git a/spark/src/test/resources/sql-tests/expressions/hash/sha2.sql b/spark/src/test/resources/sql-tests/expressions/hash/sha2.sql new file mode 100644 index 00000000000..822516fbf1f --- /dev/null +++ b/spark/src/test/resources/sql-tests/expressions/hash/sha2.sql @@ -0,0 +1,62 @@ +-- 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. + +-- ConstantFolding: enabled + +-- Keep payloads in Parquet columns so SHA-2 executes even when numBits is folded. +statement +CREATE TABLE test_sha2_string(payload STRING, num_bits INT) USING parquet + +statement +INSERT INTO test_sha2_string VALUES + ('hello', 0), ('hello', 224), ('hello', 256), ('hello', 384), ('hello', 512), + ('hello', 128), ('hello', -1), ('hello', NULL), ('', 256), ('', 512), (NULL, 256) + +-- Row-varying bit widths, including invalid widths and NULL, must stay native. +query expect_native(sha2) +SELECT sha2(payload, num_bits) FROM test_sha2_string + +query expect_native(sha2) +SELECT sha2(payload, num_bits + 0) FROM test_sha2_string + +query expect_native(sha2) +SELECT sha2('hello', num_bits) FROM test_sha2_string + +-- A foldable width expression must retain native SHA-2 over the payload column. +query expect_native(sha2) +SELECT sha2(payload, 128 + 128) FROM test_sha2_string + +-- Include zero and high bytes to exercise binary payloads without string conversion. +statement +CREATE TABLE test_sha2_binary(payload BINARY, num_bits INT) USING parquet + +statement +INSERT INTO test_sha2_binary VALUES + (X'0080FF68656C6C6F', 0), (X'0080FF68656C6C6F', 224), + (X'0080FF68656C6C6F', 256), (X'0080FF68656C6C6F', 384), + (X'0080FF68656C6C6F', 512), (X'0080FF68656C6C6F', 128), + (X'0080FF68656C6C6F', -1), (X'0080FF68656C6C6F', NULL), + (X'', 256), (X'', 512), (NULL, 256) + +query expect_native(sha2) +SELECT sha2(payload, num_bits) FROM test_sha2_binary + +query expect_native(sha2) +SELECT sha2(payload, num_bits + 0) FROM test_sha2_binary + +query expect_native(sha2) +SELECT sha2(X'0080FF68656C6C6F', num_bits) FROM test_sha2_binary diff --git a/spark/src/test/resources/sql-tests/expressions/hash/sha2_ansi.sql b/spark/src/test/resources/sql-tests/expressions/hash/sha2_ansi.sql new file mode 100644 index 00000000000..f97201bebb7 --- /dev/null +++ b/spark/src/test/resources/sql-tests/expressions/hash/sha2_ansi.sql @@ -0,0 +1,40 @@ +-- Licensed to the Apache Software Foundation (ASF) under one +-- or more contributor license agreements. See the NOTICE file +-- distributed with this work for additional information +-- regarding copyright ownership. The ASF licenses this file +-- to you under the Apache License, Version 2.0 (the +-- "License"); you may not use this file except in compliance +-- with the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, +-- software distributed under the License is distributed on an +-- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +-- KIND, either express or implied. See the License for the +-- specific language governing permissions and limitations +-- under the License. + +-- Config: spark.sql.ansi.enabled=true + +-- NULL payloads must skip the ANSI cast of bits, even when the string is invalid. +-- Mixed column values keep the short-circuit check in runtime evaluation. +statement +CREATE TABLE test_sha2_ansi_string(payload STRING, bits STRING) USING parquet + +statement +INSERT INTO test_sha2_ansi_string VALUES + (NULL, 'invalid'), ('hello', '256'), ('hello', NULL), (NULL, '256') + +query expect_native(sha2) +SELECT sha2(payload, CAST(bits AS INT)) FROM test_sha2_ansi_string + +statement +CREATE TABLE test_sha2_ansi_binary(payload BINARY, bits STRING) USING parquet + +statement +INSERT INTO test_sha2_ansi_binary VALUES + (NULL, 'invalid'), (X'0080FF', '256'), (X'0080FF', NULL), (NULL, '256') + +query expect_native(sha2) +SELECT sha2(payload, CAST(bits AS INT)) FROM test_sha2_ansi_binary diff --git a/spark/src/test/scala/org/apache/spark/sql/benchmark/CometHashExpressionBenchmark.scala b/spark/src/test/scala/org/apache/spark/sql/benchmark/CometHashExpressionBenchmark.scala index 6450b822fe2..6c8273190c0 100644 --- a/spark/src/test/scala/org/apache/spark/sql/benchmark/CometHashExpressionBenchmark.scala +++ b/spark/src/test/scala/org/apache/spark/sql/benchmark/CometHashExpressionBenchmark.scala @@ -47,7 +47,12 @@ object CometHashExpressionBenchmark extends CometBenchmarkBase { HashExprConfig("sha2_224", "SELECT sha2(c_str, 224) FROM parquetV1Table"), HashExprConfig("sha2_256", "SELECT sha2(c_str, 256) FROM parquetV1Table"), HashExprConfig("sha2_384", "SELECT sha2(c_str, 384) FROM parquetV1Table"), - HashExprConfig("sha2_512", "SELECT sha2(c_str, 512) FROM parquetV1Table")) + HashExprConfig("sha2_512", "SELECT sha2(c_str, 512) FROM parquetV1Table"), + HashExprConfig( + "sha2_dynamic_bits", + """SELECT sha2(c_str, + | element_at(array(0, 224, 256, 384, 512, 128), pmod(c_int, 6) + 1)) + |FROM parquetV1Table""".stripMargin)) override def runCometBenchmark(mainArgs: Array[String]): Unit = { val values = 1024 * 1024 @@ -84,7 +89,7 @@ object CometHashExpressionBenchmark extends CometBenchmarkBase { } // Run only these cases with --wide-decimal. The fallback arm is the path used by - // wide-decimal hash on main before native support; codegen dispatch is tracked in #5835. + // wide-decimal hash on main before native support. private def runWideDecimalBenchmarks(values: Int): Unit = { runBenchmarkWithTable("Wide decimal hash", values) { rows => withTempPath { dir =>