tiberius icon indicating copy to clipboard operation
tiberius copied to clipboard

`rows_affected` length is incorrect when table has trigger

Open gaoqiangz opened this issue 4 years ago • 6 comments

CREATE TABLE t_test(
	id INT NOT NULL,
)

INSERT INTO t_test(id) VALUES(1);

CREATE TRIGGER [dbo].[tr_test]
   ON  [dbo].[t_test]
   AFTER INSERT,UPDATE,DELETE
AS 
BEGIN
	PRINT 'tr_test';
END
let rv = client.execute("UPDATE t_test SET id += 1", &[]).await?;
println!("sql affected: {:?}", rv.rows_affected()); //sql effected: [0, 1]

gaoqiangz avatar Jul 23 '21 12:07 gaoqiangz

On vacation right now. Coming back to work on Monday, taking a look asap next week.

pimeys avatar Jul 23 '21 14:07 pimeys

So, what should we expect here? Basically what we want is to get all the affected rows, so when parsing the server response, we count modifications from all procedures executed in the query, returning the results as a slice per query.

We can expand this a bit into:

CREATE TABLE t_test3
(
    id INT NOT NULL,
)
INSERT INTO t_test3(id)
VALUES (1);

CREATE TRIGGER [dbo].[tr_test3]
    ON  [dbo].[t_test3]
    AFTER INSERT,UPDATE,DELETE
    AS
BEGIN
    INSERT INTO t_test3(id) VALUES (222)
END

Now, what we get back from the server are two DONEINPROC events:

DoneInProc(
    TokenDone {
        status: BitFlags<DoneStatus> {
            bits: 0b10001,
            flags: More | Count,
        },
        cur_cmd: 195,
        done_rows: 1,
    },
);
DoneInProc(
    TokenDone {
        status: BitFlags<DoneStatus> {
            bits: 0b10001,
            flags: More | Count,
        },
        cur_cmd: 197,
        done_rows: 1,
    },
);

In your example, the first event the server returns is having done_rows set to 0 due to the trigger not modifying anything. These events do not seem to hold any information are they from a trigger or from a query the user executed, so filtering them out would be tricky. Also, you'd probably like to know the changes in total for your query, including triggers, so I don't really see yet how the result from Tiberius is incorrect.

Of course I'm open for discussion here, and maybe even understanding a bit more how other SQL Server libraries are handling this. I tried to dig into the JDBC driver, but couldn't find yet anything to help me to understand the issue a bit better...

pimeys avatar Jul 26 '21 08:07 pimeys

And, if you do not care about results per query, you can try out the ExecuteResult::total method, that returns only one number that should reflect the total number of rows affected (including triggers) in your query. This also works in a similar way with DataGrip, that returns one number but includes triggers if they modify data:

master> UPDATE t_test3 SET id += 1
[2021-07-26 10:43:47] 2 rows affected in 13 ms

pimeys avatar Jul 26 '21 08:07 pimeys

It looks really difficult to handle. I am already using the total method in project.The problem I was encountered is publish a message to the Broker queue in the trigger, which affected the total number result of rows.

gaoqiangz avatar Jul 26 '21 10:07 gaoqiangz

Maybe there is no perfect solution to this problem.

gaoqiangz avatar Jul 26 '21 10:07 gaoqiangz

I know this is a very old issue, but just something to add that might help.

I'm noob at Rust (just checking out this library) but have fairly good knowledge of MSSQL.

Just in case what you actually want is the count of rows affected by your query, and you want to ignore the number of rows affected by your trigger, the correct fix is actually to update your trigger to not return the number of rows affected.

Modifying the example posted earlier in this thread (see comment with 1 line added)

CREATE TABLE t_test3
(
    id INT NOT NULL,
)
INSERT INTO t_test3(id)
VALUES (1);

CREATE TRIGGER [dbo].[tr_test3]
    ON  [dbo].[t_test3]
    AFTER INSERT,UPDATE,DELETE
    AS
BEGIN
    SET NOCOUNT ON -- add this line
    INSERT INTO t_test3(id) VALUES (222)
END

The way the trigger is written before, even if you were using MSSQL on its own (without any programming language) and you wanted to get the number of affected rows by the query, for example with the following SQL query...

INSERT INTO t_test3(id)
VALUES (1);

SELECT @@ROWCOUNT

... the trigger will affect the value of @@ROWCOUNT unless you use SET NOCOUNT ON in your trigger.

It's standard practice to always use SET NOCOUNT ON in triggers (and often in stored procedures), precisely because they cause side effects like the one you are experiencing, and the fact you are not using it immediately stuck out to me.

Reikooters avatar Jan 22 '23 15:01 Reikooters