SQL: trigger to prevent inserting a row into a table based on a condition

Question

I have the following tables:

CREATE TABLE review 
(
    review_id NUMBER(2) NOT NULL,
    review_date DATE NOT NULL,
    review_rating NUMBER(1) NOT NULL,
    driver_no NUMBER(2) NOT NULL,
    vehicle_id NUMBER(3) NOT NULL
);

CREATE TABLE testing 
(
    testing_id NUMBER(2) NOT NULL,
    testing_start DATE NOT NULL,
    testing_end DATE NOT NULL
    driver_no NUMBER(2) NOT NULL,
    vehicle_id NUMBER(3) NOT NULL
);

Basically vehicles are tested by drivers between two dates. After the testing is complete, the driver reviews the vehicle.

I want to create a trigger which will prevent an invalid review from being added. The review is invalid if the driver reviews the vehicle before the testing end date. The review is also invalid if the driver reviews a vehicle that he has not driven.

For example, driver 1 tests vehicle 7 from 01 Feb 2019 to 07 Feb 2019. If a review is added for 05 Feb 2019, I want the trigger to prevent this from being inserted. Also, if a review is added for vehicle 5 (when vehicle 7 was the one being tested), I want the trigger to prevent this from being inserted.

This is what I have so far:

CREATE OR REPLACE TRIGGER review_check_validity
AFTER INSERT ON review
FOR EACH ROW
BEGIN
    SELECT testing_start
    FROM testing
    WHERE driver_no = :new.driver_no;

    SELECT vehicle_id
    FROM testing
    WHERE driver_no = :new.driver_no;

    IF :new.review_date < testing_end THEN
    raise_application_error(-20000, 'Review date cannot be before 
    testing end date');

    END IF;

    IF :new.vehicle_id != vehicle_id THEN
    raise_application_error(-20000, 'Driver has never driven this 
    vehicle');

    END IF;
END;
/

The trigger compiles without any errors - however when I try to test it by inserting an invalid row into the REVIEW table, I get an error message stating

Exact fetch returns more than requested number of rows

Could somebody please point out what changes I need to make to my code to achieve the desired result?

Answer 1

There are a few mistakes in your code:

• In PL/SQL select queries must have an INTO clause where the data fetched from the SELECT query is stored into some variables. -- That part is missing in your code

• You are searching in the testing table with the only driverno, which will give you all the records of that driverno(all the vehicles tested by that driveno). You need to include vehicleid along with driverno to fetch the details of testing relevant to the current review.

So Your trigger code can be re-written as follows:

CREATE OR REPLACE TRIGGER REVIEW_CHECK_VALIDITY 
BEFORE INSERT ON REVIEW -- USING BEFORE INSERT TRIGGER TO AVOID ANY UNDOs
    FOR EACH ROW
DECLARE
    LV_TEST_END_DATE   DATE;
    LV_TEST_COUNT      NUMBER;
BEGIN
    -- FETCHING RELEVANT DATA USING SINGLE QUERY
    SELECT
        COUNT(1),
        MAX(TESTING_END) -- PLEASE HANDLE THE SCENARIO WHERE THE TESTING END DATE IS NULL
    INTO
        LV_TEST_COUNT,
        LV_TEST_END_DATE
    FROM
        TESTING
    WHERE
        VEHICLE_ID = :NEW.VEHICLE_ID
        AND DRIVER_NO = :NEW.DRIVER_NO;

    IF LV_TEST_COUNT = 0 THEN
        RAISE_APPLICATION_ERROR(-20000, 'Driver has never driven this vehicle');
    ELSIF :NEW.REVIEW_DATE < LV_TEST_END_DATE THEN
        RAISE_APPLICATION_ERROR(-20000, 'Review date cannot be before testing end date');
    END IF;

END;
/

Cheers!!

Answer 2

I'd rearrange the logic here and:

1. Check if the driver has tested the vehicle, then
2. Check if the review is attempted before the testing end date for the vehicle (something you've left out).

In Oracle PL/SQL, which includes trigger code, you can't just SELECT. You have to SELECT INTO a variable. Then you can use the variable in your logic.

Equally important, when you SELECT INTO a variable the query can only return one result. Multiple rows will trigger the error you've encountered.

CREATE OR REPLACE TRIGGER review_check_validity
AFTER INSERT ON review
FOR EACH ROW
DECLARE
    testEnd DATE;
    vehicleTestCount NUMBER;
BEGIN

    SELECT COUNT(*)
      INTO vehicleTestCount
      FROM testing
      WHERE vehicle_id = :new.vehicle_id;

    IF vehicleTestCount = 0 THEN
      raise_application_error(-20000, 'Driver has never driven this vehicle');
    END IF;

    -- Assumes one test per driver per vehicle
    SELECT testing_end
      INTO testEnd
      FROM testing
      WHERE driver_no = :new.driver_no
        AND vehicle_id = :new.vehicle_id;

    IF :new.review_date < testEnd THEN
      raise_application_error(-20000, 'Review date cannot be before 
    testing end date');

    END IF;

END;
/

Finally, your table structure allows multiple tests of the same vehicle by the same driver. If it should allow this, then the review table should link to the testing table by testing_id rather than driver_no and vehicle_id.

Answer 3

I do think you should have an INTO clause in your SELECT fetches. I added some notes in your query to try to help you clearing it.

CREATE OR REPLACE TRIGGER review_check_validity
AFTER INSERT ON review
FOR EACH ROW

-- Need to declare variables for usage in your SELECT fetches.
DECLARE
var_revdate number; -- or date, if applicable
var_revvehi number; -- or varchar(n), if applicable

BEGIN
    -- In here you should have an INTO clause to assign your date parameter into 
    -- the predefined variable. As well, you need to ensure this fetch will provide
    -- only one row.

    SELECT testing_start INTO var_revdate -- why are you fetching "testing_start"?
        FROM testing
            WHERE driver_no = :new.driver_no;

    -- Same case, it can only retrieve one row. If you need to do more than one row, 
    -- you may need to use a BULK & a LOOP.

    SELECT vehicle_id INTO var_vehicid
        FROM testing
            WHERE driver_no = :new.driver_no;

    IF :new.review_date < testing_end THEN
    raise_application_error(-20000, 'Review date cannot be before testing end date');

    END IF;

    IF :new.vehicle_id != var_vehicid THEN
    raise_application_error(-20000, 'Driver has never driven this vehicle');

    END IF;
END;
/

Hope that helps.