This is my trigger when a review is deleted,
DELIMITER $$
CREATE TRIGGER after_review_delete
AFTER DELETE ON product_reviews
FOR EACH ROW
BEGIN
UPDATE product
SET
reviews_count = CASE WHEN OLD.status = 1
THEN reviews_count-1
ELSE reviews_count
END,
rating = CASE WHEN OLD.status = 1
THEN CASE
WHEN reviews_count = 0 THEN 0
ELSE (rating*(reviews_count+1)+OLD.rating)/reviews_count
END
ELSE rating
WHERE id = OLD.pid;
END$$
DELIMITER ;
Here is a little explanation of query what does it do, it checks with case
whether review status = 1
then it means it is an active review and not a pending one which rating and addition doesn't matter until it becomes active.
What I want to ask is whether that rating or reviws_count
calculation would work accurately or not, because rating
column value is dependent or reviews_count
and whether during calculating rating would I get reviews_count
initial value or the value which has been update.
I can also do like this but it will select the product again, which my application logic already does and feels a little slow,
DELIMITER $$
CREATE TRIGGER after_review_delete
AFTER DELETE ON product_reviews
FOR EACH ROW
BEGIN
-- Declare local variables to hold current values
DECLARE current_reviews_count INT;
DECLARE current_rating DECIMAL(10, 2);
-- Fetch the current reviews count and rating
SELECT reviews_count, rating INTO current_reviews_count, current_rating
FROM product
WHERE id = OLD.pid;
-- Update the product table with conditional logic
UPDATE product
SET
reviews_count = CASE
WHEN OLD.status = 1 THEN current_reviews_count - 1
ELSE current_reviews_count
END,
rating = CASE
WHEN OLD.status = 1 THEN
CASE
WHEN current_reviews_count - 1 = 0 THEN 0 -- No reviews left
ELSE (current_rating * current_reviews_count - OLD.rating) / (current_reviews_count - 1)
END
ELSE current_rating
END
WHERE id = OLD.pid;
END$$
DELIMITER ;
I want the most accurate and fastest solution, is my first approach is correct and I can safely avoid the second approach?
I have already check this answer but it wasn't asked for triggers.
This is my server version with OS info. Mysql Ver 8.0.39-0 ubuntu0.24.04.1 for Linux on x86_64 ((Ubuntu))
Your first approach seems to be correct, but there is a problem with the order of operations in the SQL and how reviews_count is used to calculate the new rating.
SQL does not guarantee the evaluation order of expressions in an UPDATE statement. This means that the reviews_count value used in the rating calculation may not reflect the value updated during the same UPDATE operation (after the reduction). As a result, this can lead to erroneous results.
If OLD.status = 1, reviews_count is decremented by 1. Then, in the same UPDATE statement, you are using the possibly still original value of reviews_count to compute the new rating. Given that reviews_count might not yet be decremented during the rating calculation, the calculation could be based on the original reviews_count, leading to an incorrect rating.
The second approach, which uses a local variable to fetch and store the current values of reviews_count and rating before performing the calculations, is safer:
Since you're explicitly fetching the values before doing the calculations, you ensure that the values used in the calculations are correct and up-to-date.
This method avoids potential pitfalls with the order of operations in the SQL UPDATE statement.
If you ask me, I would recommend optimizing the second approach. Here are a few simple steps for this;
The second approach is more reliable to ensure reviews_count and rating are calculated correctly. Although it requires an additional SELECT, the accuracy gained is often worth the tradeoff.