Closed Bug 511260 Opened 11 years ago Closed 11 years ago

Replace UNION ALL with UNION where a sync could change underlying data in the middle

Categories

(Toolkit :: Places, defect)

defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: mak, Unassigned)

References

Details

Attachments

(1 file)

due to bug 507790, during a UNION ALL underlying data in the queried tables can change, that means we can get any sort of random results (duplicates or missing results) from those queries.
Even if UNION is slightly slower, we have to replace UNION ALLs with UNIONs, to get consistent results.
Note that any query that is only run asynchronously will not run into this issue, and so it doesn't need to be changed.
sure, i won't change all of them
Attached patch patch v1.0Splinter Review
using UNION instead of UNION ALL in queries that have a COALESCE between select fields causes WARNINGs for sorting operations, i suppose due to the fact UNION tries to discard duplicates, and that process will be quite slower then what we do actually. That happens in lots of queries.

This patch is what we should do to replace UNION ALLs, Shawn what do you think?
Imho this will kill perf in a bad way, one of the queries is mDBGetChildren, that is used (among other things) to populate all bookmarks folders (included the toolbar).
Depends on: 511374
Bug 511965 should reduce cases where this happens.
if bug 511965 is ensuring functionality i'm goingto wontfix this. the perf drop would just be too big.
unfortunatly WONTFIX, performances regression would be quite bad.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.