20

I have a table Content like this:

id | text | date | idUser → User | contentType

And another table Answer:

idAnswer → Content | idQuestion → Content | isAccepted

I want to ensure that the Answer's date is bigger than the Question's date. A question is a Content with contentType = 'QUESTION'.

I tried to solve this with the following trigger, but when I try to insert an Answer there's an error: 

ERROR:  record "new" has no field "idanswer"
CONTEXT:  SQL statement "SELECT (SELECT "Content".date FROM "Content" WHERE "Content".id = NEW.idAnswer) < (SELECT "Content".date FROM "Content" WHERE "Content".id = NEW.idQuestion)"
PL/pgSQL function "check_valid_date_answer" line 2 at IF

Trigger:

CREATE TRIGGER check_valid_answer 
AFTER INSERT ON "Answer"
FOR EACH ROW EXECUTE PROCEDURE check_valid_date_answer();

Trigger function:

CREATE FUNCTION check_valid_date_answer() RETURNS trigger
    LANGUAGE plpgsql
    AS $$BEGIN
  IF (SELECT "Content".date FROM "Content"
      WHERE "Content".id = NEW.idAnswer)
   < (SELECT "Content".date FROM "Content"
      WHERE "Content".id = NEW.idQuestion) 
  THEN
    RAISE NOTICE 'This Answer is an invalid date';
  END IF;
  RETURN NEW;
END;$$;

So, my question is: do I really need to create a trigger for this? I saw that I can't use a CHECK in Answer because I need to compare with an attribute of another table. Is there any other (easier/better) way to do this? If not, why the error and how can I solve it?

Hugo Sousa
  • 1,904
  • 2
  • 15
  • 28
  • Your `create function` has `if (select`. The error message says `SELECT (SELECT `. Are you sure the code in the question corresponds to the code that generated the error? – Gordon Linoff Mar 29 '14 at 16:00
  • @Gordon Linoff That function was created in the `phpPgAdmin` graphic interface, but I copy-pasted that from the file exported from there. – Hugo Sousa Mar 29 '14 at 16:04

2 Answers2

31

Your basic approach is sound. The trigger is a valid solution. It should work except for 3 problems:

1) Your naming convention:

We would need to see your exact table definition to be sure, but the evidence is there. The error message says: has no field "idanswer" - lower case. Doesn't say "idAnswer" - CaMeL case. If you create CaMeL case identifiers in Postgres, you are bound to double-quote them everywhere for the rest of their life.

2) Abort violating insert

  • Either raise an EXCEPTION instead of a friendly NOTICE to actually abort the whole transaction.

  • Or RETURN NULL instead of RETURN NEW to just abort the inserted row silently without raising an exception and without rolling anything back.

I would do the first. This will probably fix the error at hand and work:

CREATE FUNCTION trg_answer_insbef_check()
  RETURNS trigger AS
$func$
BEGIN
   IF (SELECT c.date FROM "Content" c WHERE c.id = NEW."idAnswer")
    < (SELECT c.date FROM "Content" c WHERE c.id = NEW."idQuestion") THEN
      RAISE EXCEPTION 'This Answer is an invalid date';
   END IF;
   RETURN NEW;
END
$func$  LANGUAGE plpgsql;

The proper solution is to use legal, lower case names exclusively and avoid such problems altogether. That includes your unfortunate table names as well as the column name date, which is a reserved word in standard SQL and should not be used as identifier - even if Postgres allows it.

3) Should be a BEFORE trigger

CREATE TRIGGER insbef_check
BEFORE INSERT ON "Answer"
FOR EACH ROW EXECUTE PROCEDURE trg_answer_insbef_check();

You want to abort invalid inserts before you do anything else.

Of course you will have to make sure that the timestamps table Content cannot be changed or you need more triggers to make sure your conditions are met.
The same goes for the fk columns in Answer.

Erwin Brandstetter
  • 605,456
  • 145
  • 1,078
  • 1,228
  • Sorry, `idAnswer` is the correct column name, my bad. However, something isn't working correctly. I'm inserting an `Answer` with a date lower than the `Question` and the insert succeeds. PS: there's no `()` after the trigger name. – Hugo Sousa Mar 29 '14 at 20:19
  • @HugoSousa: Ah yes, there is a third problem: you are not actually aborting the insert. Adding 3) to the answer ... – Erwin Brandstetter Mar 29 '14 at 20:25
  • 1
    Just found the problem myself. Need to raise an `EXCEPTION`, not a `NOTICE`, it worked. Thanks a lot for the complete answer. Seems like I'm going to spend some time now changing my table and columns names :) Also, just found this post answered by you http://stackoverflow.com/questions/18409952/trigger-vs-check-constraint. As a good practice, shouln't I be doing this with a `CHECK`constraint, as stated by @Str. ? – Hugo Sousa Mar 29 '14 at 20:27
  • @HugoSousa: Exactly. Or `RETURN NULL`. – Erwin Brandstetter Mar 29 '14 at 20:29
1

I would approach this in a different way.

Recommendation:

  • use a BEFORE INSERT trigger if you want to change data before inserting it
  • use a AFTER INSERT trigger if you have to do additional work
  • use a CHECK clause if you have additional data consistency requirements.

So write a sql function that checks the condition that one date be earlier than the other, and add the check constraint. Yes, you can select from other tables in your function.

I wrote something similar (complex check) in answer to this question on SO.

Community
  • 1
  • 1
Str.
  • 1,389
  • 9
  • 14