Code review comment for lp:~thumper/launchpad/blueprint-dependencies

Revision history for this message
Robert Collins (lifeless) wrote :

So this is fine, I have a suggestion to make it better:
+def recursive_blocked_query(spec):
19 + return """
20 + RECURSIVE blocked(id) AS (
21 + SELECT %s
22 + UNION
23 + SELECT s.id
24 + FROM specificationdependency sd, blocked b, specification s
25 + WHERE sd.dependency = b.id
26 + AND s.id = sd.specification
27 + )""" % spec.id
28 +
29 +
30 +def recursive_dependent_query(spec):
31 + return """
32 + RECURSIVE dependencies(id) AS (
33 + SELECT %s
34 + UNION
35 + SELECT s.id
36 + FROM specificationdependency sd, dependencies d, specification s
37 + WHERE sd.specification = d.id
38 + AND s.id = sd.dependency
39 + )""" % spec.id

These queries do not use s other than the s.id they select which is also sd.dependency/sd.specification. So I would substitute and drop s to get
+def recursive_blocked_query(spec):
19 + return """
20 + RECURSIVE blocked(id) AS (
21 + SELECT %s
22 + UNION
23 + SELECT sd.specification
24 + FROM specificationdependency sd, blocked b
25 + WHERE sd.dependency = b.id
27 + )""" % spec.id
28 +
29 +
30 +def recursive_dependent_query(spec):
31 + return """
32 + RECURSIVE dependencies(id) AS (
33 + SELECT %s
34 + UNION
35 + SELECT sd.dependency
36 + FROM specificationdependency sd, dependencies d
37 + WHERE sd.specification = d.id
39 + )""" % spec.id

as the only thing returned is the id, this won't affect your other branch.

review: Approve

« Back to merge proposal