Skip to content

Fix the quoting of further place-holder conditions #1166

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

Merged
merged 2 commits into from
May 2, 2024

Conversation

aidanharan
Copy link
Contributor

@aidanharan aidanharan commented May 2, 2024

Fix the quoting of further place-holder conditions (eg: where("written_on < ?", time)).

Fixes the "undefined method 'value_for_database'" errors seen in https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/actions/runs/8910633467/job/24470268020

aidanharan added 2 commits May 2, 2024 18:44
@aidanharan aidanharan marked this pull request as ready for review May 2, 2024 18:02
@aidanharan aidanharan merged commit e7c4f6d into main May 2, 2024
2 of 8 checks passed
@aidanharan aidanharan deleted the basic-attribute-types branch May 2, 2024 18:02
@aidanharan aidanharan changed the title Allow basic attribute types to be query parameters Fix the quoting of further place-holder conditions May 2, 2024
@aaronsajo
Copy link

Hey @aidanharan ,
I am not sure if this is related, but we are still facing the same issue in Rails-7.2.1 and activerecord-sqlserver-adapter 7.2.1.
.where('date_column < ?', date) is throwing "undefined method `value_for_database'"

@andsip
Copy link
Contributor

andsip commented Oct 10, 2024

I also came across this today.

  • where("written_on < ?", Time.now)
  • where("written_on < ?", Date.today)
  • where("written_on < ?", DateTime.current)

So, it seems to work only for Time objects, but not Date or DateTime objects.

Adding checks for type.is_a?(Date) || type.is_a?(DateTime) in #basic_attribute_type? would fix these specific cases, but I don't know the codebase well enough to say, if this would break something else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants