-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat(spark): implement Spark datetime function last_day #16828
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
base: main
Are you sure you want to change the base?
Conversation
Hi @alamb, I’ve added the
|
I think the issue is that the function is named fn name(&self) -> &str {
"spark_last_day"
} |
Signed-off-by: Alan Tang <[email protected]>
Signed-off-by: Alan Tang <[email protected]>
Signed-off-by: Alan Tang <[email protected]>
Signed-off-by: Alan Tang <[email protected]>
impl SparkLastDay { | ||
pub fn new() -> Self { | ||
Self { | ||
signature: Signature::user_defined(Volatility::Immutable), |
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.
I think you can set the signature to be taking exactly one Date32 type.
After, the planning routine will automatically do all the validations and coercions, then:
- We can assume inside
invoke_with_args()
the input must have valid type, so thoseexec_err
s can be changed tointernal_err
just for sanity check - No need to implement coerce_types()
|
||
|
||
query ? | ||
SELECT NULL; |
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.
Is it select last_day(null)
?
perhaps we can add more bad case tests like
select last_day('foo');
select last_day(123);
select last_day();
select last_day(last_day('2016-02-07'::string, 'foo');
select last_day(last_day('2016-02-31'::string);
And ensure it's returning expected errors.
Which issue does this PR close?
datetime
functionlast_day
#16774.datafusion-spark
Spark Compatible Functions #15914.Rationale for this change
What changes are included in this PR?
Implement Spark datetime function last_day
Are these changes tested?
I added tests for
last_day
function.Are there any user-facing changes?
yes, new function.