Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
This one probably has lots of delay() safe and slave safe queries.
Comment | File | Size | Author |
---|---|---|---|
#8 | 394582-tracker-db-tng-convert-followup3.patch | 1.97 KB | Berdir |
#7 | 394582-tracker-db-tng-convert-followup2.patch | 2.9 KB | Berdir |
#4 | 394582-tracker-db-tng-convert-followup.patch | 2.02 KB | Damien Tournoud |
#1 | 394582-tracker-db-tng-convert.patch | 3.92 KB | Damien Tournoud |
Comments
Comment #1
Damien Tournoud CreditAttribution: Damien Tournoud commentedAnd here we are.
Comment #2
Dries CreditAttribution: Dries commentedThis looks good. Looking at the tracker module test, we should have good text coverage for these changes. The one exception is that the tracker module tests don't test the paging part. For extra points, update the tracker tests to use a pager?
Comment #3
Dries CreditAttribution: Dries commentedI've committed the patch in #1 to CVS. Thanks DamZ. We can talk more about tests and slave queries so not marking this 'fixed' yet.
Comment #4
Damien Tournoud CreditAttribution: Damien Tournoud commentedOk, there was two mistakes:
- the object returned by the call to ->extend() should be used when calling ->execute()... if not, the extension as no effect at all
- tracker doesn't support tablesort (no header is associated with a sql column) which made tablesort.inc throw away some notices... a fix for which I sneaked in the attached patch.
Comment #5
moshe weitzman CreditAttribution: moshe weitzman commentedI don't see the tag 'node_access' on the listing queries.
Comment #6
moshe weitzman CreditAttribution: moshe weitzman commentedAhem - still no 'node_access' tag. Lets not leave HEAD in an insecure state for months at a time.
Comment #7
Berdir- Added the tag
- Re-ordered the query a bit, early extended and chained more calls..
- Removed tablesort extender, we don't need it..
Comment #8
Berdirmoshe suggested to remove the tests, no other page is testing the paging (except of the PagerDefault tests, obviously..).
New patch.
Comment #9
moshe weitzman CreditAttribution: moshe weitzman commentedAh, node access API thanks you.
Comment #10
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks, Berdir.