Potentially huge bug with triggers

Data Abstract
,
1

Got a problem whereby a trigger has been added to a table which inserts a row into another table. Both tables involved have identity fields as primary keys.

The issue is that any attempt to insert rows into the original table now produces a “record not found” error from the framework.

I guessed that, when the framework is attempting to retrieve the newly inserted identity field for the original table, it’s actually retrieving the identity field from the secondary table the trigger is inserting into, thus it gets the wrong value and can’t locate the newly inserted record.

Tracing the SQL issued to the database appears to confirm this - you’re using @@IDENTITY rather than SCOPE_IDENTITY.

Can you please confirm this behaviour? Thanks.

2

Hello

Which exactly platform/driver you are using?

3

Delphi XE6 with RO 8.3.93.1183 using FireDAC against MSSQL

4

@@Identity is used because only it works with DAD correctly.

however, you can use SCOPE_IDENTITY instead of @@Identity:

for this update MSSQL_DoGetLastAutoInc in uDAADOInterfaces.pas:

function MSSQL_DoGetLastAutoInc(const GeneratorName: string;Query: IDAServerDataset): variant;
begin
  try
    Query.SQL := 'SELECT IsNull(@@Identity, 0) as LastInc';
    Query.Open;
    result := Query.Fields[0].Value;
  finally
    Query := nil;
  end;
end;
5

Ok I’ll give that a try.

Surely this is a very dangerous problem? Any insert trigger which inserts into another table, as is common for audit triggers, is going to fail.

Is there no way this can be made more driver-specific or controlled via some kind of setting/switch rather than editing the source?

6

how autoinc feature works in DA:

  • insert data
  • get inserted autoinc

SCOPE_IDENTITY won’t work because these two sql are placed into different scopes :frowning:

7

Sorry I’m lost now. Are you saying I can’t edit the code to use SCOPE_IDENTITY after all?

8

you can update MSSQL_DoGetLastAutoInc with SCOPE_IDENTITY and review how it will work in your project.

9

Just tried making that change and I get the same error I’m afraid.

10

you can update data via stored procedure that will revert correct autoinc

11

I don’t really want to start writing stored procedures to handle all my inserts just to get around this issue.

I’m sorry but this is a fundamental design flaw. Scope_Identity was introduced specifically because @@IDENTITY has this limitation and doesn’t work with triggers which insert into a secondary table. That the DA framework is incapable of correctly retrieving the identity value on any table with such a trigger amazes me and is going to cause me huge problems. Has no one else raised this problem before?

12

afair, we had only few support requests regarding @@Identity for last 10 years

13

I’m going to try working around the problem by changing the trigger table to use a GUID rather than an Identity field.

I still believe this is a fundamental issue though - can it be escalated up the chain for further investigation/comment?

14

another workaround is move business logic from trigger to DA server, but it will work only when only DA server has exclusive access to this DB

15

I did think of that but it’s dangerous as any manipulation of these records outside of the App Server (for example running scripts directly against the database to fix problems etc) wouldn’t generate the records in the trigger table.

I’ve managed to work around it by replacing the identity field with a GUID using newsequentialid() to prevent fragmentation problems but I still think this is a huge oversight and I’m stunned it hasn’t come up before. I really think you should review this internally at some point.