[datafusion-spark] Implement Spark luhn_check function#16580
[datafusion-spark] Implement Spark luhn_check function#16580tlm365 wants to merge 4 commits intoapache:mainfrom
luhn_check function#16580Conversation
tlm365
force-pushed
the
spark_luhn_check
branch
3 times, most recently
from
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 🚀 |
alamb
changed the title
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 🥲 |
4 participants
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.