Bug #80804 Improve error handling from call to store_key::copy() in cp_buffer_from_ref

• Submitted: 21 Mar 2016 10:41
• Modified: 20 Apr 2016 14:46
• Reporter: Olav Sandstå
• Status: Closed
• Category: MySQL Server: Optimizer
• Severity: S3 (Non-critical)
• Version: 8.0.0
• OS: Any
• CPU Architecture: Any

[21 Mar 2016 10:41] Olav Sandstå

Description:
In cp_buffer_from_ref() (in sql_executor.cc) we have the following code that calls store_key::copy() and handles errors returned:

    if (s_key->copy() & 1)
    {
      result= 1;
      break;
    }

The above code that does error handling by doing bit-was AND of the return value from copy() looks strange. It looks even more strange if we look at what the return value from store_key::copy() actually is:

  enum store_key_result store_key::copy()

where store_key_result is defined as:

  enum store_key_result { STORE_KEY_OK, STORE_KEY_FATAL, STORE_KEY_CONV };

So the above code detects error by checking that the last bit of an enum value is set. This code is not easy to read and even if it work correctly today, it can easily start to fail if someone adds a new value to the store_key_result enum (or just changes the order of the existing values for this enum).

How to repeat:
Read the source code.

Suggested fix:
Replace the bit operation on the return value from store_key::copy() with testing against actual enum values.

[20 Apr 2016 14:46] Paul DuBois

Posted by developer:
 
Fixed in 5.8.0.

Code cleanup. No changelog entry needed.

[18 Jun 2016 21:36] Omer Barnir

Posted by developer:
 
Reported version value updated to reflect release name change from 5.8 to 8.0