-
Notifications
You must be signed in to change notification settings - Fork 0
Assignment two #2
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
base: main
Are you sure you want to change the base?
Conversation
monzchan
left a comment
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.
last 3 questions have not been answered.
|
Hi Moniz,
I updated my sql assignment-two. I ran and created all activities but struggled to make everything work. I however, gained significant understand on the sql query working concepts. Please let me know if you can see the update file in my github account.
Have a great weekend.
Regards,
Jesse
On Friday, 28 November 2025 at 03:14:43 pm GMT-5, mchan ***@***.***> wrote:
@monzchan requested changes on this pull request.
last 3 questions have not been answered.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
monzchan
left a comment
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.
Great work!
| only the customer’s most recent visit. */ | ||
|
|
||
| SELECT customer_id, market_date, | ||
| ROW_NUMBER() OVER (PARTITION BY customer_id ORDER BY market_date DESC ) AS number_of_visit |
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.
The query does not filter to only the most recent visit (number_of_visit = 1). Add a line "WHERE number_of_visit = 1" may help.
| CREATE TABLE temp.sales_by_date AS | ||
| SELECT | ||
| market_date, | ||
| SUM(cost_to_customer_per_qty) AS total_sales |
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.
The assignment asked for total sales per day, here should be ROUND(SUM(quantity * cost_to_customer_per_qty),2) AS toal_sales
|
|
||
|
|
||
| DELETE FROM product_units | ||
| WHERE product_name = 'Banana Split' |
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 deletes all rows with this product_name, not just the recent one. consider this:
WITH older_record AS (
SELECT product_id, MIN(snapshot_timestamp) AS snapshot_timestamp
FROM product_units
WHERE product_id = 7
)
DELETE FROM product_units
WHERE (product_id = (SELECT product_id FROM older_record)
AND snapshot_timestamp = (SELECT snapshot_timestamp FROM older_record))
| SELECT | ||
| v.vendor_name, | ||
| pc.product_category_name, | ||
| num_customers * 5 * vi.original_price AS total_revenue |
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 formula assumes each vendor has every product and multiplies by a count, but it doesn’t correctly align with the actual product/vendor/customer relationships. This can lead to wrong revenue numbers if there are multiple products per vendor or multiple vendors per product.
What changes are you trying to make? (e.g. Adding or removing code, refactoring existing code, adding reports)
What did you learn from the changes you have made?
Was there another approach you were thinking about making? If so, what approach(es) were you thinking of?
Were there any challenges? If so, what issue(s) did you face? How did you overcome it?
How were these changes tested?
A reference to a related issue in your repository (if applicable)
Checklist