Accept FALSE as valid value from "pre_option_" hook

[martin.krcho]

It would be great if you could change the code calling the "pre_option_" hook to accept FALSE as a valid option value returned from the hook. Could this be changed to use a return value of NULL as an indicator to continue further down the get_option function? I am implementing a file-based caching solution for a bunch of options that never change. I have over 50 plug-ins on my site (yes, I need/use all of them) and even loading a simple post page cached using W3 Total Cache plugin makes 50 database calls (all options calls). I would like to avoid these because the whole point of the caching plugin is to minimise the use of server resources. Thanks in advance for consideration and perhaps some other ideas.

[SergeyBiryukov]

Related: #31047

[martin.krcho]

It is slightly related to #31047. I guess what I'm trying to say is that the pre_option_ filter does not support options with values of FALSE. Unfortunately, I have plenty of those and I would like to use the filter to short-circuit the retrieval of the option values. Is there any chance that this change could make it into the next release?

[ashfame]

@martin I remember running into this problem an year ago. Using a persistent object caching backend like memcache solves this right away or even W3TC's file based object caching for that matter. But you are right, we should be able to set false as the default and I can only imagine this causing an issue for someone who is actually setting the default as NULL by filter to avoid the DB query.

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?

Till someone else chimes in, I would mention that we wrote a plugin that did the job for us, before we starting using memcache, by saving the options which don't exist yet. When they do get saved, we filter them out from logic. It looked like it was working fine but never tested it for long to see any possible issues that might come out of it. You can check it out if you are interested - ​https://gist.github.com/ashfame/04f6243f0af27e3aee76 (I suggest you consider it as a starting point more than anything)

Also, if you are feeling a bit experimental, you can try dropping a custom object-cache.php in wp-content directory which selectively save & retrieve notoptions in a single option and rest acts non-persistently.

[martin.krcho]

@ashfame, thank you for your comments. I am using W3TC's caching, but not for the objects. However, I would like to filter the options I cache and the W3TC doesn't give me that option. I still hope the "pre_option_" filter will change. I noticed that the get_{type}_metadata filter is short-circuiting the method execution for non-NULL values. What is the best way to make this a high priority?

[boonebgorges]

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.

[ashfame]

@boonebgorges Created #31094 for attempting to cache notoptions inside database when a persistent object caching backend is not in use.

[martin.krcho]

@boonebgorges, I understand the implications of this change. However, I still need to figure out a way around this. Let's try this from a different angle: how would I go about caching options values that are not autoloaded and can return false as a valid option value? Is there anything I am missing or is it simply not possible using current latest WordPress version? Thanks

[boonebgorges]

how would I go about caching options values that are not autoloaded and can return false as a valid option value? Is there anything I am missing or is it simply not possible using current latest WordPress version?

It's not possible to do it in a reliable way. The best I can suggest is that you cache an empty string (or even a string 'false'), and convert it to false as necessary where it's being consumed. I understand that this is pretty inconvenient and ugly. But our options system is designed around a MySQL key-value store, and boolean false is translated to an empty string when it's stored in the database like this. The fact that you can put a boolean directly into the persistent cache without having it converted to a string is sort of an undocumented feature.

Long term, we might consider augmenting the options/meta schema so as to store the data type as well: a column 'type' that stores 'int', 'bool', etc. Then functions like get_option() etc would have a $strict parameter, and our cache implementation would be type-sensitive.

[martin.krcho]

I can easily store false as an empty string or string 'false', but my problem is that all the consumers are third party plugins. I have no idea how returning something other than false will affect their behaviour. I thought about adding the option value of FALSE into WordPress cache in the 'pre_option_' hook, but the cache value is also checked against FALSE further down the 'get_option'. I am not sure what to do here. I'll probably have to start changing code of third party plugins to make their options to autoload and contact the authors to sneak the change into their next release. That should be fun :(

[boonebgorges]

Can you point to an example of a publicly available plugin that expects false as a valid response from get_option()? I ask because it would almost certainly mean that the plugin has a bug in it - false === get_option( 'foo' ) will only work if get_option( 'foo' ) hits an error (when a persistent cache is not in use).

[martin.krcho]

I had a look at all the options that are causing the extra database queries and realized that I need to start caching the "notoptions" list as well as the options that are not autoloaded. Please accept my apologies for the confusion.

It would be nice to have some API to cache certain options between requests. I have a bunch of settings in themes and plugins that are not set (and I will probably never set/enable them) that are causing my system to make about 20 unnecessary database queries on every page load. When you add it all up, I end up with circa 500 MySQL requests per second.

Any ideas?

[boonebgorges]

It would be nice to have some API to cache certain options between requests. I have a bunch of settings in themes and plugins that are not set (and I will probably never set/enable them) that are causing my system to make about 20 unnecessary database queries on every page load. When you add it all up, I end up with circa 500 MySQL requests per second.

This is the purpose of a persistent object cache, like APC, Memcached, or Redis. Here's some background on using APC with WordPress: ​https://markjaquith.wordpress.com/2010/08/06/apc-object-cache-backend-for-wordpress/

Based on this discussion, I'm going to mark this as wontfix. Maybe at some point in the future we can talk about augmenting our key-value stores with information about data types, which would make it possible to do strict type checks in our options functions.

[martin.krcho]

It turned out, after hours of debugging and investigation, that I probably have an issue with using W3TC + memcached for object caching. Apologies for the confusing ticket.

[ocean90]

#33352 was marked as a duplicate.