Make WordPress Core

Opened 8 months ago

Last modified 2 months ago

#42151 new enhancement

Create a filter to add information to query data saved when SAVEQUERIES is true

Reported by: CrazyJaco Owned by:
Milestone: 5.0 Priority: normal
Severity: normal Version:
Component: Database Keywords: has-patch
Focuses: Cc:


When SAVEQUERIES is set to true in wp-config.php, an array gets created and all queries to the database get logged to that array with the query, elapsed time, and stack trace.

It would be nice if there was a filter here to add other elements to this array on a per query basis.

For instance, if I were using a plugin like ElasticPress, I would like to use this filter to add in a value that might say "Query served by ElasticPress" or something to that affect.

Attachments (4)

42151.diff (681 bytes) - added by CrazyJaco 8 months ago.
First pass.
42151.2.diff (826 bytes) - added by CrazyJaco 8 months ago.
Second pass
42151.3.diff (1.2 KB) - added by CrazyJaco 4 months ago.
3rd pass. Added public helper method
42151.4.diff (1.2 KB) - added by CrazyJaco 4 months ago.
Fixing a bug.

Download all attachments as: .zip

Change History (17)

#1 @CrazyJaco
8 months ago

I will have a patch for this shortly.

8 months ago

First pass.

#2 @CrazyJaco
8 months ago

  • Keywords has-patch added

#3 @johnbillion
8 months ago

  • Component changed from Query to Database
  • Keywords 2nd-opinion added
  • Version trunk deleted

Thanks for the patch, @CrazyJaco.

I think this is a bit of a risky change. The idea behind the saved queries and the associated debugging data is that a debugging plugin can make assumptions about what data is present in the wpdb::$queries property. If a filter is added here, then that (somewhat weak) contract is broken.

I wonder if it would be better to add an extra element to the stored query data, the value of which is a filterable array of "extra" and arbitrary information relating to the query. That way a debugging plugin can use an expose that information as it wishes, but without the need to worry about which data is present in the stored queries and in what order.

Related: #41956

#4 @CrazyJaco
8 months ago

@johnbillion Thank you for the reply! I was thinking about this last night and came to a similar realization. If two plugins added an element each to the array, they wouldn't know which element in the array was necessarily their piece of data since it is not an associative array. Changing it to one would be a breaking change so that is out of the question.

Are you suggestion something like this:

QueryData[0] => query syntax
QueryData[1] => elapsed time,
QueryData[2] => stacktrace,
Querydata[3] => apply_filter( Array( 
      custom_meta_key_1 => custom_meta_value_1,
      custom_meta_key_2 => custom_meta_value_2

so that QueryData[3] is just set to an empty filterable associative array?

#5 @johnbillion
8 months ago

  • Keywords needs-patch added; has-patch removed

Yeah I think that makes the most sense. A note in the documentation for the filter could be added which recommends that associative keys are used in the array.

8 months ago

Second pass

#6 @CrazyJaco
8 months ago

  • Keywords has-patch added; needs-patch removed

@johnbillion Added an updated patch. Let me know what you think.

Testing this with a test plugin works as I would expect, except for the first query that gets run, but I think that query might fire before plugins are loaded. All other queries show the new meta data added by my test plugin.

This is the first query: SELECT option_name, option_value FROM wp_options WHERE autoload = 'yes'

#7 @johnbillion
8 months ago

A plugin in mu-plugins should be able to catch every query. Patch looks good, I'll review properly a bit later.

#8 @CrazyJaco
8 months ago

Thank you for your time and attention!

#9 @CrazyJaco
7 months ago

Hey @johnbillion, just checking in to see if you have had a moment to review this patch/ticket properly.


#10 @CrazyJaco
4 months ago

  • Keywords needs-refresh added; 2nd-opinion has-patch removed

This ticket has sat for a while and after some thought I think the call to log a query should be extracted into a method in the class so it can be called elsewhere.

The way it currently works, you can't really log a query that goes to an external source (ElasticSearch) outside of the wordpress database and never makes it this far in the query call stack.

I will update the patch for feedback.

4 months ago

3rd pass. Added public helper method

#11 @CrazyJaco
4 months ago

  • Keywords has-patch added; needs-refresh removed

Uploaded new patch.

4 months ago

Fixing a bug.

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

2 months ago

#13 @SergeyBiryukov
2 months ago

  • Milestone changed from Awaiting Review to 5.0
Note: See TracTickets for help on using tickets.