get_post_meta doesn't return an array when requested if nothing with that key exists.

[westi]

e.g. get_post_meta($this->post_id, 'unique', false)

Where no metadata for key 'unique' exist on the post returns '' instead of array('');

[westi]

Discussed in dev-chat today.

Should return array() in this case.

[westi]

(In [11948]) Return correct results for both single and array cases. Fixes #10803.

[Denis-de-Bernardy]

I might be understanding this patch incorrectly, but it seems to me that this fix prevents it from distinguishing between a post meta that does not exist and one that has yet to be generated.

until 2.8, it's possible to do:

$children = get_post_meta($post_id, _children_cache);
if ( $children === '' )
  $children = cache_children($post_id); // might now contain array()
foreach ( $children as $child ) ...

if get_post_meta() returns an empty array instead, the above code no longer works.

since we're going to break things as a result, could we not make the mess return false instead?

[hakre]

or the ability to inject the default value on function call? then the calling code can configure the functions output in this case as needed. should make things more easy in any case.

[Denis-de-Bernardy]

that would work too, yeah. leaving by default, to avoid breaking things.

[azaozz]

Yes, having a default return value that can be changed would make it more flexible like get_option(). Perhaps the same is needed for get_usermeta().

[westi]

-1 to adding a default argument.

We don't need this - the function already had clear defined behaviour and it didn't conform to it.

The example code shared above relied on a bug in the code.

You asked the function to return you an array of results and it didn't instead it returned you the result you would have got for a request of a single value if one had not been set.

[Denis-de-Bernardy]

Nonetheless, after a certain amount of time bugs tend to become features, and this is a regression.

I don't mind the idea of breaking plugins, but if we do break things my opinion is that we should fix things properly. By properly, I mean make the meta functions return false, just like get_option() and get_transient(), instead of a potentially valid value, when the meta is not found.

[azaozz]

Having a default value is a 'convenience' for APIs that store and retrieve something. It usually saves one conditional. Instead of doing:

$my_meta = get_post_meta($post_id, 'my_field', true);
if ( empty($my_meta) )
  $my_meta = 'default-data';

we can do:

$my_meta = get_post_meta($post_id, 'my_field', true, 'default-data');

[westi]

Replying to Denis-de-Bernardy:

Nonetheless, after a certain amount of time bugs tend to become features, and this is a regression.

In some cases yes. In this case I disagree - the bug is clear and obvious and should be fixed so that the function respects its arguments.

Replying to azaozz:

Having a default value is a 'convenience' for APIs that store and retrieve something. It usually saves one conditional. Instead of doing:

Agreed, they are a convenience although I'm not sure they are useful for 80% of the uses of *_meta where we are storing meta data about a post.

Anyway - they are the subject for a new ticket rather than re-using a specific bug ticket which has been fixed.

[dd32]

I'm still failing to see how this breaks anything, It now abides by what it was supposed to do.

Furthurmore, Peoples plugins SHOULDNT break because of this.

If you rely upon a false return from the function, as long as you havnt used typechecking, it still works, false == , false == array(), empty(before()) == empty(after()), The logic stays the same, With the benefit that people can now safely use the function directly in themes (As it was originally intended IIRC).