Issues

ZF-3144: Cascading db operations do not work when 'refColumns' is not explicity set inside $_referenceMap.

Description


<?php
class Files extends Zend_Db_Table
{
    protected $_name = 'files';
    protected $_rowClass = 'File';
    protected $_referenceMap = array(
        'Email' => array(
            'columns'           => 'emailId',
            'refTableClass'     => 'Emails',
            // Without next row CASCADE doesn't work despite id is PK, and should be retrieved automaticaly
            // Manual says that if refColumns is not set it will be be discovered from table schema as PK
            'refColumns'        => 'id',
            'onDelete'          => self::CASCADE,
        ),
}

Btw I noticed, that without 'refColumns' set to 'id' CASCADE query looks like "DELETE FROM files WHERE phoneId = 0" - this is just wrong.

Comments

Please evaluate and categorize as necessary.

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

Will evaluate within 2 weeks

Is this forgotten issue?

can you describe the 2 tables and/or produce a small script (you can use sqlite ::memory:: to accomplish this) that will demonstrate the problem?

Ralph, here you go:


CREATE TABLE `products` (
  `id` tinyint(4) NOT NULL auto_increment,
  PRIMARY KEY  (`id`)
);

CREATE TABLE `files` (
  `id` tinyint(4) NOT NULL auto_increment,
  `productId` tinyint(4) NOT NULL,
  PRIMARY KEY  (`id`)
);

INSERT INTO products values (1),(2);
INSERT INTO files values (7,2),(8,2);

class Default_Model_DbTable_Files extends Zend_Db_Table
{
    protected $_name = 'files';
    protected $_referenceMap = array(
        'Product' => array(
            'columns'           => 'productId',
            'refTableClass'     => 'Default_Model_DbTable_Products',
           // 'refColumns'        => 'id',    
            'onDelete'          => self::CASCADE,
        ),
    );
} 

class Default_Model_DbTable_Products extends Zend_Db_Table
{ 
    protected $_name = 'products';  
    protected $_dependentTables = array('Default_Model_DbTable_Files');

} 

$productsTable = new Default_Model_DbTable_Products;
$productsTable->find(2)->current()->delete();

/trunk/lib/Zend/Db/Table/Abstract.php:1085 'Undefined index:  refColumns'
DELETE FROM `files` WHERE (`productId` = 0)
DELETE FROM `products` WHERE (`products`.`id` = 2)

Uncommenting // 'refColumns' => 'id', fixes problem.

Is the suggestion here to throw an exception when refColumns is not present? Seems that should be the behavior because if you don't explicitly say what the reference column is, Zend_Db_Table shouldn't make an assumption about what it's related to (it could technically be another column, not the PK).

Ralph,

I quit using ZF almost 2 years ago. Please decide on your own.

Ralph,

According to official documentation, if refColumns is not present, pk will be used. http://framework.zend.com/manual/en/…

This issue was fixed somewhere along the way.

Implemented Piotr's reproduction code as a unit test (patch attached). The result is still the same as he found:


2) Zend_Db_Table_Pdo_SqliteTest::testCascadingOperationReferenceColumnFallsBackToPrimaryKeyWhenNotSpecified
Undefined index: refColumns

/usr/local/apache2/htdocs/lib/zfdev/trunk/library/Zend/Db/Table/Abstract.php:1205
/usr/local/apache2/htdocs/lib/zfdev/trunk/library/Zend/Db/Table/Row/Abstract.php:620
/usr/local/apache2/htdocs/lib/zfdev/trunk/tests/Zend/Db/Table/TestCommon.php:1683

Ralph has said that refColumns should be explicitly specified, and the current implementation doesn't try to "autodiscover" the reference columns. However, the ZF manual says the opposite. Should we change the implementation or the manual?

As this issue has been around since at least 1.5.1, anyone using cascading operations would already have specified their reference columns, so I would vote for updating the manual page to note that specifying 'refColumns' is required for cascade operations to work. I would also change the implementation to throw an exception if 'refColumns' is not specified.

After discussion with [~ralph] on IRC, it was decided that the manual should be updated to display a note saying that as of 1.11.11 the refColumns key is required, and an exception should be thrown if it is not present. Attached is a patch implementing these changes.

Comments?

Reflecting on this, is throwing an exception the best course of action? With pre-1.12, any applications that use referenceMap without refColumns emit a notice and the cascade fails (see here above), but after this patch is applied it will throw an exception that will halt the entire application unless it's caught Would it be better to silently ignore the referenceMap entry if it doesn't have a refColumns key?

Updated manual to better reflect reality in svn r24820. Do not through an exception as this will cause a B/C break for no real benefit at this stage in ZF1's lifecycle.