New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NFR] Add bind as a separate function to QueryBuilder/PaginatorQueryBuilder #13368

Closed
trymkb opened this Issue May 7, 2018 · 3 comments

Comments

Projects
None yet
4 participants
@trymkb
Copy link

trymkb commented May 7, 2018

Currently the only way to set bind parameters via PaginatorQueryBuilder is to set it in "one or any/all" of the functions allowing it.

This might be true for other cases also, but it seems to be solved if you run execute(['bind'=>'param']) yourself in other scenarios.

Example of problem:

<?php
use Phalcon\Paginator\Adapter\QueryBuilder;

$name='R2D2';

$builder = $this->modelsManager->createBuilder()
                ->columns("id, firstname, lastname")
                ->from("Robots")
                ->orderBy("name");

if ($name) {
        $builder->where('firstname like :name:', ['name'=>$name]);
        $builder->orWhere('lastname like :name:', ['name'=>$name]);
}

$paginator = new QueryBuilder(
    [
        "builder" => $builder,
        "limit"   => 20,
        "page"    => 1,
    ]
);

Example of usage in possible solution:

<?php
use Phalcon\Paginator\Adapter\QueryBuilder;

$name='R2D2';
$params=[];
$builder = $this->modelsManager->createBuilder()
                ->columns("id, firstname, lastname")
                ->from("Robots")
                ->orderBy("name");

// ** Possible solution **
if ($name) {
        $params['name']=$name;
        $builder->where('firstname like :name:');
        $builder->orWhere('lastname like :name:');
}

$builder->bind($params);
// ** End possible solution **

$paginator = new QueryBuilder(
    [
        "builder" => $builder,
        "limit"   => 20,
        "page"    => 1,
    ]
);

Nice to have: option to check if syntax in current query matches, especially if query fails.
But mostly this is to avoid setting the same parameter numerous times, and gives a little more control over how and when things are bound.

@stale

This comment has been minimized.

Copy link

stale bot commented Aug 5, 2018

Thank you for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please feel free to either reopen this issue or open a new one. We will be more than happy to look at it again! You can read more here: https://blog.phalconphp.com/post/github-closing-old-issues

@stale stale bot added the stale label Aug 5, 2018

@stale stale bot closed this Aug 6, 2018

@Jurigag Jurigag reopened this Aug 6, 2018

@stale stale bot removed the stale label Aug 6, 2018

@stale

This comment has been minimized.

Copy link

stale bot commented Nov 4, 2018

Thank you for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please feel free to either reopen this issue or open a new one. We will be more than happy to look at it again! You can read more here: https://blog.phalconphp.com/post/github-closing-old-issues

@niden

This comment has been minimized.

Copy link
Member

niden commented Dec 12, 2018

Addressed with work from @CameronHall. Thank you @trymkb

#13653

@niden niden closed this Dec 12, 2018

niden added a commit to niden/cphalcon that referenced this issue Dec 12, 2018

[phalcon#13439] - Merge branch '4.0.x' into T13439-PSR-16
* 4.0.x:
  Updated changelog
  Fixed Paginator Query Builder tests
  Added ".zephir/" and ".temp/" to gitignore
  Cleaned up Pagination Adapters
  Fixes phalcon#13368: Query builder now supports binded params
  Updated tests to support Paginator Repository
  Updated Pagination Repository to maintain backwards compatibility
  added new feature phalcon#10957
  Implemented test for Security/Random/Base58Cest
  Fixed styling using phpcbf
  Fixed intergrations tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment