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

limit not being honored alongside search #99

Open
sircharleswatson opened this issue Jul 6, 2017 · 4 comments
Open

limit not being honored alongside search #99

sircharleswatson opened this issue Jul 6, 2017 · 4 comments

Comments

@sircharleswatson
Copy link

Hello!

I'm working on adding search to my database but for some reason, no matter what I do, the limit isn't honored when using Moebius.Query.search/2

I think this might be related to issue #98

Here's my code:

db(:my_table)
|> search(for: "term", in: [:field, :other_field])
|> limit(10)
|> MyApp.DB.run

For one particular query, I have a max of 33 results and it always returns all of them instead of limiting it to 10 results.

@robconery
Copy link
Owner

Yeah it looks like the query just goes with some SQL and doesn't follow conventions... I think. It's been a while since I dug in but here's the code for search:
https://github.com/robconery/moebius/blob/master/lib/moebius/query.ex#L257

The run command should be resolving to here:
https://github.com/robconery/moebius/blob/master/lib/moebius/database.ex#L39

The problem appears to be that nothing is "building" the query since the sql parameter is already set. That's matching this:
https://github.com/robconery/moebius/blob/master/lib/moebius/database.ex#L199

Which just fires the SQL without additional building. One way I can think of doing this is to set the type of the query to search or something, so it can be matched like this:
https://github.com/robconery/moebius/blob/master/lib/moebius/database.ex#L38

If search is matched then we could pipe the query to a transform that checks for limit (I don't think anything else is appropriate).

Would love some help on this...

@abarr
Copy link

abarr commented Apr 18, 2018

Hi Rob,

I have been learning Elixir and thought I might try Moebius. I saw this issue and thought I might try to help (This is the first time I have even thought about contributing so please be gentle).

I went through your links above and traced the steps through the pipes, it seems that you are building up the cmd with the limit function adding the value to the map/struct here:

https://github.com/robconery/moebius/blob/master/lib/moebius/query.ex#L88

It is then matched here:

https://github.com/robconery/moebius/blob/master/lib/moebius/database.ex#L232

But there is no function that accounts/matches when there is a value for limit. So the fix would be to match the limit in cmd when building the SQL. you think I am on the right track I might have a go at contributing. Let me know what you think.

@robconery
Copy link
Owner

Ha! Gentle... I've been meaning to fix this. I have a bunch of stuff scattered on the floor for v4 so I'll have a look then. Thanks :).

@abarr
Copy link

abarr commented Apr 19, 2018

OK. If you need a hand with anything let me know.

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