post__in and other array_key can't manage empty array properly

[lifeless85]

There's a conceptual mysfunction in
​http://core.trac.wordpress.org/browser/trunk/wp-includes/query.php#L1723
if we want posts that are included in an empty array we should have obviusly no posts.
Instead by adding conditions to the query, if we have an empty array we add no conditions and the query is still the same ...
i think it's a major issue.

[lifeless85]

i think a good soludion should be changing

elseif ( $q['post__in'] ) {
	$post__in = implode(',', array_map( 'absint', $q['post__in'] ));
	$where .= " AND {$wpdb->posts}.ID IN ($post__in)";
}

to

elseif ( $q['post__in'] ) {
	if ( empty( array_map( 'absint', $q['post__in'] ))){
		$where .= " AND {$wpdb->posts}.ID = NULL
	}
	else {
		$post__in = implode(',', array_map( 'absint', $q['post__in'] ));
		$where .= " AND {$wpdb->posts}.ID IN ($post__in)";
	}
}

[lifeless85]

Opz i've left out some code...

$where .= " AND {$wpdb->posts}.ID = NULL";

[chrisscott]

The problem with specifying an empty array for post__in is that fill_query_vars() sets $q['post__in'] to an empty array if it isn't specified in the query so the elseif statement never evals to true.

It seems to me like this doesn't need to be changed and instead you should be handling passing an empty array to posts__in in your code that calls the query.

[lifeless85]

Replying to chrisscott:
Ok, so this is not a solution, but, i think that handling this in my teplate / plugin / widget leads to misunderstanding.
You can't just tell me it's hard to fix so it's right... :)
Remember code is poetry...
I'll be thinking about this ...

[scribu]

You can use $wp_query->set_404();

[lifeless85]

Replying to scribu:

You can use $wp_query->set_404();

yes, tnx for your advice!

But i still need to know if the array is empty due to fill_query_vars() or if it was setted and empty, before to use $wp_query->set_404();

Maybe we can use it inside of fill_query_vars(), what's the point of filling query vars when you alredy know it will be no results ... i'll do a test in my test machine ...

[lifeless85]

first analysis:
i could set query_vars to NULL instead of empty array in the fill_query_vars() function, if they are alredy setted to empty array.

And then in the get_posts() function change the code to looks like

elseif ( $q['post__in'] === NULL) {
	$this->set_404();
} elseif ( $q['post__in'] ) {
	...
	...
}

but for some reason, inside fill_query_vars all the query_var inside of $array_keys is already setted to an empty array ... i don't get why they are already setted to empty array, if fill_query_vars is supposed to do so ... somebody can explain me this ?

[dd32]

The query var is being filled with "empty", The field in question is designed to query posts based on a specified list, If the list is empty, then its not being used.

Instead, You need to not call get_posts() or whatever if you know you dont want to retrieve posts.

If you're hooking into WP_Query for the main post request, you need to modify the query earlier to not run most likely.

If you need any help on those, i'd suggest the wp-hackers mailing list

[lifeless85]

no no no ... you didn't get the point...
assume you have this behaviour:

$sticky=get_option('sticky_posts');
$args=array(
	'showposts'=>2,
	'post__in' => $sticky,
	'caller_get_posts' => 1,
	'orderby'=>'meta_value',
	'meta_key'=>'from',
	'order'=>'ASC'
);
include('so-sidebar.php');
$args=array(
	'showposts'=>2,
	'post__not_in' => $sticky,
	'caller_get_posts' => 1,
	'orderby'=>'meta_value',
	'meta_key'=>'from',
	'order'=>'ASC'
);
include('so-sidebar.php');

and inside so-sidebar.php you have a custom loop like this:

<?
$so_query = new WP_Query($args);
if ($so_query->have_posts()) :
	while ($so_query->have_posts()) :
		$so_query->the_post();
?>
<div <?php post_class(); ?> id="post-<?php the_ID(); ?>">
	<h2><a href="<?php the_permalink() ?>"><?php the_title(); ?></a></h2>
	<div class="entry"><?php the_excerpt();?></div>
</div>
<?php
	endwhile;
endif;
?>

OK, now i guess you'll be asking yourself:

why all of this !?!?

The answer is clear.
Because if you use 'orderby'=>'meta_value' to sort your post, you can't have all stiky posts at the top (and sorted too).
So ... this is the frustrating way, i do this.

Now, this is bad, because yes i could do this in my theme:

$sticky=get_option('sticky_posts');
if( empty($stiky) ) {
	//do all the math here
}

But, this is like saing SELECT ALL ITEMS FROM AN EMPTY GROUP the result should be another EMPTY GROUP and now this is not true, this is like swearing against the LOGIC and MATH god who rule us all. you are telling me the same thing chrisscott told me before, this is an hard work, it would be easier to fix it your theme ... not exactly the kind of thing i thought i would have found here...

[dd32]

I personally believe it's cleaner for the theme to check before it mindlessly queries for data, esp. if you've got a list that you're wanting to know about.

But, this is like saing SELECT ALL ITEMS FROM AN EMPTY GROUP the result should be another EMPTY GROUP and now this is not true

not exactly the kind of thing i thought i would have found here...

What i'm trying to explain, Is the way that WordPress uses internal query parameters, post__in is designed for a specific purpose, To query the posts provided. Thats it, And while i understand your line of thinking "But its an empty list!, So i dont want anything returned",

If we change that particular query param, here's another dozen which would have to be changed to make them act the same: ​http://core.trac.wordpress.org/browser/trunk/wp-includes/query.php#L1191 Consistancy between parameters is crucial if its to be memorised by developers, the worst thing is to have to look up "How does this param take its arguements"

I have seen many themes prioritise the sticky posts to the top of the pile, and even display them in a seperate block.

Because if you use 'orderby'=>'meta_value' to sort your post, you can't have all stiky posts at the top (and sorted too). So ... this is the frustrating way, i do this.

Then it might be worth filing a bug on that specifically, That the sticky posts do not abide by the ordering? ​http://core.trac.wordpress.org/browser/trunk/wp-includes/query.php#L2329

[lifeless85]

Ok, i'm insisting on this because, i've alredy discussed on the forum about the other bug months ago, they told me not to fill a bug, told me it was better to manage that by myself on my theme, now i think i'll propose to fix that other bug too.

I belive that even if it's surely convinient now to manage that by myself, this is some kind of thing that will make developer crazy.

So why we have theme tag, custom functions and this kind of thing ? if we think people can manage everything by themself they did not need wordpress at all...

this is not a problem of mine, this is a problem on wordpress code, i alredy know how to work around on this.

There's not a dozen other parameter, the parameter that would be afflicted by this are four!!

category__in, post__in, tag__in, tag_slug__in and the cure is something like a couple of if ...

[lifeless85]

i've managed to write a simple and in my opinion acceptable solution:
changing the '''function''' fill_query_vars() in the following way:

foreach ( $array_keys as $key ) {
	if ( !isset($array[$key])) {
		if( $key == 'post__in' ){ continue; }
		$array[$key] = array();
	}
}

Notice that we should also change category__in, tag__in, tag_slug__in
And managing the new meaning of empty array inside of '''function''' &get_posts() like this :

  elseif ( is_array($q['post__in']) && empty($q['post__in']) ) {
	$where = " AND {$wpdb->posts}.ID = NULL";
	$this->set_404();
} elseif ( $q['post__in'] ) {
	$post__in = implode(',', array_map( 'absint', $q['post__in'] ));
	$where .= " AND {$wpdb->posts}.ID IN ($post__in)";
}

[lifeless85]

sorry for triple quote around the function word i wanted that in bold but i forgot about the effects of triple parentesis...

[lifeless85]

Ah! i also wanted to say, that in my opinion we should not initialize $array_keys to empty array because if we test it later by using elseif ( $q['post__in'] ), an empty array evals to false and an unset variable too.

So we should not initialize them and get advantages in the empty array meaning ...

[scribu]

I think a better solution would be for fill_query_vars() to fill postin with 'false' or 'null' rather than not setting it at all.

[lifeless85]

ok, can you also explain why ?
what about the other three query vals ? i see you stripped them from the tags, but to me looks like they suffer from same problem ...
I see 2.9.2 has been released ... sorry for setting that milestone... now that we need a patch, how can i help ? i can write it ? i should post it here or i should use some kind of svn ?

[lifeless85]

P.s. Setting a variable to null in PHP is equivalent to having it undeclared ...
​http://it.php.net/manual/en/language.types.null.php#language.types.null.syntax
and setting it to false could lead to other problems, maybe not here where we expect arrays.

But if this practice hopefully get extended to others query vars, setting a variable to false where we expect for example a boolean, is a very bad practice, we should use isset($var) instead of a simple if($var) to distinguish if it's false because it's undefined or it's false because we wanted it to be false ...

So i exclude false would be a good idea, and null is pointless ...

[scribu]

now that we need a patch, how can i help ? i can write it ? i should post it here or i should use some kind of svn ?

Read this:

​http://core.trac.wordpress.org/wiki#HowtoSubmitPatches

[lifeless85]

i'm not sure comments are appropriate in a patch ... but, hey i'm new to svn! always worked alone, so any advice on svn is greatly appreciated.

[scribu]

The WordPress codebase might enable conversation, but it's not meant to be a conversation itself. :)

Please remove the comments from your patch.

[lifeless85]

a patch to fix postin bug

[lifeless85]

tested query.php.diff on wp 2.9.2

also removed the comments from the patch.

and ... if i can ask ... how can i test this on trunk if trunk does not work ?
because i've tested there too, but with or without my patch it does not work so i can't figure out if this breack something else ... should i write this or not ? sorry for being so newbye about cooperation ...

[scribu]

If it doesn't work on trunk, it can't be commited.

Try to figure out what's changed from the 2.9 branch. (I don't have my laptop, so I can't help with testing atm)

[lifeless85]

tested on trunk revision 13172 it works

hope this time my keywords are ok ... xD sorry for making you work a lot

[nacin]

Strongly tempted to wontfix based on other comments in this thread, specifically dd32's comment about A) consistency in arguments, and B) "I personally believe it's cleaner for the theme to check before it mindlessly queries for data"... Moving to future release for now, suggested close.

[wonderboymusic]

No comments in 2 years since it was tagged "close"

[todditron]

I know this has been closed for a decade, but just ran into this as a problem in 2021.

The problem isn't that we don't have a workaround, the problem is that there is no way to anticipate that we need the workaround

In my case, I was developing a custom members site with access-protected course pages, and ended up showing ALL courses to the members without any authorization rather than ZERO courses.

Logic being
Member A - can see posts_in => [1,2] - show at most 2 posts
Member B - can see posts_in => [1] - show at most 1 posts
Member C - can see posts_in => [] - show at most 0 posts

There really wasn't anyway to predict that member C would then see ALL posts