WordPress.org

Make WordPress Core

Opened 7 months ago

Last modified 2 months ago

#41266 new enhancement

Not hard coding the table alias prefix in WP_Meta_Query would make class more extendable

Reported by: thomaslhotta Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.8
Component: Query Keywords: good-first-bug has-patch has-unit-tests
Focuses: Cc:

Description

Hi,

I am extending the WP_Meta_Query class for my own uses with custom tables that have the same structure as meta tables. It is an incredibly useful class for this. However the fact that the table alias prefix mt on line 535 is hard coded forces me to do awkward and complex string replaces if I want to use my extensions alongside built in meta queries.

I think changing the prefix from being hard coded to a property/function of the meta query object would make this class much more versatile for plugin developers.

Attachments (3)

41266-approach-1.diff (1.3 KB) - added by andizer 6 months ago.
41266-approach-2.diff (653 bytes) - added by andizer 6 months ago.
41266.patch (2.3 KB) - added by noisysocks 2 months ago.
A patch based on Approach 1 that includes unit tests

Download all attachments as: .zip

Change History (10)

#1 @boonebgorges
6 months ago

  • Keywords needs-patch good-first-bug added

I agree. This, and the lack of flexibility for the $meta_table name itself (in get_sql()), are long term annoyances for me. Could you suggest an approach in a patch?

#2 @andizer
6 months ago

I've added two patches both containing a different approach. The first patch works with a property having a default value being mt and a setter for overriding that value. The second patch works with a hook to override the default value.

If one of those patches will be merged, please also gives props to ireneyoast

#3 @janw.oostendorp
5 months ago

  • Keywords has-patch needs-unit-tests added; needs-patch removed

#4 follow-up: @ryotsun
3 months ago

@janw.oostendorp
Hello, it's just a quick question.
Which ticket are you going to choose?

#5 in reply to: ↑ 4 ; follow-up: @janw.oostendorp
3 months ago

Replying to ryotsun:

@janw.oostendorp
Hello, it's just a quick question.
Which ticket are you going to choose?

I don't understand what you are asking. Did I miss something?

#6 in reply to: ↑ 5 @ryotsun
3 months ago

Ahhh, I'm sorry.
I wanted to say "approach". Not "ticket".

2 patches are uploaded, so I just wanna know which one would be chosen.

Replying to janw.oostendorp:

Replying to ryotsun:

@janw.oostendorp
Hello, it's just a quick question.
Which ticket are you going to choose?

I don't understand what you are asking. Did I miss something?

@noisysocks
2 months ago

A patch based on Approach 1 that includes unit tests

#7 @noisysocks
2 months ago

  • Keywords has-unit-tests added; needs-unit-tests removed

Personally, I think Approach 1 is the cleaner solution. WP_Meta_Query is a class and we may as well take advantage of an object oriented approach to solving this problem.

I've added a unit test to 41266-approach-1.diff and uploaded it as 41266.diff.

Note: See TracTickets for help on using tickets.