Skip to content

Add indexes on Customers.Email and Products.Name#194

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

Add indexes on Customers.Email and Products.Name#194
Dmytro234 wants to merge 2 commits into
mate-academy:mainfrom
Dmytro234:develop

Conversation

@Dmytro234

Copy link
Copy Markdown

Login checks Customers by Email on every session, and customers search Products by Name constantly, so both benefit from an index. Orders/OrderItems are left as-is since they are write-heavy and already indexed via their foreign keys.

Login checks Customers by Email on every session, and customers
search Products by Name constantly, so both benefit from an index.
Orders/OrderItems are left as-is since they are write-heavy and
already indexed via their foreign keys.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@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 does not yet implement all the required indexes based on the usage scenarios. You correctly selected the ShopDB database and created indexes on Customers.Email and Products.Name, with index names matching the column names, which aligns with the requirements. However, the task explicitly asks you to analyze usage and create indexes for all tables/columns that need them, and the common join/filter columns are still missing.

Specifically, you should add CREATE INDEX statements (in task.sql) for:

  • Orders.CustomerID (used to fetch orders per customer and in joins with Customers)
  • Orders.Date (used for date-based reporting and recent-order queries)
  • OrderItems.OrderID (used to fetch items for a given order and in joins with Orders)
  • OrderItems.ProductID (used to analyze product sales and in joins with Products)

Make sure each index name is identical to the column name (e.g., CREATE INDEX CustomerID ON Orders (CustomerID);) to comply with the naming requirement [CHECKLIST ITEM #2]. Once these additional indexes are added, your solution will fully cover the key read patterns and satisfy the indexing requirements [CHECKLIST ITEM #1]. You’re very close—your existing choices show good understanding of login and search usage; extending the same reasoning to orders and order items will complete the task.


✨ 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
CREATE INDEX Email ON Customers (Email);

-- Customers search products by Name, so an index speeds up that search.
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.

The task requires analyzing usage and creating indexes for all tables/columns that need them. Currently you only create indexes for Customers.Email and Products.Name, but no indexes for Orders.CustomerID, Orders.Date, OrderItems.OrderID, or OrderItems.ProductID, which are likely used in joins and filters; consider adding indexes on these columns with matching names to fulfill the requirements.

Orders.CustomerID, OrderItems.OrderID, and OrderItems.ProductID
already have automatically created indexes from their FOREIGN KEY
constraints, so only Orders.Date was genuinely missing.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@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 one critical checklist requirement is still not implemented in task.sql, and this blocks the task from being considered complete. You correctly selected the ShopDB database and created indexes on Customers.Email, Products.Name, and Orders.Date with index names matching the columns, but the script is missing explicit CREATE INDEX statements for Orders.CustomerID, OrderItems.OrderID, and OrderItems.ProductID. The task and checklist specifically require these indexes to be defined in task.sql (even if foreign keys already exist), and the automated tests are failing due to having the wrong number of indexes.

To fix this, add the following statements near your other index creations:

  • CREATE INDEX CustomerID ON Orders (CustomerID);
  • CREATE INDEX OrderID ON OrderItems (OrderID);
  • CREATE INDEX ProductID ON OrderItems (ProductID);
    Make sure these appear after USE ShopDB; and that you don’t create duplicate indexes on the same columns. Once these three indexes are added with names exactly matching the column names, your solution will align with the task requirements and should pass the index-count tests. You’ve done good work on the existing indexes; this final adjustment will complete the assignment.

✨ 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
Comment on lines +9 to +11
-- Orders are commonly reported on and looked up by date, so index it.
-- (Orders.CustomerID, OrderItems.OrderID, and OrderItems.ProductID already
-- have automatically created indexes from their FOREIGN KEY constraints.)

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 task and checklist explicitly require CREATE INDEX statements for Orders.CustomerID, OrderItems.OrderID, and OrderItems.ProductID in task.sql. Relying on automatically created indexes from foreign keys does not satisfy that requirement; you should add explicit CREATE INDEX statements for each of these columns with index names identical to the column names.

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

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

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

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

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

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

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

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

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

Looks good to me, keep it up! 😎

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