Issues

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

Description

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}

Comments

Please categorize/fix as needed.

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:


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?

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.

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

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.

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.

+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.

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

Will evaluate within 2 weeks

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.

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.