Skip to content

Add indexes for ShopDB#192

Open
TrickyTenshi wants to merge 2 commits into
mate-academy:mainfrom
TrickyTenshi:develop
Open

Add indexes for ShopDB#192
TrickyTenshi wants to merge 2 commits into
mate-academy:mainfrom
TrickyTenshi:develop

Conversation

@TrickyTenshi

Copy link
Copy Markdown

No description provided.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

I am requesting changes because the current task.sql script does not fully meet the core task requirements and leaves the database setup and indexing incomplete. In task.sql, the header comment says to "Write your SQL code for the database creation here," but there are no CREATE DATABASE or CREATE TABLE statements, and yet you call USE ShopDB; at the top (around line 1); this creates a dependency on objects that are not guaranteed to exist from this script alone. Either this file needs to include the creation of ShopDB and its tables, or the comment and structure should be updated to clearly reflect that create-database.sql is solely responsible for creation.

More importantly for functionality, your indexing strategy is incomplete relative to the described workload. You added sensible indexes on Customers.Email and Products.Name (near the middle of task.sql), which align with login and product search, but you did not add indexes on heavily used foreign keys and filters such as Orders.CustomerID, Orders.Date, OrderItems.OrderID, and OrderItems.ProductID. Since orders and order items are likely joined and filtered by these columns, missing these indexes can significantly impact query performance and fails the requirement to "analyze shop usage and create all necessary indexes."

To fix this, 1) either add the CREATE DATABASE ShopDB; and the relevant CREATE TABLE statements in task.sql or clearly adjust the header comment and rely solely on create-database.sql for creation, and 2) add indexes on the key columns in Orders and OrderItems, naming each index exactly the same as its column (for example, CREATE INDEX CustomerID ON Orders(CustomerID);). You can treat date-based reporting similarly by adding an index on Orders.Date if queries typically filter or sort by date. Your current work shows good understanding of indexing for login and search; once you complete the database setup story and extend indexes to the main foreign-key and filter columns, your solution will be in solid shape.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

Footnotes

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread task.sql Outdated
Comment thread task.sql
Comment thread task.sql
Comment thread task.sql

@linubah linubah left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants