Develop#164
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because a few core task requirements are not yet met and they are causing the tests to fail. The task explicitly requires that index names must match the column names, but your indexes are named idx_products, idx_customer, and idx_orders instead of Name, Email, and CustomerID; the tests also look specifically for those index names, so this breaks the functional checks. To fix this, rename your indexes, for example: CREATE INDEX Name ON Products (Name);, CREATE INDEX Email ON Customers (Email);, and CREATE INDEX CustomerID ON Orders (CustomerID); in task.sql. The task and scenario also strongly suggest adding indexes on OrderItems.ProductID (and likely OrderItems.OrderID), with index names exactly ProductID and OrderID, to support joins and lookups on order items. Finally, make sure task.sql clearly assumes ShopDB already exists (e.g., a brief comment that it should be run after create-database.sql) and then rerun test.sql to confirm all checks pass. Once these changes are in, your solution will align well with the requirements and the intended performance considerations.
✨ 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
| # Write your SQL code for the database creation here. Good luck! | ||
| USE ShopDB; | ||
|
|
||
| CREATE INDEX idx_products ON Products (Name); |
There was a problem hiding this comment.
The task requires that index names be the same as the column names. Your indexes are named idx_products, idx_customer, and idx_orders, which do not match the column names (Name, Email, CustomerID). Rename indexes to match columns, e.g., Name, Email, CustomerID.
| @@ -1,2 +1,9 @@ | |||
| # Write your SQL code for the database creation here. Good luck! | |||
| USE ShopDB; | |||
There was a problem hiding this comment.
You should ensure the database exists before USE ShopDB. The task requires creating the ShopDB using provided create-database.sql; task.sql should assume the DB exists or at least not try to create it. If you intend task.sql to be run after create-database.sql, add a comment clarifying that. Currently it's acceptable but confirm execution order.
|
|
||
| CREATE INDEX idx_orders ON Orders (CustomerID); |
There was a problem hiding this comment.
Per the task, consider indexing any other columns used in lookups/searches. The scenario states customers search products by Name and login by Email, and Orders receive many inserts daily: indexing OrderItems.ProductID (and possibly OrderItems.OrderID) can speed joins and lookups. If you intend to add such indexes, create them and name them to match the column names.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because a critical requirement is missing that will cause tests to fail. Your task.sql correctly creates indexes on Products.Name, Customers.Email, and Orders.CustomerID with index names matching the column names, which aligns with the task requirements. However, the script does not create the required indexes on OrderItems(ProductID) and OrderItems(OrderID), and the tests explicitly expect indexes named ProductID and OrderID on the OrderItems table; without these, functionality and automated checks related to frequent joins/lookups on order items will not pass. To fix this, add two statements near your other CREATE INDEX commands, for example: CREATE INDEX ProductID ON OrderItems (ProductID); and CREATE INDEX OrderID ON OrderItems (OrderID);, then re-run test.sql to confirm everything passes. As a minor improvement (not a blocker), keep the existing USE ShopDB; and add a short comment at the top like -- Run after create-database.sql so ShopDB already exists to clarify execution order. Overall structure and syntax look good; once you add the missing OrderItems indexes, your solution should fully meet the task requirements.
✨ 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
|
|
||
| CREATE INDEX Email ON Customers (Email); | ||
|
|
||
| CREATE INDEX CustomerID ON Orders (CustomerID); |
There was a problem hiding this comment.
Indexes should also be created on OrderItems to support frequent lookups and joins (per the usage scenarios). The task and previous review expect ProductID and OrderID indexes on OrderItems (index names must match column names). Add:
CREATE INDEX ProductID ON OrderItems (ProductID);
CREATE INDEX OrderID ON OrderItems (OrderID);
This file currently lacks those indexes, so tests will fail.
| @@ -1,2 +1,9 @@ | |||
| # Write your SQL code for the database creation here. Good luck! | |||
There was a problem hiding this comment.
The file contains a stray comment line at the top that says "Write your SQL code for the database creation here." It's okay as a comment but ensure the script assumes ShopDB already exists (you already use USE ShopDB;). Consider adding a brief comment that this script should be run after create-database.sql to make intent explicit.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
APPROVAL DECISION: Your solution is approved.
You did a solid job: task.sql correctly keeps USE ShopDB; and creates all required indexes with the exact column-based names on Products(Name), Customers(Email), Orders(CustomerID), and the previously missing OrderItems(ProductID) and OrderItems(OrderID), which aligns with the project requirements and the indexing checklist. There are no critical issues to fix—the SQL should run cleanly against a freshly created ShopDB with the schema from create-database.sql. A small, optional improvement would be to add a brief comment at the top, such as -- Run after create-database.sql so ShopDB already exists, to clarify execution order for anyone running the scripts later. This is an educational setting, so I’m approving your work as is while highlighting that tiny clarity improvement as a good professional habit rather than a requirement. Overall, your indexing choices and naming are correct and show good attention to the task details—well done.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
No description provided.