-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Enhancement]implement dayofweek in FE #51453
base: main
Are you sure you want to change the base?
[Enhancement]implement dayofweek in FE #51453
Conversation
Signed-off-by: evenhuang <986025158@qq.com>
// function of dayofweek in starrocks monday is 2 and sunday is 1, so need mod 7 and plus 1. | ||
return ConstantOperator.createInt((date.getDatetime().getDayOfWeek().getValue()) % 7 + 1); | ||
} | ||
|
||
@ConstantFunction.List(list = { | ||
@ConstantFunction(name = "years_add", argTypes = {DATETIME, | ||
INT}, returnType = DATETIME, isMonotonic = true), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The most risky bug in this code is:
Using date.getDatetime().getDayOfWeek()
on an integer type could cause a runtime exception.
You can modify the code like this:
@ConstantFunction.List(list = {
@ConstantFunction(name = "dayofweek", argTypes = {DATETIME}, returnType = INT),
@ConstantFunction(name = "dayofweek", argTypes = {DATE}, returnType = INT)
})
public static ConstantOperator dayofweek(ConstantOperator date) {
// LocalDateTime.getDayOfWeek is return day of the week, such as monday is 1 and sunday is 7.
// function of dayofweek in starrocks monday is 2 and sunday is 1, so need mod 7 and plus 1.
return ConstantOperator.createInt((date.getDatetime().getDayOfWeek().getValue()) % 7 + 1);
}
// Remove this entry as it handles integers which are not suitable for getDayOfWeek
@Deprecated
@ConstantFunction(name = "dayofweek", argTypes = {INT}, returnType = INT)
Quality Gate passedIssues Measures |
[Java-Extensions Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[FE Incremental Coverage Report]✅ pass : 1 / 1 (100.00%) file detail
|
[BE Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
Why I'm doing:
If the where condition contains a partition condition that is compared with the dayofweek function, a full table scan will be required, and FE is required to implement the dayofweek function to perform partition pruning when generating the execution plan.
What I'm doing:
implement dayofweek in FE
Fixes #issue
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
FE支持DAYOFWEEK 函数
实现前:
实现后:
Bugfix cherry-pick branch check: