Oracle trigger doesn't work

Question

I'm trying to get this code sorted, and it does compile, but when I test by deleting a record if gives: Invalid Identifier

The error is probably in the :old part, but I just can't figure it out.

This is my code.

create or replace trigger t_OnDeleteCategory
create or replace trigger t_OnDeleteCategory
before delete on Categorie
for each row
declare
 v_textMail  varchar2(2000);
 v_emailAdres MailAbonnee.emailAdres%type;
 v_Voornaam MailAbonnee.voornaam%type;
 v_Achternaam MailAbonnee.Achternaam%type;
 c_cursor SYS_REFCURSOR;
 v_sql varchar2(2000);
 v_categorienaam categorie.naam%type;
begin
 v_categorienaam := :old.naam;
--1) mailbericht verzenden
v_sql := 'select voornaam, achternaam, emailAdres from MailAbonnee where id in (select mailAbonneeID from CategorieAbonnement where categorieNaam = v_categorienaam)';
open c_cursor for v_sql;
loop
    fetch c_cursor into v_Voornaam, v_Achternaam, v_emailAdres;
    exit when c_cursor%notfound;
    v_textMail := 'Beste ' || v_Voornaam || ' ' || v_Achternaam || ', uw abonnement is opgeheven voor Categorie '|| v_categorienaam || '.';
    sendMailAbonnee(v_textMail, v_emailadres);
end loop;
--2) verwijder alle abonnementen
delete from CategorieAbonnement where categorieNaam = v_categorienaam;
--3) pas alle nieuwsberichten aan
update Nieuwsbericht set categorieNaam = '' where categorieNaam = v_categorienaam;
 end; 

Answer 1

Like @JustinCave suggests, you have a problem with your dynamic SQL. v_categorienaam is a variable yet it's in the string of your SQL. Remove the dynamic SQL and your problem will go away.

In addition, you can simplify your code a lot by using a cursor for loop, something like:

create or replace trigger t_OnDeleteCategory
before delete on Categorie
for each row
declare
  v_textMail  varchar2(2000);
begin
  --1) mailbericht verzenden
  for c in (
    select voornaam, achternaam, emailAdres
    from MailAbonnee
    where id in (
      select mailAbonneeID from CategorieAbonnement where categorieNaam = :old.naam
    )
  ) loop
    v_textMail := 'Beste ' || c.voornaam || ' ' || c.achternaam || ', uw abonnement is opgeheven voor Categorie '|| c.naam || '.';
    sendMailAbonnee(v_textMail, c.emailadres);
  end loop;
  --2) verwijder alle abonnementen
  delete from CategorieAbonnement where categorieNaam = :old.naam;
  --3) pas alle nieuwsberichten aan
  update Nieuwsbericht set categorieNaam = '' where categorieNaam = :old.naam;
end;

Answer 2

In this line:

v_sql := 'select voornaam, achternaam, emailAdres from MailAbonnee where id in (select mailAbonneeID from CategorieAbonnement where categorieNaam = v_categorienaam)';

.... you're referring to v_categorienaam, but that isn't a variable in scope when the dynamic SQL is executed. You should be using a bind variable:

v_sql := 'select voornaam, achternaam, emailAdres from MailAbonnee where id in (select mailAbonneeID from CategorieAbonnement where categorieNaam = :v_categorienaam)';

and then

open c_cursor for v_sql using v_categorienaam;

But there doesn't seem to be any reason for this to be dynamic at all, as others have already pointed out. You can get rid of v_sql and just do:

open c_cursor for
  select voornaam, achternaam, emailAdres
  from MailAbonnee
  where id in (
    select mailAbonneeID
    from CategorieAbonnement
    where categorieNaam = v_categorienaam;

Which is easier to format and read, and can be validated at parse/compilation time, rather than giving you any potential errors at runtime. Colin's version is even simpler; you're not gaining anything from the explicit fetch.