google-code-export / wordpress-custom-content-type-manager

Automatically exported from code.google.com/p/wordpress-custom-content-type-manager
2 stars 1 forks source link

Additional GetPostsQuery argument to optionally avoid joining on wp_users table #480

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Original discussion here:
http://wordpress.org/support/topic/custom-field-image-select-image-takes-tons-of
-cpu

Possible solution:  (#post-4097167 on the above URL)
Hrm - search_by=true on line 165 of get_posts.php is the trouble.

I wonder if the search_by term could be set by looking at the $search_form_tpl 
variable (ie. parsing the template to see what tags are used (I'm guessing that 
is what the author/etc is being loaded in for - that the template could print 
out the authors if it wanted to?)

Or since the templates are semi-hardcoded in the post_selector/search_forms the 
search_by terms could be hard-coded in the code too? Certainly writing a parser 
that read through the template file that was going to be used, and then only 
query the database for those fields would be the best option, but I don't know 
how hard that is.

Original issue reported on code.google.com by jonda...@gmail.com on 17 Apr 2013 at 3:11

GoogleCodeExporter commented 9 years ago
The problem is that the default query joins on the wp_users table so as to 
include the post author's username.  With a large users table, this join 
becomes prohibitively expensive.  

Recommended action is to include an argument to the GetPostsQuery::get_posts() 
function that can cause the join to be omitted.  

Then, per your issue, use this option in the Ajax requests that power the 
image-selector lightboxes.

Original comment by ever...@fireproofsocks.com on 18 Apr 2013 at 2:28

GoogleCodeExporter commented 9 years ago
Committed revision 706957.  See docs for the new "join" argument: 
https://code.google.com/p/wordpress-custom-content-type-manager/wiki/get_posts

Will be released with 0.9.7.1

Original comment by ever...@fireproofsocks.com on 2 May 2013 at 1:50

GoogleCodeExporter commented 9 years ago
You can edit the config file for each type of relation field and edit the 
"join" parameter there, see: 
https://code.google.com/p/wordpress-custom-content-type-manager/wiki/Config_post
_selector_relation

Original comment by ever...@fireproofsocks.com on 2 May 2013 at 5:31

GoogleCodeExporter commented 9 years ago
Great, I look forward to testing it.  Is the current dev version reasonable to 
run on a production site (ie. how much has changed for this upcoming release?)

Original comment by jonda...@gmail.com on 2 May 2013 at 7:25

GoogleCodeExporter commented 9 years ago
Already released 0.9.7.1.  I can't guarantee anything for any site unless a 
contract is involved.

Original comment by ever...@fireproofsocks.com on 2 May 2013 at 9:42

GoogleCodeExporter commented 9 years ago
Ah, great.  I didn't see that it was already released.

Your customization/overrides stuff looks great, I hadn't realized that system 
existed.

But, I'm having trouble figuring out what file to add.

I have a custom field which is of type "image", and I only want to override 
that field, so the authors aren't looked up for every image that is shown in 
the popup.

I see this in the docs: 
http://code.google.com/p/wordpress-custom-content-type-manager/wiki/Image#Custom
izing_Manager_HTML

But, I think that is not the override I am looking for is it?

Original comment by jonda...@gmail.com on 3 May 2013 at 12:12

GoogleCodeExporter commented 9 years ago
No, you want to click on the 'Customizations' link in the wiki: 
https://code.google.com/p/wordpress-custom-content-type-manager/wiki/Customizati
ons

Specifically, 
https://code.google.com/p/wordpress-custom-content-type-manager/wiki/Config_post
_selector_relation

Add a line to your _image.php file to define the join terms:
CCTM::$post_selector['join'] = 'none';

Original comment by ever...@fireproofsocks.com on 3 May 2013 at 12:35

GoogleCodeExporter commented 9 years ago
Ok, I tried that.

I have a  uploads/cctm/config/post_selector/_image.php that is the same as 
plugins/custom-content-type-manager/config/post_selector/_image.php, but with 
the added join=none line.

But, when I do a print_r($this->search_by) on line 1255 of GetPostsForm.php, it 
returns a whole long list of things, same as before.

Is there a way to make sure that CCTM is seeing my config override file?

I did see a comment in the _image file that says "The available search 
parameters here kick in only if no where no explicit Search Paramters have been 
configured".  My custom field has search parameters on it (though I don't 
remember ever adding them, and I can't figure out how to erase them.  They are: 
"
    post_mime_type: image
    search_columns: Array (post_title, post_content)
    match_rule: contains"
I can remove the post_mime_type rule, but the others appear to be defaults, and 
restore themselves if I clear the fields.

Original comment by jonda...@gmail.com on 3 May 2013 at 1:16

GoogleCodeExporter commented 9 years ago
print_r($this->search_by) wouldn't tell you anything useful I don't think.  
Sorry, right now, I'm completely slammed, so I can't troubleshoot this further 
without actually trying it on your system.  It was just  coincidence that I 
needed to add this functionality for a different purpose entirely, so it might 
be a while before this is cleaned up and documented well enough for this 
particular use case... I may need to add some vars to the config file.

Original comment by ever...@fireproofsocks.com on 3 May 2013 at 1:26

GoogleCodeExporter commented 9 years ago
Ok, I'll keep looking.

The print_r($search_by) is useful, because it is followed by a foreach 
loop, which calls _author() which queries the user table and has an 
additional foreach loop in it, setting up a <select> object.

I wonder if the JOIN you are talking about is different than the issue I 
am having?  I haven't seen a JOIN anywhere in the code I am looking at.

Original comment by jonda...@gmail.com on 3 May 2013 at 3:45

GoogleCodeExporter commented 9 years ago
I think you're looking for something that isn't there: if I haven't coded it 
yet, you will find nothing.

99% confident this is what you need, like I said, I have absolutely zero time 
to work on documenting this and figuring out your particular use case.  The 
GetPostsQuery::get_posts() method now can control which tables it joins on, so 
if you have a million users, you it *can* avoid joining on that enormous table, 
but the Ajax requests have to be rewritten or configured to do this.  Sorry I 
just have no time to deal with this outside of the crazy projects that are 
firehosing my calendar.

Original comment by ever...@fireproofsocks.com on 3 May 2013 at 4:24

GoogleCodeExporter commented 9 years ago
Ok, I'll keep looking.  I just tracked down the missing 'h' in thumbnail, but 
then see you fixed it last night.  :)

Original comment by jonda...@gmail.com on 3 May 2013 at 5:37

GoogleCodeExporter commented 9 years ago
I fixed that damn thing THREE TIMES and somehow it kept creeping back into 
repo.  

Original comment by ever...@fireproofsocks.com on 3 May 2013 at 5:40

GoogleCodeExporter commented 9 years ago
But, now that I understand how the code works a little better, yes, the JOIN 
part you worked on in GetPostsQuery is nice, but that isn't the part that was 
taking a long time for me (or at least, even with that query fixed, displaying 
my "choose an image" form still takes more than 2 minutes to display).

The problem I am dealing with is in GetPostsForm, with the search_by defaulting 
to everything, including authors, and so then _author() is called for each post 
returned by GetPostsQuery.

Original comment by jonda...@gmail.com on 3 May 2013 at 5:43

GoogleCodeExporter commented 9 years ago
99% sure this is *exactly relevant* to your problem: the "Choose an Image" 
query uses the same query, so it'll get hamstrung with your enormous users 
table.  See my previous responses.

Original comment by ever...@fireproofsocks.com on 3 May 2013 at 5:53

GoogleCodeExporter commented 9 years ago
I guess I'm not understanding what you are saying.   It sounds to me like you 
are talking about GetPostsQuery, and that (as far as I can tell) doesn't have 
anything to do with GetPostsForm->generate, which takes each of the posts 
returned from GetPostsQuery, and then makes additional SQL calls when figuring 
out the placeholders for the image template.

Since search_by is set to true in get_posts.php, that means it loops through 
the entire author table, without regard to the join=none bit.

It seems to me that the solution is to parse the tpl first, and look for 
placeholders, before running the code that generates the stuff to fill in the 
placeholder.

Original comment by jonda...@gmail.com on 3 May 2013 at 6:00

GoogleCodeExporter commented 9 years ago
Ok, what do you think of this (attached, diff vs. revision 707551)?  This fixes 
my problem, though I wrote it pretty quickly, so I'm not confident that I know 
enough to cover all cases - I almost arbitrarily put the check in with the 
methodExists, as opposed to skipping the whole thing, and I don't know if my 
hacking on the placeholder names is appropriate, nor if the code assumes 
anywhere that some basic placeholder functions will always be called, but as a 
first draft, and hopefully to explain in code what I've been trying to say in 
English...

Also, once I've implemented this change, I removed the _image.php from my 
uploads directory, and I can't tell the difference in how fast the SQL query 
runs with or without the authors being joined in.

Original comment by jonda...@gmail.com on 3 May 2013 at 6:29

Attachments:

GoogleCodeExporter commented 9 years ago
I just realized this bug is still marked as fixed, as so has perhaps fallen off 
your radar?

Original comment by jonda...@gmail.com on 14 May 2013 at 7:01

GoogleCodeExporter commented 9 years ago
Thanks for the diff -- I see what you're after.  

Not off my radar, just slammed with other projects.

Original comment by ever...@fireproofsocks.com on 14 May 2013 at 3:39

GoogleCodeExporter commented 9 years ago
Ok, thanks, just wanted to check.  My site is working fine with this change, 
but I am only minimally testing this feature, e.g. the popup uses very little 
callbacks, so at one point I thought it was working fine, but it actually 
wasn't calling any callbacks, and I almost didn't notice.

I didn't notice any other places using this code, so I don't know if I use it 
anywhere other than the choose-an-image upload popup.

Original comment by jonda...@gmail.com on 14 May 2013 at 3:49

GoogleCodeExporter commented 9 years ago
Ooo... I'm not gonna integrate this diff: it's fundamentally the wrong approach 
to have the view layer control the controller -- backwards architecture leads 
to *huge* problems.

This gets back to issue #249 and the need to supply configurable arguments to 
GetPostsForm when forms are created.  A quickie fix here might be to put a 
limit on the number of users looked up for the author dropdown.

Original comment by ever...@fireproofsocks.com on 14 May 2013 at 4:56

GoogleCodeExporter commented 9 years ago
Yeah, I get it.  The view layer isn't quite "controlling" the control layer, as 
the control layer still gets to say what the view can do, ie. it isn't the view 
taking over the code, but "helping" the control layer be more efficient.  But, 
hey this is wordpress, just throw in a global variable, and poke holes through 
the MVC...  :)

The limit on the number of authors might work, since even if I wanted an author 
popup, presumably that isn't possible with large number of users anyway.  
Though, I have another project (.NET) that used to use a select object and 
there were a large number of usernames, and so people didn't like it (not to 
mention that they couldn't always find the users, because they might be 
alphabetized under company name, or their old name, etc.  So, I made it do an 
AJAX jquery autocomplete thing that made everything faster, and easier to find 
people, since it could search throughout the name.  But, probably overkill for 
this.

I wonder if the MVC "badness" could be made more tolerable by not looking at 
all tags like my code does, but just for a couple of the tags, or maybe see 
which ones have a large performance penalty if they are there (e.g. is author 
the only one that could hurt anything).

If I changed my "scanner" code to just look for the author tag, would that be 
acceptable to you?

Original comment by jonda...@gmail.com on 14 May 2013 at 5:52

GoogleCodeExporter commented 9 years ago
Ha: global variables.  WordPress dev is painful in so many ways.

No, any scanning of the views to control functionality isn't something I want 
to support (even though that's more or less what WP templates are built on).  
It's just too problematic in the long term and it'd be disruptive when it would 
inevitably get removed from the code base.  An Ajax request might be viable 
(e.g. text-box for 'username' with an auto-complete list), but it's tough in 
the thickbox because you're doing Ajax within Ajax and it gets really hard to 
debug and test that stuff.  Writing that thing was murder to begin with: had to 
override WP JS, and it's all built with a deprecated/abandoned JS library.

#249 is the ticket out of here: pass GetPostsForm a config array whereby 
any/all customizations could happen.  I could add a config file for that 
specifically along the lines of the other configs.

Original comment by ever...@fireproofsocks.com on 14 May 2013 at 6:44

GoogleCodeExporter commented 9 years ago
The ajax-controllers/get_posts.php file has this value:

$search_by = true;

That forces the GetPostsForm to look up everything.  I took that out and added 
CCTM::$search_by to the config file.

// From config/post_selector/_relation.php:

CCTM::$post_selector['search_columns'] = array('post_title', 'post_content');
CCTM::$post_selector['post_status'] = array('publish','inherit');
CCTM::$post_selector['orderby'] = 'ID';
CCTM::$post_selector['order'] = 'DESC';
CCTM::$post_selector['limit'] = 10;
CCTM::$post_selector['paginate'] = 1;

CCTM::$search_by[] = 'post_type';
CCTM::$search_by[] = 'post_status';
CCTM::$search_by[] = 'taxonomy';
CCTM::$search_by[] = 'taxonomy_term';
CCTM::$search_by[] = 'post_parent';
CCTM::$search_by[] = 'meta_key';
CCTM::$search_by[] = 'meta_value';

Committed revision 747694.

Original comment by ever...@fireproofsocks.com on 29 Jul 2013 at 7:53