firebird icon indicating copy to clipboard operation
firebird copied to clipboard

Dependencies check fixes

Open ilya071294 opened this issue 4 years ago • 7 comments

  1. The first commit fixes this script:

set autoddl off;

create table test (id numeric); commit;

create view v_test (id) as select id from test; commit;

drop view v_test; create or alter view v_test (id) as select id from test; drop table test; commit;

The last commit fails and the transaction should be rolled back but the test table is already broken. It happens because the phase 4 of delete_relation DFW cannot be rolled back. It's already completed by the moment of the exception. With the fix the exception will occur at the phase 3.

  1. The second commit fixes this:

set autoddl off;

create table test (id numeric); commit;

create table test1 (id numeric); commit;

set term ^;

create procedure NEW_PROCEDURE returns ( OUT integer) as begin select first 1 id from test into out; suspend; end^ commit^

alter procedure NEW_PROCEDURE returns ( OUT integer) as begin select first 1 id from test1 into out; suspend; end^

drop table test1^ commit^

set term ;^

rollback;

drop table test1; commit;

drop procedure NEW_PROCEDURE; commit;

drop table test; commit;

The last commit should work without any errors but it fails because the test table still depends on NEW_PROCEDURE even though the procedure is deleted. The test1 table is also broken here (like in the first script).

I would also suggest to add a commit where the phase 4 of delete_relation DFW is moved to the phase 6 (modifyRoutine DFW has the phase 5, and exceptions may occur there as well). Or maybe it should be moved to the post commit stage but it seems wrong.

ilya071294 avatar Aug 23 '21 10:08 ilya071294

IMHO, both scripts should fail on "drop table test", long before commit.

aafemt avatar Aug 23 '21 11:08 aafemt

IMHO, both scripts should fail on "drop table test", long before commit.

Sound logical, at first look.

hvlad avatar Sep 23 '21 13:09 hvlad

IMHO, both scripts should fail on "drop table test", long before commit.

I agree but to achieve this we need to compile a procedure/view, update and check dependencies at dsql stage.

ilya071294 avatar Oct 15 '21 07:10 ilya071294

Yes. What's the problem?

aafemt avatar Oct 15 '21 10:10 aafemt

Yes. What's the problem?

Do you suggest to move compiling and updating dependencies from dfw to dsql stage? It's not effective to do these actions at dsql stage. Especially while Firebird allows to execute several DDL statements in one transaction. And it should be done very carefully. For example, gbak requires to update dependencies at dfw stage during restore.

I would also suggest to add a commit where the phase 4 of delete_relation DFW is moved to the phase 6 (modifyRoutine DFW has the phase 5, and exceptions may occur there as well).

Any thoughts about that? I think it's the most important thing to do to avoid the possibility of table corruption.

ilya071294 avatar Oct 18 '21 10:10 ilya071294

Do you suggest to move compiling and updating dependencies from dfw to dsql stage? It's not effective to do these actions at dsql stage.

Yes. Strict dependency tracking is one of the most powerful Firebird features.

And it should be done very carefully. For example, gbak requires to update dependencies at dfw stage during restore.

Fill dependency list and all other metadata for changed objects at DDL stage. Create in-memory list of invalid objects bound to transaction. If compilation of a new object fail at DDL stage because of missing dependency - add the object to the list and repeat its recompilation during early DFW. If it fail there as well

  • raise error. It will allow creation of objects out-of-order.

DROP should check dependencies and raise error at DDL stage. It will solve this ticket. Alternatively it can add all dependent objects into the recompilation list. The idea is that metadata (system tables) must be consistent before DFW perform any real job on internal structures.

aafemt avatar Oct 18 '21 10:10 aafemt

I would also suggest to add a commit where the phase 4 of delete_relation DFW is moved to the phase 6 (modifyRoutine DFW has the phase 5, and exceptions may occur there as well).

Maybe it will help to avoid problems described in #6323.

ilya071294 avatar Oct 19 '21 13:10 ilya071294