Skip to content

Solved!#189

Open
ProstoPonchik wants to merge 3 commits into
mate-academy:mainfrom
ProstoPonchik:main
Open

Solved!#189
ProstoPonchik wants to merge 3 commits into
mate-academy:mainfrom
ProstoPonchik:main

Conversation

@ProstoPonchik

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

The code requires significant changes before it can be approved. Three critical blockers exist: First, the task explicitly requires creating the ShopDB database using create-database.sql and dropping it first with DROP DATABASE ShopDB;, but the code only has USE ShopDB; which assumes it already exists. Second, the task states the shop has a product called AwersomeProduct with ID 1, but there's no INSERT statement to create this product—it also assumes it pre-exists. Third, and most importantly, the transaction strategy is broken: INSERT INTO Orders is placed before START TRANSACTION, which means if subsequent operations fail, an orphaned order remains in the database, violating atomicity. All data modifications should be wrapped in a single transaction: move START TRANSACTION before the Orders insert and COMMIT after the warehouse UPDATE. These issues prevent the code from meeting explicit 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread task.sql Outdated
@@ -1,11 +1,17 @@
-- Use our database
USE ShopDB;

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 creating the ShopDB database using SQL script in create-database.sql, and deleting it first if it already exists using DROP DATABASE ShopDB;. This code only has USE ShopDB; which assumes the database already exists.

Comment thread task.sql
Comment on lines +4 to +5
INSERT INTO Orders (CustomerID, Date)
VALUES (1, '2023-01-01');

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 states the shop has one product called AwersomeProduct with ID 1, but there's no INSERT statement to create this product. The code assumes it already exists.

Comment thread task.sql Outdated
Comment on lines 4 to 8
INSERT INTO Orders (CustomerID, Date)
VALUES (1, '2023-01-01');

-- Start the transaction
START TRANSACTION;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Based on business rules analysis: the order creation, order item creation, and warehouse update should ALL be in the same transaction to ensure atomicity. If the order insert fails after the transaction commits, the database would be left inconsistent. Move START TRANSACTION before the INSERT INTO Orders and COMMIT after the UPDATE Products.

@ProstoPonchik

Copy link
Copy Markdown
Author

I think the first two points are not correct.

The task says to create the ShopDB database using create-database.sql before running task.sql. So task.sql is not expected to contain DROP DATABASE or CREATE DATABASE statements. It should only use the already created ShopDB database.

Also, AwersomeProduct and the customer with ID 1 are already inserted in create-database.sql. The task.sql file should not recreate this initial data.

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

Review Summary

Your solution is missing two critical requirements. First, the task explicitly requires dropping and recreating the ShopDB database using DROP DATABASE IF EXISTS ShopDB; and creating it from the create-database.sql script, but your code only has USE ShopDB; which assumes the database already exists. Second, and most critically, the INSERT INTO Orders statement is placed before START TRANSACTION, which breaks transaction atomicity—if any subsequent operation (OrderItems insert or Products update) fails, an orphaned order will remain in the database. All three data modifications (Orders, OrderItems, and Products UPDATE) must be wrapped in a single transaction block to ensure they succeed or fail together. Please move START TRANSACTION before the Orders INSERT and place COMMIT after the Products UPDATE.


✨ 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
@@ -1,11 +1,19 @@
-- Use our database
USE ShopDB;
USE ShopDB;

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 dropping and recreating the ShopDB database using DROP DATABASE ShopDB; and the create-database.sql script. Currently, only USE ShopDB; is present, which assumes the database already exists. Add DROP DATABASE IF EXISTS ShopDB; and CREATE DATABASE ShopDB; statements before the USE statement.

Comment thread task.sql
Comment on lines +7 to +8
INSERT INTO Orders (CustomerID, Date)
VALUES (1, '2023-01-01');

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 INSERT INTO Orders is placed before START TRANSACTION, breaking atomicity. If the OrderItems insert or Products update fails, the order will remain orphaned in the database. Move START TRANSACTION before this INSERT and the COMMIT after the UPDATE to ensure all operations succeed or fail together.

@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