Solution#194
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there is a critical issue that prevents your SQL script from running successfully and one more that risks data consistency. First, in task.sql on the INSERT INTO Orders statement (around line 3), you use the column name CustomersID, but the schema defines the column as CustomerID; this mismatch causes a runtime error (Unknown column 'CustomersID') and must be corrected to CustomerID so the insert can execute. Second, your transaction currently starts after the order is created, so the INSERT INTO Orders is not part of the same atomic unit as inserting into OrderItems and updating Products.WarehouseAmount; if a failure happens after the order insert but before the commit, you can end up with an order that has no items or stock changes. To align with the business rules that order creation, its items, and the stock update should be consistent, move START TRANSACTION; so that it wraps the order insert, the LAST_INSERT_ID() retrieval, the OrderItems insert, and the Products update, and then COMMIT; at the end. Aside from these issues, the overall flow (creating the order first, then adding one item for product ID 1 with count 1, and decreasing WarehouseAmount by the ordered quantity) is correct and close to the desired solution. Once you fix the column name and expand the transaction to cover all related operations, your script should meet the task requirements solidly.
✨ 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 (CustomersID, Date) VALUES (1, '2023-01-01'); |
There was a problem hiding this comment.
The Orders table is described as having a CustomerID column, not CustomersID. Using the wrong column name will cause this INSERT to fail; update the column name to match the schema from create-database.sql.
|
|
||
| -- Start the transaction | ||
| START TRANSACTION; | ||
| START TRANSACTION; |
There was a problem hiding this comment.
Currently you start the transaction after inserting the order. For better consistency with the business rules, consider starting the transaction before creating the order so that the order creation, order item insertion, and stock update are all part of one atomic transaction.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because, although your SQL logic for creating the order and updating stock is correct, it does not match the transactional behavior required by the tests. The automated check reports that the Orders table is being updated inside the transaction, while the task’s tests expect the update to Orders to happen after the transaction is committed. In other words, your START TRANSACTION ... COMMIT block should not include the Orders update; that statement must be moved below the COMMIT; so it runs outside the transaction. This mismatch is a critical blocker because it causes the functional test (test.sh) for transaction behavior to fail, even though the data changes themselves look correct.
To fix this, identify the part of task.sql where you insert/update Orders and adjust the structure so that: 1) any required transactional work (like inserting into OrderItems and updating Products) stays inside START TRANSACTION; ... COMMIT;, and 2) the Orders update statement runs only after the transaction has finished. Once you separate these concerns as expected by the test, your solution should satisfy both the functional and transactional requirements. You already handled the schema correctly and used LAST_INSERT_ID() and MySQL transaction syntax well, so this is mainly about aligning the ordering of statements with the specified test behavior. Keep that transaction boundary in mind as a key learning point for future database tasks.
✨ 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
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 continue progressing while you learn, even though there is still one important transactional detail to fix. You did a solid job using the ShopDB database, creating the order for customer 1 on 2023-01-01, inserting an OrderItems row with ProductID = 1 and Count = 1, and correctly decreasing WarehouseAmount inside a transaction. The remaining issue is that your INSERT INTO Orders and the SET @OrderID = LAST_INSERT_ID(); statement are both placed before START TRANSACTION;, but the requirements and tests expect them to run after COMMIT; so that the order is created only after the OrderItems and Products changes have been safely committed [CHECKLIST ITEM #11], [CHECKLIST ITEM #12], [CHECKLIST ITEM #18]. To fix this, move the INSERT INTO Orders ... and the SET @OrderID = LAST_INSERT_ID(); lines so they appear below COMMIT;, while keeping only the INSERT INTO OrderItems ... and UPDATE Products ... between START TRANSACTION; and COMMIT;. This will also better reflect the idea that the order is finalized only after its items and stock changes are successfully handled [CHECKLIST ITEM #14]. Overall, your understanding of the core logic is strong, and with this small transactional reordering, your solution will fully match the intended behavior.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| -- And some data should be created inside the transaction | ||
| USE ShopDB; | ||
|
|
||
| INSERT INTO Orders (CustomerID, Date) VALUES (1, '2023-01-01'); |
There was a problem hiding this comment.
Per checklist item #18 and the previous review, the Orders table insert is still executed before the transaction rather than after COMMIT;. The tests expect the order creation to happen outside and after the transactional block that handles OrderItems and Products, so you should move this statement below COMMIT;.
| USE ShopDB; | ||
|
|
||
| INSERT INTO Orders (CustomerID, Date) VALUES (1, '2023-01-01'); | ||
| SET @OrderID = LAST_INSERT_ID(); |
There was a problem hiding this comment.
LAST_INSERT_ID() here depends on the order row inserted above, but both are currently before START TRANSACTION;. According to the requirements, the order creation and retrieval of its ID should happen after the transaction that inserts into OrderItems and updates Products has been committed, so this statement should also be moved to follow COMMIT;.
No description provided.