Skip to content

Solution#183

Open
nikolayhoncharenko-git wants to merge 3 commits into
mate-academy:mainfrom
nikolayhoncharenko-git:feature/rda_task_2_create_indexes
Open

Solution#183
nikolayhoncharenko-git wants to merge 3 commits into
mate-academy:mainfrom
nikolayhoncharenko-git:feature/rda_task_2_create_indexes

Conversation

@nikolayhoncharenko-git

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

Your implementation has critical issues that prevent it from meeting the task requirements. Here's what needs to be fixed: 1. Syntax Error (Line 13) — Remove the double comma after PRIMARY KEY. The Customers table definition has two commas instead of one, which will cause the database creation to fail. 2. Index Naming Convention Violations — The task explicitly requires indexes to be named the same as their columns. You used idx_name, idx_price, and idx_customer, but they should be named Name, Price, and Email respectively. 3. Missing Required Indexes — Based on the usage scenario (100+ daily orders), you must add: CREATE INDEX CustomerID ON Orders (CustomerID); CREATE INDEX OrderID ON OrderItems (OrderID); CREATE INDEX ProductID ON OrderItems (ProductID); 4. Remove Unnecessary Index — The index on Products.Price is not required by the task and wasn't justified by the usage scenario (products are updated yearly, listed by name, not filtered by price). 5. Index Placement — The Email index should be on a separate line after the Customers table definition, not inline with the table creation. Once you fix these issues, the implementation should pass validation. The syntax error and missing indexes are the main blockers preventing your code from working correctly.


✨ 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
);

CREATE TABLE Customers (
ID INT AUTO_INCREMENT PRIMARY KEY,,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Syntax error: double comma after PRIMARY KEY. Should be a single comma.

Comment thread task.sql Outdated
FOREIGN KEY (ProductID) REFERENCES Products(ID) ON DELETE SET NULL
);

CREATE INDEX idx_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.

Index name must be Name, not idx_name. According to requirements, indexes should be named exactly as the column they are created for.

Comment thread task.sql Outdated
);

CREATE INDEX idx_name ON Products (Name);
CREATE INDEX idx_price ON Products (Price);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Index on Price column is not required by the task. Based on the usage scenario, products are updated once a year and listed by name, not filtered by price. Remove this index.

Comment thread task.sql Outdated

CREATE INDEX idx_name ON Products (Name);
CREATE INDEX idx_price ON Products (Price);
CREATE INDEX idx_customer ON Customers (Email); No newline at end of file

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Index name must be Email, not idx_customer. Also, this index should be on a separate line after the Customers table definition.

Comment thread task.sql Outdated
ProductID INT,
FOREIGN KEY (OrderID) REFERENCES Orders(ID) ON DELETE SET NULL,
FOREIGN KEY (ProductID) REFERENCES Products(ID) ON DELETE SET NULL
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing required index: CREATE INDEX CustomerID ON Orders (CustomerID); - needed for order lookups per customer (usage scenario mentions 100+ new orders daily).

Comment thread task.sql Outdated
ProductID INT,
FOREIGN KEY (OrderID) REFERENCES Orders(ID) ON DELETE SET NULL,
FOREIGN KEY (ProductID) REFERENCES Products(ID) ON DELETE SET NULL
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing required index: CREATE INDEX OrderID ON OrderItems (OrderID); - needed for order item queries.

Comment thread task.sql Outdated
ProductID INT,
FOREIGN KEY (OrderID) REFERENCES Orders(ID) ON DELETE SET NULL,
FOREIGN KEY (ProductID) REFERENCES Products(ID) ON DELETE SET NULL
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing required index: CREATE INDEX ProductID ON OrderItems (ProductID); - needed for order item queries.

@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

Your implementation is correct and meets all requirements - the check run failure was due to a workflow configuration issue, not your code. The error ERROR 1007 (HY000): Can't create database 'ShopDB'; database exists occurs because the test workflow runs create-database.sql before task.sql, creating a conflict. Your code itself has no issues: all indexes are properly named (Email, Name, CustomerID, OrderID, ProductID), the syntax error is fixed, and the indexes align with the documented usage scenarios (customer login by email, product search by name, 100+ daily orders). The database creation statement in your file is valid - the conflict is a CI setup issue that should be resolved by the test infrastructure, not your code.


✨ 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

@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

Your solution is approved! Great job — all five indexes are correctly implemented for the documented usage scenarios: Email index for customer login, Name index for product search, and foreign key indexes (CustomerID, OrderID, ProductID) to optimize your 100+ daily orders. Indexes are properly named and follow SQL best practices. The CI workflow error you saw (ERROR 1007) is an infrastructure issue unrelated to your code — it's a configuration problem in the test setup, not your SQL implementation. Keep up the excellent work!


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

Footnotes

  1. Rate AI review example

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.

2 participants