moebius icon indicating copy to clipboard operation
moebius copied to clipboard

limit not being honored alongside search

Open sircharleswatson opened this issue 7 years ago • 4 comments

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.

sircharleswatson avatar Jul 06 '17 09:07 sircharleswatson

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...

robconery avatar Jul 06 '17 18:07 robconery

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.

abarr avatar Apr 18 '18 06:04 abarr

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 :).

robconery avatar Apr 18 '18 14:04 robconery

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

abarr avatar Apr 19 '18 02:04 abarr