feat: Implement map-to-string casting - #4647
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Allow map nulltype key value to execute natively for cast map to string by enabling nulltype support in cast and codegen input output paths. Update SQL expectation for empty map from fallback to native query. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| @@ -672,6 +673,8 @@ private[codegen] object CometBatchKernelCodegenInput { | |||
| if (elementNullable) " if (isNullAt(i)) return null;\n" | |||
There was a problem hiding this comment.
This is intentional. NullType has no typed getter to override (there is no getNull API), so we emit no scalar getter here. Null semantics are handled via isNullAt (always true for NullVector) and generic get(..., NullType) dispatch, which returns null.
|
Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days. |
The rendering matches Spark's Four things. Does this honor
Is the map case gated by that check today? If not, a user with the legacy config gets a silently different string. If it is gated, a test with the config on asserting a fallback would pin it. Hidden entries outside a slice or under a null row The function casts the entire This is the same class of problem #5457 is fixing for nested casts generally. Is it reachable here? If the fix lands in
def isSupportedDataType(dt: DataType): Boolean = dt match {
case NullType => trueThat is in Is there an expression where a The propagation test was inverted
That is correct, but it means the original property is no longer covered. Could you add back a case with a genuinely unsupported nested type, for example |
|
@andygrove thank for review.
Yes. MapType is covered by the existing complex-type legacy guard, so it routes through CodegenDispatchFallback instead of the native map-to-string cast when the config is enabled. I also added an empty-map case to cast_complex_types_to_string_legacy.sql. |
I added regression tests for both sliced maps and hidden entries under null map rows. As you said, both currently fail. I also verified that both pass when combined with #5457, so this will be resolved once that PR is merged. |
I added runtime Janino tests covering a top-level NullType-to-primitive cast, reading an Array element through elementGetterCall in a primitive expression, and writing Array output through emitSpecializedGetterExpr. All three tests compile and pass. |
I restored the Unsupported propagation coverage using Map<Int, CalendarInterval>. The test verifies that the nested CalendarInterval-to-String Unsupported result and reason are propagated through the outer Map arm, and it passes. The above changes were made with the assistance of an AI agent. |
Which issue does this PR close?
Closes #4631.
Rationale for this change
This PR implements native map-to-string casting in Comet to match Spark formatting (
{k -> v, ...}).As part of completing this feature, it also covers the empty-map case (
cast(map() as string)) where Catalyst infersMap<NullType, NullType>.What changes are included in this PR?
native/spark-expr.MapType -> StringTypeinCometCast.NullType -> StringTypein cast support checks.Map<NullType, NullType>:NullTypeNullVector, null-only write/getter handling)cast(map() as string)runs natively instead of fallback.How are these changes tested?
PROFILES='-Pspark-4.0' make format./mvnw -q test -DwildcardSuites=CometSqlFileTestSuite -Dsuites='org.apache.comet.CometSqlFileTestSuite cast_complex_types_to_string' -Dtest=none