laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.42k stars 10.99k forks source link

Pagination does not works with binding named param in whereRaw #10830

Closed AlexRadch closed 8 years ago

AlexRadch commented 8 years ago

When I make usual where with param, the pagination works fine

$employees = $employees->where('firstName', 'LIKE',
    '%'.DbHelper::escapeLike($filters['filter-name']).'%');

but when I replace where on whereRaw with named binding, the pagination does not works

$employees = $employees->whereRaw('concat_ws(" ", lastName, firstName, middleName) LIKE :filterName',
    ['filterName' => '%'.DbHelper::escapeLike($filters['filter-name']).'%']);

Error in log:

Next exception 'Illuminate\Database\QueryException' with message 'SQLSTATE[HY093]: Invalid parameter number: mixed named and positional parameters (SQL: select count(*) as aggregate from `employees` where `employees`.`deleted_at` is null and `lastName` LIKE Ю% and concat_ws(" ", lastName, firstName, middleName) LIKE :filterName)' in C:\htdocs\laravel\webserver\www\vendor\laravel\framework\src\Illuminate\Database\Connection.php:651
AlexRadch commented 8 years ago

Pagination works with whereRaw position binding

$employees = $employees->whereRaw('concat_ws(" ", lastName, firstName, middleName) LIKE ?',
    ['%'.DbHelper::escapeLike($filters['filter-name']).'%']);
rkgrep commented 8 years ago

Did you try to use parameters without names?

$employees = $employees->whereRaw('concat_ws(" ", lastName, firstName, middleName) LIKE ?',
    ['%'.DbHelper::escapeLike($filters['filter-name']).'%']);
AlexRadch commented 8 years ago

Yes, Ii works with unnamed parameters. But with named doesn't!

rkgrep commented 8 years ago

Invalid parameter number: mixed named and positional parameters

That's because PDO does not support mixing of named and unnamed parameters. While Laravel always appends only unnamed ones.

AlexRadch commented 8 years ago

@rkgrep @GrahamCampbell Why you close this issue? In Laravel documentation wrote example with named parameters! I am new in laravel, about one week. And I think examples from documentation should works. With pagination also. May be change documentation examples, and close issue after that?

AlexRadch commented 8 years ago

@rkgrep @GrahamCampbell find whereRaw here http://laravel.com/docs/5.1/queries#advanced-where-clauses

GrahamCampbell commented 8 years ago

Hmmm, well if our docs are wrong, then I'll reopen this.

rkgrep commented 8 years ago

And I think examples from documentation should works.

Can't find what's wrong with the docs? There is only one example with whereRaw in whereExists and there are no named parameters.

Actually, there are no examples with named parameters at all in docs. What we can do - is to mention that named parameters should not be used at all.

Hmmm, well if our docs are wrong, then I'll reopen this.

Maybe, it is better to open an issue in docs

GrahamCampbell commented 8 years ago

Maybe, it is better to open an issue in docs

We've disabled issues on that repo.

salarmehr commented 8 years ago

Also see: http://stackoverflow.com/questions/33721048/laravel-pagination-fails-in-building-the-correct-query

seydu commented 8 years ago

I have the same problem with named parameters. I get an error even when I don't mix named and position parameters.

I had a look at the query builder. It flattens the bindings before using them. The helper that is used to flatten the array ignores the keys and this loses the named parameters. As it stands I don't see any decent way of using named parameters in the query builder.

rkgrep commented 8 years ago

I don't mix named and position parameters

This cannot be properly detected during runtime. You should understand, that query builder is not supposed to work with named parameters as it always expects equal count of placeholders and bindings within its default behavior. If you use raw queries and want to use named parameters, execute statements directly or try to use $query->toSql() method and append named bindings manually.

seydu commented 8 years ago

I agree that one cannot use named and positional parameters in the same query. It does not make sense. But I expected to be able to use named parameters only. I understand that as it stands now I cannot use named parameters. The point I was making is that we should be able to do so. I have a filter query that is processed by many parts of my application (to add joins, conditions, ....). It is safer to have named parameters to avoid position mismatch. Anyway, the query builder should allow to use named parameters, unless it is extremely complex to implement. What prevent the query builder from being able to process named parameters is two things it does that remove the keys (in getBindings and addBindings). I extended the query builder and redefined these two methods. Now I can use named parameters.

rkgrep commented 8 years ago

@seydu I'm just giving my own opinion. As I tried to use named parameters, it was not hard to implement using them, however I came into unexpected consequences when query was being modified in different parts of application (controllers, repoitories, etc).

You are free to make a PR anyway - it might be accepted.

seydu commented 8 years ago

Using named parameters should make it easier to have many components of your application collaborate to build a query. If you want to avoid conflicts add a unique identifier to your names. Your components will not be passed the Eloquent query itself, but a wrapper that encapsulates the logic that let them manipulate the underlying Eloquent query.

I want to use the changes I made before I make a PR. I have already found two places where an instance of the query builder is created. There could be more, I have to check it first.