Represent wheres and raw wheres in the same way

[matthewmcgarvey]

Fixes #459

Avram::Where::Raw and Avram::Where::SqlClause did not have overlapping ancestors so they were stored in separate lists in Avram::QueryBuilder and had different APIs. When the sql string was put together, raw wheres were appended to the where clause. When "OR" support is added, this will no longer be acceptable.

It simplifies the API to have them be used in the same way. This adds a common ancestor of Avram::Where::Condition that has the one method of #prepare. The only problem of doing this is that SqlClauses where being passed a placeholder string that had an incrementing number. Rather than keeping with the push style and checking which types were save to give the placeholder to, I switched the where conditions to take in a supplier of those placeholders so that it could get one if it wanted or just ignore it and not cause the number to increment.

PS: Naming is hard, any ideas on how to name the different abstract classes better?

[jwoertink]

Ah, I see what you're doing here. That's nice. I'll come back for a review again later, but initial glance over I like the direction. I'll also try and think of a good name for that abstract class. Thanks for doing this!

[jwoertink]

Ok, so for the most part I like what this is doing, and I think I'll need it before being able to merge in #442. I think my only hesitation here is if will cause any type-safe issues by these being merged in.... For example, when you query, you never actually call Avram::Where directly... you'll do UserQuery.new.username() or you'll do UserQuery.new.where("").... I may just be overthinking it now that I write it out 😝 But what are your thoughts?

[jwoertink]

I know this type comes from what was defined in Condition before, but any idea why these types specifically? I ask because I'm wondering if with the latest pg and db, will this blow up when someone uses UUID? 🤔 Also, Int64 I guess... I'm assuming this is the value of a DB column which could be many different types... Or maybe I'm wrong and this value is something else?

[matthewmcgarvey]

I'm not able to hunt down why we convert everything to these types

ultimately, we call out to here https://github.com/crystal-lang/crystal-db/blob/bd456028644853f852f5adfe053142e6ad885e52/src/db/statement.cr#L69-L71
which does not have type signatures so it's not very helpful. Our process ultimately delegates to the #to_db method for each type which in most things converts to a string (Ints, UUIDs, etc).

[jwoertink]

It'll probably be ok, but lets keep this in mind if we notice something weird popup. I feel like this should have more types on it, but if it works as is then we can just roll with it.

[matthewmcgarvey]

@jwoertink I don't think any type safety was lost/gained. Most queries will never interact with the #where that takes in these base classes, but when they do they won't have to remember whether they should use #where or #raw_where.

[jwoertink]

👍