CakePHP Delete with Multiple Conditions

Question

This is driving me mad. Why does Cake need to make simple things overly complex..

I am hoping to get Cake to generate SQL that looks like this..

I expect the SQL that runs to be

DELETE `ProductVouchers` 
FROM `product_vouchers` AS `ProductVouchers`   
WHERE `ProductVouchers`.`id` = 16 
AND `ProductVouchers`.`client_id` IS NULL; 

I am using delete() like this

$this->ProductVouchers->delete([
    'ProductVouchers.id'=>$id,
    'ProductVouchers.client_id'=>"NULL"
]);

The log shows

DELETE `ProductVouchers` 
FROM `product_vouchers` AS `ProductVouchers`   
WHERE `ProductVouchers`.`id` IN (16, 'NULL')

Trying

$this->ProductVouchers->delete([
   'ProductVouchers.id'=>$id,
   'ProductVouchers.client_id'=>NULL
]);

Log shows

DELETE `ProductVouchers` 
FROM `product_vouchers` AS `ProductVouchers`   
WHERE `ProductVouchers`.`id` IN (16, NULL) 

Trying:

 $this->ProductVouchers->delete([
    'ProductVouchers.id'=>$id,
    'ProductVouchers.client_id IS NULL'
 ]);

Log shows nothing which is even more wrong!

EDIT:

As pointed out in the answer below the delete() method is incorrect as it only accepts the primary key as an integer. So using deleteAll()

 $this->ProductVouchers->deleteAll([       
   'ProductVouchers.id'=>$id,
   'ProductVouchers.client_id'=>'NULL'
 ]);

This also produces nothing in the log or any error. Trying:

 $this->ProductVouchers->deleteAll([
   'ProductVouchers.id'=>$id,
   'ProductVouchers.client_id'=>NULL
 ]);

This produces...

 DELETE `ProductVouchers` 
 FROM `product_vouchers` AS `ProductVouchers`   
 WHERE `ProductVouchers`.`id` = (17)

Which is both wrong and weird (whats the point of the brackets??)

Solution 1

Model::delete expects an id

Delete is a method for deleting one row, by primary key value. The method signature for Model::delete does not expect conditions:

delete( ) public

Removes record for given ID. If no ID is given, the current ID is used. Returns true on success.

Parameters

integer|string $id optional null - ID of record to delete

boolean $cascade optional true

The behavior when called passing an array is that it won't work, one way or another (in the case here, the array values of the conditions array are understood to be ids - and used to find a record to delete - it would delete at most one row).

Model::deleteAll expects conditions

deleteAll is a method for deleting rows by conditions:

/**
 * Deletes multiple model records based on a set of conditions.
 *
 * @param mixed $conditions Conditions to match
 * @param bool $cascade Set to true to delete records that depend on this record
 * @param bool $callbacks Run callbacks
 * @return bool True on success, false on failure
 * @link http://book.cakephp.org/2.0/en/models/deleting-data.html#deleteall
 */
    public function deleteAll($conditions, $cascade = true, $callbacks = false) {

The correct call for the logic in the question would be:

$model->deleteAll(
    [
        'ProductVouchers.id' => 16, 
        'ProductVouchers.client_id' => null
    ],
    $cascade,
    $callbacks
);

This also produces nothing in the log or any error

Calling deleteAll with $cascade true (the default) means:

• Find all records matching the conditions
• Delete them one by one

If no records are found matching the conditions, there will be no delete statement.

This condition fragment:

'ProductVouchers.client_id'=>'NULL'

Will generate this sql:

WHERE ProductVouchers.client_id = 'NULL'

I.e. it will return rows where client_id is the string "NULL" - hence there is no delete statement because there are no rows with a client_id set to the string "NULL".

Which is both wrong and weird (whats the point of the brackets??)

Actually that statement is expected if there is a record with that primary key and client_id set to null (there will be a select statement immediately before it to find by id and client_id value). The parentheses are of no functional relevance, these two statements are equivalent:

DELETE FROM `product_vouchers` WHERE `ProductVouchers`.`id` = (17)
DELETE FROM `product_vouchers` WHERE `ProductVouchers`.`id` = 17

How to delete by conditions?

To generate the equivalent of this sql:

DELETE FROM `product_vouchers` AS `ProductVouchers`   
WHERE `ProductVouchers`.`id` = 16 
AND `ProductVouchers`.`client_id` IS NULL; 

Simply disable $cascade:

$model->deleteAll(
    [
        'ProductVouchers.id' => 16, 
        'ProductVouchers.client_id' => null
    ],
    false # <- single delete statement please
);

Solution 2

Simply reading the docs, the delete method only takes the identifier in the first parameter as an integer by design.

It looks like the Cake team may have accepted array formats but only use the array values to supply into the delete query (thus ignoring the keys and using the identifier every time.)

I believe you are looking for the deleteAll method, which will accept the custom conditions that you've supplied in your question:

$this->ProductVouchers->deleteAll(['ProductVouchers.id'=>$id,'ProductVouchers.client_id'=>"NULL"]);

Solution 3

The issue is related to a $belongsTo variable in the model ProductVouchers model. While this is set I cant apply a criteria to the foreign key without setting the $cascade argument to false like this:

 $this->ProductVouchers->deleteAll([
   'ProductVouchers.id'=>$id,
   'ProductVouchers.client_id IS NULL'
 ],false);

This produces working SQL

DELETE `ProductVouchers` FROM `product_vouchers` AS `ProductVouchers`             
LEFT JOIN `onepulse_dev`.`clients` AS `Client`
ON (`ProductVouchers`.`client_id` = `Client`.`clients_id`)  
WHERE `ProductVouchers`.`id` = 25 AND `ProductVouchers`.`client_id` IS NULL

EDIT:

Follow @AD7six's comprehensive answer to remove the unnecessary join.

 $this->ProductVouchers->deleteAll([
   'ProductVouchers.id'=>$id,
   'ProductVouchers.client_id'=>NULL
 ],false);

Solution 4

Try follow:

$id = 100;
$conditions = array(
    'OR' => array(
        'ProductVouchers.id' => $id,
        'ProductVouchers.client_id IS NULL'
    )
);

$this->ProductVouchers->deleteAll($conditions);

Should to create following SQL Query:

SELECT `ProductVouchers`.`id` FROM `cakephp2`.`ProductVouchers` AS `product_vouchers` WHERE ((`ProductVouchers`.`id` = 100) OR (`ProductVouchers`.`client_id` IS NULL)) GROUP BY `ProductVouchers`.`id`
DELETE `ProductVouchers` FROM `cakephp2`.`ProductVouchers` AS `product_vouchers` WHERE `ProductVouchers`.`id` IN (1, 8, 100)

The "IN (1, 8, 100)" is the result of the previous called SELECT statement.

View more solutions

Comments

[Epodax]

Why what? I'm not getting the issue?

[Mike Miller]

Updated my question - sorry wasnt very clear. product of my frustration

[Mike Miller]

I have both read the docs and tried your suggestion. Fair point re the delete method but not much success with deleteAll either

[sjagr]

@MikeMiller Can you edit in your experiences and logs with deleteAll?

[Mike Miller]

Clear enough to me. Thanks for your helpful input

[AD7six]

There is no OR in the question.

[AD7six]

The unwanted join is an artifact of how it has always worked - a workaround for that is to unbind the association before calling deleteAll (Or by shuffling the association property meaning e.g. create in your app model public function deleteAll(...) { $old = $this->belongsTo; $this->belongsTo = []; $return = parent::deleteAll(...); $this->belongsTo = $old; return $return; } - After testing and verifying this for yourself - I recommend editing your answer to have a code example - makes the answer more useful to other readers (no need to mention my nick - thanks for the thought though).