FirebirdSQL / firebird

Firebird server, client and tools
https://www.firebirdsql.org/
1.23k stars 212 forks source link

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

Open StanTody opened 1 year ago

StanTody commented 1 year ago

Hi, colleagues !

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

Here is the test environment:

Test case:

That is all - we have database with violated foreign keys

hvlad commented 1 year ago

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.

StanTody commented 1 year ago

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 !

asfernandes commented 1 year ago

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

Based in what?

mrotteveel commented 1 year ago

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).

hvlad commented 1 year ago

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 commented 1 year ago

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.

aafemt commented 1 year ago

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.

StanTody commented 1 year ago

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 !

EPluribusUnum commented 1 year ago

@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.