fix: preserve concat binary string bytes

[manuzhang]

Which issue does this PR close?

Relates to #4180.

Rationale for this change

spark.sql.function.concatBinaryAsString=true makes Spark insert BinaryType-to-StringType casts before concat. Spark preserves malformed bytes in those casts, while Comet's native Arrow string path normalizes them to UTF-8 replacement bytes. For example, Spark keeps FFFE41, but Comet produced EFBFBDEFBFBD41.

What changes are included in this PR?

• Route concat trees containing BinaryType-to-StringType casts through Spark's generated code inside the Comet pipeline.
• Add a two-value SQL config matrix covering result bytes, malformed UTF-8, empty values, and nulls.
• Add a Scala assertion for result types and direct proof that both config values use the JVM codegen dispatcher without operator fallback.
• Record the supported behavior in the Spark configuration contributor guide.

How are these changes tested?

• make core
• Focused CometSqlFileTestSuite concat_binary_as_string on Spark 3.4.3, 3.5.8, 4.0.2, and 4.1.2
• Focused CometCodegenSuite concatBinaryAsString on the same versions
• ./mvnw spotless:check -Pspark-4.1 -Pjdk17 -DskipTests -Dscalastyle.skip=true
• Dynamic documentation generation for all four Spark profiles

[andygrove]

Hi @manuzhang. Could you take a look at the epic #4764? I think the approach in this PR goes in a different direction. Would be good to get your thoughts on the bigger picture.

[andygrove]

Shouldn't this logic apply to all cast-binary-to-string, not just when called from concat?

[andygrove]

Thanks for digging into this. The diagnosis is right, and comparing bytes with hex() in the test file is exactly the right way to pin the behavior down.

My main question is about where the fix sits. The divergence comes from Cast(binary AS string), not from concat. CometCast already mixes in CodegenDispatchFallback (CometCast.scala:34), so the equivalent change there is returning Unsupported for BinaryType -> StringType at CometCast.scala:322 instead of Compatible(). That version also covers hex(cast(b AS string)) on its own, octet_length(cast(b AS string)), the CAST(CAST(X'FF' AS STRING) AS BINARY) round trip that the compatibility guide uses as its worked example, and nested shapes like concat(coalesce(cast(b AS string), ''), y) that the expr.children.exists check does not see. As the PR stands, hex(concat(cast(b AS string), x)) will match Spark while hex(cast(b AS string)) still will not, which is a confusing place for users to land.

I am not saying the answer is obviously "move it to the cast", because there is a real tradeoff there. #4763 chose the native path plus decode_utf8_spark_lossy deliberately, so that the cast stays native for the valid-UTF-8 case, which is nearly all real data. Marking binary-to-string Unsupported would send every one of those casts to the JVM dispatcher. Could you say which side of that you want to take? Both are defensible. I would just like the PR to make the choice explicitly rather than land on concat because that is where the original report happened to come from.

Two other things.

The blast radius is wider than the description suggests. Spark's ConcatTypeCoercion inserts the binary-to-string casts when concatBinaryAsString is true, or when the children are not all binary. So concat(binary_col, 'suffix') picks up a Cast(binary, StringType) child under the default false and hits the new branch too, which moves a default-config query off the native path. Worth calling out in the description and the docs section, and worth a test case.

docs/source/user-guide/latest/compatibility/index.md will need updating either way. The "Strings with non-UTF-8 bytes" section currently tells users that this whole class of expression normalizes to U+FFFD, and after this change concat no longer does.

One note for later rather than something to fix here. The preserved bytes come back into native through from_ffi at native/spark-expr/src/jvm_udf/mod.rs:239, which is the JVM-UDF import site listed under Gap B of #4764. If Gap B ever decodes at that boundary unconditionally, this fix silently reverts and the new SQL test starts failing. I will leave a note on the EPIC so that whoever picks up Gap B knows that site now has a consumer depending on it not decoding.