Bug #57243 Inconsistent use of trans_register_ha() API in InnoDB

• Submitted: 5 Oct 2010 10:55
• Modified: 30 Nov 2010 23:10
• Reporter: Konstantin Osipov (OCA)
• Status: Closed
• Category: MySQL Server: InnoDB Plugin storage engine
• Severity: S3 (Non-critical)
• Version: 5.1+
• OS: Any
• Assigned to: Sunny Bains
• CPU Architecture: Any

[5 Oct 2010 10:55] Konstantin Osipov

Description:
This has no known user-visible effect. 
InnoDB does not use trans_register_ha() MySQL Server transaction management API consistently.
The API is there to allow MySQL know which engines participate in a SQL statement or in a transaction, and invoke corresponding methods when statement/transaction ends.
See more in a long comment in handler.cc.

However, InnoDB does not use the API consistently.
For certain statements, it registers itself in MySQL, we call it "registers in statement transaction". Observe, for example, MySQL 5.5, ha_innodb.cc, innobase::start_stmt():

        /* Set the MySQL flag to mark that there is an active transaction */
        if (trx->active_trans == 0) {

                innobase_register_trx_and_stmt(ht, thd);
                trx->active_trans = 1;
        } else {
                innobase_register_stmt(ht, thd);
        }

As a result of the above, trans_register_ha(thd, FALSE, hton) is called even if trx->active_trans is TRUE (i.e. also for subsequent statement transactions).
See also ha_innobase::external_lock().

However, in another place where we may start a statement, registration
happens only for the first statement in a transaction,
innobase_query_caching_of_table_permitted():
      if (trx->active_trans == 0) {

                innobase_register_trx_and_stmt(innodb_hton_ptr, thd);
                trx->active_trans = 1;
        }

This place is a bad copy-paste from ha_innobase::transactional_table_lock().
The code is correct in ha_innobase::transactional_table_lock() because there we know that we're the first statement in a transaction (transactional_table_lock() is called for LOCK TABLES, which has an implicit COMMIT at the beginning, subsequent LOCK TABLES will also have an implicit commit).

How to repeat:
Observe the code.

Suggested fix:
Fix innobase_query_cache_of_table_permitted() to do:

        if (trx->active_trans == 0) {

                innobase_register_trx_and_stmt(ht, thd);
                trx->active_trans = 1;
        } else {
                innobase_register_stmt(ht, thd);
        }

or better yet, make the above a helper function to avoid copy-paste bugs in the future.

[25 Oct 2010 7:05] Sunny Bains

Committed to 5.5.

[13 Nov 2010 16:25] Bugs System

Pushed into mysql-trunk 5.6.99-m5 (revid:[e-mail]-20101113155825-czmva9kg4n31anmu) (version source revid:[e-mail]-20101113152450-2zzcm50e7i4j35v7) (merge vers: 5.6.1-m4) (pib:21)

[13 Nov 2010 16:37] Bugs System

Pushed into mysql-next-mr (revid:[e-mail]-20101113160336-atmtmfb3mzm4pz4i) (version source revid:[e-mail]-20101113152540-gxro4g0v29l27f5x) (pib:21)

[16 Dec 2010 22:31] Bugs System

Pushed into mysql-5.5 5.5.9 (revid:[e-mail]-20101216101358-fyzr1epq95a3yett) (version source revid:[e-mail]-20101216101358-fyzr1epq95a3yett) (merge vers: 5.5.9) (pib:24)