Skip to content

Added sql code#196

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

Added sql code#196
shafran-andriy wants to merge 8 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 the script currently does not fulfill a core functional requirement: the order never gets an OrderItems row, so the created order is empty and will fail the tests. You correctly create an Orders record for CustomerID = 1 and update Products.WarehouseAmount inside a transaction, but the required item (ProductID = 1, Count = 1) is never inserted into OrderItems. This also means the key operation “add item to order and reduce stock” is not atomic, since only the stock update is transactional and the matching OrderItems insert is missing.

Critical issues to fix:

  1. After INSERT INTO Orders (...) VALUES (...), capture the new order ID (e.g., SET @orderId = LAST_INSERT_ID();).
  2. Inside a single START TRANSACTION / COMMIT block, both:
    • INSERT INTO OrderItems (OrderID, ProductID, Count) VALUES (@orderId, 1, 1);
    • UPDATE Products SET WarehouseAmount = WarehouseAmount - 1 WHERE ID = 1;
      so they succeed or fail together.

Once you add the missing OrderItems insertion tied to the new OrderID and keep it in the same transaction as the inventory update, your solution will satisfy the business rules and should pass the tests. You already have the right idea with the transaction and inventory change; now it just needs to be completed by linking the order with its item 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

-- Some data should be created outside the transaction (here)

insert into Orders (CustomerID, Date) values (1, '2024-06-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.

According to the description, the order must have one order item (AwersomeProduct, ID: 1, count: 1) stored in OrderItems. This script never inserts into OrderItems, so the created order remains empty and does not satisfy the task requirements.

Comment thread task.sql Outdated
START TRANSACTION;

-- And some data should be created inside the transaction
update Products set WarehouseAmount = WarehouseAmount - 1 where ID = 1;

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 product quantity update is inside a transaction, which is good, but it needs to be paired with an INSERT into OrderItems in the same transaction so that adding an item and reducing WarehouseAmount succeed or fail together.

@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 there is still a critical correctness issue that breaks the core requirement of linking the order item to the newly created order. In task.sql, you currently hardcode OrderID = 1 in the INSERT INTO OrderItems statement (around the line where you insert ProductID = 1, Count = 1), which means the item may attach to an existing order with ID 1 instead of the order you just inserted. This will fail in environments where the new order’s ID is not 1, and it violates the requirement that “the SQL code creates a new order and adds a product to it” as a single, consistent operation.

To fix this, right after your INSERT INTO Orders (...) VALUES (...); statement, add something like SET @orderId = LAST_INSERT_ID(); to capture the ID of the row you just created. Then, update your INSERT INTO OrderItems inside the transaction to use @orderId instead of 1 for the OrderID column, and similarly update the related UPDATE Products (if it references OrderID) so everything is tied to that specific new order. Your use of a transaction around the OrderItems insert and the Products update is otherwise correct and matches the business rules, so once you link them with the captured @orderId, your script should fully satisfy the requirements. Keep going—you’re very close, and this adjustment will make your solution robust and correct across different database states.


✨ 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

-- And some data should be created inside the transaction

insert into OrderItems (OrderID, ProductID, Count) values (1, 1, 1);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Here you hardcode OrderID = 1. This breaks the requirement that the order item must belong to the newly created order. Capture the new order’s ID with LAST_INSERT_ID() after the INSERT INTO Orders and then use that variable (e.g., @orderId) here instead of 1.

Comment thread task.sql Outdated
-- And some data should be created inside the transaction

insert into OrderItems (OrderID, ProductID, Count) values (1, 1, 1);
update Products set WarehouseAmount = WarehouseAmount - 1 where ID = 1;

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 UPDATE is correctly inside the transaction with the OrderItems insert, but for full correctness it should be tied to the order you just created by using the captured @orderId above rather than assuming OrderID = 1 exists.

@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 — the task.sql script correctly creates a new order for CustomerID = 1 with date 2023-01-01, then uses LAST_INSERT_ID() to link the order items to the newly created order. You correctly separate the non-transactional order creation from the transactional part where you insert the order item and update Products.WarehouseAmount, which matches the requirement that stock changes and item inserts happen together in a transaction. The use of USE ShopDB; and the correct table and column names shows you’re aligning with the expected database schema. Most remaining requirements (like recreating the database, ensuring tables are empty, and PR process) are procedural and not expected to appear directly in this script.

If you want to improve this further, you could consider wrapping the order-item creation and stock update logic into a reusable stored procedure, but this is not required for the task. Overall, you’ve implemented the core logic cleanly and in line with the checklist, and you’re ready to move on. Good work applying transactions and LAST_INSERT_ID() correctly — these are important concepts to master for real-world database operations.


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

Footnotes

  1. Rate AI review example

@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