@dregad do I understand right that the plan is not to fix the regression in ADOdb, but to offer a new method that has to be called by the client (MantisBT) to get the former behavior?

@atrol Correct, but this decision is not final.

The rationale is that to achieve the "Last Insert Id" functionality, the ADOdb MSSQL driver needs to execute a second query to retrieve the id after inserting the record; the performance hit for systematically doing this is quite high (nearly 3x slower, details here. This is not a big deal for interactive systems such as MantisBT, but a big issue for batch processing.

This is a compromise with ADOdb's co-maintainer, who would like to keep the behavior as-is in 5.21; I am of a different opinion, and believe that we should disable the functionality for batch processing instead so I'm planning to discuss with him again.

@obmsch, if you would like to test, please check out https://github.com/dregad/mantisbt/tree/adodb-5.21 (you'll need to run composer install to pull the ADOdb bugfix branch)

I am of a different opinion

Same for me.
Providing a new disableLastInsertID() method and reverting to previous behaviour would fix the regression while allowing a performance enhancement for users that know what they are doing.

Without that, I expect breaking more projects than just MantisBT.

@dregad Will try when I am back to my dev box. A remark on a first glance at the changes:

        if (!$rez) {
            $rez = false;

        } elseif ($retrieveLastInsertID) {
            // Get the inserted id from the 2nd result
            if (sqlsrv_next_result($rez) && sqlsrv_fetch($rez)) {
                $this->lastInsID = sqlsrv_get_field($rez, 0, SQLSRV_PHPTYPE_INT);
            }
        }

This might not yield the correct result if triggers are involved. For that reason the old
code used a while ( sqlsrv_next_result($rez) ) loop to the get the last result.

This might not yield the correct result if triggers are involved. For that reason the old
code used a while ( sqlsrv_next_result($rez) ) loop to the get the last result.

Ah OK I didn't realize that. Will fix.

Both the ADOdb PR and the MantisBT branch have been updated.

@dregad looks good (patches applied to my 2.25.0 installation). Hope you convince Mark making disable the opt in.

@obmsch following your note 0029511:0066196

At least the mssql ones (including yours) are fixed in ADOdb 5.21.1, don't know about postgres.

Can I mark this issue as resolved then ?

@dregad just done a fresh installation MantisBT 2.25.2, ADOdb 5.21.4, PHP 7.4 (not the time to check 8.0, 8.1) with MSSQL. No problems. So no objections to resolve.

Hello Martin, many thanks for taking the time to test this once again and providing feedback.