Sergei Golubchik <serg@stripped> writes:
Hi!
Thanks for looking into this!
> this part of my_getopt.c of 5.1 is not necessary for this bugfix.
That is certainly correct. If you do look at 5.0 / 5.1 however,
you'll find that they're already different from the version I
copied from (in that they have basic error reporting and such,
for a start); I was simply hoping to prevent the trees from
further drift. Either is fine with me. Will mark in CS comments
as suggested.
> I don't like this null_reporter idea. Let init_one_value issue warnings
> normally for every value adjustment.
> Of course there should be no warnings, that is, you need to fix all
> options where default values are outside of min..max range.
Can do ... done.
>> + */
>> + if (chk && (option_limits->block_size > 1) && (*chk ==
> in))
>> + {
>> + struct my_option tmp_limits;
>> + memcpy(&tmp_limits, option_limits, sizeof(struct my_option));
>> + tmp_limits.block_size= 1;
>
> why ???
Because I was too reluctant to change the signature of
getopt_ull_limit_value(). I've done that now; it's much
prettier.
> I don't think you're supposed to do any checks or issue any warnings in
> ::update(), they belong to ::check().
You know, that looked funky to me too, but it wasn't my idea.
That's where *existing* sys_var_thd_ulong::update() both checks
*and* warns. It's also where all the other methods call
getopt_ull_limit_value() at least, which checks and adjusts
(and in pre-patch 5.0 already warns to the log, just not to
the client yet), not to mention some other checks and adjustments
(--max-variable= etc.). So there's a lot of precedent there.
Obviously, you can still be right and the existing code wrong,
but I think that's out of the scope of this bug.
Will attach updated patch once the tests finish.
thanks and regards,
Tatiana
--
Tatjana A. Nurnberg, Bugs Bunny * MySQL AB, http://www.mysql.de/ * EST
IRC: Azundris * Skype: azundris * MySQL-SIP: 4550 * eMail: azundris@stripped
.de: Firmensitz: MySQL GmbH, Radlkoferstr. 2, D-81373 Muenchen
Geschaeftsfuehrer: Hans von Bell, Kaj Arnoe - HRB Muenchen 162140