2016-01-19 4:45 GMT+01:00 Vitaly Burovoy <vitaly.buro...@gmail.com>: > On 1/4/16, Pavel Stehule <pavel.steh...@gmail.com> wrote: > > 2016-01-04 18:13 GMT+01:00 Shulgin, Oleksandr < > oleksandr.shul...@zalando.de> : > >> On Mon, Jan 4, 2016 at 6:03 PM, Pavel Stehule <pavel.steh...@gmail.com> > wrote: > >> > 2016-01-04 17:48 GMT+01:00 Shulgin, Oleksandr < > oleksandr.shul...@zalando.de>: > >> >> On Mon, Jan 4, 2016 at 4:51 PM, Robert Haas <robertmh...@gmail.com> > wrote: > >> >> > >> >> I'm also inclined on dropping that explicit check for empty string > >> >> below and let numeric_in() error out on that. Does this look OK, or > can > >> >> it confuse someone: > >> >> postgres=# select pg_size_bytes(''); > >> >> ERROR: invalid input syntax for type numeric: "" > >> > > >> > both fixed > >> > >> Hm... > >> > >> > + switch (*strptr) > >> > + { > >> > + /* ignore plus symbol */ > >> > + case '+': > >> > + case '-': > >> > + *bufptr++ = *strptr++; > >> > + break; > >> > + } > >> > >> Well, to that amount you don't need any special checks, I'm just not > sure > >> if reported error message is not misleading if we let numeric_in() > handle > >> all the errors. At least it can cope with the leading spaces, +/- and > >> empty input quite well. > >> > > > > I don't would to catch a exception from numeric_in - so I try to solve > some > > simple situations, where I can raise possible better error message. > > There are several cases where your behavior gives strange errors (see > below). > > Next batch of notes: > > src/include/catalog/pg_proc.h: > --- > + DATA(insert OID = 3317 ( pg_size_bytes... > now oid 3317 is used (by pg_stat_get_wal_receiver), 3318 is free >
fixed > > --- > + DESCR("convert a human readable text with size units to big int bytes"); > May be the best way is to copy the first sentence from the doc? > ("convert a size in human-readable format with size units into bytes") > fixed > ==== > src/backend/utils/adt/dbsize.c: > + text *arg = PG_GETARG_TEXT_PP(0); > + char *str = text_to_cstring(arg); > ... > + /* working buffer cannot be longer than original string */ > + buffer = (char *) palloc(VARSIZE_ANY_EXHDR(arg) + 1); > Is there any reason to get TEXT for only converting it to cstring and > call VARSIZE_ANY_EXHDR instead of strlen? > performance - but these strings should be short, so I can use strlen fixed > > --- > + text *arg = PG_GETARG_TEXT_PP(0); > + char *str = text_to_cstring(arg); > + char *strptr = str; > + char *buffer; > There are wrong offsets of variable names after their types (among all > body of the "pg_size_bytes" function). > See variable declarations in nearby functions ( > >> "make the new code look like the existing code around it" > http://www.postgresql.org/docs/devel/static/source-format.html > ) > > fixed > --- > + errmsg("\"%s\" is not number", > str))); > s/is not number/is not a number/ > (the second version can be found in a couple places besides translations) > fixed but this message can be little bit no intuitive - better text is "is not a valid number" > > --- > + if (*strptr != '\0') > ... > + while (*strptr && !isspace(*strptr)) > Sometimes it explicitly compares to '\0', sometimes implicitly. > Common use is explicit comparison and it is preferred due to different > compilers (their conversions to boolean). > fixed > > --- > + /* Skip leading spaces */ > ... > + /* ignore plus symbol */ > ... > + /* copy digits to working buffer */ > ... > + /* allow whitespace between integer and unit */ > I'm also inclined on dropping that explicit skipping spaces, checking > for +/- symbols, but copying all digits, spaces, dots and '+-' symbols > and let numeric_in() error out on that. > This is difficult - you have to divide string to two parts and first world is number, second world is unit. For example "+912+ kB" is correct number +912 and broken unit "+ kB". > It allows to get correct error messages for something like: > postgres=# select pg_size_bytes('.+912'); > ERROR: invalid input syntax for type numeric: ".+912" > postgres=# select pg_size_bytes('+912+ kB'); > ERROR: invalid input syntax for type numeric: "+912+ " > postgres=# select pg_size_bytes('++123 kB'); > ERROR: invalid input syntax for type numeric: "++123 " > > instead of current: > postgres=# select pg_size_bytes('.+912'); > ERROR: invalid input syntax for type numeric: "." > postgres=# select pg_size_bytes('+912+ kB'); > ERROR: invalid unit: "+ kB" > postgres=# select pg_size_bytes('++123 kB'); > ERROR: invalid input syntax for type numeric: "+" > > I redesigned this check. Now it is popostgres=# select pg_size_bytes('.+912'); ERROR: 22023: ".+912" is not a valid number stgres=# select pg_size_bytes('++123 kB'); ERROR: 22023: "++123 kB" is not a valid number > --- > + while (isspace((unsigned char) *strptr)) > ... > + while (isspace(*strptr)) > ... > + while (*strptr && !isspace(*strptr)) > ... > + while (isspace(*strptr)) > The first occurece of isspace's parameter is casting to "unsigned > char" whereas the others are not. > Note: > "The behavior is undefined if the value of ch is not representable as > unsigned char and is not equal to EOF" > > Proof: > http://en.cppreference.com/w/c/string/byte/isspace fixed > > > --- > + pfree(buffer); > + pfree(str); > pfree-s here are not necessary. See: > http://www.neilconway.org/talks/hacking/hack_slides.pdf (page 17) > Automatic memory deallocation doesn't cover all possible situations where the function can be used - for example DirectFunctionCall - so explicit deallocation can descrease a memory requirements when you call these functions from C. New version is attached Regards Pavel > > -- > Best regards, > Vitaly Burovoy >
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml new file mode 100644 index 0af01d9..c349714 *** a/doc/src/sgml/func.sgml --- b/doc/src/sgml/func.sgml *************** postgres=# SELECT * FROM pg_xlogfile_nam *** 17655,17660 **** --- 17655,17663 ---- <primary>pg_relation_size</primary> </indexterm> <indexterm> + <primary>pg_size_bytes</primary> + </indexterm> + <indexterm> <primary>pg_size_pretty</primary> </indexterm> <indexterm> *************** postgres=# SELECT * FROM pg_xlogfile_nam *** 17725,17730 **** --- 17728,17744 ---- </entry> </row> <row> + <entry> + <literal><function>pg_size_bytes(<type>text</type>)</function></literal> + </entry> + <entry><type>bigint</type></entry> + <entry> + Converts a size in human-readable format with size units into bytes. + The parameter is case-insensitive, and the supported size units + are: kB, MB, GB and TB. + </entry> + </row> + <row> <entry> <literal><function>pg_size_pretty(<type>bigint</type>)</function></literal> </entry> diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c new file mode 100644 index 2084692..643004a *** a/src/backend/utils/adt/dbsize.c --- b/src/backend/utils/adt/dbsize.c *************** *** 25,30 **** --- 25,31 ---- #include "storage/fd.h" #include "utils/acl.h" #include "utils/builtins.h" + #include "utils/guc.h" #include "utils/numeric.h" #include "utils/rel.h" #include "utils/relfilenodemap.h" *************** pg_size_pretty_numeric(PG_FUNCTION_ARGS) *** 700,705 **** --- 701,847 ---- } /* + * Convert human readable size to long int. + * + * Due to support for decimal values and case insensitivity of units + * a function parse_int cannot be used. + */ + Datum + pg_size_bytes(PG_FUNCTION_ARGS) + { + text *arg = PG_GETARG_TEXT_PP(0); + char *str, + *strptr; + char *buffer, + *bufptr; + Numeric num; + int64 result; + bool found_digit = false; + bool found_point = false; + + str = text_to_cstring(arg); + strptr = str; + + /* prepare buffer for parts of parsed input string */ + buffer = (char *) palloc(strlen(str) + 1); + bufptr = buffer; + + /* Skip leading spaces */ + while (isspace((unsigned char) *strptr)) + strptr++; + + switch (*strptr) + { + /* ignore plus symbol */ + case '+': + case '-': + *bufptr++ = *strptr++; + break; + } + + while(*strptr != '\0') + { + if (isdigit((unsigned char) *strptr)) + { + *bufptr++ = *strptr++; + found_digit = true; + } + else if (*strptr == '.') + { + if (found_point) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("\"%s\" is not in expected format", str))); + + *bufptr++ = *strptr++; + found_point = true; + } + else + /* any non digit char, the unit is starting */ + break; + } + + if (!found_digit) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("\"%s\" is not in expected format", str))); + + *bufptr = '\0'; + + /* don't allow empty string */ + if (*buffer == '\0') + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("\"%s\" is not in expected format", str))); + + num = DatumGetNumeric(DirectFunctionCall3(numeric_in, + CStringGetDatum(buffer), 0, -1)); + + /* allow whitespace between integer and unit */ + while (isspace((unsigned char) *strptr)) + strptr++; + + /* Handle possible unit */ + if (*strptr != '\0') + { + int multiplier; + Numeric mul_num; + const char *hintmsg; + const char *unitstr; + + bufptr = buffer; + + /* + * unitstr holds complete string related to unit part. Used + * as part of error message. The parse_memory_unit is called + * with this value, when we detected invalid format, and we + * would to emit error result to ger hintstr. Elsewhere is + * used space trimmed buffer. + */ + unitstr = strptr; + + /* copy chars to buffer and stop on first space or end string */ + while (*strptr != '\0' && !isspace((unsigned char) *strptr)) + *bufptr++ = *strptr++; + + *bufptr = '\0'; + + /* We allow ending spaces. Skip all spaces. */ + while (isspace((unsigned char) *strptr)) + strptr++; + + /* Use buffer as unitstr, when we didn't find more words. */ + if (*strptr == '\0') + unitstr = buffer; + + /* Still, the unit can be invalid: too long, unknown */ + if (!parse_memory_unit(unitstr, &multiplier, &hintmsg)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid unit: \"%s\"", unitstr), + errhint("%s", _(hintmsg)))); + + /* + * Now, the multiplier is in KB units. It should be multiplied by 1024 + * before usage. + */ + mul_num = DatumGetNumeric(DirectFunctionCall1(int8_numeric, + Int64GetDatum(multiplier * 1024L))); + + num = DatumGetNumeric(DirectFunctionCall2(numeric_mul, + NumericGetDatum(mul_num), + NumericGetDatum(num))); + } + + result = DatumGetInt64(DirectFunctionCall1(numeric_int8, NumericGetDatum(num))); + + pfree(buffer); + pfree(str); + + PG_RETURN_INT64(result); + } + + /* * Get the filenode of a relation * * This is expected to be used in queries like diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c new file mode 100644 index 38ba82f..4020df7 *** a/src/backend/utils/misc/guc.c --- b/src/backend/utils/misc/guc.c *************** convert_from_base_unit(int64 base_value, *** 5238,5243 **** --- 5238,5272 ---- /* + * Parse value as some known memory unit to their size in bytes. + * Used in pg_size_bytes function. Against convert_to_base_unit, a string + * comparation is case insensitive. + */ + bool + parse_memory_unit(const char *unit, int *multiplier, + const char **hintmsg) + { + int i; + + for (i = 0; *memory_unit_conversion_table[i].unit; i++) + { + const unit_conversion *conv = &memory_unit_conversion_table[i]; + + if (conv->base_unit == GUC_UNIT_KB && + strcasecmp(unit, conv->unit) == 0) + { + *multiplier = conv->multiplier; + return true; + } + } + + *hintmsg = memory_units_hint; + + return false; + } + + + /* * Try to parse value as an integer. The accepted formats are the * usual decimal, octal, or hexadecimal formats, optionally followed by * a unit name if "flags" indicates a unit is allowed. diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h new file mode 100644 index 244aa4d..839934a *** a/src/include/catalog/pg_proc.h --- b/src/include/catalog/pg_proc.h *************** DATA(insert OID = 2286 ( pg_total_relati *** 3567,3572 **** --- 3567,3574 ---- DESCR("total disk space usage for the specified table and associated indexes"); DATA(insert OID = 2288 ( pg_size_pretty PGNSP PGUID 12 1 0 0 0 f f f f t f v s 1 0 25 "20" _null_ _null_ _null_ _null_ _null_ pg_size_pretty _null_ _null_ _null_ )); DESCR("convert a long int to a human readable text using size units"); + DATA(insert OID = 3331 ( pg_size_bytes PGNSP PGUID 12 1 0 0 0 f f f f t f v s 1 0 20 "25" _null_ _null_ _null_ _null_ _null_ pg_size_bytes _null_ _null_ _null_ )); + DESCR("convert a size in human-readable format with size units into bytes"); DATA(insert OID = 3166 ( pg_size_pretty PGNSP PGUID 12 1 0 0 0 f f f f t f v s 1 0 25 "1700" _null_ _null_ _null_ _null_ _null_ pg_size_pretty_numeric _null_ _null_ _null_ )); DESCR("convert a numeric to a human readable text using size units"); DATA(insert OID = 2997 ( pg_table_size PGNSP PGUID 12 1 0 0 0 f f f f t f v s 1 0 20 "2205" _null_ _null_ _null_ _null_ _null_ pg_table_size _null_ _null_ _null_ )); diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h new file mode 100644 index 477fde1..023d6a2 *** a/src/include/utils/builtins.h --- b/src/include/utils/builtins.h *************** extern Datum pg_relation_size(PG_FUNCTIO *** 462,467 **** --- 462,468 ---- extern Datum pg_total_relation_size(PG_FUNCTION_ARGS); extern Datum pg_size_pretty(PG_FUNCTION_ARGS); extern Datum pg_size_pretty_numeric(PG_FUNCTION_ARGS); + extern Datum pg_size_bytes(PG_FUNCTION_ARGS); extern Datum pg_table_size(PG_FUNCTION_ARGS); extern Datum pg_indexes_size(PG_FUNCTION_ARGS); extern Datum pg_relation_filenode(PG_FUNCTION_ARGS); diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h new file mode 100644 index e1de1a5..3bfe0f4 *** a/src/include/utils/guc.h --- b/src/include/utils/guc.h *************** extern int NewGUCNestLevel(void); *** 357,362 **** --- 357,364 ---- extern void AtEOXact_GUC(bool isCommit, int nestLevel); extern void BeginReportingGUCOptions(void); extern void ParseLongOption(const char *string, char **name, char **value); + extern bool parse_memory_unit(const char *unit, int *multiplier, + const char **hintmsg); extern bool parse_int(const char *value, int *result, int flags, const char **hintmsg); extern bool parse_real(const char *value, double *result); diff --git a/src/test/regress/expected/dbsize.out b/src/test/regress/expected/dbsize.out new file mode 100644 index aa513e7..aa652fb *** a/src/test/regress/expected/dbsize.out --- b/src/test/regress/expected/dbsize.out *************** SELECT size, pg_size_pretty(size), pg_si *** 35,37 **** --- 35,98 ---- 1000000000000000.5 | 909 TB | -909 TB (12 rows) + SELECT pg_size_bytes(size) FROM + (VALUES('1'), ('1kB'), ('1MB'), (' 1 GB'), ('1.5 GB '), + ('1TB'), ('3000 TB')) x(size); + pg_size_bytes + ------------------ + 1 + 1024 + 1048576 + 1073741824 + 1610612736 + 1099511627776 + 3298534883328000 + (7 rows) + + -- case insensitive units are supported + SELECT pg_size_bytes(size) FROM + (VALUES('1'), ('1kb'), ('1mb'), (' 1 Gb'), ('1.5 gB '), + ('1tb')) x(size); + pg_size_bytes + --------------- + 1 + 1024 + 1048576 + 1073741824 + 1610612736 + 1099511627776 + (6 rows) + + -- negative numbers are supported + SELECT pg_size_bytes(size) FROM + (VALUES('-1'), ('-1kb'), ('-1mb'), (' -1 Gb'), ('-1.5 gB '), + ('-1tb')) x(size); + pg_size_bytes + ---------------- + -1 + -1024 + -1048576 + -1073741824 + -1610612736 + -1099511627776 + (6 rows) + + --should fail + SELECT pg_size_bytes('1 AB'); + ERROR: invalid unit: "AB" + HINT: Valid units for this parameter are "kB", "MB", "GB", and "TB". + SELECT pg_size_bytes('1 AB A'); + ERROR: invalid unit: "AB A" + HINT: Valid units for this parameter are "kB", "MB", "GB", and "TB". + select pg_size_bytes('9223372036854775807.9'); + ERROR: bigint out of range + select pg_size_bytes('1024 bytes'); + ERROR: invalid unit: "bytes" + HINT: Valid units for this parameter are "kB", "MB", "GB", and "TB". + select pg_size_bytes('.+912'); + ERROR: ".+912" is not in expected format + select pg_size_bytes('+912+ kB'); + ERROR: invalid unit: "+ kB" + HINT: Valid units for this parameter are "kB", "MB", "GB", and "TB". + select pg_size_bytes('++123 kB'); + ERROR: "++123 kB" is not in expected format diff --git a/src/test/regress/sql/dbsize.sql b/src/test/regress/sql/dbsize.sql new file mode 100644 index c118090..8e8c3af *** a/src/test/regress/sql/dbsize.sql --- b/src/test/regress/sql/dbsize.sql *************** SELECT size, pg_size_pretty(size), pg_si *** 10,12 **** --- 10,36 ---- (10.5::numeric), (1000.5::numeric), (1000000.5::numeric), (1000000000.5::numeric), (1000000000000.5::numeric), (1000000000000000.5::numeric)) x(size); + + SELECT pg_size_bytes(size) FROM + (VALUES('1'), ('1kB'), ('1MB'), (' 1 GB'), ('1.5 GB '), + ('1TB'), ('3000 TB')) x(size); + + -- case insensitive units are supported + SELECT pg_size_bytes(size) FROM + (VALUES('1'), ('1kb'), ('1mb'), (' 1 Gb'), ('1.5 gB '), + ('1tb')) x(size); + + -- negative numbers are supported + SELECT pg_size_bytes(size) FROM + (VALUES('-1'), ('-1kb'), ('-1mb'), (' -1 Gb'), ('-1.5 gB '), + ('-1tb')) x(size); + + --should fail + SELECT pg_size_bytes('1 AB'); + SELECT pg_size_bytes('1 AB A'); + select pg_size_bytes('9223372036854775807.9'); + select pg_size_bytes('1024 bytes'); + + select pg_size_bytes('.+912'); + select pg_size_bytes('+912+ kB'); + select pg_size_bytes('++123 kB');
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers