Skip to content

solution#120

Open
Chornoknysh wants to merge 4 commits into
mate-academy:mainfrom
Chornoknysh:develop
Open

solution#120
Chornoknysh wants to merge 4 commits into
mate-academy:mainfrom
Chornoknysh:develop

Conversation

@Chornoknysh

Copy link
Copy Markdown

No description provided.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

Decision: REQUEST_CHANGES — a few missing constraints and engine settings are critical to data integrity and can cause test failures.

Why: Your schema meets the 3NF decomposition and inserts are correct, but several required constraints and settings are missing. These are critical blockers because they violate explicit checklist items (NOT NULL/UNIQUE, non-negative quantity) and can break referential integrity if the storage engine doesn’t enforce FKs.

Critical issues to fix (with where/why/how):

  1. Countries.Name and Products.Name not enforced as unique/non-null (task.sql — CREATE TABLE for Countries, Products)
    • Why: Checklist 2.6/3.2 requires unique identifiers; NULLs/duplicates break integrity and tests.
    • How: Set Name to VARCHAR(50) NOT NULL UNIQUE in both tables.
  2. Missing NOT NULL on FK and key columns (task.sql — Warehouses.CountryID; Inventory.ProductID, Inventory.WarehouseID)
    • Why: Checklist 3.4 expects NOT NULL on required references; NULL breaks referential semantics.
    • How: Declare these columns NOT NULL.
  3. Inventory quantity allows NULL/negative values (task.sql — Inventory.WarehouseAmount)
    • Why: Checklist 3.3/3.4 requires non-negative, non-null quantities.
    • How: Change to WarehouseAmount INT UNSIGNED NOT NULL (or an appropriate unsigned numeric type).
  4. Engine not specified for FK enforcement (all CREATE TABLE statements)
    • Why: MySQL enforces FKs only with InnoDB; without it, constraints may be ignored on some servers (Checklist 2.4).
    • How: Add ENGINE=InnoDB (optionally DEFAULT CHARSET=utf8mb4).

Important but non-blocking improvements:

  • IDs as surrogate keys with auto-increment (Countries/Products/Warehouses): use INT AUTO_INCREMENT PRIMARY KEY (Checklist 3.3). If you switch, remove explicit IDs from INSERTs or adjust accordingly.
  • Consider explicit FK actions: e.g., ON DELETE RESTRICT ON UPDATE CASCADE for clarity (Checklist 2.4).
  • Warehouses: make Name and Address NOT NULL; consider UNIQUE(Name) or UNIQUE(Name, Address) depending on intended uniqueness (Checklist 3.2).
  • Decide on DROP DATABASE IF EXISTS ShopDB; vs the exact DROP DATABASE ShopDB; if your grader requires the literal statement. The current form is safer and idempotent.
  • Minor: fix product name typo AwersomeProductAwesomeProduct if unintended.

You’re very close — the design is solid. Add these constraints/engine settings and you should fully meet the requirements. Keep it up!


✨ 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
-- Create database and tables

-- 🔄 Пересоздаємо базу
DROP DATABASE IF EXISTS ShopDB;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Line uses DROP DATABASE IF EXISTS which is idempotent and safer. The task description mentioned dropping ShopDB before creation (the spec used DROP DATABASE ShopDB;). If your grader expects the exact statement, change to DROP DATABASE ShopDB;. Otherwise, keeping IF EXISTS is preferable because it avoids an error when the DB does not exist (and still satisfies the intent of removing any pre-existing DB).

Comment thread task.sql Outdated
Comment on lines +10 to +11
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Countries table: ID INT PRIMARY KEY is valid but the checklist recommends surrogate keys (INT AUTO_INCREMENT) for IDs (3.3). Also add constraints on the name column: Name should be NOT NULL and UNIQUE to satisfy requirement 2.6 / 3.2 (unique country names). Consider changing to ID INT AUTO_INCREMENT PRIMARY KEY and Name VARCHAR(50) NOT NULL UNIQUE.

Comment thread task.sql Outdated
Comment on lines +16 to +17
);

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: The ID column is currently INT PRIMARY KEY — consider INT AUTO_INCREMENT PRIMARY KEY for a surrogate key (requirement 3.3). Also make Name NOT NULL and UNIQUE (Products.Name should be unique in test data per 3.2). This enforces data integrity and matches checklist constraints (2.3, 3.2).

Comment thread task.sql Outdated
Comment on lines 22 to 26
Address VARCHAR(100),
CountryID INT,
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: Add stronger constraints: ID can be INT AUTO_INCREMENT PRIMARY KEY; Name and Address should be NOT NULL. Also make CountryID NOT NULL and consider adding a uniqueness constraint — either UNIQUE(Name) if names are unique, or UNIQUE(Name, Address) to handle same-name warehouses in different places (check checklist 3.2). Finally, ensure the CREATE TABLE uses ENGINE=InnoDB so the FK to Countries is enforced (requirement 2.4).

Comment thread task.sql Outdated
Comment on lines 30 to 36
WarehouseID INT,
WarehouseAmount INT,
WarehouseName VARCHAR(50),
WarehouseAddress VARCHAR(50),
CountryID INT,
FOREIGN KEY (CountryID) REFERENCES Countries(ID) ON DELETE NO ACTION,
PRIMARY KEY (ID)
PRIMARY KEY (ProductID, WarehouseID),
FOREIGN KEY (ProductID) REFERENCES Products(ID),
FOREIGN KEY (WarehouseID) REFERENCES Warehouses(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.

Inventory table (junction): Good that you modeled amount via a junction table with a composite PK (ProductID, WarehouseID). Improvements needed: make ProductID and WarehouseID NOT NULL; ensure the quantity column is non-negative and NOT NULL (e.g., WarehouseAmount INT UNSIGNED NOT NULL), per checklist 3.3/3.4. Also add ENGINE=InnoDB to ensure FK enforcement and consider explicit ON DELETE/ON UPDATE behavior for your FKs (e.g., ON DELETE RESTRICT or ON DELETE CASCADE depending on desired semantics).

Comment thread task.sql Outdated
Comment on lines +42 to +48
(2, 'Country2');

-- Продукт
INSERT INTO Products (ID, Name) VALUES
(1, 'AwersomeProduct');

-- Склади

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

INSERTs and test data: Insert order (countries → products → warehouses → inventory) is correct and IDs line up. Note: if you change ID columns to AUTO_INCREMENT, either remove explicit ID values from INSERTs or explicitly set SET SESSION sql_mode = 'NO_AUTO_VALUE_ON_ZERO' / use INSERT with IDs intentionally. Also I noticed the product name AwersomeProduct — if that was meant to be AwesomeProduct, fix the spelling to preserve intended semantics.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

Main decision: Requesting changes because a few checklist-required names/statements are not met and will likely cause grading failures.

Critical issues to fix (blockers):

  1. Line ~2: DROP statement uses DROP DATABASE IF EXISTS ShopDB;. The checklist item 1.2 calls for the literal DROP DATABASE ShopDB;. If your grader is strict about exact text, replace it. Why: strict graders may search for that exact string. How: change to DROP DATABASE ShopDB;.
  2. Line ~18 (CREATE TABLE Products): Column is Name VARCHAR(50) NOT NULL UNIQUE. Requirement (3.1/3.2/4.3) is ProductName. Why: tests expect exact column names. How: rename to ProductName VARCHAR(50) NOT NULL UNIQUE and update every reference:
    • INSERT INTO Products: change column list from (ID, Name) to (ID, ProductName).
    • Any JOINs/VIEW: replace p.Name with p.ProductName.
  3. Line ~39 (CREATE TABLE Inventory): Column is WarehouseAmount INT UNSIGNED NOT NULL. Requirement (3.1/3.2/4.3) is Quantity. Why: tests check for Quantity. How: rename to Quantity INT UNSIGNED NOT NULL and update references:
    • INSERT INTO Inventory: use (ProductID, WarehouseID, Quantity); values stay numerically the same.
    • VIEW: replace i.WarehouseAmount with i.Quantity.

Minor/clarifying tips:

  • After renaming columns, ensure the compatibility VIEW selects p.ProductName and i.Quantity. If the old consumer expects the legacy column names, you can alias as needed (e.g., i.Quantity AS WarehouseAmount) while keeping the internal schema correct.
  • Re-run the script end-to-end to catch any lingering references.

What you did well:

  • Strong 3NF design with proper PKs/FKs, InnoDB, unsigned quantities, and a helpful compatibility VIEW. Creation order and test data mapping look solid.

Once these naming and literal DROP fixes are applied, the submission should pass the checklist and function as intended. Keep it up!


✨ 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
-- Create database and tables

-- 1. Пересоздаємо базу
DROP DATABASE IF EXISTS ShopDB;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Potential strict-grader issue: You use DROP DATABASE IF EXISTS ShopDB;. The checklist item 1.2 explicitly requires dropping ShopDB using the literal DROP DATABASE ShopDB; (without IF EXISTS). If the grader expects the exact statement, please replace this line with DROP DATABASE ShopDB;. If the grader accepts IF EXISTS as equivalent, no change is needed. (Checklist reference: 1.2 / 4.4)

Comment thread task.sql
CREATE TABLE Products (
ID INT NOT NULL AUTO_INCREMENT,
Name VARCHAR(50) NOT NULL UNIQUE,
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 Products table currently declares the column as Name VARCHAR(50) NOT NULL UNIQUE. The checklist requires the Products table to use the column name ProductName. Please rename to ProductName VARCHAR(50) NOT NULL UNIQUE and update all references (INSERT INTO Products, the VIEW, and any JOINs) accordingly. (Checklist references: 3.1, 3.2, 4.3)

Comment thread task.sql
Comment on lines +57 to +58
(1, 'AwersomeProduct');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

INSERT into Products currently uses (ID, Name) — after renaming the Products column you must update this INSERT to (ID, ProductName) and set the VALUES accordingly. Ensure the inserted product continues to match the original dataset. (Checklist references: 1.6, 3.5)

Comment thread task.sql
ProductID INT NOT NULL,
WarehouseID INT NOT NULL,
WarehouseAmount INT UNSIGNED NOT NULL,
PRIMARY KEY (ProductID, WarehouseID),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Inventory column is named WarehouseAmount INT UNSIGNED NOT NULL. The checklist requires the association table column to be named Quantity (not WarehouseAmount). Please rename to Quantity INT UNSIGNED NOT NULL and update all INSERTs and the VIEW to use Quantity. (Checklist references: 3.1, 3.2, 4.3)

Comment thread task.sql
Comment on lines +64 to +66
(1, 1, 2),
(1, 2, 5);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

INSERT statements into Inventory currently list values for WarehouseAmount. Update the INSERT column list to (ProductID, WarehouseID, Quantity) and change the VALUES to match (the numeric values stay the same). Example: (1, 1, 2) remains but must be inserted into Quantity. (Checklist references: 1.6, 3.5)

Comment thread task.sql Outdated
Comment on lines +72 to +73
i.WarehouseAmount,
w.Name AS WarehouseName,

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 VIEW selects p.Name AS ProductName and i.WarehouseAmount. After renaming Products.NameProductName and Inventory.WarehouseAmountQuantity, update the view to use p.ProductName and i.Quantity. This keeps backward compatibility while using the normalized column names internally. (Checklist reference: 1.6 / compatibility view)

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

Educational approval: You’re on the right track with a solid 3NF schema, correct FK relationships, non-negative Quantity via INT UNSIGNED, proper DDL/DML ordering, and a helpful compatibility VIEW. Nice work aligning the schema to the checklist core items and keeping it MySQL-executable end-to-end.

Critical issues to address next:

  • Checklist 3.3 (uniqueness for warehouses): In task.sql, inside CREATE TABLE Warehouses (around the table definition block), there’s no UNIQUE constraint on warehouse identity. Without this, duplicates can slip in. Consider adding one of: UNIQUE (Name) if names are globally unique, or UNIQUE (Name, Address) if uniqueness is defined by the combination. This prevents update anomalies and should satisfy automated checks.
  • Data consistency: On or near line 58, the product name AwersomeProduct appears misspelled. If tests expect the original value (likely AwesomeProduct), update the INSERT to match exactly to avoid mismatches.

Minor improvements (recommended):

  • Checklist 4.3: Name your constraints explicitly for clarity and verifiability, e.g., CONSTRAINT pk_countries PRIMARY KEY (ID), CONSTRAINT pk_products PRIMARY KEY (ID), CONSTRAINT pk_warehouses PRIMARY KEY (ID), CONSTRAINT pk_inventory PRIMARY KEY (ProductID, WarehouseID).

You’ve done the heavy lifting well—just tighten the warehouse uniqueness and verify the product name spelling. Keep going; you’re very close to a fully compliant and robust submission!


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

Footnotes

  1. Rate AI review example

Comment thread task.sql
-- 2.3 Warehouses
CREATE TABLE Warehouses (
ID INT NOT NULL AUTO_INCREMENT,
Name VARCHAR(50) 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.

The Name column for Warehouses is defined but there is no UNIQUE constraint. Checklist item 3.3 requires warehouse identity be uniquely defined to prevent duplicates/update anomalies. Add a uniqueness constraint — either on Name alone or as a composite with Address.

Example options to add inside the CREATE TABLE block:

  • UNIQUE KEY uq_warehouses_name (Name), -- if warehouse name alone is unique
  • UNIQUE KEY uq_warehouses_name_addr (Name, Address), -- if name+address defines uniqueness

Pick the option that matches your domain expectations and update the table DDL accordingly.

Comment thread task.sql
Comment on lines +12 to +40
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;

-- 2.2 Products
CREATE TABLE Products (
ID INT NOT NULL AUTO_INCREMENT,
ProductName VARCHAR(50) NOT NULL UNIQUE,
PRIMARY KEY (ID)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;

-- 2.3 Warehouses
CREATE TABLE Warehouses (
ID INT NOT NULL AUTO_INCREMENT,
Name VARCHAR(50) NOT NULL,
Address VARCHAR(100) NOT NULL,
CountryID INT NOT NULL,
PRIMARY KEY (ID),
CONSTRAINT fk_warehouses_country
FOREIGN KEY (CountryID) REFERENCES Countries(ID)
ON DELETE RESTRICT
ON UPDATE CASCADE
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;

-- 2.4 Inventory
CREATE TABLE Inventory (
ProductID INT NOT NULL,
WarehouseID INT NOT NULL,
Quantity INT UNSIGNED NOT NULL,
PRIMARY KEY (ProductID, WarehouseID),
CONSTRAINT fk_inventory_product

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 keys are present but are not named with explicit constraint identifiers. Checklist item 4.3 recommends explicitly naming primary keys (where possible) to improve verifiability. Consider changing the anonymous PRIMARY KEY declarations to named constraints, for example:

  • CONSTRAINT pk_countries PRIMARY KEY (ID)
  • CONSTRAINT pk_products PRIMARY KEY (ID)
  • CONSTRAINT pk_warehouses PRIMARY KEY (ID)
  • CONSTRAINT pk_inventory PRIMARY KEY (ProductID, WarehouseID)

This is not required for execution but makes automated checks and reviews clearer.

Comment thread task.sql

INSERT INTO Products (ID, ProductName) VALUES
(1, 'AwersomeProduct');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Double-check the product name inserted here: AwersomeProduct. The checklist requires that the test data reconstruct the same logical dataset as the original ProductInventory. If the original name was AwesomeProduct (or another spelling), update this value to match the original to avoid test mismatches.

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