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

Searching for 'marco' finds too much #51

Open
luxflux opened this issue Apr 10, 2013 · 10 comments
Open

Searching for 'marco' finds too much #51

luxflux opened this issue Apr 10, 2013 · 10 comments

Comments

@luxflux
Copy link

luxflux commented Apr 10, 2013

Following scoped_search definition:

scoped_search :on => [:id, :name, :content, :created_at, :updated_at]

If you search for marco it finds every record which has been created or updated on 1st of March 2013.

Caused by the following SQL:

... OR (
  `records`.`created_at` >= '2013-03-01 00:00:00'
  AND `records`.`created_at` < '2013-03-02 00:00:00'
) OR (
  `records`.`updated_at` >= '2013-03-01 00:00:00'
  AND `records`.`updated_at` < '2013-03-02 00:00:00'
) ...

It works, if I remove created_at and updated_at from the definition.

The bug is in defintion.rb#L217. DateTime#parse parses 'marco' as the first of March...:

irb(main):025:0> DateTime.parse('marco')
=> Fri, 01 Mar 2013 00:00:00 +0000

My purpose would be to use DateTime#parse only if there is an operator in front...

@abenari
Copy link
Collaborator

abenari commented Apr 10, 2013

You can define the following:
scoped_search :on => [:id, :name, :content]
scoped_search :on => [:created_at, :updated_at], :only_explicit => true

@brocktimus
Copy link
Collaborator

Is marco march in another language?

I just had a try and english and german returned the same results:

irb(main):007:0> Date.parse 'march'
=> #<Date: 2013-03-01 ((2456353j,0s,0n),+0s,2299161j)>
irb(main):008:0> Date.parse 'marsch'
=> #<Date: 2013-03-01 ((2456353j,0s,0n),+0s,2299161j)>

If that's the case it might just be possible to disable those other
languages if you don't want them parsed? Unsure if that is a good fix or
not...

On 10/04/13 22:14, abenari wrote:

You can define the following:
scoped_search :on => [:id, :name, :content]
scoped_search :on => [:created_at, :updated_at], :only_explicit => true


Reply to this email directly or view it on GitHub
#51 (comment).

@luxflux
Copy link
Author

luxflux commented Apr 11, 2013

It happens as soon as you use mar, apr or even with a number...:

1.9.3p392 :006 > DateTime.parse 'aprilia'
 => #<DateTime: 2013-04-01T00:00:00+00:00 ((2456384j,0s,0n),+0s,2299161j)>
1.9.3p392 :007 > DateTime.parse '20001'
 => #<DateTime: 2020-01-01T00:00:00+00:00 ((2458850j,0s,0n),+0s,2299161j)>

@luxflux
Copy link
Author

luxflux commented Apr 11, 2013

Using the definition mentioned by @abenari works. But I think this needs a change anyway as DateTime.parse will always return strange values.

@abenari
Copy link
Collaborator

abenari commented Apr 11, 2013

If you keep the date columns only explicit, it also give you great flexibility in searching for dates

@wvanbergen
Copy link
Owner

Maybe we should only attempt DateTime.parse if the query word starts with a digit?

@brocktimus
Copy link
Collaborator

I seem to recall some of the tests explicitly testing that it worked
with things like "Today" and "Yesterday". So your idea is possible,
would just require removing that functionality.

On 15/04/13 15:05, Willem van Bergen wrote:

Maybe we should only attempt |DateTime.parse| if the query word starts
with a digit?


Reply to this email directly or view it on GitHub
#51 (comment).

@wvanbergen
Copy link
Owner

The date functionality also has timezone issues: #43. Not sure what the best way forward is to make this more robust.

@wvanbergen
Copy link
Owner

@brocktimus Yes, or we handle Today and Yesterday explicitly.What other values do we want to support?

@brocktimus
Copy link
Collaborator

You're right. Today and Yesterday are handled explicitly. I'm not sure what values we'd like to support, my main use for this library is the really basic text searching stuff. I'm also normally super explicit, but that is more me than an average user.

I think the behaviour at the moment is a bit unintuitive. If anything I would expect 'March' to span all of March. Does it ever work that well as a search library for dates or times when it isn't explicitly mentioned? I would be tempted to do a simple regex like /\d[:- ]\d/ to check it has at least two digits and a separator before parsing it into DateTime.parse unless the field is explicitly mentioned.

Somewhat unrelated but should we be using Time.zone.parse instead of DateTime.parse ?

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

No branches or pull requests

4 participants