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

Media 2024 queries #3738

Merged
merged 5 commits into from
Nov 9, 2024
Merged

Conversation

stefanjudis
Copy link
Contributor

@stefanjudis stefanjudis commented Aug 17, 2024

This PR includes the updated queries for the media chapter. #3596

@stefanjudis stefanjudis mentioned this pull request Aug 17, 2024
10 tasks
@tunetheweb tunetheweb changed the title Add web almanac 2024 SQL queries Media 2024 queries Aug 21, 2024
@tunetheweb tunetheweb added the analysis Querying the dataset label Aug 21, 2024
@tunetheweb tunetheweb added this to the 2024 Analysis milestone Aug 21, 2024
Fixing inconsistencies in linter
@mgifford
Copy link
Contributor

How do you get around the of blocked regex 'TABLESAMPLE' error? I don't actually know what it does mind you...

@stefanjudis
Copy link
Contributor Author

@mgifford I'm sorry, I don't understand. What error?

@tunetheweb
Copy link
Member

TABLESAMPLE SYSTEM (0.001 PERCENT) says only run this on 0.001% of the rows in the table. This is good during query development as faster (and cheaper!) to run than on the full dataset.

However, eventually we will want all queries run on the full dataset. Therefore there is a linting rule warning this is being used and we should not merge this PR until that is fixed.

So it's fine to ignore it for now (especially as this PR is still in Draft status) if you're still working on those queries. It just means you won't see a nice green tick in the linting check for now and also that you might miss other linting errors unless you explicitly look at those checks to see if anything else needs fixing too.

@stefanjudis
Copy link
Contributor Author

Aaaah, I didn't realize there was a linting error. Thanks @tunetheweb!

I've run most of the queries against the full data set already. and will double check that this is true!

removing  TABLESAMPLE SYSTEM (0.001 PERCENT)
Copy link
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me. Still using some older tables in some queries but let's not perfection be the enemy of the good!

Let me know if good to merge, but I think you might still be working on some queries?

@eeeps
Copy link
Contributor

eeeps commented Nov 9, 2024

I've been writing off of the results of these - as far as I can tell they are good to merge!

@tunetheweb tunetheweb marked this pull request as ready for review November 9, 2024 19:45
@tunetheweb tunetheweb merged commit fd87798 into HTTPArchive:main Nov 9, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analysis Querying the dataset
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants