View Issue Details

IDProjectCategoryView StatusLast Update
0028068mantisbtdb mssqlpublic2023-10-31 16:36
Reporterobmsch Assigned Todregad  
PrioritynormalSeverityblockReproducibilityalways
Status closedResolutionfixed 
Product Version2.26.0 
Target Version2.26.0Fixed in Version2.26.0 
Summary0028068: Impossible to insert child records with ADOdb 5.21.0 on mssql
Description

As reported by @obmsch in 0028015:0065205, with ADOdb 5.21.0 on MS SQL Server all MantisBT operations that require creating a base record and one or more related records in other tables fail.

For example, but not limited to:

  • create new issue (mantis_bug_table, mantis_bug_text_table)
  • add new note (mantis_bugnote_table, mantis_bugnote_text_table)
  • add subproject (mantis_project_file_table, mantis_project_hierarchy_table)
Additional Information

Upstream bug report https://github.com/ADOdb/ADOdb/issues/692

TagsNo tags attached.

Relationships

child of 0028015 closeddregad Update ADOdb to 5.21.4 

Activities

atrol

atrol

2021-03-13 05:55

developer   ~0065238

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

dregad

dregad

2021-03-13 06:06

developer   ~0065239

Last edited: 2021-03-13 06:09

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

dregad

dregad

2021-03-13 06:10

developer   ~0065240

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

atrol

atrol

2021-03-13 06:43

developer   ~0065241

Last edited: 2021-03-14 07:08

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.

obmsch

obmsch

2021-03-14 11:22

reporter   ~0065242

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

dregad

dregad

2021-03-14 12:35

developer   ~0065243

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.

dregad

dregad

2021-03-14 12:52

developer   ~0065244

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

obmsch

obmsch

2021-03-15 08:41

reporter   ~0065246

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

dregad

dregad

2022-02-07 04:05

developer   ~0066222

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

obmsch

obmsch

2022-02-07 08:49

reporter   ~0066224

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

dregad

dregad

2022-02-08 04:43

developer   ~0066226

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