On 2/16/16, Dean Rasheed <dean.a.rash...@gmail.com> wrote: > On 16 February 2016 at 05:01, Pavel Stehule <pavel.steh...@gmail.com> > wrote: >> 2016-02-15 10:16 GMT+01:00 Dean Rasheed <dean.a.rash...@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 agree with Pavel, it leads to a comparison in numeric, which is overhead. int8 can always be casted to numeric on-demand, but not vice versa. The main reasons to return int8 instead of numeric are performance and inability to imagine use case where so big numbers could take place. > 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. :-D Yes, it is fail. I missed it... > 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. I guess the best way is to simply make "bytes" a valid size unit even in GUC by adding it to the memory_unit_conversion_table with reflecting it in memory_units_hint and removing an extra checking from pg_size_bytes. > 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. > > I'll try to hack something up, and see what it looks like. > > Regards, > Dean -- Best regards, Vitaly Burovoy -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers