firebird icon indicating copy to clipboard operation
firebird copied to clipboard

Foreign keys broken after check constraint violation in procedure - reproducible case

Open StanTody opened this issue 3 years ago • 1 comments

Hi, colleagues !

I develop small business management software and recently I faced case, when foreign keys gets violated unconditionally.

Here is the test environment:

  • Firebird 3.0.10 (tested with both 32 and 64 bit)
  • OS: Windows
  • test database, firebird.conf, data and additional resources are in attached archive): BrokenForeignKeyTestCaseAndData.zip

Test case:

  • install UDFs library DLL from subfolder \UDF from attached ZIP
  • restore DB from backup BrakeForeignKeyTestDB.fbk in subfolder \Data from attached ZIP
  • open database
  • use SQLs from BrakeForeignKeySequence.sql from subfolder \Data from attached ZIP for bellow steps:
    • insert test data,
    • execute main statement - exception for check constraint violation is thrown; rollback the transaction
    • check for violated foreign keys by last SQLs

That is all - we have database with violated foreign keys

StanTody avatar Oct 11 '22 15:10 StanTody

I can confirm it. The problem is in handling savepoints after SUSPEND, looks like some actions were not undone after exception happens. It is already fixed in v4.

BTW, it is bad practice to change data by selectable procedure, although this is not excuse for bug presence.

hvlad avatar Oct 13 '22 16:10 hvlad

Thank you for replay ! Can we hope for fix in FB 3 or we need upgrade ?

P.S. About SUSPEND in change data procedure - can you advice me for better way to obtain multiple results from such type procedures ? Primary goal of procedure is changing data, and it returns unknown count of results. Thanks in advance !

StanTody avatar Oct 14 '22 09:10 StanTody

BTW, it is bad practice to change data by selectable procedure, although this is not excuse for bug presence.

Based in what?

asfernandes avatar Oct 14 '22 10:10 asfernandes

BTW, it is bad practice to change data by selectable procedure, although this is not excuse for bug presence.

Based in what?

I think Vlad is referring to the fact that execution of a selectable stored procedure depends on actually fetching rows, and that behaviour is not 100% deterministic if you don't fetch all rows (due to intermediate server-side and client-side fetch buffers).

mrotteveel avatar Oct 14 '22 10:10 mrotteveel

Thank you for replay ! Can we hope for fix in FB 3 or we need upgrade ?

That fix was a part of significant refactoring and I'm afraid it will be too hard to "extract" required part of it. Probably @dyemanov have another opinion.

P.S. About SUSPEND in change data procedure - can you advice me for better way to obtain multiple results from such type procedures ? Primary goal of procedure is changing data, and it returns unknown count of results. Thanks in advance !

The problem is not in SUSPEND itself but in data changes between SUSPEND's. If you find a way to make all changes first and output results then - it should be safe.

hvlad avatar Oct 14 '22 10:10 hvlad

BTW, it is bad practice to change data by selectable procedure, although this is not excuse for bug presence.

Based in what?

I think Vlad is referring to the fact that execution of a selectable stored procedure depends on actually fetching rows, and that behaviour is not 100% deterministic if you don't fetch all rows (due to intermediate server-side and client-side fetch buffers).

Exactly. Also, many users not aware that exception in procedure will automatically undo changes since last suspend only.

hvlad avatar Oct 14 '22 10:10 hvlad

About SUSPEND in change data procedure - can you advice me for better way to obtain multiple results from such type procedures ? Primary goal of procedure is changing data, and it returns unknown count of results. Thanks in advance !

Reroute the data into a temporary table instead of client. Then you can select from it after the procedure finishes its work.

aafemt avatar Oct 14 '22 10:10 aafemt

If you find a way to make all changes first and output results then - it should be safe.

Thank you, Vlad - this can be done and will save my a... head :) if solves the problem.

We all greatly appreciate your work - keep going and many thanks !

StanTody avatar Oct 14 '22 10:10 StanTody

@aafemt , @StanTody , also ORDER BY on proc return cause PLAN SORT which leads to full server side procedure result fetch and resultset materialization before returning the first record to client.

EPluribusUnum avatar Oct 14 '22 10:10 EPluribusUnum