ZF-1103: Cascading delete functionality is not recursive

Description

Cascade delete now deletes only one level of data. For example:

table object: id parent name

data: ||id||parent||name|| |1|null|domain1| |2|1|section1| |3|2|item1|

$objects->find(1)->current()->delete() it deletes domain1 and section1 and not item1. Probably issue is in _cascadeDelete method that deletes rows in dependent table using table->delete($where), not by selecting a rowset and calling ->delete for each row.

In DBMS cascade operations affect all depended tables and their subtables.

Comments

Assign to Bill Karwin.

Came across this bug also when I removed a parent of another table which in turn was the parent of a third table from which the affected rows wasn't removed.

On row 887 of the Zend_Table_Abstract in version 1.0.1 within the function _cascadeDelete a row is constructed like this:



It should instead be constructed as something like this:

$toDelete = $this->fetchAll($where); foreach($toDelete as $row) { $rowsAffected += $row->delete(); } ```

By the way - shouldn't this be flagged as a bug rather than an improvement?

Another effect of the current way of deleting rows in depending tables is that it bypasses eventual pre- and post-operations that may have been added to those tables row-classes. Is that an issue perhaps?

Please categorize/fix as needed.

This doesn't appear to have been fixed in 1.5.0. Please update if this is not correct.

Seconding Pelle Wessman's comment regarding the status of this issue; this issue should be marked as a Bug, as the current method of cascading results in unexpected behaviour (ie. it behaves in a manner contrary to every other "cascade" operation I can think of, RDBMS or otherwise).

This indeed is a bug. I was 100% sure that it works like normal cascades in RDBMSs do, until reading it.

Cascades on the PHP level are also useful if you're using triggers in MySQL, because native MySQL cascades don't trigger them on cascaded changes.

Reassigning to Ralph since he's the new maintainer of Zend_Db

Ill attempt to confirm this with unit tests within 2 weeks.

Ralph Schindler, did you confirm this?

Not confirmed yet. But just a thought: what is to prevent circular references from being an issue?

Do you need more information to resolve this issue?

I'd be happy to provide some test cases or anything else you may need. I believe this to be a rather serious issue with Zend_Db_Table.

Yes, can you provide a simple test case that demonstrates the problem? Do you know if it can be demonstrated with the existing database in the unit tests?

I have the same issue. Below is an example of how you can replicate it ...

============================================================================

The SQL Tables and some sample data:


CREATE TABLE IF NOT EXISTS `testtable` (
  `rowid` int(11) NOT NULL AUTO_INCREMENT,
  `name` varchar(255) DEFAULT NULL,
  PRIMARY KEY (`rowid`)
);

CREATE TABLE IF NOT EXISTS testtable_sub (
  rowid int(11) NOT NULL AUTO_INCREMENT,
  testtable_id int(11),
  name varchar(255) DEFAULT NULL,
  PRIMARY KEY (`rowid`),
  KEY tts (testtable_id)
);


CREATE TABLE IF NOT EXISTS testtable_sub_sub (
  rowid int(11) NOT NULL AUTO_INCREMENT,
  testtable_sub_id int(11),
  name varchar(255) DEFAULT NULL,
  PRIMARY KEY (`rowid`),
  KEY ttss (testtable_sub_id)
);


CREATE TABLE IF NOT EXISTS testtable_sibling (
  rowid int(11) NOT NULL AUTO_INCREMENT,
  testtable_id int(11),
  name varchar(255) DEFAULT NULL,
  PRIMARY KEY (`rowid`),
  KEY ttsb (testtable_id)
);


INSERT INTO testtable VALUES( 1, "Person 1" );
INSERT INTO testtable VALUES( 2, "Person 2" );

INSERT INTO testtable_sibling VALUES( 3, 1, "Person 1 (sibling)" );
INSERT INTO testtable_sibling VALUES( 5, 2, "Person 2 (sibling)" );

INSERT INTO testtable_sub VALUES( 5, 1, "Person 1 (sub)" );
INSERT INTO testtable_sub VALUES( 7, 2, "Person 2 (sub)" );

INSERT INTO testtable_sub_sub VALUES( 1, 5, "Person 1 (sub sub 1)" );
INSERT INTO testtable_sub_sub VALUES( 2, 5, "Person 1 (sub sub 2)" );
INSERT INTO testtable_sub_sub VALUES( 3, 7, "Person 2 (sub sub 2)" );

============================================================================

The Models/Classes (I have them in separate files, but am just copying/pasting here for speed)


<?php
class Testtable extends Table
{
    # This table
    protected $_name = 'testtable';
    protected $_primary = array( 'rowid' );
    protected $_sequence = true;

    # Dependents
    protected $_dependentTables = array( 'TesttableSibling','TesttableSub' );

    # References
    protected $_referenceMap = array(

    );
}

?>

<?php
class TesttableSibling extends DB_Table_Abstract
{
    # This table
    protected $_name = 'testtable_sibling';
    protected $_primary = array( 'rowid' );
    protected $_sequence = true;

    # Dependents
    protected $_dependentTables = array();

    # References
    protected $_referenceMap = array(
        'TT'        => array(
            'columns'       => 'testtable_id',
            'refTableClass' => 'Testtable',
            'refColumns'    => 'rowid',
            'onDelete'      => self::CASCADE,
            'onUpdate'      => self::CASCADE,
        ),
    );


}

?>

<?php
class TesttableSub extends DB_Table_Abstract
{
    # This table
    protected $_name = 'testtable_sub';
    protected $_primary = array( 'rowid' );
    protected $_sequence = true;

    # Dependents
    protected $_dependentTables = array( 'TesttableSubSub' );

    # References
    protected $_referenceMap = array(
        'TT'        => array(
            'columns'       => 'testtable_id',
            'refTableClass' => 'Testtable',
            'refColumns'    => 'rowid',
            'onDelete'      => self::CASCADE,
            'onUpdate'      => self::CASCADE,
        ),
    );

}

?>

<?php
class TesttableSubSub extends DB_Table_Abstract
{
    # This table
    protected $_name = 'testtable_sub_sub';
    protected $_primary = array( 'rowid' );
    protected $_sequence = true;

    # Dependents
    protected $_dependentTables = array();

    # References
    protected $_referenceMap = array(
        'TT'        => array(
            'columns'       => 'testtable_sub_id',
            'refTableClass' => 'TesttableSub',
            'refColumns'    => 'rowid',
            'onDelete'      => self::CASCADE,
            'onUpdate'      => self::CASCADE,
        ),
    );


}

?>

============================================================================

Test Case


    $TestTable = new Testtable();

    $record = $TestTable->getRecord( array (
        "rowid" => $this->view->params['oldid']
    ) );

    $record->delete();

Create an object of TestTable, grab one row (find by rowid) and then call $row->delete()

It will delete the record from testtable, testtable_sub and testtable_sibling (both are immediate children of testttable), but any records in testtable_sub_sub, which is a child of testtable_sub are not deleted as you would expect them to be, following the deletion of their parent rows from testtable_sub by the cascade operation.

If you set these same tables up in mySQL as INNODB, using Foreign Keys and cascading deletes, everything happens as it should when you delete the parent record ... would be nice to have reliable functionality at the Zend Level as well for those cases where you cannot rely on the DB to think for you.

HTH, John

Attachment is against trunk.

To use this feature, set the 'onDelete' key of any reference map element to self::CASCADE_RECURSE instead of self::CASCADE.

Changing the _getPrimaryKey() function in Zend_Db_Table_Row_Abstract to have public scope instead of protected is problematic because the Zend coding standard's naming convention does not allow an underscore to preceed a public method's name.

Perhaps you change the _getPrimaryKey() back to protected and add a public getPrimaryKey() that calls the protected function?

I can do that, yes. We do have some places in ZF that have public underscored methods, they are mainly there as a way to "enforce" or demonstrate that they are to be used internally only. Imagine if PHP had class level (or namespace visibility).

But I agree with you, and I can change this. I guess it's not a big deal if getPrimaryKey() is visibly available to consumers of the class.

How does it work for you though? :)

-ralph

In my initial tests, the recursive deletes are working on my test data.

Two Minor Changes: 1) There are three typos in the comments, both with the spelling of CASCADE. You have spelled it CASECADE. 2) The method comment for _cascadeDelete says "Called by parent table's class during delete() method.", but it should actually be something along the lines of: Called by table row during delete() method, and the parent table's class during _cascadeDelete() method for recursive cascading deletes.


I did have one possibly unrelated question/issue. I tried creating a table relationship where the refColumn defined in the child table's referenceMap is not one of the parent table's primary keys. A cascading and/or cascadingRecursive delete does not work because when the table's _cascadeDelete() method is called, only the primary keys are passed as the second parameter. I'm wondering why only the primary keys were sent and not all of the data. This would allow the following scenario:


DROP TABLE IF EXISTS `profiles`;
CREATE TABLE `profiles` (
  `id` int(11) NOT NULL auto_increment,
  `name` varchar(30) NOT NULL,
  `photoId` int(11) NOT NULL,
  PRIMARY KEY  (`id`),
  KEY `photo` (`photoId`)
);

INSERT INTO `profiles` (`id`, `name`, `photoId`) VALUES
(101, 'Profile #1', 201),
(102, 'Profile #2', 202);

DROP TABLE IF EXISTS `photos`;
CREATE TABLE `photos` (
  `id` int(11) NOT NULL auto_increment,
  `title` varchar(50) NOT NULL,
  `path` varchar(255) NOT NULL,
  PRIMARY KEY  (`id`)
);

INSERT INTO `photos` (`id`, `title`, `path`) VALUES
(201, 'Profile #1 Photo', '/images/one.jpg'),
(202, 'Profile #2 Photo', '/images/two.jpg');

When a profile is deleted, I want the corresponding profile photo (stored in photos table) deleted as well.

Here's my models:


<?php
class Application_Model_Profiles extends Zend_Db_Table_Abstract
{
    protected $_name    = 'profiles';

    protected $_dependentTables = array(
        'Application_Model_Photos',
    );

}

<?php
class Application_Model_Photos extends Zend_Db_Table_Abstract
{
    /** Table name */
    protected $_name    = 'photos';

    protected $_referenceMap    = array(
        'Profile' => array(
            'columns'           => 'id',
            'refTableClass'     => 'Application_Model_Profiles',
            'refColumns'        => 'photoId',
            'onDelete'          => self::CASCADE_RECURSE,
        ),
    );
}

The reason I organized the data like this is to allow several different entities (profiles, albums, users, groups, etc.) to store their photo in one table. The cascading deletes, however, do not work because the photoId column from the Photos table doesn't exist in the _cascadeDelete method because only the profile's ID column is passed.

I know this is off-topic for the recursive cascading deletes, but perhaps this new capability could be added simultaneously.

I ran into this issue. My solution is a bit simpler than the proposed one. It's posted on Stack Overflow. My solution only touches the {{_cascadeDelete}} function.

I see a steady trend of this getting put off. What is needed to get this included in a release? It's a bug. True, recursive, cascading deletes are not performed.

Is there any reason this never made it into the framework?

I tested the patch suggested by Pelle Wessmann and it worked for me. +1 for integrating into the framework.

Attached reproducing test case which uses the single-table schema proposed by the original ticket creator:


CREATE TABLE `zfalt_cascade_recursive` (
  `item_id` int(11) NOT NULL,
  `item_parent` int(11) DEFAULT NULL,
  `item_data` varchar(100) NOT NULL,
  PRIMARY KEY (`item_id`)
);

INSERT INTO `zfalt_cascade_recursive` (`item_id`, `item_parent`, `item_data`) VALUES
(1, NULL, '1'),
(2, 1, '1.2'),
(3, 1, '1.3'),
(4, 3, '1.3.4'),
(5, 3, '1.3.5'),
(6, NULL, '6');

PHPUnit Result:


PHPUnit 3.4.15 by Sebastian Bergmann.

Zend Framework
 Zend Framework - Zend
   Zend Framework - Zend_Db
     Zend_Db_Table_MysqliTest
   F
  
   Zend_Db_Table_Pdo_MysqlTest
   F
  
   Zend_Db_Table_Pdo_SqliteTest
   F
  
Time: 10 seconds, Memory: 418.00Mb

There were 3 failures:

1) Zend_Db_Table_MysqliTest::testTableCascadeRecurseDelete
Failed asserting that 
Zend_Db_Table_Row Object ( item_id = 4 )
 is null.

/var/www/ZFv1/trunk/tests/Zend/Db/Table/TestCommon.php:1643

2) Zend_Db_Table_Pdo_MysqlTest::testTableCascadeRecurseDelete
Failed asserting that 
Zend_Db_Table_Row Object ( item_id = 4 )
 is null.

/var/www/ZFv1/trunk/tests/Zend/Db/Table/TestCommon.php:1643

3) Zend_Db_Table_Pdo_SqliteTest::testTableCascadeRecurseDelete
Failed asserting that 
Zend_Db_Table_Row Object ( item_id = 4 )
 is null.

/var/www/ZFv1/trunk/tests/Zend/Db/Table/TestCommon.php:1643

FAILURES!
Tests: 3, Assertions: 39, Failures: 3.

As you can see, it only deletes the parent row and the first level dependent rowset.

Updated title and description

Attached fix appears to work as expected. After applying the patch, change the onDelete parameter value in the file {{tests/Zend/Db/Table/_files/My/ZendDbTable/TableCascadeRecursive.php}} from {{self::CASCADE}} to {{self::CASCADE_RECURSE}} and the tests will pass.

A similar fix and unit test suite will need to be implemented for {{onUpdate}} as well.

Attached completed patch for onDelete recursive cascade (fix + unit test + manual page update). Will open a separate ticket to implement similar fix for onUpdate

Fixed in SVN r24831.