[DC-701] Aggregates functions do not return proper values when using many relationships and limits Created: 24/May/10  Updated: 17/Apr/14

Status: Reopened
Project: Doctrine 1
Component/s: None
Affects Version/s: 1.2.2
Fix Version/s: None

Type: Bug Priority: Blocker
Reporter: will ferrer Assignee: Jonathan H. Wage
Resolution: Unresolved Votes: 0
Labels: None
Environment:

XP Xamp


Attachments: Text File DC_701_adds_disbaleLimitSubquery_testcase.patch     Text File DC_701_adds_hydrateArrayShallow_testcase.patch     Text File DC_701_fix_adds_arrayShallow.patch     Text File DC_701_fix_adds_disableLimitSubQuery.patch    

 Description   

Hi All

I have encountered a problem that seems very core to the way that doctrine works – if you apply an aggregate function to a column in a table and then join to another table via a many relationship while also using a limit, like so:

$q = Doctrine_Query::create();
$q->from('Customer Customer'); 
$q->addSelect('COUNT(Customer.id) as COUNT_customer_id');
$q->addSelect('Customer_Order.id as order_id'); 
$q->leftJoin('Customer.Order Customer_Order'); // Many relationship here
$q->limit(20);

It produces this correct DQL:

SELECT COUNT(Customer.id) as COUNT_customer_id, Customer_Order.id as order_id 
FROM Customer Customer 
  LEFT JOIN Customer.Order Customer_Order 
LIMIT 20

However the SQL it produces will not return an accurate count – the count is restricted by the limit:

SELECT COUNT(p.id) AS p__0, p2.id AS p2__1 
FROM product_customers p 
  LEFT JOIN product_orders p2 ON p.id = p2.customer_id 
WHERE p.id IN ('1', '2', '3', '4', '5', '6', '7', '8', '9', '10', '11', '12', '13', '14', '15', '16', '17', '18', '19', '20')

This produces a count of 21 instead of what it should be (1000).

The reason for this is because Doctrine's internal functionality does an intermediary query where it gets gets the ids needed from the customer table in order to use them as the IN portion of the constraints on the final query – like so:

SELECT DISTINCT p3.id FROM product_customers p3 LIMIT 20

It may seem strange that I am applying a limit to a query which will aggregate to 1 to row . In the actual queries that alerted me to this problem I am using a group by. The reason I am not reporting this bug with a group by in my example however is that you can not use a group by with a many relationship and a limit in doctrine 1.2.2 as it works currently (see bug: DC-594). However I am personally able to use a limit with a group by and many relationship since I am using a version of the code I patched my self to repair bug DC-594.

Here is my sample schema in case its helpful.

detect_relations: false
package: Example
options:
  type: INNODB
  charset: utf8
Order:
  tableName: orders
  columns:
    order_id:
      type: integer(4)
      primary: true
      notnull: true
    customer_id:
      type: integer(4)
    order_date: timestamp
  relations:
    Customer:
      type: one
      local: customer_id
      foreign: customer_id
  options:
    type: InnoDB
Customer:
  tableName: customers
  columns:
    customer_id:
      type: integer(4)
      primary: true
      notnull: true
      autoincrement: true
    firstname:
      type: string(45)
    lastname:
      type: string(45)
    streetaddress:
      type: string(45)
    city:
      type: string(45)
    state:
      type: string(45)
    postalcode:
      type: string(45)
  relations:
    Order:
      type: many
      local: customer_id
      foreign: customer_id
  options:
    type: InnoDB

Thanks much.

Will Ferrer

edit: split SQL code to make the discussion readable without huge horizontal scrolling...



 Comments   
Comment by Jonathan H. Wage [ 08/Jun/10 ]

Is this still a problem? I am not sure if this can be patched easily. The limit subquery algorithm is flawed deeply but also a core part of the current way Doctrine 1 works.

Comment by will ferrer [ 08/Jun/10 ]

Hi Jon

This bug is still an issue for me but I think it may be VERY hard to patch since it seems very core to the way Doctrine 1 works.

The project I am working on at the moment is a visual query builder that is highly reliant on Doctrine. In order to prevent users from making queries that would trigger this bug I am currently giving an alert when ever a query that would trigger this bug is created.

It would be great if this were patchable but if not I should be able to get by.

Does Doctrine 2 fix this problem?

Thanks for the help.

Will Ferrer

Comment by Jonathan H. Wage [ 08/Jun/10 ]

In Doctrine 2 the limit subquery algorithm does not exist. It is now up to the developer to write the query to handle the scenario instead of Doctrine "trying" to automate it for you. Since the situation where it is needed it is so rare, and each case can be slightly different it is better to let the developer handle it in those cases.

Comment by will ferrer [ 08/Jun/10 ]

Hi Jon

Does that mean that Doctrine 2 can no longer intelligently handle limits with many relationships, and doesn't have the ability to hydrate a return with sub arrays in it for many relationships?

Thanks again for your help.

Will Ferrer

Comment by Jonathan H. Wage [ 09/Jun/10 ]

That is correct. We do not automatically try and limit the relationships with a "limit subquery" as it causes more problems then it helps.

Comment by will ferrer [ 10/Jun/10 ]

Hi Jon

I was thinking about what you said here – that the "limit subquery" causes more harm than good. It occur to me that I really do like being able to use this method (it comes in very handy very often) but some times it would really help to be able to turn it off (the bug in this thread is a good example such a time). So I figured why not just make an option to turn it off and have the best of both worlds? I looked at the code and found it was really very easy to put in a property which can be set on the query object to enable or disable the use of the limit subquery.

I made the change in my code base and found it very useful for several situations I was dealing with in my own project.

The one thing I couldn't do is make a test case for this new feature – I couldn't find an existing test case that was actually triggering the limit subquery as I understood it to work (I couldn't find a test case that would put an WHERE IN constraint with all the ids gathered in the limit subquery. I tried some test cases that I THOUGHT would activate this feature but it didn't seem that they did).

After adding this feature to my code base I also wanted a new hydrator – one that worked like scalar but would put in array key names that were the same as the ones you would see in HYDRATE_ARRAY. I noticed that HYDRATE_SCALAR already had the ability to do this but it required that a value of false be passed into the $aliasPrefix argument of the _gatherRowData function. I made a custom hydrator I call HYDRATE_ARRAY_SHALLOW that passes in this false value. I then realized this might be handy to add to doctrine so I integrated it into my code base and made a few test cases for it.

There are 2 patches I am now attaching this to thread:

disableLimitSubquery_2010-06-10_Doctrine_1.2_SVN.patch – this patch adds the disableLimitSubquery property to query objects so that users can turn off the use of the subquery for those times when they don't want to use that behavior (fixing the bug in this thread and probably others)

and

disableLimitSubquery_and_HYDRATE_ARRAY_SHALLOW_2010-06-10_Doctrine_1.2_SVN.patch – this is the same as the first patch but also contains the HYDRATE_ARRAY_SHALLOW hydration type I mentioned above (which tends to go well with disableLimitSubquery turned on).

I put these in 2 patches incase you liked 1 feature but disliked the other.

If you like either of this features I would be very happy to see them added to doctrine.

Also if you have any advice on how to improve these features (or info on how to make a good test case for disableLimitSubquery) please let me know.

Hope you are well.

Will Ferrer

Comment by will ferrer [ 10/Jun/10 ]

Reopened because I added a patch that I think in essence fixes the issues.

Comment by Jonathan H. Wage [ 01/Sep/10 ]

Thanks for the issue and patches.

Fixed here http://github.com/doctrine/doctrine1/commit/2ad78e62e360133efc04bf6897bf679c7f3d833b

Comment by will ferrer [ 01/Sep/10 ]

individual patch for this issue with out other features included

Comment by will ferrer [ 01/Nov/10 ]

adding test cases for these features

Comment by will ferrer [ 01/Nov/10 ]

reopened because I posted some test cases to add to doctrine along with the patchs previously posted

Generated at Sun Nov 23 07:43:12 UTC 2014 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.