Is there any reason to use PhabricatorTokenGivenQuery here?

There's an argument against using it: some types of Queries require related objects to exist, even if querying with the Omnipotent viewer.

For example, TransactionQuery classes refuse to load transactions if the objects aren't loadable, even if the rows exist. The general idea is that the overwhelming majority of application code is way simpler if it doesn't need to check that every object is complete/valid, and we normally can't policy check these objects anyway.

In this case we need to load those "broken/invalid" rows, so destroyTransactions() queries the database directly instead of using an appropriate Query class.

It looks like TokenGivenQuery has similar logic, and refuses to load tokens if the objects don't load, so I would expect it to never "see" any tokens (since we just destroyed their objects).

There are at least two reasons why this might be bad:

• $token->delete() might expect that the token is in some state that only Query puts it in. I don't think it does today, but, for example, CalendarEvent has some special postprocessing in the Query and loading an object via the Query vs via load() can give you objects in meaningfully different states.
• If there are a million results, we should ideally iterate over them in pages, not bulk-load them all at once (e.g., deleting an object with 1M tokens might exhaust RAM or something right now). I suppose that in this specific case, since we're deleting the rows, we could just issue the same query over and over again with a LIMIT until it comes back empty, although that's a little sketchy. In the general case, if we wanted to build a "query iterator with constraints", it might make more sense to put it on Query than on the raw DAO. But maybe not. Maybe just letting LiskMigrationIterator add a condition to the WHERE clause would be good enough.

In the case of a large result set, probably it would be better to just use queryfx to DELETE FROM %T WHERE objectPHID = %s?

Yeah, but only if we're certain delete() has no other logic and that we'll definitely remember to fix this if it ever does. In the general case, some delete() methods do something.