diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c index cdf1f4a..8ca36ab 100644 --- a/src/backend/access/common/reloptions.c +++ b/src/backend/access/common/reloptions.c @@ -1198,7 +1198,7 @@ parse_one_reloption(relopt_value *option, char *text_str, int text_len, { relopt_real *optreal = (relopt_real *) option->gen; - parsed = parse_real(value, &option->values.real_val); + parsed = parse_real(value, &option->values.real_val, 0, NULL); if (validate && !parsed) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index bb6052a..5226733 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -755,13 +755,13 @@ const char *const config_type_names[] = * For each supported conversion from one unit to another, we have an entry * in the table. * - * To keep things simple, and to avoid intermediate-value overflows, + * To keep things simple, and to avoid possible roundoff error, * conversions are never chained. There needs to be a direct conversion * between all units (of the same type). * - * The conversions from each base unit must be kept in order from greatest - * to smallest unit; convert_from_base_unit() relies on that. (The order of - * the base units does not matter.) + * The conversions for each base unit must be kept in order from greatest to + * smallest human-friendly unit; convert_xxx_from_base_unit() rely on that. + * (The order of the base-unit groups does not matter.) */ #define MAX_UNIT_LEN 3 /* length of longest recognized unit string */ @@ -770,9 +770,7 @@ typedef struct char unit[MAX_UNIT_LEN + 1]; /* unit, as a string, like "kB" or * "min" */ int base_unit; /* GUC_UNIT_XXX */ - int64 multiplier; /* If positive, multiply the value with this - * for unit -> base_unit conversion. If - * negative, divide (with the absolute value) */ + double multiplier; /* Factor for converting unit -> base_unit */ } unit_conversion; /* Ensure that the constants in the tables don't overflow or underflow */ @@ -787,45 +785,40 @@ static const char *memory_units_hint = gettext_noop("Valid units for this parame static const unit_conversion memory_unit_conversion_table[] = { - /* - * TB -> bytes conversion always overflows 32-bit integer, so this always - * produces an error. Include it nevertheless for completeness, and so - * that you get an "out of range" error, rather than "invalid unit". - */ - {"TB", GUC_UNIT_BYTE, INT64CONST(1024) * 1024 * 1024 * 1024}, - {"GB", GUC_UNIT_BYTE, 1024 * 1024 * 1024}, - {"MB", GUC_UNIT_BYTE, 1024 * 1024}, - {"kB", GUC_UNIT_BYTE, 1024}, - {"B", GUC_UNIT_BYTE, 1}, - - {"TB", GUC_UNIT_KB, 1024 * 1024 * 1024}, - {"GB", GUC_UNIT_KB, 1024 * 1024}, - {"MB", GUC_UNIT_KB, 1024}, - {"kB", GUC_UNIT_KB, 1}, - {"B", GUC_UNIT_KB, -1024}, - - {"TB", GUC_UNIT_MB, 1024 * 1024}, - {"GB", GUC_UNIT_MB, 1024}, - {"MB", GUC_UNIT_MB, 1}, - {"kB", GUC_UNIT_MB, -1024}, - {"B", GUC_UNIT_MB, -(1024 * 1024)}, - - {"TB", GUC_UNIT_BLOCKS, (1024 * 1024 * 1024) / (BLCKSZ / 1024)}, - {"GB", GUC_UNIT_BLOCKS, (1024 * 1024) / (BLCKSZ / 1024)}, - {"MB", GUC_UNIT_BLOCKS, 1024 / (BLCKSZ / 1024)}, - {"kB", GUC_UNIT_BLOCKS, -(BLCKSZ / 1024)}, - {"B", GUC_UNIT_BLOCKS, -BLCKSZ}, - - {"TB", GUC_UNIT_XBLOCKS, (1024 * 1024 * 1024) / (XLOG_BLCKSZ / 1024)}, - {"GB", GUC_UNIT_XBLOCKS, (1024 * 1024) / (XLOG_BLCKSZ / 1024)}, - {"MB", GUC_UNIT_XBLOCKS, 1024 / (XLOG_BLCKSZ / 1024)}, - {"kB", GUC_UNIT_XBLOCKS, -(XLOG_BLCKSZ / 1024)}, - {"B", GUC_UNIT_XBLOCKS, -XLOG_BLCKSZ}, + {"TB", GUC_UNIT_BYTE, 1024.0 * 1024.0 * 1024.0 * 1024.0}, + {"GB", GUC_UNIT_BYTE, 1024.0 * 1024.0 * 1024.0}, + {"MB", GUC_UNIT_BYTE, 1024.0 * 1024.0}, + {"kB", GUC_UNIT_BYTE, 1024.0}, + {"B", GUC_UNIT_BYTE, 1.0}, + + {"TB", GUC_UNIT_KB, 1024.0 * 1024.0 * 1024.0}, + {"GB", GUC_UNIT_KB, 1024.0 * 1024.0}, + {"MB", GUC_UNIT_KB, 1024.0}, + {"kB", GUC_UNIT_KB, 1.0}, + {"B", GUC_UNIT_KB, 1.0 / 1024.0}, + + {"TB", GUC_UNIT_MB, 1024.0 * 1024.0}, + {"GB", GUC_UNIT_MB, 1024.0}, + {"MB", GUC_UNIT_MB, 1.0}, + {"kB", GUC_UNIT_MB, 1.0 / 1024.0}, + {"B", GUC_UNIT_MB, 1.0 / (1024.0 * 1024.0)}, + + {"TB", GUC_UNIT_BLOCKS, (1024.0 * 1024.0 * 1024.0) / (BLCKSZ / 1024)}, + {"GB", GUC_UNIT_BLOCKS, (1024.0 * 1024.0) / (BLCKSZ / 1024)}, + {"MB", GUC_UNIT_BLOCKS, 1024.0 / (BLCKSZ / 1024)}, + {"kB", GUC_UNIT_BLOCKS, 1.0 / (BLCKSZ / 1024)}, + {"B", GUC_UNIT_BLOCKS, 1.0 / BLCKSZ}, + + {"TB", GUC_UNIT_XBLOCKS, (1024.0 * 1024.0 * 1024.0) / (XLOG_BLCKSZ / 1024)}, + {"GB", GUC_UNIT_XBLOCKS, (1024.0 * 1024.0) / (XLOG_BLCKSZ / 1024)}, + {"MB", GUC_UNIT_XBLOCKS, 1024.0 / (XLOG_BLCKSZ / 1024)}, + {"kB", GUC_UNIT_XBLOCKS, 1.0 / (XLOG_BLCKSZ / 1024)}, + {"B", GUC_UNIT_XBLOCKS, 1.0 / XLOG_BLCKSZ}, {""} /* end of table marker */ }; -static const char *time_units_hint = gettext_noop("Valid units for this parameter are \"ms\", \"s\", \"min\", \"h\", and \"d\"."); +static const char *time_units_hint = gettext_noop("Valid units for this parameter are \"us\", \"ms\", \"s\", \"min\", \"h\", and \"d\"."); static const unit_conversion time_unit_conversion_table[] = { @@ -834,18 +827,21 @@ static const unit_conversion time_unit_conversion_table[] = {"min", GUC_UNIT_MS, 1000 * 60}, {"s", GUC_UNIT_MS, 1000}, {"ms", GUC_UNIT_MS, 1}, + {"us", GUC_UNIT_MS, 1.0 / 1000}, {"d", GUC_UNIT_S, 60 * 60 * 24}, {"h", GUC_UNIT_S, 60 * 60}, {"min", GUC_UNIT_S, 60}, {"s", GUC_UNIT_S, 1}, - {"ms", GUC_UNIT_S, -1000}, + {"ms", GUC_UNIT_S, 1.0 / 1000}, + {"us", GUC_UNIT_S, 1.0 / (1000 * 1000)}, {"d", GUC_UNIT_MIN, 60 * 24}, {"h", GUC_UNIT_MIN, 60}, {"min", GUC_UNIT_MIN, 1}, - {"s", GUC_UNIT_MIN, -60}, - {"ms", GUC_UNIT_MIN, -1000 * 60}, + {"s", GUC_UNIT_MIN, 1.0 / 60}, + {"ms", GUC_UNIT_MIN, 1.0 / (1000 * 60)}, + {"us", GUC_UNIT_MIN, 1.0 / (1000 * 1000 * 60)}, {""} /* end of table marker */ }; @@ -5960,17 +5956,35 @@ ReportGUCOption(struct config_generic *record) /* * Convert a value from one of the human-friendly units ("kB", "min" etc.) * to the given base unit. 'value' and 'unit' are the input value and unit - * to convert from. The converted value is stored in *base_value. + * to convert from (there can be trailing spaces in the unit string). + * The converted value is stored in *base_value. + * It's caller's responsibility to round off the converted value as necessary + * and check for out-of-range. * * Returns true on success, false if the input unit is not recognized. */ static bool -convert_to_base_unit(int64 value, const char *unit, - int base_unit, int64 *base_value) +convert_to_base_unit(double value, const char *unit, + int base_unit, double *base_value) { + char unitstr[MAX_UNIT_LEN + 1]; + int unitlen; const unit_conversion *table; int i; + /* extract unit string to compare to table entries */ + unitlen = 0; + while (*unit != '\0' && !isspace((unsigned char) *unit) && + unitlen < MAX_UNIT_LEN) + unitstr[unitlen++] = *(unit++); + unitstr[unitlen] = '\0'; + /* allow whitespace after unit */ + while (isspace((unsigned char) *unit)) + unit++; + if (*unit != '\0') + return false; /* unit too long, or garbage after it */ + + /* now search the appropriate table */ if (base_unit & GUC_UNIT_MEMORY) table = memory_unit_conversion_table; else @@ -5979,12 +5993,9 @@ convert_to_base_unit(int64 value, const char *unit, for (i = 0; *table[i].unit; i++) { if (base_unit == table[i].base_unit && - strcmp(unit, table[i].unit) == 0) + strcmp(unitstr, table[i].unit) == 0) { - if (table[i].multiplier < 0) - *base_value = value / (-table[i].multiplier); - else - *base_value = value * table[i].multiplier; + *base_value = value * table[i].multiplier; return true; } } @@ -5992,14 +6003,15 @@ convert_to_base_unit(int64 value, const char *unit, } /* - * Convert a value in some base unit to a human-friendly unit. The output - * unit is chosen so that it's the greatest unit that can represent the value - * without loss. For example, if the base unit is GUC_UNIT_KB, 1024 is - * converted to 1 MB, but 1025 is represented as 1025 kB. + * Convert an integer value in some base unit to a human-friendly unit. + * + * The output unit is chosen so that it's the greatest unit that can represent + * the value without loss. For example, if the base unit is GUC_UNIT_KB, 1024 + * is converted to 1 MB, but 1025 is represented as 1025 kB. */ static void -convert_from_base_unit(int64 base_value, int base_unit, - int64 *value, const char **unit) +convert_int_from_base_unit(int64 base_value, int base_unit, + int64 *value, const char **unit) { const unit_conversion *table; int i; @@ -6020,15 +6032,15 @@ convert_from_base_unit(int64 base_value, int base_unit, * assume that the conversions for each base unit are ordered from * greatest unit to the smallest! */ - if (table[i].multiplier < 0) + if (table[i].multiplier <= 1.0) { - *value = base_value * (-table[i].multiplier); + *value = rint(base_value / table[i].multiplier); *unit = table[i].unit; break; } - else if (base_value % table[i].multiplier == 0) + else if (base_value % (int64) table[i].multiplier == 0) { - *value = base_value / table[i].multiplier; + *value = rint(base_value / table[i].multiplier); *unit = table[i].unit; break; } @@ -6038,6 +6050,44 @@ convert_from_base_unit(int64 base_value, int base_unit, Assert(*unit != NULL); } +/* + * Convert a floating-point value in some base unit to a human-friendly unit. + * + * Same as above, except we have to do the math a bit differently, and + * there's a possibility that we don't find any exact divisor. + */ +static void +convert_real_from_base_unit(double base_value, int base_unit, + double *value, const char **unit) +{ + const unit_conversion *table; + int i; + + *unit = NULL; + + if (base_unit & GUC_UNIT_MEMORY) + table = memory_unit_conversion_table; + else + table = time_unit_conversion_table; + + for (i = 0; *table[i].unit; i++) + { + if (base_unit == table[i].base_unit) + { + /* + * Accept the first conversion that divides the value evenly; or + * if there is none, use the smallest (last) target unit. + */ + *value = base_value / table[i].multiplier; + *unit = table[i].unit; + if (*value == rint(*value)) + break; + } + } + + Assert(*unit != NULL); +} + /* * Try to parse value as an integer. The accepted formats are the @@ -6082,26 +6132,14 @@ parse_int(const char *value, int *result, int flags, const char **hintmsg) /* Handle possible unit */ if (*endptr != '\0') { - char unit[MAX_UNIT_LEN + 1]; - int unitlen; - bool converted = false; + double cval; if ((flags & GUC_UNIT) == 0) return false; /* this setting does not accept a unit */ - unitlen = 0; - while (*endptr != '\0' && !isspace((unsigned char) *endptr) && - unitlen < MAX_UNIT_LEN) - unit[unitlen++] = *(endptr++); - unit[unitlen] = '\0'; - /* allow whitespace after unit */ - while (isspace((unsigned char) *endptr)) - endptr++; - - if (*endptr == '\0') - converted = convert_to_base_unit(val, unit, (flags & GUC_UNIT), - &val); - if (!converted) + if (!convert_to_base_unit((double) val, + endptr, (flags & GUC_UNIT), + &cval)) { /* invalid unit, or garbage after the unit; set hint and fail. */ if (hintmsg) @@ -6114,13 +6152,15 @@ parse_int(const char *value, int *result, int flags, const char **hintmsg) return false; } - /* Check for overflow due to units conversion */ - if (val != (int64) ((int32) val)) + /* Round to int, then check for overflow due to units conversion */ + cval = rint(cval); + if (cval > INT_MAX || cval < INT_MIN) { if (hintmsg) *hintmsg = gettext_noop("Value exceeds integer range."); return false; } + val = (int64) cval; } if (result) @@ -6136,24 +6176,47 @@ parse_int(const char *value, int *result, int flags, const char **hintmsg) * If okay and result is not NULL, return the value in *result. */ bool -parse_real(const char *value, double *result) +parse_real(const char *value, double *result, int flags, const char **hintmsg) { double val; char *endptr; + /* To suppress compiler warnings, always set output params */ if (result) - *result = 0; /* suppress compiler warning */ + *result = 0; + if (hintmsg) + *hintmsg = NULL; errno = 0; val = strtod(value, &endptr); if (endptr == value || errno == ERANGE) return false; - /* allow whitespace after number */ + /* allow whitespace between number and unit */ while (isspace((unsigned char) *endptr)) endptr++; + + /* Handle possible unit */ if (*endptr != '\0') - return false; + { + if ((flags & GUC_UNIT) == 0) + return false; /* this setting does not accept a unit */ + + if (!convert_to_base_unit(val, + endptr, (flags & GUC_UNIT), + &val)) + { + /* invalid unit, or garbage after the unit; set hint and fail. */ + if (hintmsg) + { + if (flags & GUC_UNIT_MEMORY) + *hintmsg = memory_units_hint; + else + *hintmsg = time_units_hint; + } + return false; + } + } if (result) *result = val; @@ -6336,13 +6399,16 @@ parse_and_validate_value(struct config_generic *record, case PGC_REAL: { struct config_real *conf = (struct config_real *) record; + const char *hintmsg; - if (!parse_real(value, &newval->realval)) + if (!parse_real(value, &newval->realval, + conf->gen.flags, &hintmsg)) { ereport(elevel, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("parameter \"%s\" requires a numeric value", - name))); + errmsg("invalid value for parameter \"%s\": \"%s\"", + name, value), + hintmsg ? errhint("%s", _(hintmsg)) : 0)); return false; } @@ -8687,48 +8753,44 @@ GetConfigOptionByNum(int varnum, const char **values, bool *noshow) values[1] = _ShowOption(conf, false); /* unit */ - if (conf->vartype == PGC_INT) + switch (conf->flags & (GUC_UNIT_MEMORY | GUC_UNIT_TIME)) { - switch (conf->flags & (GUC_UNIT_MEMORY | GUC_UNIT_TIME)) - { - case GUC_UNIT_BYTE: - values[2] = "B"; - break; - case GUC_UNIT_KB: - values[2] = "kB"; - break; - case GUC_UNIT_MB: - values[2] = "MB"; - break; - case GUC_UNIT_BLOCKS: - snprintf(buffer, sizeof(buffer), "%dkB", BLCKSZ / 1024); - values[2] = pstrdup(buffer); - break; - case GUC_UNIT_XBLOCKS: - snprintf(buffer, sizeof(buffer), "%dkB", XLOG_BLCKSZ / 1024); - values[2] = pstrdup(buffer); - break; - case GUC_UNIT_MS: - values[2] = "ms"; - break; - case GUC_UNIT_S: - values[2] = "s"; - break; - case GUC_UNIT_MIN: - values[2] = "min"; - break; - case 0: - values[2] = NULL; - break; - default: - elog(ERROR, "unrecognized GUC units value: %d", - conf->flags & (GUC_UNIT_MEMORY | GUC_UNIT_TIME)); - values[2] = NULL; - break; - } + case 0: + /* no unit specified */ + values[2] = NULL; + break; + case GUC_UNIT_BYTE: + values[2] = "B"; + break; + case GUC_UNIT_KB: + values[2] = "kB"; + break; + case GUC_UNIT_MB: + values[2] = "MB"; + break; + case GUC_UNIT_BLOCKS: + snprintf(buffer, sizeof(buffer), "%dkB", BLCKSZ / 1024); + values[2] = pstrdup(buffer); + break; + case GUC_UNIT_XBLOCKS: + snprintf(buffer, sizeof(buffer), "%dkB", XLOG_BLCKSZ / 1024); + values[2] = pstrdup(buffer); + break; + case GUC_UNIT_MS: + values[2] = "ms"; + break; + case GUC_UNIT_S: + values[2] = "s"; + break; + case GUC_UNIT_MIN: + values[2] = "min"; + break; + default: + elog(ERROR, "unrecognized GUC units value: %d", + conf->flags & (GUC_UNIT_MEMORY | GUC_UNIT_TIME)); + values[2] = NULL; + break; } - else - values[2] = NULL; /* group */ values[3] = _(config_group_names[conf->group]); @@ -9257,10 +9319,9 @@ _ShowOption(struct config_generic *record, bool use_units) const char *unit; if (use_units && result > 0 && (record->flags & GUC_UNIT)) - { - convert_from_base_unit(result, record->flags & GUC_UNIT, - &result, &unit); - } + convert_int_from_base_unit(result, + record->flags & GUC_UNIT, + &result, &unit); else unit = ""; @@ -9279,8 +9340,18 @@ _ShowOption(struct config_generic *record, bool use_units) val = conf->show_hook(); else { - snprintf(buffer, sizeof(buffer), "%g", - *conf->variable); + double result = *conf->variable; + const char *unit; + + if (use_units && result > 0 && (record->flags & GUC_UNIT)) + convert_real_from_base_unit(result, + record->flags & GUC_UNIT, + &result, &unit); + else + unit = ""; + + snprintf(buffer, sizeof(buffer), "%g%s", + result, unit); val = buffer; } } diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h index c07e7b9..2712a77 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -361,7 +361,8 @@ extern void BeginReportingGUCOptions(void); extern void ParseLongOption(const char *string, char **name, char **value); extern bool parse_int(const char *value, int *result, int flags, const char **hintmsg); -extern bool parse_real(const char *value, double *result); +extern bool parse_real(const char *value, double *result, int flags, + const char **hintmsg); extern int set_config_option(const char *name, const char *value, GucContext context, GucSource source, GucAction action, bool changeVal, int elevel,