Edit task.sql#191
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because the core transactional requirement is not yet met and tests are failing as a result. Right now, INSERT INTO Orders and INSERT INTO OrderItems run outside the transaction, and only the UPDATE Products is wrapped by START TRANSACTION ... COMMIT, so creating an order item and updating WarehouseAmount are not atomic as required. You also hardcode OrderID = 1 when inserting into OrderItems, which will break as soon as the Orders table is non-empty or uses auto-increment IDs; instead you should insert the order inside the transaction and then use the generated ID (e.g. LAST_INSERT_ID() in MySQL) for the order item. Another functional issue is that you subtract a fixed 1 from WarehouseAmount rather than the actual Count value of the order item, which violates the rule that ordering 5 items should decrease stock by 5. To fix this, start the transaction before inserting into Orders, insert the order, capture its ID, insert the corresponding OrderItems row with that ID and Count, and then update Products.WarehouseAmount by the same Count in a single transaction, using numeric literals for IDs (WHERE ID = 1) and adjusting the OrderItems insert to match the actual schema (include ID if it is not auto-increment). Once these transactional and ID-handling issues are resolved, your script will match the business rules much more robustly, and the rest of your approach (creating an order for customer 1 with the specified product) is on the right track.
✨ 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
| USE ShopDB; | ||
|
|
||
| -- Some data should be created outside the transaction (here) | ||
| INSERT INTO Orders (CustomerID, Date) VALUES (1, '2026-06-14'); |
There was a problem hiding this comment.
INSERT INTO Orders is executed outside any transaction. According to the business rules, at least the order creation and related item/stock updates should be atomic; consider moving this insert inside the transaction block.
|
|
||
| -- Some data should be created outside the transaction (here) | ||
| INSERT INTO Orders (CustomerID, Date) VALUES (1, '2026-06-14'); | ||
| INSERT INTO OrderItems (OrderID, ProductID, Count) |
There was a problem hiding this comment.
This insert hardcodes OrderID = 1 and runs outside the transaction. If the Orders table already contains rows or uses auto-increment IDs, this may point to the wrong order and breaks atomicity with the stock update; use the last inserted order ID and keep this inside the transaction.
| -- Some data should be created outside the transaction (here) | ||
| INSERT INTO Orders (CustomerID, Date) VALUES (1, '2026-06-14'); | ||
| INSERT INTO OrderItems (OrderID, ProductID, Count) | ||
| VALUES (1, 1, 1); |
There was a problem hiding this comment.
Here you assume order ID 1 exists and corresponds to the newly created order. Instead, use LAST_INSERT_ID() (or equivalent) so the order item is correctly linked to the order regardless of existing data.
| VALUES (1, 1, 1); | ||
|
|
||
| -- Start the transaction | ||
| START TRANSACTION; |
There was a problem hiding this comment.
The transaction currently protects only the UPDATE Products. The requirement says creating an OrderItem and updating WarehouseAmount should be tied together; consider starting the transaction before inserting into Orders/OrderItems so they are all committed or rolled back together.
| START TRANSACTION; | ||
|
|
||
| -- And some data should be created inside the transaction | ||
| UPDATE Products SET WarehouseAmount = WarehouseAmount - 1 |
There was a problem hiding this comment.
You subtract a fixed 1 from WarehouseAmount, which matches Count = 1 now, but the requirement states that stock should be decreased by the item count. It would be better to tie this to the actual Count value, especially if the script is later modified to handle other quantities.
|
|
||
| -- And some data should be created inside the transaction | ||
| UPDATE Products SET WarehouseAmount = WarehouseAmount - 1 | ||
| WHERE ID = '1'; |
There was a problem hiding this comment.
Using '1' as a string for the product ID is risky if ID is an integer column; prefer WHERE ID = 1 to match the schema type and avoid implicit casts.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because the current script does not align with the test expectations around transaction boundaries, which is a core requirement. Your transactional logic for creating the order, inserting the order item, and updating the product stock is otherwise solid, but the automated tests are failing because Orders is being updated inside the transaction, while the task expects Orders updates to happen outside that transaction block. To fix this, you need to restructure task.sql so that the transaction only wraps the operations that must be atomic (like stock changes or other specified operations), and move any UPDATE Orders ... statements to execute before or after START TRANSACTION/COMMIT. Additionally, the order date you are inserting (2026-06-14) may not match what the provided tests expect; you should change it to the example date from the task (likely 2023-01-01) so that date-based checks pass. As a minor improvement, you can also keep @qty and the OrderItems.Count column in sync by deriving the quantity from a single source so they cannot diverge if you change the value later. Overall, you’re very close—once you move the Orders update outside the transaction and align the date with the expected value, your solution should meet all 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
| START TRANSACTION; | ||
|
|
||
| -- And some data should be created inside the transaction | ||
| INSERT INTO Orders (CustomerID, Date) VALUES (1, '2026-06-14'); |
There was a problem hiding this comment.
The task description’s example uses 2023-01-01 for the order date, and the provided test.sql may rely on that specific value; using 2026-06-14 could cause tests to fail even though it is logically allowed, so consider switching to the documented example date.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution as an educational override so you can keep progressing while you refine the last detail. You did a solid job moving the order creation outside the transaction, using the correct date 2023-01-01, working in ShopDB, and wrapping the order item insert plus stock update in a transaction as required. The remaining issue is with quantity consistency [CHECKLIST ITEM #19]: in task.sql, the quantity 1 is hard-coded in the INSERT INTO OrderItems (..., Count) while you also set @qty = 1 later, which means they could get out of sync if one is changed and the other is not. To improve this, define @qty once (for example, right before the INSERT) and then use @qty both in the OrderItems.Count value and in the UPDATE Products SET WarehouseAmount = WarehouseAmount - @qty, so there is a single source of truth for the quantity. Everything else in your script aligns well with the transactional and business rules, and this is a good step toward writing robust and maintainable SQL. Keep going—you’re very close to a fully polished solution.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| INSERT INTO OrderItems (OrderID, ProductID, Count) | ||
| VALUES (@order_id, 1, 1); |
There was a problem hiding this comment.
Here the quantity is hard-coded as 1, but you also separately set @qty = 1 below; per checklist item #19, consider defining @qty once before this INSERT and using @qty here so the order item count and stock update always stay in sync if the quantity changes.
| SET @qty = 1; | ||
| UPDATE Products SET WarehouseAmount = WarehouseAmount - @qty |
There was a problem hiding this comment.
Because the quantity is hard-coded here and also assigned separately in SET @qty = 1, there is a risk they diverge if one is changed; instead, set @qty once and use it in both the INSERT and this UPDATE to derive the quantity from a single source (checklist item #19).
No description provided.