Skip to content

Commit 734e9fe

Browse files
author
Sven Sandberg
committed
WL#14194 step 1.3: Fix min/max values for sysvars with block_size!=1
WL#14194: Replace old terms in replication system variables, options, and strings Background: System variables have a feature known as "block_size". Numeric system variables can only be set to values that are multiples of block_size. Most variables use a block size of 1. Numeric variables are additionally limited to a maximum and a minimum value. Problem: In case the minimum and maximum values are not specified as multiples of the block_size, the effective minimum and maximum are different than the ones specified. This typically happens when using a block_size that is a power of two, for instance 1024, and a maximum that is a numeric limit such as MAXINT, which may have values such as 2^64-1, i.e., one-less than a multiple of the block_size. This has no observable effect, other than the maximum value displayed in performance_schema.system_variables_info being a value that is not actually allowed to use for the variable. This is a litte inconsistent, and also made it hard to write the scripted tests in later steps of the worklog. Also, the indentation was wrong all over Sys_var_integer. Fix: - Round the maximum down to the next smaller multiple of block_size. - Round the minimum up to the next grater multiple of block_size. - Fix the indentation. RB:26085
1 parent 50015e7 commit 734e9fe

File tree

1 file changed

+128
-108
lines changed

1 file changed

+128
-108
lines changed

sql/sys_vars.h

Lines changed: 128 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -176,126 +176,146 @@ struct CMD_LINE {
176176
Backing store: uint, ulong, ha_rows, ulonglong, long, depending on the
177177
Sys_var_*
178178
*/
179+
// clang-format off
179180
template <typename T, ulong ARGT, enum enum_mysql_show_type SHOWT, bool SIGNED>
180181
class Sys_var_integer : public sys_var {
181-
public : Sys_var_integer(
182-
const char *name_arg, const char *comment, int flag_args, ptrdiff_t off,
183-
size_t size MY_ATTRIBUTE((unused)), CMD_LINE getopt, T min_val, T max_val,
184-
T def_val, uint block_size, PolyLock *lock = nullptr,
182+
public:
183+
Sys_var_integer(
184+
const char *name_arg, const char *comment, int flag_args,
185+
ptrdiff_t off, size_t size MY_ATTRIBUTE((unused)), CMD_LINE getopt,
186+
T min_val, T max_val, T def_val, uint block_size,
187+
PolyLock *lock = nullptr,
185188
enum binlog_status_enum binlog_status_arg = VARIABLE_NOT_IN_BINLOG,
186189
on_check_function on_check_func = nullptr,
187190
on_update_function on_update_func = nullptr,
188-
const char *substitute = nullptr, int parse_flag = PARSE_NORMAL) :
189-
sys_var(&all_sys_vars, name_arg, comment, flag_args, off, getopt.id,
190-
getopt.arg_type, SHOWT, def_val, lock, binlog_status_arg,
191-
on_check_func, on_update_func, substitute, parse_flag){
192-
option.var_type = ARGT; option.min_value = min_val;
193-
option.max_value = max_val; option.block_size = block_size;
194-
option.u_max_value = (uchar **)max_var_ptr();
195-
if (max_var_ptr()) * max_var_ptr() = max_val;
196-
197-
// Do not set global_var for Sys_var_keycache objects
198-
if (offset >= 0) global_var(T) = def_val;
199-
200-
assert(size == sizeof(T)); assert(min_val < max_val);
201-
assert(min_val <= def_val); assert(max_val >= def_val);
202-
assert(block_size > 0); assert(def_val % block_size == 0);}
203-
bool do_check(THD *thd, set_var *var) override {
204-
bool fixed = false;
205-
longlong v;
206-
ulonglong uv;
207-
208-
v = var->value->val_int();
209-
if (SIGNED) /* target variable has signed type */
210-
{
211-
if (var->value->unsigned_flag) {
212-
/*
213-
Input value is such a large positive number that MySQL used an
214-
unsigned item to hold it. When cast to a signed longlong, if the
215-
result is negative there is "cycling" and this is incorrect (large
216-
positive input value should not end up as a large negative value in
217-
the session signed variable to be set); instead, we need to pick the
218-
allowed number closest to the positive input value, i.e. pick the
219-
biggest allowed positive integer.
220-
*/
221-
if (v < 0)
222-
uv = max_of_int_range(ARGT);
223-
else /* no cycling, longlong can hold true value */
224-
uv = (ulonglong)v;
225-
} else
226-
uv = v;
227-
/* This will further restrict with VALID_RANGE, BLOCK_SIZE */
228-
var->save_result.ulonglong_value =
191+
const char *substitute = nullptr, int parse_flag = PARSE_NORMAL)
192+
: sys_var(&all_sys_vars, name_arg, comment, flag_args, off,
193+
getopt.id, getopt.arg_type, SHOWT, def_val, lock,
194+
binlog_status_arg, on_check_func, on_update_func,
195+
substitute, parse_flag) {
196+
option.var_type = ARGT;
197+
if ((min_val % block_size) != 0)
198+
min_val += block_size - (min_val % block_size);
199+
option.min_value = min_val;
200+
option.max_value = max_val - (max_val % block_size);
201+
option.block_size = block_size;
202+
option.u_max_value = (uchar **)max_var_ptr();
203+
if (max_var_ptr()) * max_var_ptr() = max_val;
204+
205+
// Do not set global_var for Sys_var_keycache objects
206+
if (offset >= 0) global_var(T) = def_val;
207+
208+
assert(size == sizeof(T));
209+
assert(min_val <= def_val);
210+
assert(def_val <= max_val);
211+
assert(block_size > 0);
212+
assert(option.min_value % block_size == 0);
213+
assert(def_val % block_size == 0);
214+
assert(option.max_value % block_size == 0);
215+
}
216+
bool do_check(THD *thd, set_var *var) override {
217+
bool fixed = false;
218+
longlong v;
219+
ulonglong uv;
220+
221+
v = var->value->val_int();
222+
if (SIGNED) { /* target variable has signed type */
223+
if (var->value->unsigned_flag) {
224+
/*
225+
Input value is such a large positive number that MySQL used
226+
an unsigned item to hold it. When cast to a signed longlong,
227+
if the result is negative there is "cycling" and this is
228+
incorrect (large positive input value should not end up as a
229+
large negative value in the session signed variable to be
230+
set); instead, we need to pick the allowed number closest to
231+
the positive input value, i.e. pick the biggest allowed
232+
positive integer.
233+
*/
234+
if (v < 0)
235+
uv = max_of_int_range(ARGT);
236+
else /* no cycling, longlong can hold true value */
237+
uv = (ulonglong)v;
238+
} else
239+
uv = v;
240+
/* This will further restrict with VALID_RANGE, BLOCK_SIZE */
241+
var->save_result.ulonglong_value =
229242
getopt_ll_limit_value(uv, &option, &fixed);
230-
} else {
231-
if (var->value->unsigned_flag) {
232-
/* Guaranteed positive input value, ulonglong can hold it */
233-
uv = (ulonglong)v;
234243
} else {
235-
/*
236-
Maybe negative input value; in this case, cast to ulonglong makes it
237-
positive, which is wrong. Pick the closest allowed value i.e. 0.
238-
*/
239-
uv = (ulonglong)(v < 0 ? 0 : v);
240-
}
241-
var->save_result.ulonglong_value =
244+
if (var->value->unsigned_flag) {
245+
/* Guaranteed positive input value, ulonglong can hold it */
246+
uv = (ulonglong)v;
247+
} else {
248+
/*
249+
Maybe negative input value; in this case, cast to ulonglong
250+
makes it positive, which is wrong. Pick the closest allowed
251+
value i.e. 0.
252+
*/
253+
uv = (ulonglong)(v < 0 ? 0 : v);
254+
}
255+
var->save_result.ulonglong_value =
242256
getopt_ull_limit_value(uv, &option, &fixed);
243-
}
257+
}
244258

245-
if (max_var_ptr()) {
246-
/* check constraint set with --maximum-...=X */
247-
if (SIGNED) {
248-
longlong max_val = *max_var_ptr();
249-
if (((longlong)(var->save_result.ulonglong_value)) > max_val)
250-
var->save_result.ulonglong_value = max_val;
251-
/*
252-
Signed variable probably has some kind of symmetry. Then it's good
253-
to limit negative values just as we limit positive values.
254-
*/
255-
max_val = -max_val;
256-
if (((longlong)(var->save_result.ulonglong_value)) < max_val)
257-
var->save_result.ulonglong_value = max_val;
258-
} else {
259-
ulonglong max_val = *max_var_ptr();
260-
if (var->save_result.ulonglong_value > max_val)
261-
var->save_result.ulonglong_value = max_val;
259+
if (max_var_ptr()) {
260+
/* check constraint set with --maximum-...=X */
261+
if (SIGNED) {
262+
longlong max_val = *max_var_ptr();
263+
if (((longlong)(var->save_result.ulonglong_value)) > max_val)
264+
var->save_result.ulonglong_value = max_val;
265+
/*
266+
Signed variable probably has some kind of symmetry. Then
267+
it's good to limit negative values just as we limit positive
268+
values.
269+
*/
270+
max_val = -max_val;
271+
if (((longlong)(var->save_result.ulonglong_value)) < max_val)
272+
var->save_result.ulonglong_value = max_val;
273+
} else {
274+
ulonglong max_val = *max_var_ptr();
275+
if (var->save_result.ulonglong_value > max_val)
276+
var->save_result.ulonglong_value = max_val;
277+
}
262278
}
263-
}
264279

265-
return throw_bounds_warning(thd, name.str,
266-
var->save_result.ulonglong_value != (ulonglong)v,
267-
var->value->unsigned_flag, v);
268-
}
269-
bool session_update(THD *thd, set_var *var) override {
270-
session_var(thd, T) = static_cast<T>(var->save_result.ulonglong_value);
271-
return false;
272-
}
273-
bool global_update(THD *, set_var *var) override {
274-
global_var(T) = static_cast<T>(var->save_result.ulonglong_value);
275-
return false;
276-
}
277-
bool check_update_type(Item_result type) override { return type != INT_RESULT; }
278-
void session_save_default(THD *thd, set_var *var) override {
279-
var->save_result.ulonglong_value = static_cast<ulonglong>(
280+
return throw_bounds_warning(
281+
thd, name.str, var->save_result.ulonglong_value != (ulonglong)v,
282+
var->value->unsigned_flag, v);
283+
}
284+
bool session_update(THD *thd, set_var *var) override {
285+
session_var(thd, T) = static_cast<T>(var->save_result.ulonglong_value);
286+
return false;
287+
}
288+
bool global_update(THD *, set_var *var) override {
289+
global_var(T) = static_cast<T>(var->save_result.ulonglong_value);
290+
return false;
291+
}
292+
bool check_update_type(Item_result type) override {
293+
return type != INT_RESULT;
294+
}
295+
void session_save_default(THD *thd, set_var *var) override {
296+
var->save_result.ulonglong_value = static_cast<ulonglong>(
280297
*pointer_cast<const T *>(global_value_ptr(thd, nullptr)));
281-
}
282-
void global_save_default(THD *, set_var *var) override {
283-
var->save_result.ulonglong_value = option.def_value;
284-
}
285-
void saved_value_to_string(THD *, set_var *var, char *def_val) override {
286-
if (SIGNED)
287-
longlong10_to_str((longlong)var->save_result.ulonglong_value, def_val, -10);
288-
else
289-
longlong10_to_str((longlong)var->save_result.ulonglong_value, def_val, 10);
290-
}
291-
292-
private:
293-
T *max_var_ptr() {
294-
return scope() == SESSION ? (T *)(((uchar *)&max_system_variables) + offset)
295-
: nullptr;
296-
}
297-
}
298-
;
298+
}
299+
void global_save_default(THD *, set_var *var) override {
300+
var->save_result.ulonglong_value = option.def_value;
301+
}
302+
void saved_value_to_string(THD *, set_var *var, char *def_val) override {
303+
if (SIGNED)
304+
longlong10_to_str((longlong)var->save_result.ulonglong_value,
305+
def_val, -10);
306+
else
307+
longlong10_to_str((longlong)var->save_result.ulonglong_value,
308+
def_val, 10);
309+
}
310+
311+
private:
312+
T *max_var_ptr() {
313+
return scope() == SESSION
314+
? (T *)(((uchar *)&max_system_variables) + offset)
315+
: nullptr;
316+
}
317+
};
318+
// clang-format on
299319

300320
typedef Sys_var_integer<int32, GET_UINT, SHOW_INT, false> Sys_var_int32;
301321
typedef Sys_var_integer<uint, GET_UINT, SHOW_INT, false> Sys_var_uint;

0 commit comments

Comments
 (0)