Yii Framework Forum: CDbCriteria in CActiveDataProvider is combining with others on same page - Yii Framework Forum

Jump to content

Page 1 of 1
  • You cannot start a new topic
  • You cannot reply to this topic

CDbCriteria in CActiveDataProvider is combining with others on same page Rate Topic: -----

#1 User is offline   warenzema 

  • Newbie
  • Yii
  • Group: Members
  • Posts: 2
  • Joined: 28-April 12

Posted 28 April 2012 - 11:36 PM

Ran into a weird bug that I can't quite understand why things are happening the way they are. I'm fairly new to Yii (3 months) and have just started really getting into using CGridview with my latest project.

As part of this project, I tried to combine what I learned of scopes with CGridView and CDbCriteria. Relevant to my issue, here are the methods in my CActiveRecord model that I use (OrderProductTask).

	public function for_order($order_id){
		if($order_id){
			$this->getDbCriteria()->mergeWith(array(
		        'condition'=>'order_product.order_id=:order_id',
				'params'=>array(':order_id'=>$order_id),
		    	'together'=>true,
		    	'with'=>array('order_product'),
		    ));
		}
	    return $this;
	}
	
	public function with_criteria($criteria){
		$this->getDbCriteria()->mergeWith($criteria);
		
		return $this;
	}


The issue I am having is with the custom method with_criteria(). I followed the model of how named scopes work, and created with_criteria() to take a custom CDbCriteria to give me better flexibility in creating dataProviders for CGridView, without having to define a scope for every little thing I may want to look up.

The issue I am having is that the criteria I pass in this way seems to be added onto previous criteria on the page, resulting in only the first CGridView to have any results, in many cases.

<?php

foreach(Order::model()->findAll() as $order){
	/*
		$criteria = new CDbCriteria();
		$criteria->compare('order_product.order_id', 0);
		
		$dataProvider = new CActiveDataProvider(OrderProductTask::model()->together()->with(array(
			'order_product',
		))->with_criteria($criteria)
		);
		*/
		
		echo '<h2>'.$order->order_id.'</h2>';
	
		$criteria = new CDbCriteria;
		$criteria->compare('order_product.order_id', $order->order_id);
	
		$dataProvider = new CActiveDataProvider(OrderProductTask::model()->together()->with(array(
			'order_product',
		))->with_criteria($criteria)
		);
		
		
		$this->widget('zii.widgets.grid.CGridView', array(
			'dataProvider'=>$dataProvider,
			'columns'=>array(
				'task_id',
				'order_product.order_id',
			),
		));
}
?>


In the logs I am seeing the following queries:

Quote

Querying SQL: SELECT `t`.`order_product_task_id` AS `t0_c0`,
`t`.`order_product_id` AS `t0_c1`, `t`.`task_id` AS `t0_c2`,
`t`.`order_product_task_status_id` AS `t0_c3`,
`t`.`assigned_to_user_id` AS `t0_c4`, `t`.`assigned_dt` AS `t0_c5`,
`t`.`due_date` AS `t0_c6`, `order_product`.`order_product_id` AS `t1_c0`,
`order_product`.`order_id` AS `t1_c1`,
`order_product`.`parent_order_product_id` AS `t1_c2`,
`order_product`.`product_id` AS `t1_c3`, `order_product`.`product_size_id`
AS `t1_c4`, `order_product`.`product_timeframe_id` AS `t1_c5`,
`order_product`.`due_date` AS `t1_c6`, `order_product`.`final_price_n` AS
`t1_c7` FROM `order_product_task` `t` LEFT OUTER JOIN `order_product`
`order_product` ON
(`t`.`order_product_id`=`order_product`.`order_product_id`) WHERE
(order_product.order_id=:ycp0) LIMIT 10


and..

Quote

Querying SQL: SELECT `t`.`order_product_task_id` AS `t0_c0`,
`t`.`order_product_id` AS `t0_c1`, `t`.`task_id` AS `t0_c2`,
`t`.`order_product_task_status_id` AS `t0_c3`,
`t`.`assigned_to_user_id` AS `t0_c4`, `t`.`assigned_dt` AS `t0_c5`,
`t`.`due_date` AS `t0_c6`, `order_product`.`order_product_id` AS `t1_c0`,
`order_product`.`order_id` AS `t1_c1`,
`order_product`.`parent_order_product_id` AS `t1_c2`,
`order_product`.`product_id` AS `t1_c3`, `order_product`.`product_size_id`
AS `t1_c4`, `order_product`.`product_timeframe_id` AS `t1_c5`,
`order_product`.`due_date` AS `t1_c6`, `order_product`.`final_price_n` AS
`t1_c7` FROM `order_product_task` `t` LEFT OUTER JOIN `order_product`
`order_product` ON
(`t`.`order_product_id`=`order_product`.`order_product_id`) WHERE
((order_product.order_id=:ycp0) AND (order_product.order_id=:ycp1)) LIMIT
10


The WHERE clause continues to grow and grow with each loop:

Quote

((((order_product.order_id=:ycp0) AND (order_product.order_id=:ycp1)) AND
(order_product.order_id=:ycp2)) AND (order_product.order_id=:ycp3))


etc, etc...

If I uncomment the block of code at the top (where I set the order_id = 0), then not even the first result displays anything, as it is looking for an order_id of 0 for all the CGridViews. If I change the uncommented code to use for_order($order->order_id) instead of with_criteria($criteria), the problem vanishes as long as the 'order_id=0' code at the top remains commented out. If it is not commented, then still, no results are displayed for any table.

Since both $criteria and $dataProvider are being overwritten with each pass of the loop, I don't understand why the criteria is compounding like it is.


Update:

Apparently the issue isn't with my with_criteria() scope at all. Using the following code I get correct CGridViews for each order_id in the loop, but the additional table at the end, which queries different aspects of OrderProductTask, has no results if the loop executes first.

<?php

foreach(Order::model()->findAll() as $order){
		
		
		echo '<h2>'.$order->order_id.'</h2>';
	
		$criteria = new CDbCriteria;
		$criteria->compare('order_product.order_id', $order->order_id);
	
		$dataProvider = new CActiveDataProvider(OrderProductTask::model()->together()->with(array(
			'order_product',
		))->for_order($order->order_id)//->with_criteria($criteria)
		);
		
		
		$this->widget('zii.widgets.grid.CGridView', array(
			'dataProvider'=>$dataProvider,
			'columns'=>array(
				'task_id',
				'order_product.order_id',
			),
		));
}

$dataProvider = new CActiveDataProvider(OrderProductTask::model()->for_order_product(53));

$this->widget('zii.widgets.grid.CGridView', array(
	'dataProvider'=>$dataProvider,
	'columns'=>array(
		'task_id',
		'order_product_id',
	),
));
?>


The WHERE clause of the query for the last table is:

Quote

WHERE
((order_product.order_id=:order_id) AND (t.order_product_id = :op_id))


which, as you can see includes a constraint on the order_id, despite nothing in the scope for_order_product() having any reference to the order id.

	public function for_order_product($op_id){
		if($op_id){
			$this->getDbCriteria()->mergeWith(array(
				'condition'=>'t.order_product_id = :op_id',
				'params'=>array(':op_id'=>$op_id),
			));
		}
		return $this;
	}


(Yes, I verified that if I comment out the foreachloop at the start that order_product_id 53 does populate the last table with results.)

Using Yii 1.1.10

Am I seriously misunderstanding how this is supposed to work, or is something broken?
0

#2 User is offline   saegeek 

  • Standard Member
  • PipPip
  • Yii
  • Group: Members
  • Posts: 244
  • Joined: 09-December 09
  • Location:Montpellier - France

Posted 29 April 2012 - 01:54 AM

I don't know anything about your bug but have just a suggest.
This is really unoptimized and bad for RAM and SQL server:
foreach(Order::model()->findAll() as $order){
....

use this instead:
$o=Order::model()->findAll();
foreach($o as $order){
....

Try this before anything
And [for] their saying, "Indeed, we have killed the Messiah, Jesus, the son of Mary, the messenger of God ." And they did not kill him, nor did they crucify him; but [another] was made to resemble him to them. And indeed, those who differ over it are in doubt about it. They have no knowledge of it except the following of assumption. And they did not kill him, for certain.Rather, God raised him to Himself.
0

#3 User is offline   warenzema 

  • Newbie
  • Yii
  • Group: Members
  • Posts: 2
  • Joined: 28-April 12

Posted 29 April 2012 - 02:39 AM

View Postsaegeek, on 29 April 2012 - 01:54 AM, said:

I don't know anything about your bug but have just a suggest.
This is really unoptimized and bad for RAM and SQL server:
foreach(Order::model()->findAll() as $order){
....

use this instead:
$o=Order::model()->findAll();
foreach($o as $order){
....

Try this before anything


That part was in there just to demonstrate the bug with very-stripped down code. It is not related to the issue.

I just managed to figure out the issue, it's due to the fact that this the class is called statically. That is why the criteria is persisting. I added this to my model and it solved this issue. Time will tell if it causes other issues. (It probably will.)

public static function model($className=__CLASS__)
	{
		$model=new $className(null);
        $model->_md=new CActiveRecordMetaData($model);
        $model->attachBehaviors($model->behaviors());
        return $model;
        
		return parent::model($className);
	}


I just copied the code from code.google.com/p/yii/source/browse/tags/1.1.10/framework/db/ar/CActiveRecord.php#372 to bypass the 'save' feature. I suspect this was implemented for optimization, so with luck my fix won't mess anything up for me later on.

Update:

http://www.yiiframew...setScope-detail

Using resetScopes() worked for me. Much better solution than changing how model() works. Just wish it hadn't taken me so long to figure out what was going on.
0

#4 User is offline   pavlepredic 

  • Junior Member
  • Pip
  • Yii
  • Group: Members
  • Posts: 38
  • Joined: 30-August 11

Posted 10 May 2012 - 08:30 AM

View Postsaegeek, on 29 April 2012 - 01:54 AM, said:

I don't know anything about your bug but have just a suggest.
This is really unoptimized and bad for RAM and SQL server:
foreach(Order::model()->findAll() as $order){
....

use this instead:
$o=Order::model()->findAll();
foreach($o as $order){
....

Try this before anything



What exactly is the difference between the two methods and why is the second one better?
0

#5 User is offline   saegeek 

  • Standard Member
  • PipPip
  • Yii
  • Group: Members
  • Posts: 244
  • Joined: 09-December 09
  • Location:Montpellier - France

Posted 11 May 2012 - 11:10 AM

Because @ each foreach loop PHP will re-launch the "model->findAll() function" and this function will reask for SQL records (@ each loop !)
So foreach will surely give you always the same $order (the first SQL record)
And [for] their saying, "Indeed, we have killed the Messiah, Jesus, the son of Mary, the messenger of God ." And they did not kill him, nor did they crucify him; but [another] was made to resemble him to them. And indeed, those who differ over it are in doubt about it. They have no knowledge of it except the following of assumption. And they did not kill him, for certain.Rather, God raised him to Himself.
0

#6 User is offline   pavlepredic 

  • Junior Member
  • Pip
  • Yii
  • Group: Members
  • Posts: 38
  • Joined: 30-August 11

Posted 14 May 2012 - 03:19 AM

View Postsaegeek, on 11 May 2012 - 11:10 AM, said:

Because @ each foreach loop PHP will re-launch the "model->findAll() function" and this function will reask for SQL records (@ each loop !)
So foreach will surely give you always the same $order (the first SQL record)


That's not correct. Method findAll() will return an array and foreach will then loop over that array. Assigning that array to a new variable will simply produce a copy of it, so that will be slightly less efficient, I think.
0

#7 User is offline   saegeek 

  • Standard Member
  • PipPip
  • Yii
  • Group: Members
  • Posts: 244
  • Joined: 09-December 09
  • Location:Montpellier - France

Posted 16 May 2012 - 03:33 PM

View Postpavlepredic, on 14 May 2012 - 03:19 AM, said:

Method findAll() will return an array


You're right, but this array will be recreated at each loop.
And this is really bad for perfomances
And [for] their saying, "Indeed, we have killed the Messiah, Jesus, the son of Mary, the messenger of God ." And they did not kill him, nor did they crucify him; but [another] was made to resemble him to them. And indeed, those who differ over it are in doubt about it. They have no knowledge of it except the following of assumption. And they did not kill him, for certain.Rather, God raised him to Himself.
0

Share this topic:


Page 1 of 1
  • You cannot start a new topic
  • You cannot reply to this topic

1 User(s) are reading this topic
0 members, 1 guests, 0 anonymous users