Skip to content
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

[ADD] - reverse function #327

Merged
merged 4 commits into from
Aug 26, 2021
Merged

[ADD] - reverse function #327

merged 4 commits into from
Aug 26, 2021

Conversation

zmrdltl
Copy link
Collaborator

@zmrdltl zmrdltl commented Aug 26, 2021

Uploaded function reverse
To operate : Only a table exists and only string type should be received as a parameter.

@panarch panarch self-requested a review August 26, 2021 08:17
@panarch panarch added the enhancement New feature or request label Aug 26, 2021
@codecov-commenter
Copy link

codecov-commenter commented Aug 26, 2021

Codecov Report

❗ No coverage uploaded for pull request base (main@3019bef). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #327   +/-   ##
=======================================
  Coverage        ?   90.95%           
=======================================
  Files           ?      126           
  Lines           ?     7714           
  Branches        ?        0           
=======================================
  Hits            ?     7016           
  Misses          ?      698           
  Partials        ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3019bef...08bfca3. Read the comment docs.

Copy link
Member

@panarch panarch left a comment

Choose a reason for hiding this comment

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

I was looking forward to use this code coverage test in the PR review, now it is available :)
Thanks to @MRGRAVITY817 🥇

Looks very good 👍 Only a single request here, adding a NULL value test.

ref https://codecov.io/gh/gluesql/gluesql/src/1325672384517576577cebd86ea3b6057c4ad914/src/executor/evaluate/mod.rs

image

You can also get help from this code coverage report.
There is lack of test of REVERSE function taking NULL value as a param.

@zmrdltl
Copy link
Collaborator Author

zmrdltl commented Aug 26, 2021

Added what you requested. plus, error about param which is int. Is that ok?

@panarch panarch self-requested a review August 26, 2021 13:10
Copy link
Member

@panarch panarch left a comment

Choose a reason for hiding this comment

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

Awesome 👍 👍 👍
Merging!

@panarch panarch merged commit 23e0e87 into gluesql:main Aug 26, 2021
@zmrdltl zmrdltl deleted the feature-function_reverse branch August 26, 2021 13:22
@panarch
Copy link
Member

panarch commented Aug 26, 2021

close #265

devgony pushed a commit to devgony/gluesql that referenced this pull request Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants