LEFT JOIN in 2.7 to JOIN in 2.8; Why?

[Mike Schinkel-5]

Question:

Does anyone know why the change was made in 2.8 on line 2141 of /wp-includes/query.php to this:

$join .= " JOIN $wpdb->postmeta ON ($wpdb->posts.ID = $wpdb->postmeta.post_id) ";

When in 2.7 on line 2136 of /wp-includes/query.php it was this?

$join .= " LEFT JOIN $wpdb->postmeta ON ($wpdb->posts.ID = $wpdb->postmeta.post_id) ";

I found it because it broke a plugin of mine where I was testing for 'LEFT JOIN ...' in the 'posts_join_paged' hook.

I was able to fix my plugin by removing "LEFT " but I'm wondering:

1.) Why was it changed in the first place? Was there a good reason that's non-obvious to me? If not it might break other's plugins for no good reason.

2.) If no good reason can it be changed back? Testing for "LEFT JOIN ..." is more exact than testing or "JOIN ..."; or example there is a tiny chance I could match a "RIGHT JOIN ... " when I didn't intend to but not if I'm testing explicitly for "LEFT JOIN ..."

Thanks in advance.

-Mike Schinkel
Custom Wordpress Plugins
http://mikeschinkel.com/custom-wordpress-plugins 
_______________________________________________
wp-hackers mailing list
wp-hackers@...
http://lists.automattic.com/mailman/listinfo/wp-hackers

[Austin Matzko]

On Tue, Jun 16, 2009 at 8:33 PM, Mike Schinkel
<mikeschinkel@...> wrote:
> Does anyone know why the change was made in 2.8 on line 2141 of /wp-includes/query.php to this:

According to this ticket, it was done for better performance:
http://core.trac.wordpress.org/ticket/9720

> 2.) If no good reason can it be changed back? Testing for "LEFT JOIN ..." is more exact than testing or "JOIN ..."; or example there is a tiny chance I could match a "RIGHT JOIN ... " when I didn't intend to but not if I'm testing explicitly for "LEFT JOIN ..."

You could use a negative lookbehind, which will match "LEFT JOIN" and
"JOIN", but not "RIGHT JOIN":

preg_match('/(?<!RIGHT )JOIN/', $text)
_______________________________________________
wp-hackers mailing list
wp-hackers@...
http://lists.automattic.com/mailman/listinfo/wp-hackers

[Mike Schinkel-5]

"Austin Matzko" <if.website@...> wrote:
> According to this ticket, it was done for better performance:
> http://core.trac.wordpress.org/ticket/9720

Thanks for finding that.  OTOH, it looks like Ryan reverted it anyway because of other regression bugs:

   http://core.trac.wordpress.org/ticket/9851

> You could use a negative lookbehind, which will match "LEFT JOIN" and "JOIN", but not "RIGHT JOIN":
> preg_match('/(?<!RIGHT )JOIN/', $text)

Thanks for that. I'm still not a regex whiz so unless it's obvious I don't think to use regex.
OTOH, isn't preg_match() significantly slower than strpos()?

Also, for future finders of this thread the follow is what you'd need in hook post_join_paged($join) - note the space after the closing paren, not before:

   preg_match('/(?<!RIGHT) JOIN/', $join)

-Mike Schinkel
Custom Wordpress Plugins
http://mikeschinkel.com/custom-wordpress-plugins
_______________________________________________
wp-hackers mailing list
wp-hackers@...
http://lists.automattic.com/mailman/listinfo/wp-hackers

[Lynne Pope]

Thanks for posting this Mike. I had a similar problem and hadn't isolated
it.

My question (note - "question" not criticism) is why this wasn't in 2.8
final? The change was reverted on 25th May but this does not appear in RC1
or final. Just confused over the build process so would appreciate it if
anyone can explain please. Was this revision not committed to the trunk by
mistake?

Thanks.
Lynne
_______________________________________________
wp-hackers mailing list
wp-hackers@...
http://lists.automattic.com/mailman/listinfo/wp-hackers

[Peter Westwood]

On 17 Jun 2009, at 07:01, Lynne Pope wrote:

> Thanks for posting this Mike. I had a similar problem and hadn't  
> isolated
> it.
>
> My question (note - "question" not criticism) is why this wasn't in  
> 2.8
> final? The change was reverted on 25th May but this does not appear  
> in RC1
> or final. Just confused over the build process so would appreciate  
> it if
> anyone can explain please. Was this revision not committed to the  
> trunk by
> mistake?

Not sure what you mean here.

The original change in [11259] changed 4 instanced of LEFT JOIN to  
just JOIN.

On the 25th of May in [11452] one of the changes was reverted but the  
rest were kept.

The change that was reverted was because of the issue it caused with  
custom taxonomies - #9851

The packages that are made available for download are built from svn  
so this change would have been included.

Looking at the RC1 zipfile I can see this single LEFT JOIN in wp-
includes/query.php:

                        if ( $post_status_join ) {
                                $join .= " LEFT JOIN $wpdb->posts AS p2 ON ($wpdb-
 >posts.post_parent = p2.ID) ";
                                foreach ( $statuswheres as $index => $statuswhere )
                                        $statuswheres[$index] = "($statuswhere OR ($wpdb-
 >posts.post_status = 'inherit' AND " . str_replace($wpdb->posts,  
'p2', $statuswhere) . "))";
                        }

westi
--
Peter Westwood
http://blog.ftwr.co.uk | http://westi.wordpress.com
C53C F8FC 8796 8508 88D6 C950 54F4 5DCD A834 01C5

_______________________________________________
wp-hackers mailing list
wp-hackers@...
http://lists.automattic.com/mailman/listinfo/wp-hackers

[Lynne Pope]

2009/6/17 Peter Westwood <peter.westwood@...>

>
>  The original change in [11259] changed 4 instanced of LEFT JOIN to just
> JOIN.
>
> On the 25th of May in [11452] one of the changes was reverted but the rest
> were kept.
>
> The change that was reverted was because of the issue it caused with custom
> taxonomies - #9851
>
> The packages that are made available for download are built from svn so
> this change would have been included.
>
> Looking at the RC1 zipfile I can see this single LEFT JOIN in
> wp-includes/query.php:
>
>                        if ( $post_status_join ) {
>                                $join .= " LEFT JOIN $wpdb->posts AS p2 ON
> ($wpdb->posts.post_parent = p2.ID) ";
>                                foreach ( $statuswheres as $index =>
> $statuswhere )
>                                        $statuswheres[$index] =
> "($statuswhere OR ($wpdb->posts.post_status = 'inherit' AND " .
> str_replace($wpdb->posts, 'p2', $statuswhere) . "))";
>                        }
>
>

Yep, you are completely right Westi - my bad (doing diffs on wrong lines -
doh!).
The LEFT JOIN has not been reverted in postmeta queries.

That's me back to the drawing board ;)

Cheers,
Lynne
_______________________________________________
wp-hackers mailing list
wp-hackers@...
http://lists.automattic.com/mailman/listinfo/wp-hackers