[5.0] Defer Query Builder Bindings Until The Query Is Being Compiled

## The problem

The old design of adding bindings as clauses are added to the query worked most of the time had two major issues:
 1. For some complex queries, like unions or sub queries involving joins, the bindings fail to apply or are applied in the wrong order. See #5833 for examples.
 1. Cleanly removing global scopes that introduced bindings is impossible to guarantee, especially if other global scopes are in use.

## The fix
To fix this we take a very simple approach to defer assigning the query bindings until the query is being compiled. We introduce a CompiledQuery class that holds the sql string and the bindings array. It also takes care of a lot of the complexity of the concatenating and trimming sql string so the Grammar doesn't have to care about those details or imploding arrays of strings. This has two effects:
 1. Query bindings are guaranteed to be added in the same order that the sql string is concatenated.
 1. When removing global scopes you don't need to worry about removing bindings. Just remove the appropriate query clauses and the bindings will be taken care of at compile time.

## Unit tests
Note that all unit tests pass as is with only a minor refactoring to allow us to assert the sql string and bindings in one helper method. I've added a significant number of new tests some of which failed without the fixes.

## API changes
There are a fewAPI change:

 1. Remove all the `Query\Builder` binging methods `getBindings`, `getRawBindings`, `addBinding`, `setBindings` and `mergeBindings` which have no meaning in the world of compile deferred bindings. This API was most being used by developers of Global Scopes so they could attempt to remove the bindings. A problem they don't need to solve anymore.
 1. Rename `Query\Builder` `toSql` to `compile`

We could add some backwards compatible `toSql` and `getBindings` methods and mark them as deprecated but I'm not sure its really safe to do so. Anyone using those old APIs need to think hard about how to use the new `compile` method.

## Removing Bindings and Global Scopes
The early assignment of bindings makes the removal of global scope particularly challenging. See for example this SO question: [laravel-how-to-disable-a-global-scope-in-order-to-include-inactive-objects](http://stackoverflow.com/questions/25312386/laravel-how-to-disable-a-global-scope-in-order-to-include-inactive-objects). The problem is that there is no guaranteed way to identify the bindings a global scope added and its therefore hard to ensure that you remove them cleanly. Most techniques are relying on storing the index into the where and binding arrays when the scope was added and then using that when removing. However, that technique, and all others, are extremely brittle if more than one global scope are involved.
Why are these tests named things like “testRealHas”? What does that mean? Is there a “fake” has?



On October 5, 2014 at 9:16:53 PM, Jon Palmer (notifications@github.com) wrote:

In src/Illuminate/Database/Query/Builder.php:

> @@ -269,11 +254,26 @@ public function selectSub($query, $as)
>     * @param  mixed  $column
>     * @return $this
>     */
> -  public function addSelect($column)
> +  public function addSelect($column, $bindings = [])
> +  {
> +          $columns = is_array($column) ? $column : [$column];
> +
> +          $this->columns[] = compact('columns', 'bindings');
> +
> +          return $this;
> +  }
storing columns now looks a lot more like wheres as they have their own bindings.

—
Reply to this email directly or view it on GitHub.