-
-
Notifications
You must be signed in to change notification settings - Fork 824
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(WIP) Sybunt - Rob's work #8823
Conversation
…oft credits. This also includes some tidy up of the code, getting rid of the overriding of the signature
Remove unused return variable, fix comments, remove duplicate key
This reduced the time in testing from 190+ seconds (killed at that point) to ~1 second for a report with a small result set on a large DB
…nt group queries. The process of switching them over is not hard but they need to be looked at one-be-one
This reduced the time in testing from 190+ seconds (killed at that point) to ~1 second for a report with a small result set on a large DB
Have blocked off some time to look at this on Saturday. |
@eileenmcnaughton Thanks Eileen. |
Argh... I've got troubles adding 8823 as a patch to a clean dmaster [just civibuild it 5min ago]: karins-macbook-pro:civicrm sysadmin$ cat 8823.patch | patch -p1 --dry-run |
@KarinG you will have to pull the whole branch - ie. add me as a remote & then pull my sybunt branch - there are a bunch of other changes I made in #8820 which this runs on top of. Hopefully I can get that merged at some point as it fixes the performance issues for group filter - although not on all reports |
Ah - ok - got it:
|
Hey @robbrandt and @eileenmcnaughton - I'm all set up and am looking at the new SYBUNT report. Here's what I've done thus far (see screenshot):
|
@KarinG @robbrandt I rebased Rob's sybunt report updates - this branch has a bunch of my fixes not yet merged so it adds a bit to the list- but adding a PR so you can test on a sandbox
I haven't dug into Rob's changes too much