feat: [branch-1.0] add codegen dispatch fallback for CometCast incompatible/unsupported cases (#5079) - #5140
Merged
Conversation
…orted cases (including legacy config paths) (apache#5079) (cherry picked from commit 76b9d2e)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Cherry-pick of #5079 (merged to
mainas76b9d2eee) ontobranch-1.0.Rationale for this change
CometCastwas the last serde on the incompatible/unsupported path that had no codegen dispatch. Without it, a single cast that Comet declines to run natively forces the whole projection back to Spark, so the surrounding native operators are lost too. The cases affected are all reachable with default or common configuration:spark.sql.legacy.castComplexTypesToString.enabled=true(struct/array/map to string) wasUnsupported, forcing full fallback.CORRECTEDspark.sql.legacy.timeParserPolicy(LEGACY,EXCEPTION) on string to date/timestamp casts.MapTypeto string, and struct/array containing aMapType, which have no native implementation.Incompatiblecases (negative-scale decimal to string, float/double to decimal rounding).branch-1.0already carriesCodegenDispatchFallbackand the serdes that use it, so this is the same mechanism already shipping forIn/InSet, strings, datetime, arrays, and maps —CometCastwas simply missed. Getting it into 1.0.0 avoids a release where a legacy cast config silently costs users native execution of an entire projection.What changes are included in this PR?
A
git cherry-pick -xof76b9d2eee. The content is identical to the commit onmain; the twodev/diffs/*.diffhunks auto-merged (only surroundingcomet.versioncontext differs on this branch, sincebranch-1.0is at1.0.0rather than1.1.0-SNAPSHOT), and no adaptation was needed.CometCastmixes inCodegenDispatchFallback, so casts it cannot match Spark bit-for-bit run Spark's owndoGenCodeinside the Comet pipeline instead of falling the projection back.spark.sql.legacy.castComplexTypesToString.enabled=truenow routes through the dispatcher; the complex-type check is deduplicated againstDataTypeSupport.isComplexType, and theSQLConflookup is reordered to run only when the source type is complex.VariantType(Spark 4, SPARK-45827) is rejected inCometCast.getSupportLevel/isSupportedvia a newisVariantTypeinCometTypeShim(stubbedfalseon Spark 3.x). Neither the native serde nor the codegen dispatcher can carryVariantTypein the data args or return type, so these casts must reach Spark. The guard also covers theLiteralshort-circuit branch, where constant folding would otherwise produce aLiteral[VariantType]that no downstream serde can serialize.expect_fallbackmarkers dropped from the non-legacycast_complex_types_to_stringcases that now stay in the Comet pipeline, plus a missingquerydirective restored above the array-of-map case (it was previously an orphanSELECTthat never ran);CometCastSuitechecks the Comet operator rather than a Spark fallback reason forArrayType(DateType)to an unsupportedArrayType;CometExpressionSuite'sexplain cometexpectations updated for theCalendarIntervalTypeto string cast, which no longer emits a fallback reason.dev/diffs/4.0.2.diffand4.1.2.diff: loosenVariantEndToEndSuite's codegen-mode plan-type assertion to also acceptCometNativeColumnarToRowExec, since under Comet the plan is wrapped rather than havingWholeStageCodegenExecat the top.How are these changes tested?
By the tests carried in the cherry-pick. Verified locally on this branch with a debug native build:
CometSqlFileTestSuitefiltered tocast_complex_types_to_string— 3 tests pass (legacy file, plus non-legacy with dictionary encoding on and off).CometCastSuite— 163 tests pass on the default profile, and 162 pass (1 canceled) under-Pspark-3.5, which exercises thespark-3.xisVariantTypestub.CometExpressionSuite— 138 tests pass, covering the updatedexplain cometexpectations../mvnw spotless:check scalastyle:checkclean oncomet-spark.git apply --checkagainstv4.0.2andv4.1.2checkouts.