Skip to content

study: finish task#157

Open
yapelenua wants to merge 3 commits intomate-academy:mainfrom
yapelenua:(study)-creating-table
Open

study: finish task#157
yapelenua wants to merge 3 commits intomate-academy:mainfrom
yapelenua:(study)-creating-table

Conversation

@yapelenua
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 your SQL script does not yet meet several core requirements of the task, and these will cause the tests to fail. You correctly create the ShopDB database and all four required tables with proper names and AUTO_INCREMENT primary keys, which is a solid start. However, many columns use disallowed data types: Name and FirstName/LastName/Email are VARCHAR(255) instead of VARCHAR(50) or VARCHAR(100), Description and Address use TEXT instead of a VARCHAR of allowed length, and Price uses DECIMAL(10,2) instead of an allowed type like INT (e.g., storing cents). These must all be changed so every column is strictly one of: INT, DATE, VARCHAR(50), or VARCHAR(100).

Your foreign keys are also missing the required ON DELETE SET NULL behavior, and the related columns are declared NOT NULL, which makes it impossible to set them to NULL when a referenced row is deleted. Specifically, Orders.CustomerID, OrderItems.OrderID, and OrderItems.ProductID should be nullable (remove NOT NULL) and their constraints must be written exactly like: FOREIGN KEY (CustomerID) REFERENCES Customers(ID) ON DELETE SET NULL (and similarly for OrderIDOrders(ID) and ProductIDProducts(ID)). Once you update all column types to the allowed ones and adjust each foreign key and its column nullability to use ON DELETE SET NULL, your script should satisfy the task requirements; optional improvements like CREATE DATABASE IF NOT EXISTS and consistent formatting can then be considered as polish. You are close in structure—fixing these type and foreign key details will bring your solution fully in line with the specification.


✨ 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

task.sql Outdated

CREATE TABLE Products (
ID INT PRIMARY KEY AUTO_INCREMENT,
Name VARCHAR(255) NOT NULL,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Name uses VARCHAR(255) which is not allowed. Use VARCHAR(50) or VARCHAR(100) as required by the task.

task.sql Outdated
CREATE TABLE Products (
ID INT PRIMARY KEY AUTO_INCREMENT,
Name VARCHAR(255) NOT NULL,
Description TEXT,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Description uses TEXT (disallowed). Use VARCHAR(100) (or VARCHAR(50)) because only INT, DATE, VARCHAR(50), or VARCHAR(100) are permitted.

task.sql Outdated
ID INT PRIMARY KEY AUTO_INCREMENT,
Name VARCHAR(255) NOT NULL,
Description TEXT,
Price DECIMAL(10, 2) NOT NULL,
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 DECIMAL(10, 2) which is not in the allowed types. Change to an allowed type (for example INT) per the task constraints.

task.sql Outdated

CREATE TABLE Customers (
ID INT PRIMARY KEY AUTO_INCREMENT,
FirstName VARCHAR(255) NOT NULL,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

FirstName uses VARCHAR(255) (disallowed). Use VARCHAR(50) or VARCHAR(100) instead.

task.sql Outdated
CREATE TABLE Customers (
ID INT PRIMARY KEY AUTO_INCREMENT,
FirstName VARCHAR(255) NOT NULL,
LastName VARCHAR(255) NOT NULL,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LastName uses VARCHAR(255) (disallowed). Use VARCHAR(50) or VARCHAR(100) instead.

task.sql Outdated
ID INT PRIMARY KEY AUTO_INCREMENT,
CustomerID INT NOT NULL,
Date DATE NOT NULL,
FOREIGN KEY (CustomerID) REFERENCES Customers(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 foreign key for CustomerID is missing ON DELETE SET NULL. Use the exact construction: FOREIGN KEY (CustomerID) REFERENCES Customers(ID) ON DELETE SET NULL as required by the task.

task.sql Outdated

CREATE TABLE OrderItems (
ID INT PRIMARY KEY AUTO_INCREMENT,
OrderID INT NOT NULL,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

OrderID is declared INT NOT NULL but if you use ON DELETE SET NULL on its FK it must be nullable. Remove NOT NULL to allow setting it to NULL on delete.

task.sql Outdated
CREATE TABLE OrderItems (
ID INT PRIMARY KEY AUTO_INCREMENT,
OrderID INT NOT NULL,
ProductID INT NOT NULL,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ProductID is declared INT NOT NULL but if you use ON DELETE SET NULL on its FK it must be nullable. Remove NOT NULL to allow setting it to NULL on delete.

task.sql Outdated
ID INT PRIMARY KEY AUTO_INCREMENT,
OrderID INT NOT NULL,
ProductID INT NOT NULL,
FOREIGN KEY (OrderID) REFERENCES Orders(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 foreign key on OrderID is missing ON DELETE SET NULL. Change to FOREIGN KEY (OrderID) REFERENCES Orders(ID) ON DELETE SET NULL.

task.sql Outdated
OrderID INT NOT NULL,
ProductID INT NOT NULL,
FOREIGN KEY (OrderID) REFERENCES Orders(ID),
FOREIGN KEY (ProductID) REFERENCES Products(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 foreign key on ProductID is missing ON DELETE SET NULL. Change to FOREIGN KEY (ProductID) REFERENCES Products(ID) ON DELETE SET NULL.

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 there are still critical schema issues that block the database from being created successfully and violate the task constraints. The Orders.CustomerID column is declared as INT NOT NULL while its foreign key uses ON DELETE SET NULL, which MySQL does not allow; the column must be nullable so CustomerID INT (or CustomerID INT NULL) is required for the FK to work [CHECKLIST ITEM #10]. The CI error you see (Column 'CustomerID' cannot be NOT NULL ... SET NULL) comes directly from this mismatch, so fixing it is mandatory for the script to run. Additionally, Products.Description and Customers.Address are currently defined as TEXT, but the assignment restricts you to INT, DATE, VARCHAR(50) and VARCHAR(100); change both to VARCHAR(100) (or VARCHAR(50) if you prefer) to satisfy the data type requirement [CHECKLIST ITEM #7]. Once you update CustomerID to be nullable and replace the TEXT columns with allowed VARCHAR types, the schema should both execute cleanly and align with the task rules. The rest of your design (database name, tables, primary keys, and foreign key structures) looks solid, so you are very close—these targeted fixes should resolve the blocking issues.


✨ 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

task.sql Outdated
CREATE TABLE Products (
ID INT PRIMARY KEY AUTO_INCREMENT,
Name VARCHAR(50) NOT NULL,
Description TEXT,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Description uses TEXT, which is not allowed. Change it to VARCHAR(100) (or VARCHAR(50)) so the column uses one of the permitted types (INT, DATE, VARCHAR(50), VARCHAR(100)). This addresses checklist item #7 and correction item #13.

task.sql Outdated
FirstName VARCHAR(50) NOT NULL,
LastName VARCHAR(50) NOT NULL,
Email VARCHAR(100) NOT NULL,
Address TEXT
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Address uses TEXT, which is not an allowed type. Replace it with VARCHAR(100) (or VARCHAR(50)) to comply with the permitted types. This addresses checklist item #7 and correction item #13.

task.sql Outdated

CREATE TABLE Orders (
ID INT PRIMARY KEY AUTO_INCREMENT,
CustomerID INT NOT NULL,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CustomerID is declared INT NOT NULL but the foreign key uses ON DELETE SET NULL. For that behavior to work the column must be nullable — remove NOT NULL so it is INT. This fixes checklist items #10 and correction items #14/#15.

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 approving your solution because your SQL script correctly creates the ShopDB database and all four required tables (Products, Customers, Orders, OrderItems) with the right columns and only the allowed data types. Your primary keys are properly defined with AUTO_INCREMENT, and you used the exact FOREIGN KEY (...) REFERENCES ...(...) ON DELETE SET NULL pattern with nullable foreign key columns, which matches the requirements. There are no blocking issues for the assignment based on the current specification.

As an educational note, one improvement you might consider is adding CREATE DATABASE IF NOT EXISTS ShopDB; and CREATE TABLE IF NOT EXISTS ... so that re-running the script does not cause errors if objects already exist. Another optional enhancement is explicitly setting ENGINE=InnoDB on your tables to ensure foreign keys are always supported, especially on differently configured MySQL servers. Overall, your schema design is clear, consistent, and satisfies the assignment goals—well done on getting the relationships and data types exactly right.


✨ 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