#24684 closed enhancement (maybelater)
posts_clauses Query filter should have array as argument
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 3.5.2 |
Component: | Query | Keywords: | needs-patch |
Focuses: | Cc: |
Description
Proposal to make it easier to modify the main, core-built query string.
In its current state, the posts_clauses
filter and its siblings (posts_where
, etc.) all have a string to modify. This can be really tough as one has to
- use a Regex
- consider every possible (edge) case
just to replace, add or remove parts of the SQL query. The reason for this is that get_posts()
concatenates to the strings.
What I'd like to propose is exchanging the $where .= 'foobar';
string concatenations with an array core would add to: $where[] = 'foobar';
. This would make it much easier to search through the different parts and replace/add/remove parts of it before the final query string is built.
I'd like to propose this as two step process:
- I'd add a patch that replaces the string concatenations with the array building process. The current arguments would stay in place, but the arrays would be
implode()
d for the filter calls. Then we could add another, temporary filter that's namedposts_clauses_arrays
, that would hold all those arrays as subarrays of$pieces
. - If every developer is familiar with this, we could change the arguments for the sibling filters and possibly remove the
posts_clauses
-filter in favor of theposts_clauses_array
-filter.
Change History (11)
#4
in reply to:
↑ 3
@
12 years ago
- Keywords close removed
Replying to wonderboymusic:
This would just end up being a 3rd argument to the
post_clauses
filter (back compat) and would be a major rewrite ofget_posts()
- unless a patch materializes that nails it, I don't see this picking up any steam
You misread something: There would be no 3rd argument and no major rewrite. Actually the patch would just replace all the string concatenating with array pushing. All the filters and the vars would then just get converted from Array > String using a simple join()
/implode()
before they'd get called.
The new filter would run after the current filters so nothing would break and everything would be backwards compatible. Plugins could slowly move towards using the newer (easier and faster) filter.
#5
follow-up:
↓ 6
@
12 years ago
no major rewrite
Actually the patch would just replace all the string concatenating with array pushing
What isn't major about that?
#6
in reply to:
↑ 5
@
12 years ago
Replying to wonderboymusic:
no major rewrite
Actually the patch would just replace all the string concatenating with array pushing
What isn't major about that?
I'd have to agree with Kaiser, but I would say get_posts is in desperate need of a major refactor. Or at least breaking up into separate methods
Infact, I know with certainty it is required, most computer scientists will advise that a method should have an NPath complexity of less than 200, and get_posts has an NPath complexity of:
1,435,733,941,397,422,709,124,940,625,188,500,371,668,992,000,000
or 1x1049, which is 25 orders of magnitude greater than the number of stars in the visible universe
When a method needs more unit tests to verify it works than there are stars in the universe and grains of sand on all the beaches in the solar system, I can surmise that swapping out some string concatenation for some array insertions is trivial in comparison.
That stupendous number could easily be brought down very quickly, and the changes necessary will go a long way towards making that method more maintainable. Perhaps Kaiser has a patch ready I'm unaware of, but anything that modifies get_posts is likely to be a good thing, after all, that number can't get much higher.
For those watching who are unaware http://www.codingswag.com/2013/05/cyclomatic-and-npath-complexity-explained/
#8
@
12 years ago
Out of curiosity, if each unit test is 1.5kb and we use 1TB samsung drives at .4kg each, the unit tests needed would weigh 80 Undecillion KG ( 38 zeros ).
Or 4 solar masses. The lightest black hole found was 3.8 solar masses, and I hear disk platters are dense
#9
@
11 years ago
- Milestone Awaiting Review deleted
- Resolution set to maybelater
- Status changed from new to closed
There is a lot to do here to even come up with a proof of concept that is backwards-compatible. No one has produced a patch in the past 8 months.
#10
@
11 years ago
FWIW, there's a Trac comment with POC from MikeSchinkel proposing this at a larger scale, which isn't unreasonable, just seemingly a significant undertaking. I can certainly see this being desirable, and think we can probably reopen this as some point with focus. No need to go xkcd-what-if on unit tests right now. :)
This would just end up being a 3rd argument to the
post_clauses
filter (back compat) and would be a major rewrite ofget_posts()
- unless a patch materializes that nails it, I don't see this picking up any steam