Creating a procedure with PLSQL

Question

I am trying to write a PLSQL function that implements a constraint that an employee cannot be both a driver and a mechanic at the same time, in other words, the same E# from TRKEMPLOYEE cannot be in TRKDRIVER and TRKMECHANIC at the same time. If the contents of the DB violate that constraint, list the E# and NAME of all employees that are both mechanics and drivers. The abovementioned tables are something like as follows:

TRKEMPLOYEE(E#              NUMBER(12)      NOT NULL
    NAME            VARCHAR(50)     NOT NULL,
    DOB             DATE                    ,
    ADDRESS         VARCHAR(300)    NOT NULL,
    HIREDATE        DATE            NOT NULL,
CONSTRAINT TRKEMPLOYEE_PKEY PRIMARY KEY(E#))

TRKDRIVER(E#              NUMBER(12)      NOT NULL
L#              NUMBER(8)       NOT NULL,
STATUS          VARCHAR(10)     NOT NULL,
CONSTRAINT TRKDRIVER_PKEY PRIMARY KEY(E#),
CONSTRAINT TRKDRIVER_FKEY FOREIGN KEY(E#) REFERENCES TRKEMPLOYEE(E#))

TRKMECHANIC(E#              NUMBER(12)      NOT NULL
L#              NUMBER(8)       NOT NULL,
STATUS          VARCHAR(10)     NOT NULL,
EXPERIENCE      VARCHAR(10)     NOT NULL,
CONSTRAINT TRKMECHANIC_PKEY PRIMARY KEY(E#),
CONSTRAINT TRKMECHANIC_FKEY FOREIGN KEY(E#) REFERENCES TRKEMPLOYEE(E#))

I have attempted to write a function but keep getting a compile error in line 1 column 7. Can someone tell me why my code doesn't work? My code is as follows

CREATE OR REPLACE FUNCTION Verify()
IS DECLARE
  E# TRKEMPLOYEE.E#%TYPE;
  CURSOR C1 IS SELECT E# FROM TRKEMPLOYEE;
BEGIN
  OPEN C1;
  LOOP
   FETCH C1 INTO EMPNUM;
   IF(EMPNUM IN(SELECT E# FROM TRKMECHANIC )AND EMPNUM IN(SELECT E# FROM TRKDRIVER))
     SELECT E#, NAME FROM TRKEMPLOYEE WHERE E#=EMPNUM;
   ELSE
     dbms_output.put_line(“OK”);
   ENDIF
   EXIT WHEN C1%NOTFOUND;
  END LOOP;
  CLOSE C1;
END;
/

Any help would be appreciated. Thanks.

Answer 1

You are creating a function but missing the RETURN declaration. If you don't want to return a result, the use create or replace procedure.

Additionally, if you don't have any parameters, remove the brackets () and the DECLARE keyword is not correct either.

---

The the way you check the tables is not really efficient. You can get the count of all employees violating your business constraint with a single query:

select emp.e#, 
       emp.name,
       count(drv.e#) + count(mec.e#) as cnt
from trkemployee emp
   left join trkdriver drv on drv.e# = emp.e#
   left join trkmechanic mec on mec.e# = emp.e#
group by emp.e#, emp.name
having count(drv.e#) + count(mec.e#) > 1;

Additionally the EXIT WHEN C1%NOTFOUND; should be right after the FETCH statement. Otherwise you are staying in the loop even though the cursor didn't fetch anything.

If you take my simplified statement, you can loop once through all employees and print OK, nor Not OK depending on the count:

CREATE OR REPLACE procedure Verify
IS 
  empnum number(12);
  cnt    integer;
  empname varchar(50);

  CURSOR C1 IS 
      select emp.e#,
             emp.name,
             count(drv.e#) + count(mec.e#) as cnt  
      from trkemployee emp
         left join trkdriver drv on drv.e# = emp.e#
         left join trkmechanic mec on mec.e# = emp.e#
      group by emp.e#, emp.name;
BEGIN
  OPEN C1;
  LOOP
    FETCH C1 INTO empnum, empname, cnt;
    EXIT WHEN C1%NOTFOUND;

    if cnt > 1 then 
       dbms_output.put_line(to_char(empnum)||' NOK');
    else
       dbms_output.put_line(to_char(empnum)||' OK');
    end if;

  END LOOP;
CLOSE C1;
END;
/

---

Solving the problem without a stored procedure

I was thinking about a way how this business rule can be enforced with only constraints in the database:

create table trkemployee
(
    E#              NUMBER(12)      NOT NULL,
    NAME            VARCHAR(50)     NOT NULL,
    DOB             DATE                    ,
    ADDRESS         VARCHAR(300)    NOT NULL,
    HIREDATE        DATE            NOT NULL,
    emp_type        char(1)         NOT NULL,
    constraint trkemployee_pkey primary key(e#, emp_type),
    constraint chk_emp_type check (emp_type in ('d','m'))
);

create table TRKDRIVER
(
  e#              number(12)      not null,
  emp_type        char(1)         default 'd' not null,
  l#              number(8)       not null,
  status          varchar(10)     not null,
  constraint trkdriver_pkey primary key(e#),
  constraint trkdriver_fkey foreign key(e#,emp_type) references trkemployee(e#, emp_type),
  constraint check_drv_type check (emp_type = 'd')
);

create table trkmechanic
(
  e#              number(12)      not null,
  emp_type        char(1)         default 'm' not null,
  l#              number(8)       not null,
  status          varchar(10)     not null,
  experience      varchar(10)     not null,
  constraint trkmechanic_pkey primary key(e#),
  constraint trkmechanic_fkey foreign key(e#,emp_type) references trkemployee(e#, emp_type),
  constraint check_mec_type check (emp_type = 'm')
);

The idea is to include the employee type in its foreign key, and to make sure that the dependent tables cannot use the wrong type. When inserting a new trkemployee the type must be specified. The check constraint on the detail tables will reject any row with the wrong type.

To "move" an employee from one type to the other (if this is possible at all), the old detail row must be deleted first. Only then the employee's type can be changed to the other type. And finally the new detail could be inserted.

Answer 2

Mixing the answers given by a_horse_with_no_name and Ben and then 'Simplifying' a little bit gives-

CREATE OR REPLACE FUNCTION Verify
 return varchar2 IS                
BEGIN       
  FOR c_rec in (select emp.e#,
                      count(drv.e#) + count(mec.e#) as cnt  
                 from trkemployee emp
                 left join trkdriver drv on drv.e# = emp.e#
                 left join trkmechanic mec on mec.e# = emp.e#
                 group by emp.e#)
   LOOP
    if c_rec.cnt > 1 then 
       return ('BAD '||c_rec.e#);
    else
       return ('OK '||c_rec.e#);
    end if;    
  END LOOP;
EXCEPTION
  WHEN ....THEN
  --as the wise man once said "Do some meaningful exception handling here"...
END;
/

Looks like an overkill to have to DECLARE, OPEN, FETCH and CLOSE a cursor.

OR a PROCEDURE like

CREATE OR REPLACE PROCEDURE Verify
IS                
BEGIN       
  FOR c_rec in (select emp.e#,
                      count(drv.e#) + count(mec.e#) as cnt  
                 from trkemployee emp
                 left join trkdriver drv on drv.e# = emp.e#
                 left join trkmechanic mec on mec.e# = emp.e#
                 group by emp.e#)
   LOOP
    if c_rec.cnt > 1 then 
       dbms_output.put_line('BAD '||c_rec.e#);
    else
       dbms_output.put_line('OK '||c_rec.e#);
    end if;    
  END LOOP;
EXCEPTION
  WHEN ....THEN
  --as the wise man once said "Do some meaningful exception handling here"...
END;
/

Answer 3

Okay, there's a lot wrong with this function.

As a_horse_with_no_name notes if you have no parameters then remove the () and by definition a function requires a RETURN statement.

However, that's not all:

1. You can't use a SELECT in a IF statement.
2. You have to put the result in a variable or reference it as part of a cursor.
3. There's no need for the DECLARE block in a function or a procedure.
4. Your IF statement needs a THEN... But, I don't think you need this as you're only doing something in the ELSE.
5. You're fetching the results of the cursor into a non-existent variable.

I would hazard a guess that you want a procedure as I don't see what you could return. I would also guess that if you don't want to do anything with your first IF then you could put the other selects into the cursor.

Lastly I'm going to assume that you pass it the e# of the employee you want to check.

As all 3 tables are unique on e# you can use LEFT OUTER JOINS to put all your SQL in a single cursor

create or replace function verify ( Pemployee trkemployee.e#%type 
            ) return varchar2 is

   l_mechanic trkemployee.e#%type;
   l_driver  trkemployee.e#%type;

begin

   -- Doing things in SQL is more efficient.
    select m.e#, d.e#
      into l_mechanic, l_driver
      from trkemployee e
      left outer join trkmechanic m
        on e.e# = m.e#
      left outer join trkdriver d
        on e.e# = d.e#
           ;

   if l_driver is not null and l_mechanic is not null then
      return 'BAD';
   else
      return 'OK';
   end if;

end;
/