MySQL Bugs: #54569: Some options are not allowed to take argument when passed with loose- prefix

Bug #54569	Some options are not allowed to take argument when passed with loose- prefix
Submitted:	16 Jun 2010 21:49	Modified:	20 Nov 2010 17:44
Reporter:	Elena Stepanova	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: General	Severity:	S3 (Non-critical)
Version:	5.5.4-m3, 5.5.5-m3	OS:	Any
Assigned to:	Kristofer Pettersson	CPU Architecture:	Any
Tags:	regression

Description:
(Setting the category to 'InnoDB Plugin' as only observed it with InnoDB plugin 1.1 so far, although maybe the root of the problem is generic.)

Some boolean options cause parsing failures when they are given with prefix loose- and an argument, either in the command line or in configuration file.

---------------
Examples:

1. loose- and argument for some options cause error:

--loose-innodb_use_native_aio=0
=>
[ERROR] mysqld: option 'innodb-use-native-aio' cannot take an argument
[ERROR] Parsing options for plugin 'InnoDB' failed.

2. For some other options it is OK:

--loose-innodb_table_locks=1
=> no errors

3. Argument but no loose- prefix is OK:

--innodb_use_native_aio=0
=> no errors

4. Prefix but no argument is OK:

--loose-innodb_use_native_aio
=> no errors

---------------

Same problem exists for some more options (I did not check all):

innodb_use_sys_malloc
innodb_locks_unsafe_for_binlog
innodb_file_per_table
innodb_adaptive_flushing
...

And does not exist for some others:

innodb_adaptive_hash_index
innodb_support_xa
...

How to repeat:
Start mysqld with --loose-innodb_file_per_table=1 
or --loose-innodb_use_native_aio=0

Suggested fix:
Make the behavior consistent

This is not a bug.

The options that in question have PLUGIN_VAR_NOCMDARG bit set, so they suppose not taking any argument:

static MYSQL_SYSVAR_BOOL(use_native_aio, srv_use_native_aio,
  PLUGIN_VAR_NOCMDARG | PLUGIN_VAR_READONLY,  <=============
  "Use native AIO if supported on this platform.",
  NULL, NULL, TRUE);

static MYSQL_SYSVAR_BOOL(locks_unsafe_for_binlog, innobase_locks_unsafe_for_binlog,
  PLUGIN_VAR_NOCMDARG | PLUGIN_VAR_READONLY,  <=============
  "Force InnoDB to not use next-key locking, to use only row-level locking.",
  NULL, NULL, FALSE);

Those options ran OK do not have this bit, for example, "innodb_table_lock":

static MYSQL_THDVAR_BOOL(table_locks, PLUGIN_VAR_OPCMDARG,
  "Enable InnoDB locking in LOCK TABLES",
  /* check_func */ NULL, /* update_func */ NULL,
  /* default */ TRUE);

The actual argument check happens in mysys/my_getopt.c handle_options(), error message raised:

    321         if (must_be_var && optp->arg_type == NO_ARG) <====
    322         {
    323           if (my_getopt_print_errors)
    324             my_getopt_error_reporter(ERROR_LEVEL,
    325                                      "%s: option '%s' cannot take an argument",
    326                                      my_progname, optp->name);
    327           return EXIT_NO_ARGUMENT_ALLOWED;
    328         }

So this is expected.

When you specify the option without "loose" prefix and with argument, such as "innodb_use_native_aio=0", this option will not recognized at all, and simply skipped, so you do not see the error message:

   251           if (!opt_found)  <=== the "innodb_use_native_aio=0" not recognized
    252           {
    253             if (my_getopt_skip_unknown)
    254             {
    255               /* Preserve all the components of this unknown option. */
    256               do {
    257                 (*argv)[argvpos++]= *first++;
    258               } while (first <= pos);
    259               continue;  <=== simply skipped.
    260             }

4: opt_str = 0xbffff665 "innodb_use_native_aio=0"
(gdb) p opt_found
$22 = 0
(gdb) p my_getopt_skip_unknown
$23 = 1 '\001'

So user should use "skip", "enable", "disable" prefixes for these  PLUGIN_VAR_NOCMDARG options. And error message clearly state the arguement is not allowed.

Jimmy - this is a real bug, and a regression. According to design loose_innodb_use_sys_malloc=0 and innodb_use_sys_malloc=0 should have the same result if the engine can recognize it. In this case, it does not. I have verified that innodb_use_sys_malloc=0 (or =1) works, but loose_innodb_use_sys_malloc=0 does not.

It is a regression because loose_innodb_use_sys_malloc=0/1 works in 5.1, but does not work in 5.5.

The bug may be in the server layer. Please check it out. You may check the InnoDB 1.0 under 5.1.

Calvin, here is the difference between 5.1 and 5.5

mysys/my_getopt.c

handle_options()
{
-         if (must_be_var && (optp->var_type & GET_TYPE_MASK) == GET_NO_ARG)
+         if (must_be_var && optp->arg_type == NO_ARG)
          {
               if (my_getopt_print_errors)
                      my_getopt_error_reporter(ERROR_LEVEL,
                                     "%s: option '%s' cannot take an argument",
                                     my_progname, optp->name);
               return EXIT_NO_ARGUMENT_ALLOWED;
           }

}

Notice optp->var_type !=  GET_NO_ARG, but optp->arg_type == NO_ARG
gdb) p optp->var_type
$57 = 2                <======== GET_BOOL
(gdb) p optp->arg_type
$58 = NO_ARG

So we are getting a BOOLEAN datatype data for a option with NO_ARG bit.

In 5.1, it checks optp->var_type instead of optp->arg_type when deciding whether to print error. So someone might thinking it is not appropriate and changed to  optp->arg_type instead.

Just from the plain meaning of NO_ARG or PLUGIN_VAR_NOCMDARG, it is normal to think these options should not have argument. User should use "enable/disable/skip etc." prefixes to modify these variables. If that is case, then it should be documented. However, if there is long tradition of provide 1/0 option to these NO_ARG options, then it need to consider to change back.

In addition, use loose- and without loose- prefix, the result are also different. The difference is simply due to the "must_be_var" is set or not.

Only with loose-prefix, "must_be_var" is set to 1. As shown in following code chunk, you can see  "must_be_var" set depending on whether these is special prefix (such as loose-) for the variable:

handle_options()
{
        if (!(opt_found= findopt(opt_str, length, &optp, &prev_found)))
        {       
          /*      
            Didn't find any matching option. Let's see if someone called
            option with a special option prefix
          */      
          if (!must_be_var)
          {       
            if (optend)
              must_be_var= 1; /* option is followed by an argument */ <===

              for (i= 0; special_opt_prefix[i]; i++)
              {
                /*
                  We were called with a special prefix, we can reuse opt_found
                */
              if ((opt_found= findopt(opt_str, length, &optp, &prev_found)))
              ...
              }

}

Thus the variable setting has different behavior  with this check:

        if (must_be_var && optp->arg_type == NO_ARG)
          {
               if (my_getopt_print_errors)
          }

All these are in server code mysys/my_getopt.c. Two issues to decide

1) whether the change from  optp->var_type to optp->arg_type is correct, whether you want all NO_ARG option really comes with no arg.
2) The difference behavior with loose- prefix set or not, the behavior needs to be consistent.

My analysis is slightly different. I believe that maybe Sergei G introduced the failing if-statement without proper analysis of the code. If I apply this patch:

-	if (must_be_var && optp->arg_type == NO_ARG)
-	{
-	  if (my_getopt_print_errors)
-            my_getopt_error_reporter(ERROR_LEVEL, 
-                                     "%s: option '%s' cannot take an argument",
-                                     my_progname, optp->name);
-	  return EXIT_NO_ARGUMENT_ALLOWED;
-	}

I can get this debug trace which seems like perfectly valid behavior:

Known variable and patch is applied
----------------------------------------
Pass 1..n:
cur_arg= --loose-innodb_use_native_aio=0, argc= 11
-- didn't find a registered opt variable to match the key.
-- There is an equal sign after the key so we can expect a value; must_be_var=1
-- The 'loose'-prefix has been identified; lets remove it and try again.
-- Despite removing the prefix from the key we didn't find a matching option variable.
-- Preserve all the components of this unknown option.

Pass n+1:
-- Loaded option: Use_native_aio (Use native AIO if supported on this platform.) is a bool option.
cur_arg= --innodb_use_native_aio=0, argc= 2
-- Found registerd option; arg_type is NO_ARG
-- Option var type is BOOL; Setting option value to true or false depending on argument value 1 or 0.

Pass 1..n:
cur_arg= --innodb_use_native_aio=0, argc= 10
-- didn't find a registered opt variable to match the key.
-- There is an equal sign after the key so we can expect a value; must_be_var=1
-- Despite removing the prefix from the key we didn't find a matching option variable.
-- Preserve all the components of this unknown option.

Pass n+1:
-- load innodb
-- Loaded use_native_aio (Use native AIO if supported on this platform.) and it's a bool option.
cur_arg= --innodb_use_native_aio=0, argc= 2
-- Option arg_type is NO_ARG
-- Option var type is BOOL; Setting option value to true or false depending on argument value 1 or 0.

For totally unknown variables using the patch
----------------------------------------------
Pass 1..n
cur_arg= --loose-innodb_not_such_var=2, argc= 2
-- didn't find a registered opt variable to match the key.
-- There is an equal sign after the key so we can expect a value; must_be_var=1
-- The 'loose'-prefix has been identified; lets remove it and try again.
-- Despite removing the prefix from the key we didn't find a matching option variable.
-- Preserve all the components of this unknown option.

Pass n+1
cur_arg= --loose-innodb_not_such_var=2, argc= 1
-- didn't find a registered opt variable to match the key.
-- There is an equal sign after the key so we can expect a value; must_be_var=1
-- The 'loose'-prefix has been identified; lets remove it and try again.
-- Despite removing any prefix from the key we didn't find a matching option variable.
The supplied argument is a key-value pair (must_be_var=1).
=> [Warning] /home/thek/bzr/mysql-5.5-bugteam/build/sql/mysqld: unknown variable 'loose-innodb_not_such_var=2'
-- The loose prefix makes us count down the available arguments and continue (skip ahead) the loop.

Pass 1..n
cur_arg= --innodb_not_such_var=2, argc= 2
-- didn't find a registered opt variable to match the key.
-- There is an equal sign after the key so we can expect a value; must_be_var=1
-- Despite removing any prefix from the key we didn't find a matching option variable.
-- Preserve all the components of this unknown option.

Pass n+1
cur_arg= --innodb_not_such_var=2, argc= 2
-- didn't find a registered opt variable to match the key.
-- There is an equal sign after the key so we can expect a value; must_be_var=1
-- Despite removing any the prefix from the key we didn't find a matching option variable.
-- The supplied argument is a key-value pair (must_be_var=1).
=> [ERROR] /home/thek/bzr/mysql-5.5-bugteam/build/sql/mysqld: unknown variable 'innodb_not_such_var=2'
-- We still have a loose prefix so we return EXIT_UNKNOWN_VARIABLE

Without the patch for totally unknown variables
------------------------------------------------
cur_arg= --loose-innodb_not_such_var=2, argc= 2
-- didn't find a registered opt variable to match the key.
-- There is an equal sign after the key so we can expect a value; must_be_var=1
-- The 'loose'-prefix has been identified; lets remove it and try again.
-- Despite removing the prefix from the key we didn't find a matching option variable.
-- Preserve all the components of this unknown option.

cur_arg= --loose-innodb_not_such_var=2, argc= 1
-- didn't find a registered opt variable to match the key.
-- There is an equal sign after the key so we can expect a value; must_be_var=1
-- The 'loose'-prefix has been identified; lets remove it and try again.
-- Despite removing the prefix from the key we didn't find a matching option variable.
-- The supplied argument is a key-value pair (must_be_var=1).
=> [Warning] unknown variable 'loose-innodb_not_such_var=2'
-- The loose prefix makes us count down the available arguments and continue (skip ahead) the loop.

Pass 1..n
cur_arg= --innodb_not_such_var=2, argc= 11
-- didn't find a registered opt variable to match the key.
-- There is an equal sign after the key so we can expect a value; must_be_var=1
-- Despite removing the prefix from the key we didn't find a matching option variable.
-- Preserve all the components of this unknown option.

Pass n+1
cur_arg= --innodb_not_such_var=2, argc= 1
-- didn't find a registered opt variable to match the key.
-- There is an equal sign after the key so we can expect a value; must_be_var=1
-- Despite removing the prefix from the key we didn't find a matching option variable.
-- The supplied argument is a key-value pair (must_be_var=1).
=> [ERROR] unknown variable 'innodb_not_such_var=2'
-- We still have a loose prefix so we return EXIT_UNKNOWN_VARIABLE

Without the patch for known variables
--------------------------------------
Pass 1..n:
cur_arg= --loose-innodb_use_native_aio=0, argc= 11
-- didn't find a registered opt variable to match the key.
-- There is an equal sign after the key so we can expect a value; must_be_var=1
-- The 'loose'-prefix has been identified; lets remove it and try again.
-- Despite removing the prefix from the key we didn't find a matching option variable.
-- Preserve all the components of this unknown option.

Pass n+1:
-- loaded InnoDB 
-- Loaded use_native_aio (Use native AIO if supported on this platform.) and it's a bool option.
cur_arg= --loose-innodb_use_native_aio=0, argc= 3
-- didn't find a registered opt variable to match the key.
-- There is an equal sign after the key so we can expect a value; must_be_var=1
-- The 'loose'-prefix has been identified; lets remove it and try again.
=> [ERROR] mysqld: option 'innodb-use-native-aio' cannot take an argument
=> [ERROR] Parsing options for plugin 'InnoDB' failed.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/120361

3095 Kristofer Pettersson	2010-10-08
      Bug#54569 Some options are not allowed to take argument when passed with loose- prefix
      
      Boolean options cause parsing failures when they are given
      with prefix loose- and an argument, either in the command
      line or in configuration file.
      
      The reason was a faulty logic which forced the parsing
      to throw an error when an argument of type NO_ARG was
      used together with an argument which has been identified
      as a key-value pair. Despite the attribute NO_ARG these
      options actually take arguments if they are of type
      BOOL.
     @ mysys/my_getopt.c
        * removed if-statement which prevented logic for handling boolean types with arguments to be executed.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/121804

3095 Kristofer Pettersson	2010-10-25
      Bug#54569 Some options are not allowed to take argument when passed with loose- prefix
      
      Boolean options cause parsing failures when they are given
      with prefix loose- and an argument, either in the command
      line or in configuration file.
      
      The reason was a faulty logic which forced the parsing
      to throw an error when an argument of type NO_ARG was
      used together with an argument which has been identified
      as a key-value pair. Despite the attribute NO_ARG these
      options actually take arguments if they are of type
      BOOL.
     @ include/my_getopt.h
        * More comments to help future refactoring
     @ mysys/my_getopt.c
        * removed if-statement which prevented logic for handling boolean types with arguments to be executed.
        * Added comments to aid in future refactoring.

Pushed into mysql-trunk 5.6.99-m5 (revid:alexander.nozdrin@oracle.com-20101113155825-czmva9kg4n31anmu) (version source revid:alexander.nozdrin@oracle.com-20101113152450-2zzcm50e7i4j35v7) (merge vers: 5.6.1-m4) (pib:21)

Pushed into mysql-next-mr (revid:alexander.nozdrin@oracle.com-20101113160336-atmtmfb3mzm4pz4i) (version source revid:vasil.dimov@oracle.com-20100629074804-359l9m9gniauxr94) (pib:21)

Noted in 5.5.8 changelog.

Boolean command options caused an error if given with an option value
and the loose- option prefix.

Noted in 5.5.8 changelog.

Boolean command options caused an error if given with an option value
and the loose- option prefix.

Pushed into mysql-5.5 5.5.9 (revid:jonathan.perkin@oracle.com-20101216101358-fyzr1epq95a3yett) (version source revid:jonathan.perkin@oracle.com-20101216101358-fyzr1epq95a3yett) (merge vers: 5.5.9) (pib:24)