Skip to content

Created indexes#190

Open
shafran-andriy wants to merge 2 commits into
mate-academy:mainfrom
shafran-andriy:main
Open

Created indexes#190
shafran-andriy wants to merge 2 commits into
mate-academy:mainfrom
shafran-andriy:main

Conversation

@shafran-andriy

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 a couple of core task requirements are not yet met, which would block this from being considered complete. First, the index names do not exactly match the column names: the requirement says indexes must be named the same as their columns, but you created indexes email and name while the actual columns are Email and Name (e.g., on the lines where you have CREATE INDEX email ON Customers (Email); and CREATE INDEX name ON Products (Name);) – update the index names to Email and Name to satisfy this. Second, the task explicitly asks you to analyze the usage scenario and create all necessary indexes; currently only Customers.Email and Products.Name are indexed, but the described workload with many daily orders strongly implies the need for indexes on foreign key columns such as Orders.CustomerID, OrderItems.OrderID, and OrderItems.ProductID [CHECKLIST ITEM #2, #3]. These missing indexes mean common joins and lookups on orders and order items will be inefficient and do not fully align with the stated requirement to optimize based on usage. As a minor improvement, you might also adjust comments in task.sql so they clearly describe that this file is for index creation, and optionally note that create-database.sql must be run before this script [CHECKLIST ITEM #1, #12]. Once you rename the indexes to match column names exactly and add indexes for the key foreign key columns suggested by the scenario, your solution will align well with the requirements and be in good 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
USE ShopDB;


create index email ON Customers (Email);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The requirement states that index names must be the same as the column names; here the index is named email while the column name is Email, so adjust the index name to match the column exactly.

Comment thread task.sql Outdated


create index email ON Customers (Email);
create index name ON Products (Name);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Similarly, this index is named name while the column is Name; rename the index so its name matches the column exactly, and consider whether you also need additional indexes (e.g., on foreign keys like CustomerID, OrderID, ProductID) based on the described usage scenario.

@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 approving your solution because all core functional requirements for this task are met. Your task.sql correctly targets the ShopDB database with USE ShopDB; and focuses solely on index creation rather than schema or database creation. You created indexes on the appropriate columns used for lookups and joins (Customers.Email, Products.Name, Orders.CustomerID, OrderItems.OrderID, OrderItems.ProductID), and you followed the constraint that index names must exactly match the column names. Functionally, the script should run cleanly after create-database.sql has been executed and will provide the intended performance benefits for the described workload.

The only remaining issue is very minor and non-blocking: the top comment still mentions “database creation” even though this file is now dedicated to index creation. As an optional improvement, you could update that comment to say this script assumes ShopDB has been created (via create-database.sql) and that it is specifically for index creation, which will avoid confusion for anyone running the scripts in order. Overall, this is a solid and focused implementation—well done.


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

Footnotes

  1. Rate AI review example

Comment thread task.sql
@@ -1,2 +1,9 @@
# Write your SQL code for the database creation here. Good luck!

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This comment still refers to "database creation", but this file is now specifically for index creation. It’s not functionally wrong, but you might reword it to avoid confusion for anyone running your scripts in order (and optionally mention that create-database.sql must be executed before this script).

@Losiev Losiev 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.

Well done!👏

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