-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-29854][SQL][TESTS] Add tests to check lpad/rpad throw an exception for invalid length input #28604
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
Conversation
|
@cloud-fan @gengliangwang I've checked the proposal in #27024 again and I think it's been already fixed in the master. So, we could close SPARK-29854 now. Just in case, I opened this PR to add trivial tests to check that. But, closing this PR looks okay if not needed. Could you check this? |
|
|
||
| setupTestData() | ||
|
|
||
| test("lpad/rpad should throw an exception for invalid length input if the ANSI mode enabled") { |
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.
Thank you, @maropu .
Can we add these new test coverage into string-functions.sql instead of SQLQuerySuite?
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.
For ansi, you can create a new string-functions.sql under ansi directory.
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.
ok
dongjoon-hyun
left a comment
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.
+1, LGTM. Thank you, @maropu and @HyukjinKwon .
sql/core/src/test/resources/sql-tests/inputs/string-functions.sql
Outdated
Show resolved
Hide resolved
|
Test build #122953 has finished for PR 28604 at commit
|
|
Test build #122960 has finished for PR 28604 at commit
|
|
Test build #122965 has finished for PR 28604 at commit
|
|
retest this please |
|
Test build #122978 has finished for PR 28604 at commit
|
|
retest this please |
|
Test build #123000 has finished for PR 28604 at commit
|
…tion for invalid length input ### What changes were proposed in this pull request? This PR intends to add trivial tests to check #27024 has already been fixed in the master. Closes #27024 ### Why are the changes needed? For test coverage. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Added tests. Closes #28604 from maropu/SPARK-29854. Authored-by: Takeshi Yamamuro <[email protected]> Signed-off-by: Takeshi Yamamuro <[email protected]> (cherry picked from commit 7ca73f0) Signed-off-by: Takeshi Yamamuro <[email protected]>
|
Thanks, all! Merged to master/3.0. |
|
Hi, @maropu . |
|
Since it's trivial, I regenerated it and made a hotfix followup commit to recover |
|
Ah, sorry for that... Thanks for the update, anyway. |
|
No problem~, @maropu . |
|
I should be more careful when merging PRs into branch-3.0. |
What changes were proposed in this pull request?
This PR intends to add trivial tests to check #27024 has already been fixed in the master.
Closes #27024
Why are the changes needed?
For test coverage.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Added tests.