test: cover crc32 on binary inputs

[peterxcli]

Which issue does this PR close?

No linked issue; this adds coverage for binary inputs to crc32.

Rationale for this change

The existing crc32 tests use strings or values cast to strings, so they do not exercise arbitrary binary input such as invalid UTF-8 or embedded zero bytes.

What changes are included in this PR?

Extend the SQL fixture with binary columns and literals covering invalid UTF-8, embedded zero bytes, multibyte UTF-8, empty input, NULL, and a checksum above the signed 32-bit range. Compare results with Spark and require Comet execution.

How are these changes tested?

The crc32.sql fixture passes locally through CometSqlFileTestSuite on Spark 3.5.9 and 4.1.3 with JDK 17. All four queries compare results with Spark and require Comet execution. make format (Spark 3.5) and Apache RAT passed.

[sunchao]

Correctness

The existing crc32 fixture covers strings and casts to strings. This change adds a six-row binary table and two queries to exercise arbitrary bytes directly, including invalid UTF-8, embedded NULs, multibyte UTF-8, empty input, NULL and a checksum above the signed 32-bit range.

The maintained Spark 3.5 and 4.0 implementations accept BinaryType, hash the entire byte array with java.util.zip.CRC32, and return the unsigned 32-bit checksum in a LongType. NULL input returns NULL. Neither implementation has an ANSI-mode or timezone branch. The 123456789 vector produces 3421780262, and empty input produces zero.

The current Comet serde emits the native crc32 scalar function. Binary literals are serialized as bytes without a text conversion. I checked the exact datafusion-spark 55.1.0 package against the checksum in Cargo.lock: its implementation hashes byte slices, preserves NULLs, and widens the checksum to Int64. Comet registers that function for both type resolution and execution.

The SQL harness disables constant folding, rebuilds a separate Spark baseline, compares results and requires Comet operators for every plain query. With the current direct scalar serde, the binary column and non-NULL literals exercise the native route. The literal NULL expression may simplify under Spark's separate null-propagation rule, so the NULL row in the binary table supplies runtime null coverage. The fixture has no version filter or ignored queries. Maintained Spark 3.4 and 4.1 source branches were unavailable locally, so the canonical comparison is limited to 3.5 and 4.0.

Validation

At the review cutoff, CI had 41 successful checks and 11 skipped, with none failed or running. I read the original expression-job logs: expressions/hash/crc32.sql passed on Spark 3.4, 3.5, 4.0, 4.1 and 4.2. Other canceled or ignored tests in those suites are not counted as coverage.

Those jobs checked out merge 7e0e8329, whose parents are base 2d1aab3a and reviewed head 2166d615. All 13 reviewed fixture, harness, dependency and implementation files match the head. The merge differs only in three unrelated shuffle files. The native builder and all five consumers report the same artifact digest. I did not run a local JVM/native build. A separate Python checksum calculation confirmed the fixture's unsigned-range and empty-input vectors, but is not Comet runtime evidence.

No verified P1/P2 findings.

Performance

The PR adds no production-path work. The added fixture uses six rows, one binary-column query and one combined literal query. That is a small, focused increase in test work, with table cleanup handled by the existing harness. There is no performance claim or new kernel implementation requiring a microbenchmark.

Design

Extending the existing SQL fixture keeps the string and binary behavior together while exercising the actual Spark-to-Comet serialization and native execution path. Hex literals express the exact bytes without relying on a source file's text encoding. The column case covers nullable batch input, and the literal case covers scalar arguments. No additional test framework or production changes are needed.

Abstraction & complexity

The change consists of 17 SQL lines and reuses the existing answer and operator checks. The parser discovers the new table for scoped cleanup. The explicit byte vectors and combined literal query are easy to inspect, and I found no unnecessary abstraction or actionable simplification.

[andygrove]

Thanks. Good to have the invalid-UTF-8 and embedded-NUL cases hashed as bytes, plus the unsigned checksum that overflows signed 32-bit.