Page MenuHomePhabricator

Add support for querying feed transactions by author and date range

Authored by epriestley on May 21 2019, 12:25 PM.



Depends on D20531. Ref T13294. Enable finding raw transactions in particular date ranges or with particular authors.

Test Plan

Diff Detail

rP Phabricator
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.May 21 2019, 12:25 PM
epriestley requested review of this revision.May 21 2019, 12:27 PM
amckinley accepted this revision.May 21 2019, 6:40 PM
amckinley added inline comments.

Really this is for the previous diff, but shouldn't we fire all of these in parallel?

This revision is now accepted and ready to land.May 21 2019, 6:40 PM
epriestley added inline comments.May 21 2019, 7:37 PM

Running queries in parallel is theoretically possible, but requires mysqli_async(), which not all installs have and we don't really use for anything, and execute() does a lot of other work so it would need to be unravelled.

So "yes", but we have no practical ability to do parallel queries right now and this probably isn't a particularly strong case for developing that capability since I expect it to be rarely used for answering pretty narrow audit-related questions.

If some other use case arose to motivate development of parallel query capabilities, coming back here and converting this would probably make sense, although we do get some benefit of doing these serially by shrinking the date window progressively below as results come back. After a bit we should have a fairly cheap query (examining a very small time window).

There's currently no dateCreated key on the transaction tables so this probably doesn't actually help us very much today, but if these queries are slow in practice we could likely add that key to largely fix the problem (it would just trigger a bit of a migration mess).

I expect this page may be "kind of slowish" on real large installs but likely not explosively bad, but I guess we'll see.