nexedi / erp5

@jm: Why "UNION ALL", especially in the priority query ? Why only one "UNION" is tagged as "ALL" in each query ?

this was also not obvious when I looked at !836 . I assumed that UNION ALL just adds results and UNION adds and discard duplicates, so we just need to discard duplicates once, because of the cost of discarding duplicates.

(edit because of grammar mistake)

added 1 commit

• d6bce47b - CMFActivity: Implement node families.

Compare with previous version

I assumed that UNION ALL just adds results and UNION adds and discard duplicates, so we just need to discard duplicates once, because of the cost of discarding duplicates.

I see, so the rule was not "ALL on one" but "no ALL on one". Patch updated & pushed.

%s UNION ALL %s UNION %s UNION ALL %

is better because deduplication happens on small sets

Done as part of reordering unions to make the family subquery optional.

I don't understand why you reordered. This was not required to make the family subquery optional, and my above comment is still not addressed.

(I assume that the family subquery always returns a distinct set than the other. And note that UNION ALL has same precedence as UNION.)

I don't understand why you reordered [...] my above comment is still not addressed.

Because I did not understand how/why you did assumptions on set sizes, and as I do not have time to fight I just applied based on which subquery is on the right of each.

I assume that the family subquery always returns a distinct set than the other.

This thought process I understand, and indeed reordering is not good to take advantage of this property.

And yes, it must necessarily be distinct: node id (self and others) is always strictly larger than zero, zero is zero, and families are always strictly less than zero. Applied.

Does MariaDB find quickly that UNION ALL (SELECT ... WHERE 0 AND ...) has to be ignored ?

I'd have rather do:

' UNION ALL ' + subquery(-1, 'node IN (%s)' % ','.join(map(str, node_set))) if node_set else '',

(+ obvious change above)

That would be also less Python work if node_set is empty.

And same remarks below.

Does MariaDB find quickly that UNION ALL (SELECT ... WHERE 0 AND ...) has to be ignored ?

It finds it, yes, but it seems from ongoing measures that it's not fast, or maybe not efficient in the union around it. I happen to have just developped & pushed just this change. I won't be able to benchmark it befor tomorrow though.

added 1 commit

• 464704a6 - CMFActivity: Implement node families.

Compare with previous version

resolved all discussions

resolved all discussions

added 1 commit

• ee311124 - CMFActivity: Implement node families.

Compare with previous version

added 1 commit

• 621b401c - CMFActivity: Implement node families.

Compare with previous version

Note that I ordered (el)if statements by probability. With the way you reworked this part, I'd test 'same' first.

But the node = 'same' is not pretty. From the original code, I'd have only replaced the raise by kw['node'] = ...; break.

the node = 'same' is not pretty.

+1

From the original code, I'd have only replaced the raise by kw['node'] = ...; break.

Fine for me. I thought the intent was to only set in one place (at "loop" end).

Done.

added 1 commit

• a13d9764 - CMFActivity: Implement node families.

Compare with previous version

resolved all discussions

added 1 commit

• 643e1a5b - CMFActivity: Implement node families.

Compare with previous version

merged

(I hadn't reviewed the UI & tests)

I've just look at the new UI and I have 2 questions:

• what's the danger about renaming a family ?
• shouldn't the Available Nodes: list show only processing nodes ?

(I hadn't reviewed the UI & tests)

Sorry, I had to push: the code is now in production. I waited until the very last day I could.

what's the danger about renaming a family ?

Breaking scripts which spawn activities using that family name, or maybe getting a mix of activities on one "real" id (-1, -2, ...).

Basically, "danger" is intended to apply whenever the family is in active use (there are activities in the queue, or code refering to it). Once both are false, one can delete or rename a family without risks.

shouldn't the Available Nodes: list show only processing nodes ?

I thought doing so would produce weird results when nodes are removed from "processing": the node is still selected in a family, but not selectable anymore.

Also, it allows pre-creating families before letting the nodes work, if needed.