[wp-trac] [WordPress Trac] #31061: Accept FALSE as valid value from "pre_option_" hook

[WordPress Trac]

#31061: Accept FALSE as valid value from "pre_option_" hook
--------------------------------+------------------------------
 Reporter:  martin.krcho        |       Owner:
     Type:  enhancement         |      Status:  new
 Priority:  normal              |   Milestone:  Awaiting Review
Component:  Options, Meta APIs  |     Version:  4.1
 Severity:  normal              |  Resolution:
 Keywords:  close               |     Focuses:  performance
--------------------------------+------------------------------
Changes (by boonebgorges):

 * keywords:   => close


Comment:

 It's definitely annoying not to be able to set `false` as the default
 value, but I'm not sure there's a way to change this behavior in a way
 that won't break many plugins. Consider the following pattern:

 {{{
 function wp31061_pre_option_foo( $retval ) {
     if ( is_user_logged_in() ) {
         return 'whatever';
     } else {
         return false;
     }
 }
 add_filter( 'pre_option_foo', 'wp31061_pre_option_foo' );
 }}}

 Changing the existing check to something other than `false` would break
 this kind of thing.

 One mitigating factor here is that it's arguably unwise to use or expect
 boolean values with `get_option()`. Under "normal" circumstances (ie,
 where values are being pulled from the options table via MySQL), the
 return value will be either an object, an array, or a string. `add_option(
 'foo', true )` will result in a value of `'1'` on the way out, and
 `add_option( 'foo', false )` will be `''` coming out. (This is different
 with a persistent cache. See
 https://core.trac.wordpress.org/ticket/22192#comment:10 and the referenced
 unit tests for more of the delightful details.)

 Another way of putting the same point is that, under normal circumstances,
 `get_option()` returns `false` on *error*, and `''` (an empty string) when
 no corresponding value is found in the database. So if your primary goal
 is to filter certain known-to-be-absent options to avoid database hits,
 you should be returning `''`.

 > Another thought, will it make sense to save non-existent queries in a
 single DB option to save extra DB queries just like how we autoload
 options?

 This is worth exploring. I can't believe there's not already a ticket for
 it. We have a 'notoptions', but without a persistent cache, it's only good
 for the current pageload. There would be various
 synchronization/invalidation issues to tackle if we synced these keys to a
 database option, but I don't think they're insurmountable. My inclination
 is to close the current ticket as wontfix, and to open a separate one for
 the possibility of storing 'notoptions' keys in the db.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/31061#comment:5>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform