fix: Support scalar inputs for datetime expressions when constant folding is disabled#4769
Open
andygrove wants to merge 1 commit into
Open
fix: Support scalar inputs for datetime expressions when constant folding is disabled#4769andygrove wants to merge 1 commit into
andygrove wants to merge 1 commit into
Conversation
…ding is disabled When Spark's ConstantFolding rule is disabled, all-literal datetime expressions (hour, minute, second, unix_timestamp) reach Comet's native engine as ColumnarValue::Scalar instead of being folded in the JVM. The native engine previously errored with "<expr>(scalar) should be fold in Spark JVM side." Add a Scalar match arm to extract_date_part! and unix_timestamp that broadcasts the scalar to a single-element array, evaluates it, and converts the result back to a scalar. Refactor the unix_timestamp array logic into eval_array so both the array and scalar paths share it. Remove the ignore annotations from the existing hour/minute/second/unix_timestamp SQL file tests so they cover the fix against Spark. Closes apache#3336
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
2 participants
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?
Closes #3336
This continues the work in #3448 by @0lai0, rebased onto current
mainwith the merge conflicts inunix_timestamp.rsresolved (the file gained aTimestampNTZbranch anddiv_floorhandling onmainsince that PR was opened). The original commit authorship is preserved.Rationale for this change
When Spark's
ConstantFoldingrule is disabled, all-literal datetime expressions such ashour(timestamp('2024-01-15 12:30:45'))reach Comet's native engine asColumnarValue::Scalarinstead of being folded away in the JVM. The native engine had no scalar match arm for these functions and returned"<expr>(scalar) should be fold in Spark JVM side.", failing the query.What changes are included in this PR?
extract_date_part.rs: add aColumnarValue::Scalarmatch arm to theextract_date_part!macro (covershour,minute,second). The scalar is broadcast to a single-element array, evaluated, and converted back to a scalar viaScalarValue::try_from_array.unix_timestamp.rs: refactor the array evaluation intoeval_arrayand add a scalar arm that reuses it. The merge keepsmain'sTimestamp(Microsecond, None)(TimestampNTZ) branch anddiv_floorbehavior.ignore(https://github.com/apache/datafusion-comet/issues/3336)annotations from the existinghour,minute,second, andunix_timestampSQL file tests so the literal cases are exercised.How are these changes tested?
The existing SQL file tests for
hour,minute,second, andunix_timestamprun withConstantFoldingdisabled and compare Comet's output against Spark. Their literal-argument cases were previously ignored for this issue and are now enabled. All 104 tests in the datetime SQL file suite pass locally, along with the nativedatetime_funcsunit tests.