ZF-2773: Zend_Db_Table_Row_Abstract::__isset() returns true for null values

Issue Type: Bug Created: 2008-03-01T06:27:32.000+0000 Last Updated: 2009-08-06T07:50:42.000+0000 Status: Postponed Fix version(s): - Next Major Release ()

Reporter: Jack Sleight (jacksleight) Assignee: Ralph Schindler (ralph) Tags: - Zend_Db_Table

Related issues: Attachments:


Zend_Db_Table_Row_Abstract::__isset() uses array_key_exists rather than isset, which results in null values returning true. Quote from Bill Karwin on the mailing list:

{quote} The way it works now makes it a little more like property_exists(). That is, if the Row object has a property, even if its value is null, then the method returns true. This is useful information, and should be supported.

But I can understand that supporting this with the magic __isset() method might not be the best solution, since isset() has pretty well defined semantics, including returning false if the property has a null value (I have issues with this behavior of isset(), but the fact is that it does work that way). {quote}


Posted by Wil Sinclair (wil) on 2008-03-25T20:43:56.000+0000

Please categorize/fix as needed.

Posted by Darby Felton (darby) on 2008-04-02T15:49:55.000+0000

If it is needed to know whether a property exists and is it is not of a null value, users simply can test against this, as follows:

<pre class="highlight">
if (isset($row->property) && $row->property !== null) { ... }

Perhaps another solution would be to augment the row abstract class with a method implementing this feature for simplicity's sake, since it is likely a common use case.

Other ideas for resolution?

Posted by Jack Sleight (jacksleight) on 2008-04-02T17:10:28.000+0000

I think the only resolution should be changing the current __isset() mefthod to return false when the value is null i.e. use isset() rather than array_key_exists(). As Bill said, isset() has very well defined semantics, it is expected to return false when the value is null. Zend_Db_Table_Row_Abstract shouldn't be any different.

If the current functionality is still required then that should be in a new, separate method.

Posted by Darby Felton (darby) on 2008-04-03T07:17:35.000+0000

I'm not sure that breaking backward compatibility is worth the change.

Posted by Jack Sleight (jacksleight) on 2008-04-04T07:36:31.000+0000

Hmm, I dunno. I cant see that anyone would be directly calling __isset(), it would of course be being called when people use the actual isset() PHP function on the object, and everyone knows that isset() returns false when the value's null. I don't see it being a major backward compatibility issue.

Although personally, I'd change it even if it did significantly break backward compatibility, simply because at present, it makes the isset() PHP function behave differently from the documented behaviour. It is essentially broken in its current state.

Just my two cents.

Posted by Darby Felton (darby) on 2008-04-04T08:30:01.000+0000

For what it's worth, I also agree that the change should be made. That is, I believe that __isset() should be changed to be consistent with PHP's isset() function. Another method could be added, like propertyExists($name), that would provide the ability to return true if and only if a property exists, even if it is null (the existing behavior of __isset()).

That said, I'm afraid that breaking backward compatibility is not possible for us before 2.0.0, so I mark this issue as to fix for next major release.

Posted by Jaka Jancar (jaka) on 2008-08-18T07:35:31.000+0000

+1 for changing the behavior in the next major version. I just lost half an hour debugging a problem caused by this :/

While theoretically I think the current behavior is more logical and find it silly that ??$a = NULL; isset($a)?? returna false, ZF should be consistent with PHP's silliness.

Posted by Wil Sinclair (wil) on 2009-01-06T10:49:56.000+0000

No action on this issue for too long. I'm reassigning to Ralph for re-evaluation and categorization.

Posted by Ralph Schindler (ralph) on 2009-01-10T11:23:17.000+0000

Will evaluate within 2 weeks

Posted by Kirk Madera (aredamkrik) on 2009-05-20T15:31:07.000+0000

Strongly agreed that isset() should follow php behavior. This caused me to question my basic php knowledge for a sec until I realized what was going on. Also agreed that it could be added into the next major release to avoid backward compatibility problems.

Posted by Ralph Schindler (ralph) on 2009-08-06T07:50:41.000+0000

Given there is a massive use of isset($row->field) and an already expected behavior, we are gonna have to push this off till the next major release.

Have you found an issue?

See the Overview section for more details.


© 2006-2018 by Zend, a Rogue Wave Company. Made with by awesome contributors.

This website is built using zend-expressive and it runs on PHP 7.