Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resource Hints 2021 query amends #2408

Merged
merged 4 commits into from
Oct 27, 2021
Merged

Conversation

kevinfarrugia
Copy link
Contributor

@kevinfarrugia kevinfarrugia commented Oct 26, 2021

Progress on #2158

  • Renamed consoleLog_missing_crossorigin.sql to consoleLog_incorrect_crossorigin.sql
  • Renamed consoleLog_missing_crossorigin_type.sql to consoleLog_incorrect_crossorigin_type.sql
  • Included WHERE hints.preload to consoleLog queries to only look at pages which have at least one rel="preload"
  • Added new query to check adoption of imagesrcset and imagesizes on rel="preload"

hint.name AS name,
hint.attribute AS attribute,
COUNTIF(hint.value IS NOT NULL) AS freq,
SUM(COUNT(0)) OVER (PARTITION BY _TABLE_SUFFIX, hint.name, hint.attribute) AS total,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partitioning by everything in the GROUP BY is effectively the same as just COUNT(0). Did you mean to partition only by client and name?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the total value you're going for, eg total number of instances per client/name/attribute combo? instances per client/name combo?

If the first one, I think SUM(COUNT(0)) OVER () is equivalent to COUNT(0) since there's only ever one count per combination to sum. If the second one, you'd need to remove the attribute from the partition.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you Rick. I have pushed an update with what I believe is the best implementation. Using client/name combo will show the number of times the attribute was used when the rel=preload per client.

Note that this query isn't very important and I only added it to substantiate my claim that imagesrcset isn't widely used.

@rviscomi rviscomi added analysis Querying the dataset ASAP This issue is blocking progress labels Oct 26, 2021
@rviscomi rviscomi added this to the 2021 Analysis milestone Oct 26, 2021
@rviscomi rviscomi assigned kevinfarrugia and unassigned rviscomi Oct 26, 2021
@rviscomi rviscomi merged commit b419165 into main Oct 27, 2021
@rviscomi rviscomi deleted the resource-hints-2021-sql-part-iv branch October 27, 2021 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analysis Querying the dataset ASAP This issue is blocking progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants