php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Request #56758 memcache::get() should return NULL on failure, not false
Submitted: 2005-12-29 09:06 UTC Modified: 2006-09-20 08:41 UTC
From: stuart at gentoo dot org Assigned:
Status: Wont fix Package: memcache (PECL)
PHP Version: 5.1.1 OS: Gentoo
Private report: No CVE-ID: None
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If this is not your bug, you can add a comment by following this link.
If this is your bug, but you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: stuart at gentoo dot org
New email:
PHP Version: OS:

 

 [2005-12-29 09:06 UTC] stuart at gentoo dot org
Description:
------------
memcache::get() in memcache-2.0.0 returns FALSE on error.  This makes it impossible to retrieve the value FALSE if stored in memcache, because it's not possible to tell a genuine error apart from the value FALSE having been returned.

If memcache::get() returned NULL on error, this would solve the problem.  The patch pasted below works nicely locally.

Best regards,
Stu

Reproduce code:
---------------
diff -u --recursive memcache-2.0.0/memcache.c memcache-2.0.0-patched/memcache.c
--- memcache-2.0.0/memcache.c   2005-12-21 18:21:55.000000000 +0000
+++ memcache-2.0.0-patched/memcache.c   2005-12-29 12:15:38.000000000 +0000
@@ -1706,17 +1706,17 @@
        }

        if (!mmc_get_pool(mmc_object, &pool TSRMLS_CC) || !pool->num_servers) {
-               RETURN_FALSE;
+               RETURN_NULL();
        }

        if (Z_TYPE_P(key) != IS_ARRAY) {
                if (mmc_exec_retrieval_cmd(pool, key, &return_value TSRMLS_CC) < 0) {
-                       RETURN_FALSE;
+                       RETURN_NULL();
                }
        }
        else {
                if (mmc_exec_retrieval_cmd_multi(pool, key, &return_value TSRMLS_CC) < 0) {
-                       RETURN_FALSE;
+                       RETURN_NULL();
                }
        }
 }



Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2005-12-29 09:14 UTC] tony2001 at phpclub dot net
What happens if you want to store NULL?
How are you going to distinct between failure and success in this case?
 [2005-12-29 09:41 UTC] mikael at synd dot info
Current behavior is to return FALSE both on error and when key was not found. 

Returning FALSE on failure and NULL on no-result-available feels more natural, at least for functions where a not-found state is not an indication of error. Other extensions (turck-mmcache, mysql, oci8, pdo) I've looked at varies in how they indicate error versus not-found.

Changing this would break interface compat for people using === for comparison though. Any thoughts?
 [2005-12-29 17:02 UTC] stuart at gentoo dot org
Hi,

If you return FALSE for any reason other than FALSE was the value stored using Memcache::set() et al, how can I retrieve a value FALSE legitimately stored in memcached?

I guess it's just my personal style, but I find it very natural to treat NULL as being either an error or 'not found'.  isset() works similiar:

  $myVar = null;
  var_dump(isset($myVar));

isset() returns FALSE if a variable has been set to NULL, even though the variable does actually exist.

It'd be great to have some form of errorCode or errorMessage method, so that real errors can be quickly diagnosed and corrected in production environments, but maybe that's a different issue? :)

=== b/c break can be solved by releasing Memcache 2.1.0?

Best regards,
Stu
 [2006-01-16 17:34 UTC] xing at mac dot com
my 2 cents. 

I like to use === FALSE better as it is much more, natural, to me. I do not believe any function should return NULL to represent FALSE/not-found since they are so different. Then again, we see this all the time in external php libraries where developers sometimes use NULL and other times use FALSE. 

In the end, it doesn't matter as long as the devels stay consistent.

As far as getting the distinction between "not found" and "error", I say that in future memcahced versions, exceptions are thrown for errors and let the developer deal with the error case.
 [2006-09-20 05:43 UTC] kamel at vpod dot tv
An alternative solution is to modify the memcache_get function to take a third parameters (reference) where the value will be stored.
I've implemented it and it works nicely, now i'm able to store  NULL and FALSE values.

Example usage code:
if (memcache_get($m, $key, $val))
{
  print "Got it: $val\n";
}
else
{
  print "Not found :-(\n";
}

If someone is interested i can post a patch for memcache-2.0.4
 [2006-09-20 08:41 UTC] mikael at synd dot info
For the users who need to separate FALSE, NULL and not-found/error, it can be implemented in userspace by always serializing the value before calling Memcache::set()

class ExtendedMemcache extends Memcache {
  function get($key) {
    if (!($buffer = parent::get($key)))
      return false;
    return unserialize($buffer);
  }

  function getInto($key, &$value) {
    if (!($buffer = parent::get($key)))
      return false;
    return $value = unserialize($buffer);
  }

  function set($key, $value) {
    return parent::set($key, serialize($value));
  }
}

As to separating not-found from hard errors, cache clients should treat errors as if the key wasn't non-found and fall back to secondary (persistent, database, ..) storage anyway, with error reporting performed internally through the regular trigger_error(). Closing the bugreport.

//Mikael
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Fri Mar 29 08:01:27 2024 UTC