Support for "having" clause

[AeonFr]

I would like to propose support for the having clause.

I managed to pull out a working demo in this fork of the repo but found some issues:

1. The Mysql::translateWhere() function has two responsibilities, and it's confusing
2. The having() methods (and helper aliases) was created in the SelectBase class, to keep support for nested having clauses. These methods should be placed inside the Select clause instead (nested sub-queries should be created as Select objects, to have the having() method available).
3. There's no validation (you can use having without a group by). Is not clear if validation should occur in the Mysql class, or if the having() method should throw exception if groupBy() wasn't called previously

I would like to hear some opinions on 3. For the moment I'm not implementing any validation whatsoever, because:

• For once, it would be annoying to have an exception thrown if you attempt to use having() without calling groupBy() first. It probably should not depend on the order (user might want to add it later for whatever reason?).
• Other alternative would be to throw the exception in the ->get() function or in the translator, but I'm not clear those functions should have that responsibility.

In an unrelated topic, please let me know if you consider the PR I send to add support for the whereNotIn() method is good enough to be approved, I haven't done a PR to a big project like this one before, and I wonder if it meets the standard (I also wonder why it doesn't build, I don't understand very well "Travis"). I will consider feedback from this PR to make a future PR with this feature.

Thanks

[mario-deluna]

Hello

Your fork looks very good!

One of the main reasons I've build Hydrahon is to have a query builder library im able to use in legacy application where there is no way to upgrade them to modern PHP versions.

This is one reason why im trying my best to keep this backwards compatible with PHP versions long out of support. This unfortunately comes with many compromises when it comes to the current version of v1.

Now to your fork, you did not add any breaking changes and I would really like "having" this feature (pun intended.)

1. The Mysql::translateWhere() function has two responsibilities, and it's confusing

Agree, you added the method parseConditional I would relay this naming to the translator by simply renaming translateWhere to translateConditional.

2. The having() methods (and helper aliases) was created in the SelectBase class, to keep support for nested having clauses. These methods should be placed inside the Select clause instead (nested sub-queries should be created as Select objects, to have the having() method available).

Ideally the conditionals would be splittet into traits (WhereConditionTrait, HavingConditionTrait) and then have specific objects only exposing the available methods in the current context. Your implementation in the SelectBase class is the current way to go but should be revisited for v2.

3. There's no validation (you can use having without a group by). Is not clear if validation should occur in the Mysql class, or if the having() method should throw exception if groupBy() wasn't called previously

This is something that unfortunately is not very consistent in this library and also quite hard to pin down.

First of all I believe a query builder should not be responsible to ensure the generated query will work. But, it should prevent you from creating something structural broken while keeping the validation overhead minimal.

For once, it would be annoying to have an exception thrown if you attempt to use having() without calling groupBy() first. It probably should not depend on the order (user might want to add it later for whatever reason?).

Exactly, the order should not be enforced.

Other alternative would be to throw the exception in the ->get() function or in the translator, but I'm not clear those functions should have that responsibility.

As of my knowledge having without a group statement is valid SQL but invalid in MySQL so it would make sense to validate that in the MySQL translator. The check is cheap and can be only executed while translating the "havings"

[AeonFr]

I managed to move the having() method to Select by making the nested queries be initialized like this:

$subquery = new static;

instead of like this:

$subquery = new SelectBase;

I think this change also allows makes the library easier to extend, so once I saw it worked I didn't looked back. (In fact I think I changed it in a few more methods of what was strictly necessary for this to work).

I saw the changes needed to integrate my commit into master and tried to keep the same legacy syntax to support PHP 5.4+. In particular I have doubts about the static keyword and the array type hint in function declarations, but they seem to be supported. And I used array() instead of []. It now seems like the Travis check is passing 🙂

I really enjoy using this library because I'm slowly extending/migrating a legacy codebase, using a more modern approach. This library helps a lot in the "micro framework" we had built.

Thanks for the work, it was a huge help for us (me and my coworkers). I hope the contribution helps.