Refactor this using CDbCriteria (or another Yii method)

I have the following query that is becoming long and clunky, and need to refactor this into a ‘Yii style’ way of doing things… the query is quite mammouth in my view and I want to try to minismise & make it ‘cleaner’ with Yii CDbCriteria if possible…

Any advice would be appreciated :




        return ShopProducts::model()->findAllByAttributes(array(

                'organisation_id' => $this->organisation_id,

            ),

            'IF(


                availability=3,

                (SELECT count(*) FROM shop_product_years WHERE shop_product_years.Guid = t.Guid AND shop_product_years.year_id = '.Myuser::model()->currentUser->year_id.') > 0,

				(availability=:availability OR availability = '.ShopProducts::ALL.')

			)

			AND (multi_buy = 1

				OR (multi_buy = 0

					AND (

						SELECT count(*)

						FROM shop_purchases

						WHERE shop_purchases.user_id = '.Yii::app()->user->id.'

						AND shop_purchases.GUID = t.Guid

					) = 0

					AND (

						SELECT count(*)

						FROM shop_purchases_queue

						WHERE shop_purchases_queue.user_id = '.Yii::app()->user->id.'

						AND shop_purchases_queue.GUID = t.Guid

					) = 0

					OR (

						SELECT count(*)

						FROM shop_purchases

						WHERE shop_purchases.user_id = '.Yii::app()->user->id.'

						AND shop_purchases.GUID = t.Guid

						AND shop_purchases.refunded IS NULL

					) = 0

				)

			)


                /* Classroom */

				OR (

				    SELECT count(*)

                    FROM shop_product_classrooms

                    WHERE

				    shop_product_classrooms.Guid = t.Guid

			        AND shop_product_classrooms.classroom_id IN ( '. $user_classroom_ids .' )

                )


			AND active='.ShopProducts::ACTIVE.'

			AND product_type_id='.ShopProducts::SCHOOL_PRODUCT_TYPE_ID,

            array(

                ':availability' => (Myuser::model()->currentUser->organisation->secondary_start_year === 0) ? '('.ShopProducts::SECONDARY_ONLY.' OR '.ShopProducts::PRIMARY_ONLY.')' : ((Myuser::model()->currentUser->isSecondary) ? ShopProducts::SECONDARY_ONLY : ShopProducts::PRIMARY_ONLY)

            ));



[list=1][]I use scopes to avoid this;[]I avoid subqueries.

For instance, to check that there is no related record, you can create a “HAS_ONE” relation (outer join) and test that the id found on the relation is “null”. This should be more efficient than making a count too because a count requires that all records are “retrieved”, whereas in the HAS_ONE relation, the search is resolved on the first valid record.[]Use relations.[]Check out ‘relatedsearchbehavior’ extension - it does not help a lot for this query I think, but it does implement some scopes automatically.[/list]

Just to add another perspective: I generally avoid Yii relations and scopes.

Most of my database interactions are in plain SQL using CDbCommand->execute or ->queryAll. For some, the Yii way will be more readable. For me though, for anything that is not a plain AR update or delete I prefer writing the queries myself.

Looking at your query I think there is a lot of room for optimisation in the SQL itself:

  • WHERE A OR B: consider UNION

  • WHERE (select COUNT(*)…)=0: consider LEFT JOIN with WHERE key IS NULL

  • WHERE (select COUNT(*)…)>0: consider JOIN, LEFT JOIN or even EXISTS if you must