[VL] Re-enable SPARK-36286 timestamp cast test with Velox 18502 - #12771
Open
Smallfu666 wants to merge 1 commit into
Open
[VL] Re-enable SPARK-36286 timestamp cast test with Velox 18502#12771Smallfu666 wants to merge 1 commit into
Smallfu666 wants to merge 1 commit into
Conversation
Remove the test exclusion for "SPARK-36286: invalid string cast to timestamp" across all Spark version shims (spark33/34/35/40/41). The test was disabled because Velox accepted trailing-T strings as valid timestamps; Velox PR 18502 fixes the parser to reject them. Set UPSTREAM_VELOX_PR_ID="18502" in get-velox.sh so Gluten CI builds its pinned Velox baseline with the proposed fix applied, validating the re-enabled test against the actual Velox change. Once 18502 is merged and Gluten's pinned Velox baseline contains the fix, the PR ID pin can be removed.
|
Run Gluten Clickhouse CI on x86 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR re-enables the SPARK-36286 regression test for invalid string-to-timestamp casting in the Velox backend across all Spark shims, and attempts to validate the behavior by applying an upstream Velox PR patch during the Velox build.
Changes:
- Removed the
.exclude("SPARK-36286: invalid string cast to timestamp")filter fromVeloxTestSettings.scalain Spark 3.3/3.4/3.5/4.0/4.1 shims. - Set
UPSTREAM_VELOX_PR_ID="18502"inep/build-velox/src/get-velox.shto auto-apply the upstream Velox fix during CI builds.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| gluten-ut/spark33/src/test/scala/org/apache/gluten/utils/velox/VeloxTestSettings.scala | Re-enables the SPARK-36286 test by removing its exclusion. |
| gluten-ut/spark34/src/test/scala/org/apache/gluten/utils/velox/VeloxTestSettings.scala | Re-enables the SPARK-36286 test by removing its exclusion. |
| gluten-ut/spark35/src/test/scala/org/apache/gluten/utils/velox/VeloxTestSettings.scala | Re-enables the SPARK-36286 test by removing its exclusion. |
| gluten-ut/spark40/src/test/scala/org/apache/gluten/utils/velox/VeloxTestSettings.scala | Re-enables the SPARK-36286 test by removing its exclusion. |
| gluten-ut/spark41/src/test/scala/org/apache/gluten/utils/velox/VeloxTestSettings.scala | Re-enables the SPARK-36286 test by removing its exclusion. |
| ep/build-velox/src/get-velox.sh | Pins application of Velox PR #18502 during source preparation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
27
to
29
| # Developer use only for testing Velox PR. | ||
| UPSTREAM_VELOX_PR_ID="" | ||
| UPSTREAM_VELOX_PR_ID="18502" | ||
|
|
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.
What
Re-enables the
SPARK-36286: invalid string cast to timestamptest that was disabled across all Spark version shims (spark33/34/35/40/41).The test was disabled because Velox accepted
CAST('2015-03-18T' AS TIMESTAMP)as a valid date with zeroed time (2015-03-18 00:00:00), while Spark returnsNULL(aTseparator must be followed immediately by the time component).Velox PR facebookincubator/velox#18502 fixes the parser to reject trailing-T strings. This PR re-enables the Gluten regression test to validate that fix.
Change
.exclude("SPARK-36286: invalid string cast to timestamp")fromVeloxTestSettings.scalain all 5 shims.UPSTREAM_VELOX_PR_ID="18502"inep/build-velox/src/get-velox.shso Gluten CI builds its pinned Velox baseline with the proposed fix applied.Testing
UPSTREAM_VELOX_PR_ID="18502"is intentionally set so that Gluten CI:Once #18502 is merged and Gluten's pinned Velox baseline contains the fix, the PR ID pin can be removed.
Related