Done#195
Conversation
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 successfully recreate the required test data and fails with foreign key errors, which blocks the task from completing. You correctly normalized the schema into Products, Warehouses, Countries, and ProductInventory, but in task.sql you never insert any rows into Products or Warehouses before inserting into ProductInventory (see around your INSERT INTO ProductInventory statements, e.g., near line 45), so the ProductID and WarehouseID values point to non‑existent parent rows. This violates the foreign key constraints and also means the logical test data (product names and warehouse names/addresses) from the original ProductInventory table is not actually present in the new structure, which breaks the requirement to preserve the same test data. To fix this, add INSERT statements for Products and Warehouses that match the original products and warehouses (using IDs like 1, 2, etc., that correspond to the IDs you already use in ProductInventory), and place those inserts before the INSERT INTO ProductInventory statements so the parent rows exist first. Additionally, prepend DROP DATABASE IF EXISTS ShopDB; before CREATE DATABASE ShopDB; so the script can be safely re-run without manual cleanup. As a minor improvement, consider adding AUTO_INCREMENT to your primary key INT columns so repeated test runs and additional inserts work smoothly. Once these issues are addressed, your design and data loading will both align well with the 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
| CREATE TABLE Products ( | ||
| ID INT, | ||
| ProductName VARCHAR(50), | ||
| WarehouseAmount INT, | ||
| PRIMARY KEY (ID) | ||
| ); |
There was a problem hiding this comment.
Products table is defined but never populated; since ProductInventory.ProductID points here, inserting into ProductInventory will fail with a foreign key error and also does not recreate the original test data for product names as required.
| CREATE TABLE Warehouses ( | ||
| ID INT, | ||
| WarehouseName VARCHAR(50), | ||
| WarehouseAddress VARCHAR(50), | ||
| WarehouseAddress VARCHAR(50), | ||
| PRIMARY KEY (ID) | ||
| ); |
There was a problem hiding this comment.
Warehouses table is defined but never populated; ProductInventory.WarehouseID references this table, so your INSERT statements into ProductInventory will violate foreign key constraints and warehouse name/address test data will be missing.
| INSERT INTO ProductInventory (ID,ProductID,WarehouseAmount,WarehouseID,CountryID) | ||
| VALUES (1, 1, 2, 1, 1); | ||
| INSERT INTO ProductInventory (ID,ProductID,WarehouseAmount,WarehouseID,CountryID) | ||
| VALUES (2, 1, 5, 2, 2); |
There was a problem hiding this comment.
The ProductInventory inserts reference ProductID = 1 and WarehouseID = 1/2, but there are no corresponding rows in Products or Warehouses, so these statements will fail due to foreign key constraints and the logical test data is incomplete. You need to insert matching Products and Warehouses first to reflect the original data.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because one core checklist requirement is still not met and can cause the script to fail on reruns. Specifically, at the very top of task.sql (around line 1), you should add DROP DATABASE IF EXISTS ShopDB; before CREATE DATABASE ShopDB; so the script can be safely executed multiple times without an error about the database already existing ([CHECKLIST ITEM #9] and [CHECKLIST ITEM #15]). Without this, the solution does not fully satisfy the task’s explicit requirements, which is considered a blocking issue.
The rest of your normalization work looks solid: Countries, Products, Warehouses, and ProductInventory are structured correctly, and your inserts preserve the original logical data. As a minor, non-blocking improvement, you could change each primary key ID column to INT AUTO_INCREMENT in those tables to make future inserts and test data management easier ([CHECKLIST ITEM #16]), but this is optional. Once you add the DROP DATABASE IF EXISTS ShopDB; line, your solution will meet the critical requirements for this task. Nice job on the schema design so far; you’re very close to a fully compliant solution.
✨ 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
| CREATE TABLE Products ( | ||
| ID INT, | ||
| ProductName VARCHAR(50), | ||
| WarehouseAmount INT, | ||
| PRIMARY KEY (ID) |
There was a problem hiding this comment.
Similarly, consider declaring ID as INT AUTO_INCREMENT here to match the recommended improvement for primary keys.
| CREATE TABLE Warehouses ( | ||
| ID INT, | ||
| WarehouseName VARCHAR(50), | ||
| WarehouseAddress VARCHAR(50), | ||
| WarehouseAddress VARCHAR(50), | ||
| CountryID INT, | ||
| FOREIGN KEY (CountryID) REFERENCES Countries(ID) ON DELETE NO ACTION, | ||
| FOREIGN KEY (CountryID) REFERENCES Countries(ID) ON DELETE NO ACTION, | ||
| PRIMARY KEY (ID) |
There was a problem hiding this comment.
For consistency and easier test data management, it would be beneficial to make this ID column AUTO_INCREMENT as well, although not strictly required by the core task.
| CREATE TABLE ProductInventory ( | ||
| ID INT, | ||
| ProductID INT, | ||
| FOREIGN KEY (ProductID) REFERENCES Products(ID) ON DELETE CASCADE, | ||
| WarehouseAmount INT, | ||
| WarehouseID INT, | ||
| FOREIGN KEY (WarehouseID) REFERENCES Warehouses(ID) ON DELETE CASCADE, | ||
| PRIMARY KEY (ID) |
There was a problem hiding this comment.
You may also want ID here to be AUTO_INCREMENT so new inventory rows can be added without manually managing IDs, aligning with the suggested improvement.
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 learning.
You did a strong job normalizing the original ProductInventory structure into separate Countries, Products, Warehouses, and ProductInventory tables, and your INSERT statements correctly recreate the original sample data in the new 3NF schema. The main remaining issue is that your script is not yet safely rerunnable: before your CREATE DATABASE ShopDB; line (near the top of task.sql), you should add DROP DATABASE IF EXISTS ShopDB; so running the script multiple times won’t fail with a “database already exists” error. This change is required by the task’s checklist and ensures the script can be executed repeatedly during testing and development without manual cleanup.
A minor improvement you could consider is making Countries.ID an INT AUTO_INCREMENT like your other ID columns for consistency, though this is not strictly required. Overall, your understanding of normalization and foreign keys is solid—fixing the rerun behavior will make your solution both correct and robust. Keep up the good work; 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! ✨
No description provided.