Make WordPress Core

Opened 2 years ago

Last modified 19 months ago

#54642 new enhancement

Add prepared query builder support for `$wpdb` to build prepared queries across multiple location.

Reported by: vedjain's profile vedjain Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Database Keywords: has-patch
Focuses: coding-standards Cc:

Description (last modified by SergeyBiryukov)

This requests scopes adding a query builder like API to WordPress, so that queries can be built across multiple functions, without needing to disable any PHPCS sniffs.

Currently, if the query is not completely being built nearby, developers will eventually have to disable phpcs sniff, since they would need to combine or interpolate queries. Some examples of this are present in WP core as well, such as in the meta query, taxonomy etc. Although WP does a great job generally to avoid disabling phpcs, in the general plugin ecosystem, these examples are more prevalent.

Note that most of the examples of disabling phpcs can be refactored to not needing to disable. However, as far as I can tell, building a query this way (across multiple functions/places) is not considered a bad practice, if this assumption is correct then we should support doing it better.

This eventual disabling of PHPCS is risky and removes a major and effective protective layer.

Solution

The solution for WordPress would be to provide an API where developers can build a prepared query in steps, and join them before executing in a safe manner.

I looked at query builders for various ORMs such as Doctrine (Symphony), ActiveRecord (Rails), Laravel etc. While such extensive APIs would likely be overkill for WordPress, one possible solution is this hopefully simpler API that I am proposing in this PR 2062.

This PR adds a new class called WP_DB_Partial_Query which implements __toString() magic method so that it can be interpolated in sprintf calls. The objects of this class will represent a partial query and a new method $wpdb->prepare_partial() which will create and return these objects.

This $wpdb->prepare_partial() method internally calls $wpdb->prepare() and takes exactly the same arguments. This PR also introduces a new placeholder %q (although this isn't necessary and %s can be used as well) which can be used to pass these prepared partial queries. I have added some examples to use the API in this PR on my fork.

Note that this is only a PoC and implementation is not complete. Specifically, along with tests, a major component missing is to match against %q placeholders and quote the args if they are not objects of the WP_DB_Partial_Query class.

I can polish the PoC more to get in an acceptable state if this is a direction that WordPress is interested to explore at all and there are no obvious problems with the general approach that I have missed.

Change History (7)

This ticket was mentioned in PR #2062 on WordPress/wordpress-develop by vedanshujain.


2 years ago
#1

  • Keywords has-patch added

Using methods $wpdb->prepare_partial(), which takes exactly the same arguments as $wpdb->prepare() partial prepared queries can be build and supported. More detailes in the Trac ticket.

Trac ticket: 54642

vedanshujain commented on PR #2062:


2 years ago
#3

Thanks @costdev, applied the suggestions in https://github.com/WordPress/wordpress-develop/pull/2062/commits/1ae11e202cfbe287c687682deed683cef814868e.

I assume that there are no @since annotations as this is still PoC

Yup that's correct, I will add along with more checks (for %q placeholder verifications) and tests if the suggestion is accepted.

#4 @SergeyBiryukov
2 years ago

  • Description modified (diff)

This ticket was mentioned in Slack in #core by vedjain. View the logs.


23 months ago

#6 @SergeyBiryukov
23 months ago

  • Milestone changed from Awaiting Review to 6.1

Thanks for the ticket! Moving to 6.1 to get some feedback on it.

#7 @davidbaumwald
19 months ago

  • Milestone changed from 6.1 to Future Release

With 6.1 Beta 1 releasing tomorrow, this is being moved to Future Release. If any maintainer/committer wishes to assume ownership of this ticker during a specific cycle, feel free to update the milestone accordingly.

Note: See TracTickets for help on using tickets.