Caching 2020 SQL updates #1580
Merged
Caching 2020 SQL updates #1580
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
raghuramakrishnan71
added
analysis
|
@rviscomi
With this, the queries/output sheet/chapter are now in synch. |
tunetheweb
reviewed
Nov 27, 2020
There was a problem hiding this comment.
Some suggested changes @raghuramakrishnan71
Also I don't see these queries that you mentioned above:
- resource_age_party_and_type_wise.sql generated content age percentiles party, resource wise
- resource_age_party_and_type_wise_groups.sql generates percentages for specific age buckets, party and resource wise.
Are they supposed to be part of this PR?
| client, | ||
| party, | ||
| resource_type, | ||
| COUNTIF(age_weeks IS NOT NULL) AS requests_total, |
There was a problem hiding this comment.
It's not request requests_total so should this be:
Suggested change
| COUNTIF(age_weeks IS NOT NULL) AS requests_total, | |
| COUNT(0) as requests, | |
| COUNTIF(age_weeks IS NOT NULL) AS requests_with_last_modified, |
Comment on lines
18
to
23
| COUNTIF(age_weeks < 0) AS age_neg, | ||
| COUNTIF(age_weeks = 0) AS age_0wk, | ||
| COUNTIF(age_weeks >= 1 AND age_weeks <= 7) AS age_1_to_7wk, | ||
| COUNTIF(age_weeks >= 8 AND age_weeks <= 52) AS age_8_to_52wk, | ||
| COUNTIF(age_weeks >= 53 AND age_weeks <= 104) AS age_gt_1y, | ||
| COUNTIF(age_weeks >= 105) AS age_gt_2y |
There was a problem hiding this comment.
Should we do these as percentage to allow easier year on year comparison?
Suggested change
| COUNTIF(age_weeks < 0) AS age_neg, | |
| COUNTIF(age_weeks = 0) AS age_0wk, | |
| COUNTIF(age_weeks >= 1 AND age_weeks <= 7) AS age_1_to_7wk, | |
| COUNTIF(age_weeks >= 8 AND age_weeks <= 52) AS age_8_to_52wk, | |
| COUNTIF(age_weeks >= 53 AND age_weeks <= 104) AS age_gt_1y, | |
| COUNTIF(age_weeks >= 105) AS age_gt_2y | |
| SAFE_DIVIDE(COUNTIF(age_weeks < 0), COUNTIF(age_weeks IS NOT NULL)) AS age_neg_pct, | |
| SAFE_DIVIDE(COUNTIF(age_weeks = 0), COUNTIF(age_weeks IS NOT NULL)) AS age_0wk_pct, | |
| SAFE_DIVIDE(COUNTIF(age_weeks >= 1 AND age_weeks <= 7), COUNTIF(age_weeks IS NOT NULL)) AS age_1_to_7wk_pct, | |
| SAFE_DIVIDE(COUNTIF(age_weeks >= 8 AND age_weeks <= 52), COUNTIF(age_weeks IS NOT NULL)) AS age_8_to_52wk_pct, | |
| SAFE_DIVIDE(COUNTIF(age_weeks >= 53 AND age_weeks <= 104), COUNTIF(age_weeks IS NOT NULL)) AS age_gt_1y_pct, | |
| SAFE_DIVIDE(COUNTIF(age_weeks >= 105), COUNTIF(age_weeks IS NOT NULL)) AS age_gt_2y_pct |
There was a problem hiding this comment.
Included review feedback. Kept both count, pct (as graphs used a 100% stacked bar chart and calculated its own pct automatically). Instead of requests_total, renamed to requests_with_age.
- resource_age_party_and_type_wise_groups.sql was the sql file you reviewed and provided comments.
- resource_age_party_and_type_wise.sql is already in the main branch.
rviscomi
changed the title
tunetheweb
approved these changes
Nov 29, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Progress on #917