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

where(uuid/datetime: "2") silently searches for (uuid/datetime: null) (changed 5.1 -> 5.2.1) #33624

Closed
murb opened this issue Aug 15, 2018 · 3 comments

Comments

@murb
Copy link
Contributor

murb commented Aug 15, 2018

Steps to reproduce

  1. Create a new project and configure it with a PostgreSQL-database
  2. $rails g model posts title body:text uuid:uuid date_column:datetime
  3. Do the following in rails console:
      a = Post.create()
      b_uuid=SecureRandom.uuid
      b = Post.create(uuid: b_uuid)
      Post.find_by(uuid: b_uuid)
      # returns b OK
      another_random_uuid=SecureRandom.uuid
      Post.find_by(uuid: another_random_uuid)
      # returns nil OK
      Post.find_by(uuid: "not a uuid")

Expected behavior

I'd think that the last call should return nil or raise an exception.

Actual behavior

The last call returns Post a, a Post with a uuid that is nil.

Compareable behavior

Update: I wanted to compare it with Post.where(id:"a"), which returns nil, instead of c; but actually it searches for 0, which might lead to an unexpected find of a Post with id 0, but this case has been purely hypothetical for me; as ids typically start at 1 (and is kind of expected, since "a".to_i also returns 0 in plain ruby). Searching for an invalid date in e.g. where(date_column: "a"), however, used to (v5.1.5) return PG::InvalidDatetimeFormat but also silently continues with a search for nil. Below a table with some scenario's compared.

command < 5.2 SQL fragment (pg) < 5.2 SQL fragment (sqlite) 5.2 SQL fragment (pg) 5.2 SQL fragment (sqlite)
Post.where(id: "a") posts.id=0 posts.id=0 posts.id=0 posts.id=0
where(date_column: "a") PG::InvalidDatetimeFormat 👍 date_column IS 'a' (returns [])👍 date_column IS NULL👎 date_column IS NULL (returns records where date_colum == nil) 👎
Post.find_by!(date_column: "a") not tested ActiveRecord::RecordNotFound 👍 returns first record with empty date_column 👎 ActiveRecord::RecordNotFound 👍
where(uuid: "a") uuid IS NULL; but returns [] 👍 not supported uuid IS NULL; but returns values with an empty uuid 👎 not supported
find_by!(uuid: "a") ActiveRecord::RecordNotFound 👍 not supported returns first record with empty uuid 👎 not supported

Related

Old issue, from long time ago, that actually settled on the 5.1.x behaviour: #11902, raising an exception.

System configuration

Rails version: 5.2.1

Ruby version: 2.4.1

(Was still working in 5.1.5/6)

update: Initially I thought that this was a Postgres only issue; but see the table above; unexpected things are happening with SQLite as well (not tested other database backends)

@murb murb changed the title find_by(uuid: "2") silently searches for (uuid: null) where(uuid/datetime: "2") silently search for (uuid/datetime: null) Aug 17, 2018
@murb murb changed the title where(uuid/datetime: "2") silently search for (uuid/datetime: null) where(uuid/datetime: "2") silently searches for (uuid/datetime: null) (changed 5.1 -> 5.2.1) Aug 17, 2018
@rnitta
Copy link

rnitta commented Sep 12, 2018

def cast(value)
value.to_s[ACCEPTABLE_UUID, 0]
end

not UUID formatted string is casted to nil
so it occurs

I suggest AR should raise exception like:

def cast(value)
  raise SomeException unless value
  value.to_s[ACCEPTABLE_UUID, 0]
end

@rafaelfranca
Copy link
Member

We can't raise when the value can't be casted. We always return nil, but we should probably keep the same behavior. First thing to do is to try to bisect to see which commit change this behavior.

@rails-bot
Copy link

rails-bot bot commented Dec 11, 2018

This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails team are limited, and so we are asking for your help.
If you can still reproduce this error on the 5-2-stable branch or on master, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.

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

No branches or pull requests

3 participants