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

Normalize ShopDB schema #55

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

Conversation

L1mbxbx
Copy link

@L1mbxbx L1mbxbx commented Nov 29, 2024

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 SQL script! 😊 While you've made a great start, there are a few critical issues that need to be addressed to ensure the database is correctly normalized and meets the task requirements. Here's a summary of the key points:

  1. Normalization to 3NF: The ProductInventory table isn't fully normalized to the third normal form (3NF). It currently contains columns like WarehouseName, WarehouseAddress, and CountryID, which should be part of the Warehouses table. The ProductInventory table should only include ProductID, WarehouseID, and WarehouseAmount.

  2. Table Structure: Adjust the ProductInventory table to remove the columns that are not directly related to the inventory. This will help in maintaining a clean and efficient database structure.

  3. Foreign Key Constraints: Ensure that the ON DELETE NO ACTION behavior for foreign key constraints is intentional. If you want to allow deletions and automatically handle related entries, consider using ON DELETE CASCADE or ON DELETE SET NULL.

  4. Typo: There's a minor typo in the product name 'AwersomeProduct'. It should likely be 'AwesomeProduct'.

Please address these issues to ensure the database is correctly structured and meets the 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.

Address VARCHAR(50),
CountryID INT,
PRIMARY KEY (ID),
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 is specified here. Ensure that this is the intended behavior. If you want to prevent deletion of a country if it's referenced in the Warehouses table, this is correct. Otherwise, consider using ON DELETE CASCADE or ON DELETE SET NULL depending on your requirements.

task.sql Outdated
Comment on lines 33 to 34
FOREIGN KEY (ProductID) REFERENCES Products (ID) ON DELETE NO ACTION,
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.

Similar to the previous comment, verify that ON DELETE NO ACTION is the intended behavior for these foreign key constraints. If you want to allow deletion of a product or warehouse and automatically handle the related entries in ProductInventory, consider using ON DELETE CASCADE.


-- Products
INSERT INTO Products (Name)
VALUES ('AwersomeProduct');

Choose a reason for hiding this comment

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

There's 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 job on improving the SQL script! 🎉 You've successfully addressed several key issues from the previous review:

  1. Normalization to 3NF: The ProductInventory table is now correctly normalized to the third normal form (3NF). You've moved WarehouseName, WarehouseAddress, and CountryID to the Warehouses table, which is a significant improvement.

  2. Table Structure: The ProductInventory table now includes only ProductID, WarehouseID, and WarehouseAmount, aligning well with normalization requirements.

  3. Foreign Key Constraints: You've set the foreign key constraints with ON DELETE CASCADE for ProductID and WarehouseID in the ProductInventory table, which helps maintain referential integrity during deletions.

However, there are a couple of issues that need attention:

  • Typo: There's a typo in the product name 'AwersomeProduct'. Please correct it to 'AwesomeProduct' to ensure data accuracy.

  • Foreign Key Constraint for CountryID: The CountryID foreign key in the Warehouses table uses ON DELETE NO ACTION. If you want to allow deletions and automatically handle related entries, consider using ON DELETE CASCADE or ON DELETE SET NULL.

Despite these issues, I'm approving the pull request to help you move forward with your learning. Please review the comments and make the necessary corrections to improve your code. Remember, addressing these issues will enhance the functionality and maintainability of your database. 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! ✨

Address VARCHAR(50),
CountryID INT,
PRIMARY KEY (ID),
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 for CountryID in the Warehouses table uses ON DELETE NO ACTION. If you intend to allow deletions and automatically handle related entries, consider using ON DELETE CASCADE or ON DELETE SET NULL.


-- Products
INSERT INTO Products (Name)
VALUES ('AwersomeProduct');

Choose a reason for hiding this comment

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

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

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