Skip to content

[SPARK-47902][SQL]Making Compute Current Time* expressions foldable#46120

Closed
dbatomic wants to merge 5 commits intoapache:masterfrom
dbatomic:constantfoldingforcurrenttime
Closed

[SPARK-47902][SQL]Making Compute Current Time* expressions foldable#46120
dbatomic wants to merge 5 commits intoapache:masterfrom
dbatomic:constantfoldingforcurrenttime

Conversation

@dbatomic
Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

This PR is a followed of this PR that made "compute current time" family of expressions Unevaluable. Implicitly, all Unevaluable expressions are also not foldable. Hence, we have a regression comparing to state prior to 44261 where "compute current time" expressions could have been used in places where constant folding is required.

Proposed change is to keep these expression Unevaluable in a sense that eval/codeGen can't be called but to allow folding. This is a special case given that these expressions are supposed to be replaced by QO with literals (that are foldable) so proposal is to create new interface for this family of expressions.

Why are the changes needed?

Explained above.

Does this PR introduce any user-facing change?

Yes.

How was this patch tested?

Additional test is added that ensures that CurrentDate can be used in places that require folding.

Was this patch authored or co-authored using generative AI tooling?

No.

@github-actions github-actions bot added the SQL label Apr 18, 2024
oneRowScalarSubquery)
}

test("Current time functions are constant folded") {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what we need is an analyzer test to make sure we can use current_time in functions that require folding input, like rand. How about we move the test to ExpressionTypeCheckingSuite?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, please take a look.


test("check that current time is foldable") {
val rnd =
Rand(Cast(UnixTimestamp(CurrentDate(), Literal("yyyy-MM-dd HH:mm:ss")), IntegerType))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

int: we can simply do Month(CurrentDate()) to turn date into int.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, yes, sure.

@cloud-fan
Copy link
Copy Markdown
Contributor

thanks, merging to master!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants