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

Fix empty string saving for ranges #48507

Merged
merged 1 commit into from
Jun 26, 2023

Conversation

szymonlipka
Copy link
Contributor

Motivation / Background

Fixes #48486

Saving ranges with an empty range

Detail

Simply return if the string is empty, the same as it's being done for an "empty" string.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@clairity
Copy link

ha, i'd tried something similar, and while it worked on the rails console, it didn't work when put through a form submit request cycle in my rails app, so i thought it wasn't working. turns out i forgot to restart the rails server to get the server to pick up the change.

@ghiculescu
Copy link
Member

@szymonlipka
Copy link
Contributor Author

Could you add a test? It would probably go in https://github.com/rails/rails/blob/main/activerecord/test/cases/adapters/postgresql/range_test.rb

Right, done

@szymonlipka
Copy link
Contributor Author

@ghiculescu buildkite has failed, but I believe it might not be be related. Can I somehow retry the job?

@ghiculescu
Copy link
Member

Easiest way to retry would be to rebase against main.

@szymonlipka szymonlipka force-pushed the fix-empty-string-range branch 2 times, most recently from 57072d0 to 0014c58 Compare June 20, 2023 14:35
@szymonlipka
Copy link
Contributor Author

@ghiculescu the CI has passed.

@ghiculescu ghiculescu added the ready PRs ready to merge label Jun 21, 2023
@rafaelfranca rafaelfranca merged commit 5d81a7e into rails:main Jun 26, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
activerecord ready PRs ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PostgreSQL Range adapter throws NoMethodError for an empty string value
4 participants