Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.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 commentedAnd here we are.
Comment #2
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 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 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 commentedI don't see the tag 'node_access' on the listing queries.
Comment #6
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 commentedAh, node access API thanks you.
Comment #10
dries commentedCommitted to CVS HEAD. Thanks, Berdir.