New method to add parenthesis to queries

[jwoertink]

Fixes #488

When it comes to more complex queries, you may need to specify precedence for your conditions. This is done by wrapping your condition in parenthesis.

This PR adds a new where overload for your query objects that takes a block, and will wrap whatever query is defined inside the block.

# This Query
UserQuery.new
  .name("Bill")
  .or { |q|
    q.where(&.age(19).nickname("Duke"))
  }

-- Gives us this SQL
SELECT *
FROM users
WHERE name = 'Bill' OR ( age = 19 AND nickname = 'Duke' )

This opens Avram up to a lot more complex (but type-safe) queries.

[jwoertink]

This was needed because the last where might "(" and it was generating WHERE ( AND users.age...

[jwoertink]

By adding in the Avram::Where::Conjunction::None, the statements might look like ["(", "", "users.age = $1", "AND", ")", "AND"]

[jwoertink]

@stephendolan you mentioned needing to use this, can you see if this would work for your usecase?

[stephendolan]

Heck yeah! I'll pop in and try it this weekend, @jwoertink ! Thanks for putting this together.

[matthewmcgarvey]

I think I'm happy with this 👍 Just the one issue

[matthewmcgarvey]

Before I approve, @jwoertink what happens in this scenario:

UserQuery.new.name("Susan").where do |q|
  if false
    q.name("john")
   else
     q
  end
end.age(25).to_sql

I'm wondering how the clear last conjunction works if the where doesn't add any conditions.

[jwoertink]

Before I approve, @jwoertink what happens in this scenario:

UserQuery.new.name("Susan").where do |q|
  if false
    q.name("john")
   else
     q
  end
end.age(25).to_sql

I'm wondering how the clear last conjunction works if the where doesn't add any conditions.

lol 😂 another good catch... Maybe I need to rethink how this is done...

You'll end up with this:

"SELECT * FROM users WHERE users.name = $1 AND ( ) AND users.age = $2

[jwoertink]

Should it raise an exception if you use this where method, and there's no query being altered? Like, doing .where(&.itself) should throw an exception? Or do I account for that, and just remove the parens from the stack completely?

[matthewmcgarvey]

If that is valid SQL, I'm ok with it for now. I have a rough idea of how we might refactor the where storage in the future.

[jwoertink]

It is not valid SQL...

avram_dev=# SELECT * FROM users WHERE users.name = 'Susan' AND ( ) AND users.age = 25;
ERROR:  syntax error at or near ")"
LINE 1: ...ECT * FROM users WHERE users.name = 'Susan' AND ( ) AND user...
                                                             ^

[jwoertink]

Though, it probably is an edge case... I guess we could open up a separate issue and come back to refactor later?

[matthewmcgarvey]

I'd prefer not to push a feature with a know bug. Could you check if the last item in the array is an open parens and remove it if so?

[jwoertink]

Because ameba throws an error if you use a bool literal as a condition

[jwoertink]

Yeah, I was able to just check the last added Where, and see it was the PrecedenceStart. Then I can just remove it, and skip the rest.

[jwoertink]

Should this return self? 🤔

[jwoertink]

@matthewmcgarvey what are your thoughts on this? I assume no one will call this but us... but should this return self? or do you think it even matters?

[matthewmcgarvey]

I was looking at query builder yesterday and couldn't see any pattern to when we clone queryable vs query builder. I wouldn't worry about it

[jwoertink]

Cool. Thanks for the reviews!

[matthewmcgarvey]

Lots of tiny issues, but you got it there, nice job! 🎉

[stephendolan]

Finally got around to playing with this in my production app, and it worked beautifully.

This is really handy to throw into a scope so that no matter where you use it, you can guarantee that your results are grouped. Here's an example where I want to search videos by a search term across lots of attributes:

class VideoQuery < Video::BaseQuery
  def matching_search_term(search_term : String)
    where do |query|
      query.title.ilike(search_term)
        .or(&.description.ilike(search_term))
        .or(&.left_join_topics.where_topics(TopicQuery.new.label.ilike(search_term)))
    end
  end
end

[paulcsmith]

This is pretty dope!! Can you also pass a query object to where, or does it need to be a block? e.g. can you do .where(PostQuery.published) instead of where(&.published(true))?

[jwoertink]

You can do where_posts(PostQuery.published), but not for the parenthesis wrapping part. That still needs to be done with a block.