-
Notifications
You must be signed in to change notification settings - Fork 7
Mohammed A #7
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
Open
mohammedalfakih-dev
wants to merge
1
commit into
HackYourAssignment:main
Choose a base branch
from
mohammedalfakih-dev:week9/mohammed-alfakih
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Mohammed A #7
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,21 +1,59 @@ | ||
| # AI Assistance Log | ||
|
|
||
| Document one session where you used an LLM to help with a query or a design decision while completing Tasks 1-4. Replace every TODO. | ||
| ## The problem | ||
|
|
||
| > ⚠️ Never paste real customer data or PII into an LLM. The NYC taxi dataset used here is public, so sample rows are safe to share. | ||
| I was working on Task 4, question 1b, where I needed to count the number of trips per pickup borough. Borough names are stored in `vw_dim_zones`, while trip records are stored in `vw_fact_trips`, so I needed to join the fact view to the dimension view. | ||
|
|
||
| ## The problem | ||
| My first version used an inner join: | ||
|
|
||
| TODO: What were you trying to solve? Paste the relevant SQL or schema fragment. | ||
| ```sql | ||
| SELECT | ||
| z.borough, | ||
| COUNT(*) AS trip_count | ||
| FROM vw_fact_trips AS t | ||
| INNER JOIN vw_dim_zones AS z | ||
| ON t.pickup_location_id = z.location_id | ||
| GROUP BY z.borough; | ||
| ``` | ||
|
|
||
| ## The prompt | ||
|
|
||
| TODO: What did you ask the AI? Include the context you provided. | ||
| I am completing an analytics engineering SQL assignment using NYC taxi data in PostgreSQL. I created a fact view called `vw_fact_trips` and a dimension view called `vw_dim_zones`. For Task 4, question 1b, I need to count rows per pickup borough. | ||
| My first idea was to use this query: | ||
|
|
||
| ```sql | ||
| SELECT | ||
| z.borough, | ||
| COUNT(*) AS trip_count | ||
| FROM vw_fact_trips AS t | ||
| INNER JOIN vw_dim_zones AS z | ||
| ON t.pickup_location_id = z.location_id | ||
| GROUP BY z.borough; | ||
| ``` | ||
|
|
||
| However, my validation queries showed that some trips have `NULL` `pickup_location_id`, and `pickup_location_id = 999` exists in the trips table but does not exist in the zones table. Is my query still a good choice for the borough count, or would it hide some data-quality issues? What would be a better way to write this query for analytics reporting, and why? | ||
|
|
||
| ## The response | ||
|
|
||
| TODO: What did it suggest? Did it work first try? | ||
| The AI explained that my original `INNER JOIN` query would run, but it would hide trips where the pickup location does not match a row in `vw_dim_zones`. This includes trips with `NULL` `pickup_location_id` and trips with invalid pickup IDs such as `999`. | ||
|
|
||
| The AI suggested changing the query to use a `LEFT JOIN` so that all rows from `vw_fact_trips` are kept in the result. It also introduced `COALESCE(z.borough, 'Unknown') AS borough`, which replaces a missing borough value with `Unknown` instead of leaving it as `NULL`. | ||
|
|
||
| The suggested query was: | ||
|
|
||
| ```sql | ||
| SELECT | ||
| COALESCE(z.borough, 'Unknown') AS borough, | ||
| COUNT(*) AS trip_count | ||
| FROM vw_fact_trips AS t | ||
| LEFT JOIN vw_dim_zones AS z | ||
| ON t.pickup_location_id = z.location_id | ||
| GROUP BY COALESCE(z.borough, 'Unknown') | ||
| ORDER BY trip_count DESC; | ||
| ``` | ||
|
|
||
| ## Reflection | ||
|
|
||
| TODO: Did you understand *why* the suggestion worked, or did you accept it blindly? | ||
| I understood why the suggestion worked after comparing it with my validation results. Before this, I was thinking only about joining the fact view to the dimension view, so an `INNER JOIN` seemed normal. However, the validation step showed that some pickup locations were missing or invalid. | ||
|
|
||
| Using an `INNER JOIN` would silently remove those trips from the borough count. Using a `LEFT JOIN` keeps all fact rows, and `COALESCE(z.borough, 'Unknown')` makes unmatched rows visible and readable in the result. This is better for analytics reporting because it shows the data-quality issue instead of hiding it. I did not accept the suggestion blindly; I checked it against the data issues found in Task 1 and understood why it was a better choice for this query. |
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,15 @@ | ||
| # Data Dictionary | ||
|
|
||
| Document both views. State the grain in one sentence, identify the keys, and list the measures (the columns you can aggregate). Replace every TODO. | ||
|
|
||
| ## vw_fact_trips | ||
|
|
||
| - **Grain:** TODO (one sentence, e.g. "One row per ...") | ||
| - **Primary key:** TODO | ||
| - **Foreign keys:** TODO | ||
| - **Measures:** TODO (columns you would SUM or AVG) | ||
| - **Grain:** One row represents one NYC taxi trip after basic cleaning, where trips with negative `fare_amount` have been excluded. | ||
| - **Primary key:** No declared primary key is included in this view. I checked the database constraints for `nyc_taxi.raw_trips`, and no primary key or unique constraint was returned. The view is at trip-event grain, but the selected columns do not provide a guaranteed unique trip identifier. A possible natural key could be a combination of attributes such as `vendor_id`, `pickup_datetime`, `dropoff_datetime`, `pickup_location_id`, and `dropoff_location_id`, but this should not be treated as guaranteed unique without further validation. | ||
| - **Foreign keys:** `pickup_location_id` and `dropoff_location_id` reference `vw_dim_zones.location_id`. | ||
| - **Measures:** `passenger_count`, `trip_distance`, `fare_amount`, `extra`, `mta_tax`, `tip_amount`, `tolls_amount`, `improvement_surcharge`, and `total_amount`. | ||
|
|
||
| ## vw_dim_zones | ||
|
|
||
| - **Grain:** TODO | ||
| - **Primary key:** TODO | ||
| - **Foreign keys:** TODO (or "none") | ||
| - **Measures:** TODO (or "none, descriptive attributes only") | ||
| - **Grain:** One row represents one NYC taxi zone/location. | ||
| - **Primary key:** `location_id`. | ||
| - **Foreign keys:** none. | ||
| - **Measures:** none, descriptive attributes only. The descriptive columns are `borough`, `zone`, and `service_zone`. |
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,20 +1,68 @@ | ||
| -- Task 1: Data Quality Audit | ||
| -- Run every query against nyc_taxi.raw_trips / nyc_taxi.raw_zones in YOUR OWN schema (not public). | ||
| -- Run every query against nyc_taxi.raw_trips / nyc_taxi.raw_zones in YOUR OWN schema (not public). -- noqa: LT05 | ||
| -- The shared pattern is a query that returns the bad rows (or a count). | ||
| -- Zero rows back means the check passed. | ||
|
|
||
| -- 1. Duplicate check: are there rows with the same vendor_id, pickup_datetime, dropoff_datetime? | ||
| -- TODO: GROUP BY the three columns and keep only groups with HAVING COUNT(*) > 1. | ||
| -- 1. Duplicate check | ||
| SELECT | ||
| vendor_id, | ||
| pickup_datetime, | ||
| dropoff_datetime, | ||
| COUNT(*) AS duplicate_count | ||
| FROM nyc_taxi.raw_trips | ||
| GROUP BY | ||
| vendor_id, | ||
| pickup_datetime, | ||
| dropoff_datetime | ||
| HAVING COUNT(*) > 1 | ||
| ORDER BY duplicate_count DESC; | ||
|
|
||
| -- Finding: | ||
| -- Duplicate records exist. Some combinations appear 2 times | ||
|
|
||
| -- 2. Null integrity: how many rows have a NULL pickup_location_id or dropoff_location_id? | ||
| -- TODO: count the NULLs (COUNT(*) FILTER (WHERE ... IS NULL) is handy for several columns at once). | ||
| -- 2. Null integrity | ||
| SELECT | ||
| COUNT(*) FILTER ( | ||
| WHERE pickup_location_id IS NULL | ||
| ) AS null_pickup_location_id_count, | ||
| COUNT(*) FILTER ( | ||
| WHERE dropoff_location_id IS NULL | ||
| ) AS null_dropoff_location_id_count, | ||
| COUNT(*) FILTER ( | ||
| WHERE pickup_location_id IS NULL | ||
| OR dropoff_location_id IS NULL | ||
| ) AS rows_with_any_null_location_id | ||
| FROM nyc_taxi.raw_trips; | ||
|
|
||
| -- Finding: | ||
| -- 5 rows have NULL pickup_location_id. | ||
| -- 0 rows have NULL dropoff_location_id. | ||
| -- In total, 5 rows have at least one missing location ID. | ||
|
|
||
| -- 3. Range validation: what are the min and max fare_amount? Are there negative values? | ||
| -- TODO: SELECT MIN(fare_amount), MAX(fare_amount), and a count of rows where fare_amount < 0. | ||
|
|
||
| -- 3. Range validation | ||
| SELECT | ||
| MIN(fare_amount) AS min_fare_amount, | ||
| MAX(fare_amount) AS max_fare_amount, | ||
| COUNT(*) FILTER (WHERE fare_amount < 0) AS negative_fare_count | ||
| FROM nyc_taxi.raw_trips; | ||
|
|
||
| -- 4. Relationship check: which pickup_location_id values in nyc_taxi.raw_trips do NOT exist in nyc_taxi.raw_zones? | ||
| -- TODO: LEFT JOIN nyc_taxi.raw_zones ... WHERE z.location_id IS NULL (or NOT EXISTS). | ||
| -- Do NOT use NOT IN: a single NULL in the subquery hides every orphan. | ||
| -- Finding: | ||
| -- fare_amount ranges from -70 to 1422.6. | ||
| -- There are 182 rows with negative fare_amount. | ||
|
|
||
| -- 4. Relationship check | ||
| SELECT | ||
| t.pickup_location_id, | ||
| COUNT(*) AS trip_count | ||
| FROM nyc_taxi.raw_trips AS t | ||
| LEFT JOIN nyc_taxi.raw_zones AS z | ||
| ON t.pickup_location_id = z.location_id | ||
| WHERE | ||
| t.pickup_location_id IS NOT NULL | ||
| AND z.location_id IS NULL | ||
| GROUP BY t.pickup_location_id | ||
| ORDER BY trip_count DESC; | ||
|
|
||
| -- Finding: | ||
| -- pickup_location_id 999 appears in 5 trips but does not exist in nyc_taxi.raw_zones. -- noqa: LT05 |
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,22 +1,97 @@ | ||
| -- Task 4: Verification Queries. | ||
| -- Query your views and label each query with the question it answers. | ||
| -- Borough and zone names live in vw_dim_zones, so join on pickup_location_id = location_id. | ||
|
|
||
| -- 1. Volume: how many total rows in vw_fact_trips? How many rows per borough? | ||
| -- What is the most common pickup/dropoff location combination? | ||
| -- TODO | ||
| -- (Take a screenshot of the per-borough counts and save it as assets/borough_count.png.) | ||
| -- 1a. Volume: total rows in vw_fact_trips | ||
| SELECT COUNT(*) AS total_trips | ||
| FROM vw_fact_trips; | ||
|
|
||
|
|
||
| -- 2. Revenue: which pickup zone (name, not ID) generated the highest total fare_amount? | ||
| -- Which pickup zone collected the highest total fare_amount on any single day? | ||
| -- TODO | ||
| -- 1b. Volume: rows per pickup borough | ||
| -- Screenshot this result and save it as assets/borough_count.png. | ||
| SELECT | ||
| COALESCE(z.borough, 'Unknown') AS borough, | ||
| COUNT(*) AS trip_count | ||
| FROM vw_fact_trips AS t | ||
| LEFT JOIN vw_dim_zones AS z | ||
| ON t.pickup_location_id = z.location_id | ||
| GROUP BY COALESCE(z.borough, 'Unknown') | ||
| ORDER BY trip_count DESC; | ||
|
|
||
|
|
||
| -- 3. Geospatial: total number of trips and average trip_distance for each borough. | ||
| -- TODO | ||
| -- 1c. Volume: most common pickup/dropoff location combination | ||
| SELECT | ||
| z_pickup.zone AS pickup_zone, | ||
| z_dropoff.zone AS dropoff_zone, | ||
| COUNT(*) AS trip_count | ||
| FROM vw_fact_trips AS t | ||
| INNER JOIN vw_dim_zones AS z_pickup | ||
| ON t.pickup_location_id = z_pickup.location_id | ||
| INNER JOIN vw_dim_zones AS z_dropoff | ||
| ON t.dropoff_location_id = z_dropoff.location_id | ||
| GROUP BY | ||
| z_pickup.zone, | ||
| z_dropoff.zone | ||
| ORDER BY trip_count DESC | ||
| LIMIT 1; | ||
|
|
||
|
|
||
| -- 4. Time patterns: which day of the week had the highest total tip_amount? | ||
| -- What hour of the day has the highest average tip? | ||
| -- TODO | ||
| -- 2a. Revenue: pickup zone with highest total fare_amount | ||
| SELECT | ||
| z.zone AS pickup_zone, | ||
| SUM(t.fare_amount) AS total_fare | ||
| FROM vw_fact_trips AS t | ||
| INNER JOIN vw_dim_zones AS z | ||
| ON t.pickup_location_id = z.location_id | ||
| GROUP BY z.zone | ||
| ORDER BY total_fare DESC | ||
| LIMIT 1; | ||
|
|
||
|
|
||
| -- 2b. Revenue: pickup zone with highest total fare_amount on any single day | ||
| SELECT | ||
| z.zone AS pickup_zone, | ||
| DATE(t.pickup_datetime) AS trip_date, | ||
| SUM(t.fare_amount) AS total_fare | ||
| FROM vw_fact_trips AS t | ||
| INNER JOIN vw_dim_zones AS z | ||
| ON t.pickup_location_id = z.location_id | ||
| GROUP BY | ||
| z.zone, | ||
| DATE(t.pickup_datetime) | ||
| ORDER BY total_fare DESC | ||
| LIMIT 1; | ||
|
|
||
|
|
||
| -- 3. Geospatial: total number of trips and average trip_distance for each pickup borough -- noqa: LT05 | ||
| SELECT | ||
| COALESCE(z.borough, 'Unknown') AS borough, | ||
| COUNT(*) AS total_trips, | ||
| AVG(t.trip_distance) AS avg_trip_distance | ||
| FROM vw_fact_trips AS t | ||
| LEFT JOIN vw_dim_zones AS z | ||
| ON t.pickup_location_id = z.location_id | ||
| GROUP BY COALESCE(z.borough, 'Unknown') | ||
| ORDER BY total_trips DESC; | ||
|
|
||
|
|
||
| -- 4a. Time patterns: day of the week with the highest total tip_amount | ||
| SELECT | ||
| TO_CHAR(t.pickup_datetime, 'Day') AS day_of_week, | ||
| EXTRACT(DOW FROM t.pickup_datetime) AS day_num, | ||
| SUM(t.tip_amount) AS total_tip_amount | ||
| FROM vw_fact_trips AS t | ||
| GROUP BY | ||
| TO_CHAR(t.pickup_datetime, 'Day'), | ||
| EXTRACT(DOW FROM t.pickup_datetime) | ||
| ORDER BY total_tip_amount DESC | ||
| LIMIT 1; | ||
|
|
||
|
|
||
| -- 4b. Time patterns: hour of the day with the highest average tip_amount | ||
| SELECT | ||
| EXTRACT(HOUR FROM t.pickup_datetime) AS hour_of_day, | ||
| AVG(t.tip_amount) AS avg_tip_amount, | ||
| COUNT(*) AS trip_count | ||
| FROM vw_fact_trips AS t | ||
| GROUP BY EXTRACT(HOUR FROM t.pickup_datetime) | ||
| ORDER BY avg_tip_amount DESC | ||
| LIMIT 1; | ||
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is dangerous doign group by two things when you only want one , even though you hope/expect the same thing. maybe you can still put both in the select, and if not there must be a cleaner alternative (even to use FIRST would be better)