[datafusion-spark] Implement Spark luhn_check function
#16580
Conversation
e977eb8 to
6abe5d1
Compare
Remove clippy warning Remove clippy warning Remove clippy warning
|
FYI @andygrove and @shehabgamin |
|
Thank you @tlm365 🙏 |
|
Will review by Tuesday! Thank you @tlm365 🚀 |
luhn_check functionluhn_check function
|
Thanks @tlm365 and @shehabgamin 🙏 Note I filed a ticket to track this PR here: (I spent a while on it as I am trying to get a template that we can copy/paste for other functions. I think we are close) |
|
|
||
| #[test] | ||
| fn test_array_utf8() -> Result<()> { | ||
| let input = Arc::new(StringArray::from(vec![ |
There was a problem hiding this comment.
I wonder if we can come up with a good pattern to test these functions with the different kinds of strings
For example the main datafusion code uses this pattern: https://github.com/apache/datafusion/blob/main/datafusion/sqllogictest/test_files/string/README.md
Maybe we could do something similar for string functions in Spark (so we dont have to maintain 3 sets of expected outputs)
| ---- | ||
| true | ||
|
|
||
| query B |
There was a problem hiding this comment.
can we also add test with extra large numbers, not valid numbers, fractional, exponential and negative?
Looks like this went back into draft mode, feel free to ping me whenever for review! |
|
@tlm365 do you think you will have a chance to work on this PR anytime soon? |
|
@alamb Unfortunately, I don't think I'll be able to work on this PR anytime soon as I'm quite busy at the moment 🥲 |
Which issue does this PR close?
datafusion-sparkSpark Compatible Functions #15914.stringfunctionluhn_check#16612Rationale for this change
Implement spark-compatible
luhn_checkfunction.What changes are included in this PR?
Are these changes tested?
Yes.
Are there any user-facing changes?
No.