Hi, Oleksandr! On Aug 07, Oleksandr Byelkin wrote:
diff --git a/mysql-test/r/mysqld--help.result b/mysql-test/r/mysqld--help.result index fc22af6..1f2a8cf 100644 --- a/mysql-test/r/mysqld--help.result +++ b/mysql-test/r/mysqld--help.result @@ -890,6 +890,7 @@ The following options may be given as the first argument: write privileges to the mysql.user table. --secure-auth Disallow authentication for accounts that have old (pre-4.1) passwords + (Defaults to on; use --skip-secure-auth to disable.) You know what, it would be nice if the user would know what options support the --autoset prefix. I think here's a good place for it, the help for back_log can end with
gets very many connection requests in a very short time (Automatically configured unless set explicitly)
Of course, this line should be automatically generated by my_print_help(), not manually added to the help text of every autoset variable.
OK?
1063 89366 89366 28296 28296 3 +1495 89366 89366 28296 28296 3 221 56120 56120 28296 28296 3 +961 24512 24512 85239 85239 4 why? now result is sorted (and it should be because there is no order in the statement)
ok, sorry. I've missed that you added --sorted_result
diff --git a/mysys/my_getopt.c b/mysys/my_getopt.c index 88fa3d4..1d32833 100644 --- a/mysys/my_getopt.c +++ b/mysys/my_getopt.c @@ -47,14 +47,15 @@ static char *check_struct_option(char *cur_arg, char *key_name); order of their arguments must correspond to each other. */ static const char *special_opt_prefix[]= -{"skip", "disable", "enable", "maximum", "loose", 0}; +{"skip", "disable", "enable", "maximum", "loose", "autoset", 0}; So, I'm not very happy with the --autoset- prefix. Because it cannot be used from the SQL. In SQL we have to use SET variable=AUTO; or something like that. And it would be good to use the same approach on the command line and from SQL. On the other hand, =AUTO has its problems too - it conflicts with the AUTO value of the enum/set variables.
I don't have a good answer to this :( IMHO better to have both, but lets add SET...=AUTO when it will be requested.
No, please, not both. Not --autoset-variable=AUTO. Either =AUTO or --autoset. Because of the following:
+ else if (option_is_autoset) + { + if (optend) + { + my_getopt_error_reporter(ERROR_LEVEL, + "%s: automatic setup request of " + "option '--%s' cannot take an argument", + my_progname, optp->name); heh, another argument agains --autoset- prefix. With the =AUTO value one cannot possibly have this error.
So, perhaps, I'm *slighty* leaning towards the =AUTO vs --autoset.
I've started to prefer =AUTO. It's also future-proof (we can later to =AUTO from SQL).
diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 43a3f0e..21a1198 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -4316,6 +4316,27 @@ static int init_common_variables() } #endif /* HAVE_SOLARIS_LARGE_PAGES */
+ + /* Fix host_cache_size. */ + if (IS_SYSVAR_AUTOSIZE(&host_cache_size)) + { + if (max_connections <= 628 - 128) + SYSVAR_AUTOSIZE(host_cache_size, 128 + max_connections); + else if (max_connections <= ((ulong)(2000 - 628)) * 20 + 500) + SYSVAR_AUTOSIZE(host_cache_size, 628 + ((max_connections - 500) / 20)); + else + SYSVAR_AUTOSIZE(host_cache_size, 2000); + } + + /* Fix back_log */ + if (back_log == 0 || IS_SYSVAR_AUTOSIZE(&back_log)) why back_log==0 ? MySQL/OLD way to get automatic value, lest for compatibility.
Certainly not "OLD" way, because lowest allowed back_log value was 1, you've changed it to be 0 in this patch. May be it's for 5.6 compatibility?
+typedef Sys_var_integer<ulong, (GET_ULONG|GET_AUTO), SHOW_ULONG> Sys_var_aulong; Hmm, why did you make it a new type? I think the *type* of the variable doesn't depend on the auto-set behavior. The type defines storage size, how to print the value, etc. While auto-set is just a behavior modifier. I'd rather put it in flags:
- READ_ONLY GLOBAL_VAR(back_log), CMD_LINE(REQUIRED_ARG), + AUTO_SET READ_ONLY GLOBAL_VAR(back_log), CMD_LINE(REQUIRED_ARG), the problem was in how macro/constructor made, I did not found the way to do it in flags, I will recheck.
Like if (flags & AUTO_SET) option.var_type|= GET_AUTO;
@@ -2751,16 +2743,38 @@ static bool check_query_cache_type(sys_var *self, THD *thd, set_var *var) + if (var->type != OPT_GLOBAL && global_system_variables.query_cache_type == 0) { - my_error(ER_QUERY_CACHE_IS_GLOBALY_DISABLED, MYF(0)); - return true; - } + uint value= 0 /*default is off*/; + if (var->value) + { + if (var->value->result_type() == INT_RESULT) + value= var->value->val_int(); + else + { + char buff[STRING_BUFFER_USUAL_SIZE]; + String str(buff, sizeof(buff), system_charset_info), *res; + if (!(res=var->value->val_str(&str))) + return true; + if (res->length() != 3 || + my_toupper(res->charset(), res->ptr()[0]) != 'O' || + my_toupper(res->charset(), res->ptr()[1]) != 'F' || + my_toupper(res->charset(), res->ptr()[2]) != 'F') eh? query_cache_type is ENUM variable, there is no need to compare string values manually. The problem is that it called before processing, and it was bug before to check only numeric value.
Shouldn't be. See sys_var::check(): if ((var->value && do_check(thd, var)) || (on_check && on_check(this, thd, var))) First it calls do_check(), then on_check(). on_check() is your ON_CHECK callback, that is check_query_cache_type(). do_check() is Sys_var_typelib::do_check(). It converts the value from the string to a number of the enum element. Regards, Sergei