db_result should return FALSE if no result was found.

update doxygen comments

Also, I consider db_* just a wrapper against mysql_*/pgsql_* and mysql_result never returns NULL so we should not either.

also... the API change is extremely small as if(db_result()) does not change. you need to put the results in an array and check with isset -- but then this is the desired.

I don't understand why this is considered a bug.

The reason for this patch ...
Otherwise it returns NULL which is too ambiguous and makes many comparisons harder.
... doesn't seem to fly. With the patch, we'd be able to say:
When it returns FALSE, it is too ambiguous and makes many comparisons harder.

The problem is that when you store these in an array, because you will store NULLs and require the slow array_key_exists to be used.

But, as i said, this makes the db_result inconsistent with its PHP origins which is also a wrong.

Right. The reasons for this patch are 1) consistency, 2) performance. Getting FALSE back from this function will have a direct effect on how efficient we can make drupal_lookup_path.

With 81 calls to db_result() in Drupal, I fear that something which depends on db_result returning NULL will break in some subtle way. Same applies to contributions. While the change does make sense, I don't want to make subtle API changes at this point in the release process.

I read each and every === and !== in core -- they are very rare, especially if you take out the strpos related ones, 11 and 9 -- and none of them is related to a strpos.

Once again: you need a _strict typed check_ to make this any difference. Who uses strict type checks on database data?

obviously the above should read: "and none of them relates to a db_result".

A similar audit of modules tagged with DRUPAL-5 would be possible, Neil, if you ask for it. I think it is most important to get it right in core... especially so close to the heart of core, our database layer, and then document it and let contrib sort itself out.

This also matters for more tests than === or !==, like isset(), as #97317 has demonstrated.

I do think this should happen, but am not sure if it is something for 5.0 or 6. I don't want to waste contrib module authors' time as they "sort it out." I would expect this sort of thing to be rather annoying to debug if it caused a problem.

I'll leave this as RTC so other committers may see this and help decide when this should happen.

The reasons for this patch are 1) consistency, 2) performance. Getting FALSE back from this function will have a direct effect on how efficient we can make drupal_lookup_path.

So far, the only valid reason for this patch is 'better performance', not? If this is about performance, we should back up our claims and measure the gain. I'm not convinced it will be measurable, in which case we want to postpone this patch. If the gain is significant (>= 2%), I suggest we go ahead and commit this.

I was thinking this patch was good because of consistency with low-level database APIs.

I was thinking this patch was good because of consistency with low-level database APIs.

Which low-level database API. I don't think many low-level database APIs use mixed value returns, but I might be wrong. Url?

http://de.php.net/manual/en/function.mysql-result.php

Return Values

The contents of one cell from a MySQL result set on success, or FALSE on failure.

Thanks for the URL. I'm convinced now -- the patch can go in. :)

Committed to CVS HEAD. Thanks.