Skip to content

DB Normalised#163

Open
dimonalek wants to merge 4 commits into
mate-academy:mainfrom
dimonalek:main
Open

DB Normalised#163
dimonalek wants to merge 4 commits into
mate-academy:mainfrom
dimonalek:main

Conversation

@dimonalek
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

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 current task.sql cannot be executed successfully and does not fully meet the specified requirements.

Critical issues that must be fixed:

  1. At the very top of task.sql, before CREATE DATABASE ShopDB;, you need to add a statement to remove any existing database, e.g. DROP DATABASE IF EXISTS ShopDB; (the task explicitly requires dropping ShopDB before recreating it).
  2. The table creation order currently causes a foreign key error: ProductInventory is created while referencing Warehouses (and possibly Products) before those tables exist, which leads to ERROR 1824 (HY000). Reorder your DDL so that Countries is created first, then Products, then Warehouses, and only then ProductInventory with its foreign keys.
  3. Your two INSERT INTO ProductInventory ... statements reference ProductID = 1 and WarehouseID = 1 and 2, but there are no corresponding rows in Products and Warehouses. Before inserting into ProductInventory, add INSERT statements for at least one product with ProductID = 1 and for warehouses with WarehouseID = 1 and 2, using CountryID values that match existing Countries rows (e.g. 1 and 2).

As minor improvements (not blockers), consider enforcing NOT NULL on required columns, using DECIMAL for prices, and optionally making (ProductID, WarehouseID) unique or the primary key in ProductInventory to prevent duplicate inventory records. Overall, your normalization approach (separate Products, Warehouses, Countries, and ProductInventory) is on the right track; once these execution and data integrity issues are fixed, the script will 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread task.sql
FOREIGN KEY (ProductID) REFERENCES Products(ID) ON DELETE NO ACTION,
PRIMARY KEY (ID)
);

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 script is missing a DROP DATABASE ShopDB; statement before creating the database (the task explicitly asks to remove any existing ShopDB using DROP DATABASE ShopDB;). Add this at the top before CREATE DATABASE to match the requirements.

Comment thread task.sql
Name VARCHAR(50),
Description VARCHAR(100),
Price INT,
PRIMARY KEY (ID)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Products table is defined but there are no INSERT statements for Products. The ProductInventory rows reference ProductID = 1, so you must insert a matching product (e.g., ID = 1 with name/description/price) before inserting into ProductInventory.

Comment thread task.sql Outdated
Comment on lines +22 to +27
CREATE TABLE Warehouses (
ID INT,
Name VARCHAR(50),
Address VARCHAR(50),
CountryID INT,
FOREIGN KEY (CountryID) REFERENCES Countries(ID) ON DELETE NO ACTION,
FOREIGN KEY (CountryID) REFERENCES Countries(ID) ON DELETE NO ACTION,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Warehouses table exists but there are no INSERT statements for Warehouses. ProductInventory references WarehouseID = 1 and 2, so add INSERTs for warehouses with IDs 1 and 2 (including Address and CountryID matching the Countries rows) before inserting product inventory.

Comment thread task.sql Outdated
Comment on lines +45 to +48
INSERT INTO ProductInventory (ID,ProductID,WarehouseAmount,WarehouseID)
VALUES (1, 1, 2, 1);
INSERT INTO ProductInventory (ID,ProductID,WarehouseAmount,WarehouseID)
VALUES (2, 1, 5, 2);
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 two INSERT INTO ProductInventory (...) VALUES (...) statements will fail due to missing referenced Products and Warehouses rows. Ensure referenced Products and Warehouses are inserted first (or insert ProductInventory after those INSERTs). Also confirm the inserted warehouse CountryID values reference existing Countries (IDs 1 and 2).

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

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 one explicit checklist requirement about table creation order is not yet met, even though the schema itself is largely correct and runs. In task.sql, the CREATE TABLE Warehouses block currently appears before the CREATE TABLE Products block; you need to reorder the DDL so the sequence is exactly Countries -> Products -> Warehouses -> ProductInventory as required by the task ([CHECKLIST ITEM #13]). This ordering requirement is important because the automated grader and checklist expect that precise order, so leaving it as-is may cause your solution to be flagged even though the foreign keys work.

In addition to that required fix, there are a few recommended improvements to strengthen your schema (these are not blockers, but strongly encouraged): change Products.Price from INT to a money-safe type like DECIMAL(10,2) NOT NULL, add NOT NULL AUTO_INCREMENT to integer primary key columns such as Products.ID and Warehouses.ID, and enforce uniqueness of (ProductID, WarehouseID) in ProductInventory (either as a composite primary key or a UNIQUE constraint) to prevent duplicate inventory rows. After reordering the CREATE TABLE statements and, if you choose, applying these enhancements, rerun your script followed by the provided test.sql to confirm everything passes. Overall, you’re very close—once you fix the DDL order, your solution should satisfy the core 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
PRIMARY KEY (ID)
);

CREATE TABLE Warehouses (
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 (and the previous review) requires the DDL be ordered Countries -> Products -> Warehouses -> ProductInventory. Here Warehouses is created before Products which violates that requested order and may be flagged by the grader. Move the Products CREATE TABLE block so it appears before the Warehouses CREATE TABLE block.

Comment thread task.sql Outdated
ID INT,
Name VARCHAR(50),
Description VARCHAR(100),
Price INT,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Price is currently declared as INT. For monetary values use a decimal type (for example DECIMAL(10,2)) and consider adding NOT NULL to enforce presence of a price.

Comment thread task.sql
CREATE TABLE ProductInventory (
ID INT,
ProductName VARCHAR(50),
ProductID INT,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ProductInventory uses a surrogate ID as primary key (ID). To prevent duplicate inventory rows and better model the relationship, consider making (ProductID, WarehouseID) the primary key or at least a UNIQUE constraint on those two columns.

Comment thread task.sql
);

CREATE TABLE Products (
ID INT,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Primary key ID columns (e.g. Products.ID) are declared without NOT NULL or AUTO_INCREMENT. Consider adding NOT NULL and AUTO_INCREMENT to primary key integer columns to ensure uniqueness and simplify inserts (optional but recommended).

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

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 as an educational override to help you keep progressing while you continue learning, even though there are a few things to refine.
You did a solid job normalizing the schema into Countries, Products, Warehouses, and ProductInventory, and you correctly create the database ShopDB, drop it if it exists, and follow the required creation order so that all foreign keys can be resolved.
One improvement area is in the Products table (around the Price column definition, e.g., near line 25): using INT for prices can cause problems for money values, so consider switching to something like DECIMAL(10,2) so you can represent cents accurately.
In the ProductInventory table (around the column list and constraints, e.g., lines 45–55), you currently allow multiple rows for the same (ProductID, WarehouseID) pair; adding a UNIQUE (ProductID, WarehouseID) constraint or using these two columns as a composite primary key would prevent duplicate inventory records.
Also, for your primary key columns (e.g., ID in each table, around lines 10–12, 22–24, 34–36, and 46–48), consider declaring them as INT NOT NULL AUTO_INCREMENT so MySQL handles ID assignment automatically and you clearly express that these columns cannot be null.
These changes are not blocking for this task, but they will make your design more robust and closer to production-ready SQL.
Overall, this is a strong submission that meets the core requirements, and with a few datatype and constraint tweaks in a future revision, your database design skills will be even stronger.


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

Footnotes

  1. Rate AI review example

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.

2 participants