Page MenuHomePhabricator

Improve task dependency queries (Fixes T8126)

Authored by epriestley on Apr 1 2016, 8:13 AM.
Referenced Files
Unknown Object (File)
Tue, Dec 5, 7:04 PM
Unknown Object (File)
Tue, Dec 5, 10:35 AM
Unknown Object (File)
Thu, Nov 30, 1:00 AM
Unknown Object (File)
Sat, Nov 25, 4:11 AM
Unknown Object (File)
Wed, Nov 15, 11:29 AM
Unknown Object (File)
Oct 24 2023, 4:55 PM
Unknown Object (File)
Oct 8 2023, 6:23 AM
Unknown Object (File)
Oct 6 2023, 1:50 AM



To fix T8126 this filters blocking/blocked tasks to include only open tasks at join time rather than after joining, then counts the number of such tasks after grouping.

This should be about as performant as the current implementation (with the benefit of also working), however, I doubt the MySQL optimiser is smart enough to realise we're only counting tasks and comparing with zero and it therefore can stop after finding the first open blocking/blocked task. Consequently, refactoring this to use EXISTS subqueries with a LIMIT clause may give a performance boost. Let me know if you'd like me to attempt that.

And of course, let me know any other direction you'd like me to go with this.

Test Plan
  • Created four tasks, T1 to T4.
  • Added T2 and T3 as blocking tasks for T1.
  • Created Manifest queries Blocking, Non-Blocking, Blocked and Unblocked.
  • Closed T1. Checked queries. Reopened T1.
  • Closed T2. Checked queries.
  • Closed T3. Checked queries.
  • Queries always returned expected results. Tasks listed iff they had zero/greater-than-zero open blocking/blocked tasks.

Diff Detail

rP Phabricator
Lint Passed
Tests Passed
Build Status
Buildable 11378
Build 16866: Run Core Tests
Build 14168: arc lint + arc unit

Event Timeline

isfs retitled this revision from to Improve task dependency queries (Fixes T8126).
isfs updated this object.
isfs edited the test plan for this revision. (Show Details)

@epriestley, have I done the right thing submitting a review here for this? Could you spare a few minutes to accept/land it, or throw it back to me for further work? If I should be following a different procedure, please let me know. No pressure; just a gentle nudge and plea for guidance. Really appreciate all your work and respect your right to set your own priorities.


This was the original logic, but I'd like to query it.

E.g. if I choose to

  • Show blocked tasks
  • Hide blocking tasks

I would expect to see 'top level' tasks that have subtasks, but are not subtasks themselves. However, with this 'or' logic, I would actually see all tasks with subtasks as well as all tasks which aren't holding anything up.

I also believe conjunctive logic is more what the user expects from a query form. E.g. if I choose high priority tasks and list myself as assignee, I expect to see my high priority tasks, not all high priority tasks along with all tasks assigned to me.

I think this should be changed to 'AND', but perhaps for traceability it should be done in a separate change or have a separate task created.

isfs added a subscriber: chad.

@chad indicated that @epriestley reviews code from contributors, so adding him as a reviewer.

This looks good to me, and I agree that it makes a lot more sense to use AND rather than OR when building the HAVING clause.

In case you're feeling discouraged by the silence here, I should reassure you that I'm fairly confident it's not due to anything you've done wrong here.

There has been some recent refactoring of the way tasks and other object relationships are handled, e.g. D16166: Split "Edit Blocking Tasks" into "Edit Parent Tasks" and "Edit Subtasks". That and related work might affect this code and I suspect that is why this hasn't been reviewed/merged yet.

epriestley edited reviewers, added: isfs; removed: epriestley.

This should be obsoleted by D16340, which also updates the language.

I also made the AND change, and got away with slightly fewer HAVING clauses. Let me know if you're still seeing issues.

Yeah, D16340 looks good to me and should perform similarly. I'll raise any issues when I upgrade, but I don't anticipate any problems. Thanks for sorting this out!