On 16 February 2016 at 05:01, Pavel Stehule <pavel.stehule@gmail.com> wrote: > 2016-02-15 10:16 GMT+01:00 Dean Rasheed <dean.a.rasheed@gmail.com>: >> Is there any reason not to make >> pg_size_bytes() return numeric? >> > This is a question. I have not a strong opinion about it. There are no any > technical objection - the result will be +/- same. But you will enforce > Numeric into outer expression evaluation. > > The result will not be used together with function pg_size_pretty, but much > more with functions pg_relation_size, pg_relation_size, .. and these > functions doesn't return Numeric. These functions returns bigint. Bigint is > much more natural type for this purpose. > > Is there any use case for Numeric? >
[Shrug] I don't really have a strong opinion about it either, but it seemed that maybe the function might have some wider uses beyond just comparing object sizes, and since it's already computing the numeric size, it might as well just return it.
I see only one disadvantage - when we return numeric, then all following expression will be in numeric, and for some functions, or some expression the users will have to cast explicitly to bigint.
Looking at the rest of the patch, I think there are other things that need tidying up -- the unit parsing code for one. This is going to some effort to reuse the memory_unit_conversion_table from guc.c, but the result is that it has added quite a bit of new code and now the responsibility for parsing different units is handled by different functions in different files, which IMO is quite messy. Worse, the error message is wrong and misleading:
select pg_size_bytes('10 bytes'); -- OK select pg_size_bytes('10 gallons'); ERROR: invalid size: "10 gallons" DETAIL: Invalid size unit "gallons" HINT: Valid units for this parameter are "kB", "MB", "GB", and "TB".
which fails to mention that "bytes" is also a valid unit.
true
Fixing that in parse_memory_unit() would be messy because it assumes a base unit of kB, so it would require a negative multiplier, and pg_size_bytes() would have to be taught to divide by the magnitude of negative multipliers in the same way that guc.c does.
ISTM that it would be far less code, and much simpler and more readable to just parse the supported units directly in pg_size_bytes(), rather than trying to share code with guc.c, when the supported units are actually different and may well diverge further in the future.
yes, if we have different units, then independent control tables has more sense.
I'll try to hack something up, and see what it looks like.