Skip to content
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

Passing in Empty Conditions Breaks Query #2244

Closed
brian-t-allen opened this issue Mar 27, 2014 · 2 comments
Closed

Passing in Empty Conditions Breaks Query #2244

brian-t-allen opened this issue Mar 27, 2014 · 2 comments

Comments

@brian-t-allen
Copy link

Using the paginator breaks the query if you pass in empty query conditions.

$queryOptions = array();
// code to build $order, based on posted results from form
$queryOptions["order"] = $order;
// code to build $conditions, based on posted results from form
$queryOptions["conditions"] = $conditions;
$paginator = new \Phalcon\Paginator\Adapter\Model(
    array(
        "data"  => $robot->getParts($queryOptions),
        "limit" => 10,
        "page"  => $this->request->getQuery('p', 'int')
    )
);

$order is always empty or valid SQL ("id DESC" or "id DESC, name ASC").

$conditionsis always empty or valid SQL ("id < 3" or "id > 10 AND name ILIKE '%megatron%'").

But if $conditions is empty the query is SELECT [Parts].* FROM [Parts] WHERE AND [robots_id] = ?0 ORDER BY id DESC and fails.

Passing in an empty ["data"]["conditions"] key shouldn't break the query. If it's empty it shouldn't affect the WHERE clause.

@dreamsxin
Copy link
Contributor

Can you please var_dump $conditions? In Phalcon\Mvc\Model\Query\Builder has a judge :

    /** 
     * Nest the condition to current ones or set as unique
     */
    if (zend_is_true(current_conditions)) {
        PHALCON_INIT_VAR(new_conditions);
        PHALCON_CONCAT_SVSVS(new_conditions, "(", current_conditions, ") AND (", conditions, ")");
    } else {
        PHALCON_CPY_WRT(new_conditions, conditions);
    }

@brian-t-allen
Copy link
Author

var_dump($conditions) returns:

string(0) ""

Which makes the query:

WHERE AND

I understand I can remove that key, and have done so (created a function to iterate over the array passed in as data and remove any keys that are empty), but if we can add a few lines to Phalcon to do it I think we should. It shouldn't be so easy to break, and breaking it in that way is counter-intuitive.

In PHP it could be as easy as:

if (zend_is_true($current_conditions) && trim($current_conditions) != '') {

I'm guessing there is a simple equivalent in C or Zephir.

Thanks!

niden added a commit that referenced this issue Apr 18, 2014
Fix #2244 add judge conditions in `Phalcon\Mvc\Model\Manager::getRelationRecords`
@niden niden closed this as completed in dfcb39b Jun 4, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants