WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#15433 closed defect (bug) (wontfix)

Don't use 'name' query var if 'p' is already set

Reported by: scribu Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Query Keywords:
Focuses: Cc:

Description

Martin Lazarov (in wp-hackers):

Hi all,
may be it's good idea to replace this in wp-includes/query.php:1759
if ( '' != $q['name'] ) {

width this:

if ( '' != $q['name'] && !$q['p']) {

This way the query will not contains post_name='...' if there is post
ID passed. This will make query faster, because it will use only
wp_posts primary key instead of making query by index and table scan.


End the bug that i found is:
some months ago i installed fresh wordpress on the server that doesn't
have php mb_string functions, so the function
sanitize_title_with_dashes (wp-includes/formatting.php:814) just
doesn't do str to lower title values. But week ago i installed the mb
string library for php and the old slugs stop working. That's because
this code (part of wp-includes/formatting.php:814)  now works:

if (function_exists('mb_strtolower')) {
       $title = mb_strtolower($title, 'UTF-8');
}

Adding the patch at the top of this mail solves the problem for me.

Change History (14)

comment:1 scribu4 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [16384]) Ignore 'name' qv if 'p' qv is set. Fixes #15433

comment:2 follow-up: filosofo4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

This seems inconsistent with other current behavior and is somewhat arbitrary.

First, queries typically return results in which all the arguments are all true, except in the case in which they're logically impossible. If you request the posts with category X and tag Y, and there are no posts that have category X and tag Y, you don't get any results. It seems reasonable that the same would apply for the 'name' and 'p' arguments.

Second, what is the situation in which someone would query both by ID and name? It seems too obscure to warrant such an unexpected, illogical exception.

Third, it's not immediately obvious why we should favor ID over name. In fact, most sites with "pretty" permalinks on do the majority of their single-result queries by post_name, and the general trend in WP seems to be away from using IDs as identifiers to using slugs.

comment:3 scribu4 years ago

Second, what is the situation in which someone would query both by ID and name? It seems too obscure to warrant such an unexpected, illogical exception.

For example, when having both in the permalink structure:

/%post_id%/%postname%/

comment:4 scribu4 years ago

Also, it does make sense to favor 'p' over 'name' because the former is more exact.

comment:5 scribu4 years ago

I.e., you can have more than one post with the same name, because there's no UNIQUE constraint on the column.

comment:6 filosofo4 years ago

OK, granting the permalink use, it still doesn't respond to my first point, which I think is the main one. Why should we expect a request of the form

/[an actual id]/[any string, whether slug or not]/

to return a post? It violates the notion of a permalink's one-to-one correspondence.

Especially not for the case when the slug matches a different post from the ID's.

comment:7 follow-up: filosofo4 years ago

To put it another way, if
123 => 'a-post', 456 => 'another-post',

Although from a database perspective the post object is more precise, from a human perspective--which is the main reason for permalinks-as-sanitized-titles--it's not obvious that

/123/another-post/

would return the content for "a-post" and not "another-post." It should at least fail.

comment:8 scribu4 years ago

(In [16385]) Revert [16384]. See #15433

comment:9 scribu4 years ago

  • Resolution set to wontfix
  • Status changed from reopened to closed

This can be handled via filters anyway.

comment:10 scribu4 years ago

  • Milestone 3.1 deleted

comment:11 in reply to: ↑ 7 ; follow-up: mlazarov4 years ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened

Replying to filosofo:

To put it another way, if
123 => 'a-post', 456 => 'another-post',

Although from a database perspective the post object is more precise, from a human perspective--which is the main reason for permalinks-as-sanitized-titles--it's not obvious that

/123/another-post/

would return the content for "a-post" and not "another-post." It should at least fail.

wp will redirect '/123/another-post/' to '/123/a-post/' ?

That's one side of the problem. Please read all the first post. Second part of it describes second problem which now isn't solved.

comment:12 nacin4 years ago

"wp will redirect '/123/another-post/' to '/123/a-post/' ?"

That's a canonical redirect.

comment:13 in reply to: ↑ 2 gazouteast4 years ago

  • Cc gazouteast added

Replying to filosofo:

This seems inconsistent with other current behavior and is somewhat arbitrary.

Third, it's not immediately obvious why we should favor ID over name. In fact, most sites with "pretty" permalinks on do the majority of their single-result queries by post_name, and the general trend in WP seems to be away from using IDs as identifiers to using slugs.

See http://wordpress.org/support/topic/yet-another-category_id-how-to-thread?replies=1 for a real world example of when the ID is needed, not the name. In this case trying to use get_the_category() to get the category ID number, to use it as a variable in another function, such that the other function then returns data pertinent to the category of the post or archive being viewed (in this case from the same numbered Adrotate ad group, and NextGEN Gallery ID.

Because the ID number is constant, whereas the category name (and therefore the slug too) is not (due to being editable in admin), using the ID numbers for cross-function constancy is the only viable solution ... unless you advocate adding another column to the db table to introduce a new unique key?

comment:14 in reply to: ↑ 11 scribu4 years ago

  • Resolution set to wontfix
  • Status changed from reopened to closed

Replying to mlazarov:

That's one side of the problem. Please read all the first post. Second part of it describes second problem which now isn't solved.

I presume you mean the part about mb_strtolower(). This should no longer be a problem in WP 3.1-beta. See #9591

If you are still seeing the issue, feel free to reopen this ticket with steps to reproduce.

Note: See TracTickets for help on using tickets.