Change new count model squashing to discard zero counts #5702
Conversation
rowanseymour
changed the title
rowanseymour
changed the title
temba/flows/models.py
Outdated
| VALUES (%%s, %%s, GREATEST(0, (SELECT SUM("count") FROM removed)), TRUE); | ||
| SELECT * FROM ( | ||
| SELECT %%s, %%s, GREATEST(0, (SELECT SUM("count") FROM removed)) AS "count", TRUE | ||
| ) s WHERE s."count" != 0; |
There was a problem hiding this comment.
using != 0 so negative totals would still be possible to support
There was a problem hiding this comment.
we still ignore negative values and consider them 0
There was a problem hiding this comment.
GREATEST(0, (SELECT SUM("count") FROM removed))
There was a problem hiding this comment.
ahh good spot.. I'm gonna remove that. If numbers are adding to something negative we've got a problem and we shouldn't just plaster over that.
There was a problem hiding this comment.
tho I notice that GREATEST(0, NULL) == 0 so I need to check we're not leaning on that to prevent writing NULLs....
There was a problem hiding this comment.
Then we can change that to COALESCE((SELECT SUM("count") FROM removed), 0) if we need to support negative values
There was a problem hiding this comment.
Ok I added COALESCE tho the way the query is it's not technically needed because WHERE s."count" != 0 isn't true if count is NULL. But I think the query is easier to understand if you know count is a number.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5702 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 572 572
Lines 25925 25923 -2
=========================================
- Hits 25925 25923 -2 ☔ View full report in Codecov by Sentry. |
We're about to move node counts to the new count model and they have a special property - they all end up zero, and zero is already representable in the count model as non-existence. So let's not even store zeros.