search_cop icon indicating copy to clipboard operation
search_cop copied to clipboard

README error?

Open MadBomber opened this issue 1 year ago • 6 comments

I was just looking through the README when I saw this example

Book.search("Harry Potter")
# ... WHERE (books.title LIKE '%Harry%' OR books.description LIKE '%Harry%' OR ...) AND (books.title LIKE '%Potter%' OR books.description LIKE '%Potter%' ...)

Book.search("available:yes OR created_at:2014")
# ... WHERE books.available = 1 OR (books.created_at >= '2014-01-01 00:00:00' and books.created_at <= '2014-12-31 00:00:00')

Shouldn't that last time string be "23:59:59" ??? if not, then your search misses all but one second of the last day of the year.

MadBomber avatar Jul 22 '22 17:07 MadBomber

Hi @MadBomber and thanks for reporting! You are right. That's an error in the README.

Book.search("created_at:2014").to_sql
# ... (books.created_at >= '2013-12-31 23:00:00' AND \"books\".\"created_at\" <= '2014-12-31 22:59:59.999999')

SearchCop is using ActiveSupport's beginning_of_year and end_of_year for this case. Would you mind opening a PR to fix it in the README?

mrkamel avatar Jul 22 '22 17:07 mrkamel

Please note that the concrete values are of course dependent on your time zone settings

mrkamel avatar Jul 22 '22 17:07 mrkamel

WIll be happy to create the PR.

MadBomber avatar Jul 22 '22 19:07 MadBomber

Oh No - not timezones ... there be dragons there!

How about this verbage ...

"NOTE: The actual values for the created_at query range conditionals will be dependant upon the default timezone in which your app is running and the timezone of the database server. The values shown depict a default app timezone which is the same as the database timezone."

Isn't this getting into the weeds a little. Maybe there should be a new section. Something like ...

Concerning Date/TIme Queries

If you query a database column of type timestamp you may provided only a year value. This will result in the use of ActiveSupport's beginning_of_year and end_year methods to constuct the actual values used in the SQL range conditionals.

You may also specify the query value as a year and a month (YYYY-MM) which likewise with use ActiveSupport's beginning_of_month and end_of_month methods to construct the SQL range conditional.

Don't forget, when dealing with database queries that the default timezone of your application may not be the timezone of the database server. In general, best practices provide that the database server timezone should be UTC. Thankfully ActiveRecord is smart enough (most of the time ) to do date/time value conversions between the two different timezones.

MadBomber avatar Jul 22 '22 20:07 MadBomber

Thanks for the PR! It's already merged. Moreover, thanks for suggesting this new section. I'll use it if i may and keep this issue open until integrated.

mrkamel avatar Jul 22 '22 21:07 mrkamel

Glad I could help. The gem looks very interesting. I like the way it can be easoly adapted more performant tsvector tri-gram indexes.

MadBomber avatar Jul 22 '22 21:07 MadBomber