Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent behaviour of the NEW context variable in AFTER UPDATE OR DELETE triggers [CORE6144] #6393

Closed
firebird-issue-importer opened this issue Sep 16, 2019 · 11 comments

Comments

@firebird-issue-importer

Submitted by: @ilya071294

Assigned to: @ilya071294

Script to reproduce:

create table test (id integer not null primary key);
commit;
insert into test (id) values (1);
commit;
alter table test add v integer default 1 not null;
commit;
insert into test (id) values (2);

create exception ex 'Failed';

set term ^;

create or alter trigger test_null for test
after update or delete
as
begin
if (new.v is not null) then -- new.v should be NULL if the trigger runs after DELETE statement
exception ex;
end^

set term ;^

commit;

delete from test where id = 2; -- no errors
delete from test where id = 1; -- trigger throws exception

I am working on a PR.

Commits: 0c5b177 0b63117 eb31af0

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Sep 16, 2019

Commented by: @livius2

Both cases should raise an error but not your definded exception.
Bug here is that Firebird do not raise exception that "NEW" variables are not avaiable in after delete trigger at all.
In after delete trigger only OLD variables are avaiable.
Of course you can check if it is insertion or deletion inside trigger code.

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Sep 17, 2019

Commented by: @dyemanov

Karol, IIRC it's documented that unavailable context in universal triggers is returned as NULL.

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Sep 17, 2019

Modified by: @dyemanov

assignee: Dmitry Yemanov [ dimitr ]

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Sep 17, 2019

Commented by: @mrotteveel

@dmitry The documentation says: "In DELETE triggers, references to the NEW variables are invalid and will throw an exception" (see https://www.firebirdsql.org/file/documentation/reference_manuals/fblangref25-en/html/fblangref25-psql-triggers.html#fblangref25-psql-oldnew). I haven't found references for UPDATE OR DELETE triggers.

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Sep 18, 2019

Commented by: @dyemanov

That is for simple triggers. However, see README.universal_triggers:

3\. In multiple\-action triggers both OLD and NEW contexts are available\. If the
   trigger invocation forbids one of them \(e\.g\. OLD context for INSERT operation\),
   then all fields of that context will evaluate to NULL\. If you assign to
   improper context, runtime exception will be thrown\.

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Sep 18, 2019

Commented by: @ilya071294

I have prepared a PR:
#225

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Sep 19, 2019

Modified by: @dyemanov

assignee: Dmitry Yemanov [ dimitr ] => Ilya Eremin [ ilya071294 ]

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Sep 19, 2019

Modified by: @dyemanov

status: Open [ 1 ] => Resolved [ 5 ]

resolution: Fixed [ 1 ]

Fix Version: 3.0.5 [ 10885 ]

Fix Version: 4.0 Beta 2 [ 10888 ]

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Sep 22, 2019

Modified by: @pavel-zotov

status: Resolved [ 5 ] => Resolved [ 5 ]

QA Status: No test => Done successfully

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Sep 22, 2019

Modified by: @pavel-zotov

status: Resolved [ 5 ] => Closed [ 6 ]

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Sep 23, 2019

Commented by: Sean Leyne (seanleyne)

Can we have details on the new functionality?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment