Skip to content

Conversation

Standing-Man
Copy link
Contributor

@Standing-Man Standing-Man commented Jul 22, 2025

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Implement Spark luhn_check function

Are these changes tested?

Yes

Are there any user-facing changes?

Yes

@github-actions github-actions bot added sqllogictest SQL Logic Tests (.slt) spark labels Jul 22, 2025
@Standing-Man Standing-Man changed the title feat(spark): Implement Spark luhn_check function feat(spark): Implement Spark string function luhn_check Jul 22, 2025
Signed-off-by: Alan Tang <jmtangcs@gmail.com>
Signed-off-by: Alan Tang <jmtangcs@gmail.com>
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @Standing-Man -- this looks good to me

@shehabgamin does this look good to you (at a high level)?

Signed-off-by: Alan Tang <jmtangcs@gmail.com>
@Standing-Man Standing-Man requested a review from comphead July 25, 2025 00:32
@shehabgamin
Copy link
Contributor

Thank you @Standing-Man -- this looks good to me

@shehabgamin does this look good to you (at a high level)?

Will review when I'm home in the next few hours!

Copy link
Contributor

@comphead comphead left a comment

Choose a reason for hiding this comment

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

Thanks @Standing-Man it is LGTM

Copy link
Contributor

@shehabgamin shehabgamin left a comment

Choose a reason for hiding this comment

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

Well done! Just some minor things and then this looks good to me!

Signed-off-by: Alan Tang <jmtangcs@gmail.com>
@Standing-Man Standing-Man requested a review from shehabgamin July 25, 2025 06:27
Copy link
Contributor

@shehabgamin shehabgamin left a comment

Choose a reason for hiding this comment

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

My apologies I didn't notice this during my first review. We need 1 test for array input.

Approving the PR as this is simple to add.

Well done!

Signed-off-by: Alan Tang <jmtangcs@gmail.com>
@Standing-Man
Copy link
Contributor Author

Appreciate the reviews, everyone!

@comphead
Copy link
Contributor

Nice teamwork! Thanks everyone

@comphead comphead merged commit b7da86e into apache:main Jul 25, 2025
27 checks passed
@alamb
Copy link
Contributor

alamb commented Jul 25, 2025

✋ ✋

adriangb pushed a commit to pydantic/datafusion that referenced this pull request Jul 28, 2025
…6848)

* feat(spark): Implement Spark luhn_check function

Signed-off-by: Alan Tang <jmtangcs@gmail.com>

* test(spark): add more tests

Signed-off-by: Alan Tang <jmtangcs@gmail.com>

* feat(spark): set the signature to be Utf8 type

Signed-off-by: Alan Tang <jmtangcs@gmail.com>

* chore: add more types for luhn_check function

Signed-off-by: Alan Tang <jmtangcs@gmail.com>

* test: add test for array input

Signed-off-by: Alan Tang <jmtangcs@gmail.com>

---------

Signed-off-by: Alan Tang <jmtangcs@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sqllogictest SQL Logic Tests (.slt)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[datafusion-spark] Implement Spark string function luhn_check
4 participants