MySQL Bugs: #22775: Additional thread accessors like thd_in_lock

Bug #22775	Additional thread accessors like thd_in_lock_tables() required by engine plugins
Submitted:	28 Sep 2006 10:37	Modified:	1 Aug 2007 9:39
Reporter:	Paul McCullagh (Basic Quality Contributor) (OCA)	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server	Severity:	S4 (Feature request)
Version:	5.1	OS:	Any
Assigned to:	Timothy Smith	CPU Architecture:	Any
Tags:	accessors, engine, plugin, thd, thd class, thd_in_lock_tables, thread

Description:
The structure of the THD class (sql_class.h) depends on the flags used to compile MySQL (e.g SIGNAL_WITH_VIO_CLOSE, WITH_PARTITION_STORAGE_ENGINE, ERROR_INJECT_SUPPORT, DBUG_OFF).

This presents a problem for plugin engines when accessing member variables of the THD class.

If they are not compiled with the same options as the MySQL server the access is wrong.

How to repeat:
This is only a problem for engines that are dynamically linked to MySQL, for example PBXT or Solid.

I encountered the problem when accessing the 'options' field, with the SIGNAL_WITH_VIO_CLOSE not defined for the engine build, but defined for the MySQL build.

Suggested fix:
This problem can be solved by providing accessor function for the member variables. A number of such functions already exists: thd_in_lock_tables(), thd_tablespace_op(), thd_ha_data().

Currently, additional accessor functions are required for:

&thd->main_security_ctx
thd->proc_info
thd->options
thd->killed
thd->variables.tx_isolation
thd->lex->sql_command

An alternative solution would be to somehow ensure that the structure of the THD class does not change, no matter what compiler options are used. For example:

#ifdef SIGNAL_WITH_VIO_CLOSE
  Vio* active_vio;
#else
  void *unused_active_vio;
#endif

Paul, I'd rather keep THD as an "internal" structure, not exposed to plugins, making only specific fields public via accessor functions. And add these accessr functions on a case by case basis.

There is thd_proc_info() function already.

thd->variables.tx_isolation - ok
thd->killed - ok.

Why do you need the other three ?

Hi Sergei,

Access to "&thd->main_security_ctx" is not essential. I use it to access 'sctx->user' and 'sctx->host' to create a name for a thread which is used when logging errors (so the administrator has an idea what client cause the error).

'thd->options' is more critical. I use it in a test like this: "thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)", for example in ha_pbxt::external_lock(). Basically I need to determine whether the auto-commit switch is on or off.

I use 'thd->lex->sql_command' in ha_pbxt::store_lock() in the same way as InnoDB does: to test whether I can set lock_type = TL_WRITE_ALLOW_WRITE.

One other thing: currently thd_proc_info() can only be used to get and set 'proc_info'. It needs to be changed so that just a get is possible, for example: 

const char *thd_proc_info(THD *thd, const char *info)
{
  const char *old_info= thd->proc_info;
  if (info)
    thd->proc_info= info;
  return old_info;
}

Or just add a thd_get_proc_info().

Antony had this in one of his patches for WL#2936

Now that WL#2936 is pushed, I will now examine what the current state of play is.

Will complete as part of WL#3653 (currently pending arch review)

There is now a dedicated Worklog task for this, WL#3914.

This bug will be closed when that Worklog is completed.

WL#3914 is now merged into MySQL 5.1 code.