diff --git a/docs/source/user-guide/latest/expressions.md b/docs/source/user-guide/latest/expressions.md index 2ba71e93c4d..d7579bea45b 100644 --- a/docs/source/user-guide/latest/expressions.md +++ b/docs/source/user-guide/latest/expressions.md @@ -553,11 +553,11 @@ The type-name conversion functions (`bigint`, `binary`, `boolean`, `date`, `deci | --- | --- | --- | --- | | `ascii` | ✅ | Native | | | `base64` | ✅ | Native | | -| `bit_length` | ✅ | Native | | +| `bit_length` | ✅ | Hybrid | | | `btrim` | ✅ | — | | | `char` | ✅ | Native | | -| `char_length` | ✅ | Native | | -| `character_length` | ✅ | Native | | +| `char_length` | ✅ | Hybrid | | +| `character_length` | ✅ | Hybrid | | | `chr` | ✅ | Native | | | `collate` | 🔜 | — | Spark collation (umbrella [#2190](https://github.com/apache/datafusion-comet/issues/2190)) | | `collation` | ✅ | — | Constant-folded to a literal (Spark 4.0+) | @@ -574,8 +574,8 @@ The type-name conversion functions (`bigint`, `binary`, `boolean`, `date`, `deci | `instr` | ✅ | Native | | | `lcase` | ✅ | Hybrid | | | `left` | ✅ | Native | | -| `len` | ✅ | Native | | -| `length` | ✅ | Native | | +| `len` | ✅ | Hybrid | | +| `length` | ✅ | Hybrid | | | `levenshtein` | ✅ | Native | | | `locate` | ✅ | Codegen dispatch | | | `lower` | ✅ | Hybrid | | @@ -583,7 +583,7 @@ The type-name conversion functions (`bigint`, `binary`, `boolean`, `date`, `deci | `ltrim` | ✅ | Native | | | `luhn_check` | ✅ | — | Native via `StaticInvoke` (tests: luhn_check.sql) | | `mask` | ✅ | — | Routed through the JVM codegen dispatcher | -| `octet_length` | ✅ | Native | | +| `octet_length` | ✅ | Hybrid | | | `overlay` | ✅ | Codegen dispatch | | | `position` | ✅ | Codegen dispatch | | | `printf` | ✅ | Codegen dispatch | | diff --git a/spark/src/main/scala/org/apache/comet/serde/strings.scala b/spark/src/main/scala/org/apache/comet/serde/strings.scala index 665c0d3b54e..0051ac80f40 100644 --- a/spark/src/main/scala/org/apache/comet/serde/strings.scala +++ b/spark/src/main/scala/org/apache/comet/serde/strings.scala @@ -82,7 +82,10 @@ object CometUpper extends CometCaseConversionBase[Upper]("upper") object CometLower extends CometCaseConversionBase[Lower]("lower") -object CometLength extends CometScalarFunction[Length]("length") { +object CometLength extends CometScalarFunction[Length]("length") with CodegenDispatchFallback { + // The native `length` UDF has no path for BinaryType. Rather than fall the projection back to + // Spark, route the binary case through the JVM codegen dispatcher (Spark's own `doGenCode`, i.e. + // `numBytes()`) inside the Comet pipeline so the result stays native and matches Spark exactly. override def getUnsupportedReasons(): Seq[String] = Seq("`BinaryType` input is not supported") override def getSupportLevel(expr: Length): SupportLevel = expr.child.dataType match { @@ -91,7 +94,11 @@ object CometLength extends CometScalarFunction[Length]("length") { } } -object CometBitLength extends CometScalarFunction[BitLength]("bit_length") { +object CometBitLength + extends CometScalarFunction[BitLength]("bit_length") + with CodegenDispatchFallback { + // See CometLength: BinaryType has no native path, so route it through the codegen dispatcher + // (Spark's own `doGenCode`, i.e. `numBytes() * 8`) instead of falling back to Spark. override def getUnsupportedReasons(): Seq[String] = Seq("`BinaryType` input is not supported") override def getSupportLevel(expr: BitLength): SupportLevel = expr.child.dataType match { @@ -100,7 +107,11 @@ object CometBitLength extends CometScalarFunction[BitLength]("bit_length") { } } -object CometOctetLength extends CometScalarFunction[OctetLength]("octet_length") { +object CometOctetLength + extends CometScalarFunction[OctetLength]("octet_length") + with CodegenDispatchFallback { + // See CometLength: BinaryType has no native path, so route it through the codegen dispatcher + // (Spark's own `doGenCode`, i.e. `numBytes()`) instead of falling back to Spark. override def getUnsupportedReasons(): Seq[String] = Seq("`BinaryType` input is not supported") override def getSupportLevel(expr: OctetLength): SupportLevel = expr.child.dataType match { diff --git a/spark/src/test/resources/sql-tests/expressions/string/bit_length.sql b/spark/src/test/resources/sql-tests/expressions/string/bit_length.sql index a2327120163..b9fd2abcd23 100644 --- a/spark/src/test/resources/sql-tests/expressions/string/bit_length.sql +++ b/spark/src/test/resources/sql-tests/expressions/string/bit_length.sql @@ -15,6 +15,10 @@ -- specific language governing permissions and limitations -- under the License. +-- BinaryType has no native path, so it routes through the codegen dispatcher (Spark's own +-- `doGenCode`, i.e. `numBytes() * 8`) instead of falling back to Spark. +-- Config: spark.comet.exec.scalaUDF.codegen.enabled=true + statement CREATE TABLE test_bit_length(s string) USING parquet @@ -28,16 +32,15 @@ SELECT bit_length(s) FROM test_bit_length query SELECT bit_length('hello'), bit_length(''), bit_length(NULL) --- BinaryType input falls back to Spark; the native DataFusion impl rejects Binary at runtime, --- so the serde gates Binary as Unsupported (matching the existing CometLength shape). +-- BinaryType input routes through the codegen dispatcher and stays inside Comet statement CREATE TABLE test_bit_length_binary(b binary) USING parquet statement INSERT INTO test_bit_length_binary VALUES (X'48656c6c6f'), (X''), (NULL), (X'FF') -query expect_fallback(bit_length on BinaryType is not supported) +query SELECT bit_length(b) FROM test_bit_length_binary -query expect_fallback(bit_length on BinaryType is not supported) +query SELECT bit_length(X'48656c6c6f'), bit_length(CAST(NULL AS BINARY)) diff --git a/spark/src/test/resources/sql-tests/expressions/string/length.sql b/spark/src/test/resources/sql-tests/expressions/string/length.sql index 1e1e8ccc7a5..fc7807dc74f 100644 --- a/spark/src/test/resources/sql-tests/expressions/string/length.sql +++ b/spark/src/test/resources/sql-tests/expressions/string/length.sql @@ -15,6 +15,10 @@ -- specific language governing permissions and limitations -- under the License. +-- BinaryType has no native path, so it routes through the codegen dispatcher (Spark's own +-- `doGenCode`, i.e. `numBytes()`) instead of falling back to Spark. +-- Config: spark.comet.exec.scalaUDF.codegen.enabled=true + statement CREATE TABLE test_length(s string) USING parquet @@ -27,3 +31,16 @@ SELECT length(s), char_length(s) FROM test_length -- literal arguments query SELECT length('hello'), length(''), length(NULL) + +-- BinaryType input routes through the codegen dispatcher and stays inside Comet +statement +CREATE TABLE test_length_binary(b binary) USING parquet + +statement +INSERT INTO test_length_binary VALUES (X'48656c6c6f'), (X''), (NULL), (X'FF') + +query +SELECT length(b) FROM test_length_binary + +query +SELECT length(X'48656c6c6f'), length(CAST(NULL AS BINARY)) diff --git a/spark/src/test/resources/sql-tests/expressions/string/octet_length.sql b/spark/src/test/resources/sql-tests/expressions/string/octet_length.sql index e650950347c..5add3b10020 100644 --- a/spark/src/test/resources/sql-tests/expressions/string/octet_length.sql +++ b/spark/src/test/resources/sql-tests/expressions/string/octet_length.sql @@ -15,6 +15,10 @@ -- specific language governing permissions and limitations -- under the License. +-- BinaryType has no native path, so it routes through the codegen dispatcher (Spark's own +-- `doGenCode`, i.e. `numBytes()`) instead of falling back to Spark. +-- Config: spark.comet.exec.scalaUDF.codegen.enabled=true + statement CREATE TABLE test_octet_length(s string) USING parquet @@ -28,16 +32,15 @@ SELECT octet_length(s) FROM test_octet_length query SELECT octet_length('hello'), octet_length(''), octet_length(NULL) --- BinaryType input falls back to Spark; the native DataFusion impl rejects Binary at runtime, --- so the serde gates Binary as Unsupported (matching the existing CometLength shape). +-- BinaryType input routes through the codegen dispatcher and stays inside Comet statement CREATE TABLE test_octet_length_binary(b binary) USING parquet statement INSERT INTO test_octet_length_binary VALUES (X'48656c6c6f'), (X''), (NULL), (X'FF') -query expect_fallback(octet_length on BinaryType is not supported) +query SELECT octet_length(b) FROM test_octet_length_binary -query expect_fallback(octet_length on BinaryType is not supported) +query SELECT octet_length(X'48656c6c6f'), octet_length(CAST(NULL AS BINARY))