Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Solution #60

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

MykhailoIvchenko
Copy link

No description provided.

Copy link

@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.

Thank you for your hard work on this task! 😊 While you've made good progress towards normalizing the database, there are a few critical issues that need to be addressed to fully meet the task requirements:

  1. Normalization to 3NF: The ProductInventory table should not directly reference attributes like WarehouseName, WarehouseAddress, and CountryID. Instead, it should rely on WarehouseID, with proper relationships to other tables. This is crucial for achieving the third normal form (3NF).

  2. Missing DROP DATABASE Statement: The task specifies that the script should include DROP DATABASE ShopDB; to handle cases where the database already exists. This is important for ensuring the script can be run multiple times without errors.

  3. Test Data Population: Ensure that the INSERT statements are correctly populating data according to the new normalized schema. This is essential for verifying the database structure.

Additionally, consider reviewing the foreign key constraints and using ON DELETE CASCADE where appropriate to maintain data integrity when related records are deleted.

Please address these issues to ensure the database is correctly structured and meets the task requirements. Keep up the great work, and don't hesitate to reach out if you need further guidance! 🌟

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

🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.

task.sql Outdated
ID INT AUTO_INCREMENT,
Name VARCHAR(50),
CountryID INT,
FOREIGN KEY (CountryID) REFERENCES Countries(ID) ON DELETE NO ACTION,

Choose a reason for hiding this comment

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

The foreign key constraint ON DELETE NO ACTION might cause issues if a country is deleted while cities still reference it. Consider using ON DELETE CASCADE if you want to automatically delete dependent cities when a country is deleted.

task.sql Outdated
ID INT AUTO_INCREMENT,
StreetName VARCHAR(100),
CityID INT,
FOREIGN KEY (CityID) REFERENCES Cities(ID) ON DELETE NO ACTION,

Choose a reason for hiding this comment

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

Similar to the previous comment, using ON DELETE NO ACTION for the foreign key constraint might cause issues if a city is deleted while addresses still reference it. Consider using ON DELETE CASCADE if appropriate.

task.sql Outdated
ID INT AUTO_INCREMENT,
Name VARCHAR(50),
AddressID INT,
FOREIGN KEY (AddressID) REFERENCES Addresses(ID) ON DELETE NO ACTION,

Choose a reason for hiding this comment

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

For the foreign key constraint on AddressID, consider using ON DELETE CASCADE if you want to automatically delete dependent warehouses when an address is deleted.

task.sql Outdated
ProductName VARCHAR(50),
WarehouseAmount INT,
WarehouseID INT,
FOREIGN KEY (WarehouseID) REFERENCES Warehouses(ID) ON DELETE NO ACTION,

Choose a reason for hiding this comment

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

The foreign key constraint ON DELETE NO ACTION for WarehouseID might cause issues if a warehouse is deleted while products still reference it. Consider using ON DELETE CASCADE if appropriate.

task.sql Outdated
Comment on lines 67 to 69
VALUES ('AwersomeProduct', 2, 1);
INSERT INTO ProductInventory (ProductName,WarehouseAmount, WarehouseID)
VALUES ('AwersomeProduct', 5, 2);

Choose a reason for hiding this comment

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

There seems to be a typo in the product name 'AwersomeProduct'. It should likely be 'AwesomeProduct'.

Copy link

@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

Great effort on your SQL script! 🎉 While your .gitignore file is perfectly set up, there are a few critical areas in your SQL script that need attention:

  1. Missing DROP DATABASE Statement: It's important to include DROP DATABASE ShopDB; at the beginning of your script. This ensures that the script can be executed multiple times without encountering errors due to the database already existing.

  2. Normalization to 3NF: You've done a good job normalizing the ProductInventory table by using WarehouseID with proper foreign key relationships. This aligns with the third normal form (3NF) principles.

  3. Test Data Population: Ensure that your INSERT statements correctly populate data according to the normalized schema. There's a minor issue with using a non-existent WarehouseID of 2 in the ProductInventory table. Please correct this to match the available data.

  4. Foreign Key Constraints: Your foreign key constraints are well-defined, and the use of ON DELETE CASCADE is appropriate for maintaining data integrity.

The decision to approve is made to help you move forward with your learning journey. Please review the comments and address the issues to improve your code. Remember, addressing critical issues like the missing DROP DATABASE statement is essential to avoid similar problems in the future. Keep up the good work and continue learning from these experiences! 🚀

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

task.sql Outdated
@@ -1,33 +1,72 @@
-- Drop the old database
CREATE DATABASE ShopDB;

Choose a reason for hiding this comment

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

You should include DROP DATABASE ShopDB; before this line to handle cases where the database already exists.

Comment on lines +69 to +72
INSERT INTO ProductInventory (ProductName, WarehouseAmount, WarehouseID)
VALUES ('AwersomeProduct', 2, 1);
INSERT INTO ProductInventory (ProductName, WarehouseAmount, WarehouseID)
VALUES ('AwersomeProduct', 5, 2);

Choose a reason for hiding this comment

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

Ensure that the test data is correctly populated according to the normalized schema. Verify that all foreign key references are valid.

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