Order by calculated field value from select statement will raise a notice #27

Closed
gallien opened this Issue May 26, 2011 · 8 comments

Comments

Projects
None yet
6 participants
Contributor

gallien commented May 26, 2011

Example Query with an order by calculated "score":

    $score = $qb->expr()->diff(10, $qb->expr()->sum($qb->expr()->sum($qb->expr()->sum($qb->expr()->abs($qb->expr()->diff(':cat1', 'tc1.priority')), $qb->expr()->abs($qb->expr()->diff(':cat2', 'tc2.priority'))),
                 $qb->expr()->sum($qb->expr()->abs($qb->expr()->diff(':cat3', 'tc3.priority')), $qb->expr()->abs($qb->expr()->diff(':cat4', 'tc4.priority')))),
                 $qb->expr()->abs($qb->expr()->diff(':cat5', 'tc5.priority'))));

    $qb->select(array('DISTINCT t', $score . ' AS score')) 
       ->from('Tour', 't')
       ->innerJoin('t.periods', 'tp')
       ->innerJoin('t.user', 'u', 'WITH', 'u.status = :ustatus')
       ->innerJoin('t.country', 'c')
       ->innerJoin('t.categories', 'tc1', 'WITH', 'tc1.category = 1')
       ->innerJoin('t.categories', 'tc2', 'WITH', 'tc2.category = 14')
       ->innerJoin('t.categories', 'tc3', 'WITH', 'tc3.category = 41')
       ->innerJoin('t.categories', 'tc4', 'WITH', 'tc4.category = 42')
       ->innerJoin('t.categories', 'tc5', 'WITH', 'tc5.category = 54')
       ->where($qb->expr()->eq('t.status', ':tstatus'))
       ->andWhere($qb->expr()->gte($score, '5'))
       ->orderBy('score', 'DESC')
       ->setParameter('ustatus', 'A')
       ->setParameter('tstatus', 'V');

Will raise following notice:

Notice: Trying to get property of non-object /var/www/library/DoctrineExtensions/Paginate/LimitSubqueryWalker.php on line 78

The LimitSubqueryWalker seems to be ignore the use case of sorting non-entity fields.

I'm experimenting the same issue, I tried to take a look at the code:

            foreach ($AST->orderByClause->orderByItems as $item) {
                $pathExpression = new PathExpression(
                                PathExpression::TYPE_STATE_FIELD | PathExpression::TYPE_SINGLE_VALUED_ASSOCIATION,
                                $item->expression->identificationVariable,
                                $item->expression->field
                );
                $pathExpression->type = PathExpression::TYPE_STATE_FIELD;
                $AST->selectClause->selectExpressions[] = new SimpleSelectExpression($pathExpression);
            }

It looks like $item->expression is a string where an instance of PathExpression is needed. I tried to dive in the deep of the Walkers but I'm not enough experimented with AST and such things to thing a workaround.

However if someone would point me to the right direction, I'll be glad to work on this issue I actually need to solve.

Contributor

gallien commented Sep 22, 2011

I don't understand the usage of "createLimitSubQuery" at all, so I commented out the "hint":

$subQuery//->setHint(Query::HINT_CUSTOM_TREE_WALKERS, array('DoctrineExtensions\Paginate\LimitSubqueryWalker'))
->setFirstResult($offset)
->setMaxResults($itemCountPerPage);

Then all queries with aggregation or sorting by non-entity fields will work fine.

mroowqa commented Sep 29, 2011

I have the same problem but commenting the "hint" don't work for me:(

whads commented Oct 10, 2011

The problem is that the subquery removes the original select statement so when you order by the calculated variable, it doesn't exists. I fixed that with this changes:

--- /home/jordi/Baixades/beberlei-DoctrineExtensions-3279a82/lib/DoctrineExtensions/Paginate/LimitSubqueryWalker.php    2011-06-21 10:00:07.000000000 +0200
+++ libs/DoctrineExtensions/Paginate/LimitSubqueryWalker.php    2011-10-10 09:20:05.817720940 +0200
@@ -67,22 +67,33 @@
         );
         $pathExpression->type = PathExpression::TYPE_STATE_FIELD;
 
-        $AST->selectClause->selectExpressions = array(
+        $selectExpressions = array(
             new SimpleSelectExpression($pathExpression)
         );
 
         if (isset($AST->orderByClause)) {
             foreach ($AST->orderByClause->orderByItems as $item) {
-                $pathExpression = new PathExpression(
+                if (is_string($item->expression)) {
+                    foreach ($AST->selectClause->selectExpressions as $selectExpression) {
+                        if ($item->expression == $selectExpression->fieldIdentificationVariable) {
+                            $selectExpressions[] = $selectExpression;
+                            break;
+                        }
+                    }
+                }
+                else {
+                    $pathExpression = new PathExpression(
                                 PathExpression::TYPE_STATE_FIELD | PathExpression::TYPE_SINGLE_VALUED_ASSOCIATION,
                                 $item->expression->identificationVariable,
                                 $item->expression->field
-                );
-                $pathExpression->type = PathExpression::TYPE_STATE_FIELD;
-                $AST->selectClause->selectExpressions[] = new SimpleSelectExpression($pathExpression);
+                    );
+                    $pathExpression->type = PathExpression::TYPE_STATE_FIELD;
+                    $selectExpressions[] = new SimpleSelectExpression($pathExpression);
+                }
             }
         }
 
+        $AST->selectClause->selectExpressions = $selectExpressions;
         $AST->selectClause->isDistinct = true;
     }
Contributor

sandermarechal commented Oct 11, 2011

Talk about coincidence. I implemented a nearly similar solution just now: #53

Seldaek commented Oct 31, 2011

This should be fixed now that #53 was merged. Please verify.

@gallien gallien closed this Oct 31, 2011

Contributor

gallien commented Oct 31, 2011

Yes, it seems to be fixed with the latest version. Thanks.

yup, thanks for the patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment