Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-07-09 Thread David Rowley
On Thu, 8 Jul 2021 at 05:44, David Christensen wrote: > Enclosed is the patch to change the return type to numeric, as well as one > for expanding units to > add PB and EB. I ended up not changing the return type of pg_size_bytes(). > I figured that PB and EB are probably good enough additions

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-07-08 Thread Dean Rasheed
On Thu, 8 Jul 2021 at 14:38, David Rowley wrote: > > I gave it a bit of exercise by running pgbench and calling this procedure: > > It ran 8526956 times, so with the loop that's 8.5 billion random > numbers. No variations between the two functions. I got the same > after removing the 0 - to test

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-07-08 Thread David Rowley
tOn Thu, 8 Jul 2021 at 20:23, Dean Rasheed wrote: > > > On Thu, 8 Jul 2021 at 13:31, David Rowley wrote: > > Here's a patch which I believe makes pg_size_pretty() and > > pg_size_pretty_numeric() match in regards to negative values. > > LGTM, except I think it's worth also making the numeric

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-07-08 Thread David Rowley
On Thu, 8 Jul 2021 at 07:31, Tom Lane wrote: > > David Christensen writes: > > Enclosed is the patch to change the return type to numeric, as well as one > > for expanding units to > > add PB and EB. > > Can we really get away with changing the return type? That would > by no stretch of the

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-07-08 Thread Dean Rasheed
On Thu, 8 Jul 2021 at 05:30, David Rowley wrote: > > On Thu, 8 Jul 2021 at 13:31, David Rowley wrote: > > It feels like if we're going to fix this negative rounding thing then > > we should maybe do it and backpatch a fix then rebase this work on top > > of that. Yes, that was my thinking too.

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-07-07 Thread David Rowley
On Thu, 8 Jul 2021 at 13:31, David Rowley wrote: > It feels like if we're going to fix this negative rounding thing then > we should maybe do it and backpatch a fix then rebase this work on top > of that. Here's a patch which I believe makes pg_size_pretty() and pg_size_pretty_numeric() match in

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-07-07 Thread David Rowley
On Thu, 8 Jul 2021 at 01:32, Dean Rasheed wrote: > Hmm, this looked easy, but... > > It occurred to me that there ought to be regression tests for the edge > cases where it steps from one unit to the next. So, in the style of > the existing regression tests, I tried the following: > > SELECT

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-07-07 Thread David Christensen
Tom Lane writes: > David Christensen writes: >> Enclosed is the patch to change the return type to numeric, as well as one >> for expanding units to >> add PB and EB. > > Can we really get away with changing the return type? That would > by no stretch of the imagination be free; one could

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-07-07 Thread Tom Lane
David Christensen writes: > Enclosed is the patch to change the return type to numeric, as well as one > for expanding units to > add PB and EB. Can we really get away with changing the return type? That would by no stretch of the imagination be free; one could expect breakage of a few user

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-07-07 Thread David Christensen
David Rowley writes: > On Wed, 7 Jul 2021 at 02:46, David Christensen > wrote: >> if we do decide to expand the units table there will be a >> few additional changes (most significantly, the return value of >> `pg_size_bytes()` will need to switch >> to `numeric`). > > I wonder if it's worth

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-07-07 Thread Dean Rasheed
On Wed, 7 Jul 2021 at 03:47, David Rowley wrote: > > Updated patch attached. > Hmm, this looked easy, but... It occurred to me that there ought to be regression tests for the edge cases where it steps from one unit to the next. So, in the style of the existing regression tests, I tried the

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-07-06 Thread David Rowley
On Wed, 7 Jul 2021 at 02:46, David Christensen wrote: > if we do decide to expand the units table there will be a > few additional changes (most significantly, the return value of > `pg_size_bytes()` will need to switch > to `numeric`). I wonder if it's worth changing pg_size_bytes() to return

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-07-06 Thread David Rowley
On Wed, 7 Jul 2021 at 02:54, Tom Lane wrote: > > Minor nit: use "const char *text" in the struct declaration, so > that all of the static data can be placed in fixed storage. Thanks for pointing that out. > David Rowley writes: > > (I'm not sure why pgindent removed the space between != and

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-07-06 Thread David Rowley
On Wed, 7 Jul 2021 at 00:51, Dean Rasheed wrote: > 10. half_round(21) == 11 :: 20 >> 1 = 10 > > The correct result should be 10 (it would be very odd to claim that > 10241 bytes should be displayed as 11kb), but the half-rounding keeps > rounding up at each stage. > > That's a general property of

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-07-06 Thread Tom Lane
David Rowley writes: > Does anyone want to have a look over this? If not, I plan to push it > in the next day or so. Minor nit: use "const char *text" in the struct declaration, so that all of the static data can be placed in fixed storage. > (I'm not sure why pgindent removed the space

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-07-06 Thread David Christensen
David Rowley writes: > On Mon, 5 Jul 2021 at 20:00, David Rowley wrote: >> I don't really like the fact that I had to add the doHalfRound field >> to get the same rounding behaviour as the original functions. I'm >> wondering if it would just be too clever just to track how many bits >> we've

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-07-06 Thread Dean Rasheed
On Tue, 6 Jul 2021 at 13:15, David Rowley wrote: > > Can you give an example where calling half_rounded too many times will > give the wrong value? Keeping in mind we call half_rounded the number > of times that the passed in value would need to be left-shifted by to > get the equivalent

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-07-06 Thread David Rowley
On Tue, 6 Jul 2021 at 23:39, Dean Rasheed wrote: > When I first read this: > > +/* half-round until we get down to unitBits */ > +while (rightshifts++ < unit->unitBits) > +size = half_rounded(size); > > it looked to me like it would be invoking

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-07-06 Thread Dean Rasheed
On Tue, 6 Jul 2021 at 10:20, David Rowley wrote: > > I made another pass over this and ended up removing the doHalfRound > field in favour of just doing rounding based on the previous > bitshifts. > When I first read this: +/* half-round until we get down to unitBits */ +

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-07-06 Thread David Rowley
On Mon, 5 Jul 2021 at 20:00, David Rowley wrote: > I don't really like the fact that I had to add the doHalfRound field > to get the same rounding behaviour as the original functions. I'm > wondering if it would just be too clever just to track how many bits > we've shifted right by in

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-07-05 Thread David Rowley
On Wed, 30 Jun 2021 at 05:11, David Christensen wrote: > 1) A basic refactor of the existing code to easily handle expanding the > units we use into a table-based format. This also includes changing the > return value of `pg_size_bytes()` from an int64 into a numeric, and > minor test

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-06-29 Thread David Christensen
shinya11.k...@nttdata.com writes: >>I had not really looked at the patch, but if there's a cleanup portion to the >>same >>patch as you're adding the YB too, then maybe it's worth separating those out >>into another patch so that the two can be considered independently. > > I agree with this

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-06-15 Thread David Christensen
>> I had not really looked at the patch, but if there's a cleanup portion to >> the same >> patch as you're adding the YB too, then maybe it's worth separating those out >> into another patch so that the two can be considered independently. > > I agree with this opinion. It seems to me that we

RE: [PATCH] expand the units that pg_size_pretty supports on output

2021-06-15 Thread Shinya11.Kato
>I had not really looked at the patch, but if there's a cleanup portion to the >same >patch as you're adding the YB too, then maybe it's worth separating those out >into another patch so that the two can be considered independently. I agree with this opinion. It seems to me that we should think

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-06-15 Thread David Rowley
On Wed, 16 Jun 2021 at 02:58, David Christensen wrote: > That said, it seems like having the code structured in a way that we can > expand via adding an element to a table instead of the existing way it's > written with nested if blocks is still a useful refactor, whatever we decide > the

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-06-15 Thread David Christensen
On Tue, Jun 15, 2021 at 8:26 AM David Rowley wrote: > On Tue, 15 Jun 2021 at 21:24, wrote: > > Hmmm, I didn't think YB was necessary, but what do others think? > > For me personally, without consulting Wikipedia, I know that Petabyte > comes after Terabyte and then I'm pretty sure it's Exabyte.

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-06-15 Thread David Christensen
On Tue, Jun 15, 2021 at 8:31 AM Isaac Morland wrote: > On Tue, 15 Jun 2021 at 05:24, wrote: > >> >> I don't see the need to extend the unit to YB. >> >> What use case do you have in mind? >> > >> >Practical or no, I saw no reason not to support all defined units. I >> assume we’ll >> >get to a

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-06-15 Thread Isaac Morland
On Tue, 15 Jun 2021 at 05:24, wrote: > >> I don't see the need to extend the unit to YB. > >> What use case do you have in mind? > > > >Practical or no, I saw no reason not to support all defined units. I > assume we’ll > >get to a need sooner or later. :) > > Thank you for your reply! > Hmmm, I

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-06-15 Thread David Rowley
On Tue, 15 Jun 2021 at 21:24, wrote: > Hmmm, I didn't think YB was necessary, but what do others think? For me personally, without consulting Wikipedia, I know that Petabyte comes after Terabyte and then I'm pretty sure it's Exabyte. After that, I'd need to check. Assuming I'm not the only

RE: [PATCH] expand the units that pg_size_pretty supports on output

2021-06-15 Thread Shinya11.Kato
>> I don't see the need to extend the unit to YB. >> What use case do you have in mind? > >Practical or no, I saw no reason not to support all defined units. I assume >we’ll >get to a need sooner or later. :) Thank you for your reply! Hmmm, I didn't think YB was necessary, but what do others

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-06-14 Thread David Christensen
> I don't see the need to extend the unit to YB. > What use case do you have in mind? Practical or no, I saw no reason not to support all defined units. I assume we’ll get to a need sooner or later. :) David

RE: [PATCH] expand the units that pg_size_pretty supports on output

2021-06-13 Thread Shinya11.Kato
>From: David Christensen >Sent: Friday, June 4, 2021 4:18 AM >To: PostgreSQL-development >Subject: Re: [PATCH] expand the units that pg_size_pretty supports on output > >New versions attached to address the initial CF feedback and rebase on HEAD as >of now. >

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-06-03 Thread David Christensen
New versions attached to address the initial CF feedback and rebase on HEAD as of now. 0001-Expand-the-units-that-pg_size_pretty-numeric-knows-a.patch - expands the units that pg_size_pretty() can handle up to YB. 0002-Expand-the-supported-units-in-pg_size_bytes-to-cover.patch - expands the

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-05-30 Thread Asif Rehman
The following review has been posted through the commitfest application: make installcheck-world: not tested Implements feature: not tested Spec compliant: not tested Documentation:not tested Hi David, I was reviewing this patch and the compilation failed with

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-04-28 Thread David Christensen
A second patch to teach the same units to pg_size_bytes(). Best, David On Wed, Apr 14, 2021 at 11:13 AM David Christensen < david.christen...@crunchydata.com> wrote: > Hi folks, > > Enclosed is a patch that expands the unit output for > pg_size_pretty(numeric) going up to Yottabytes; I

Re: [PATCH] expand the units that pg_size_pretty supports on output

2021-04-15 Thread Michael Paquier
On Wed, Apr 14, 2021 at 11:13:47AM -0500, David Christensen wrote: > Enclosed is a patch that expands the unit output for > pg_size_pretty(numeric) going up to Yottabytes; I reworked the existing > numeric output code to account for the larger number of units we're using > rather than just adding

[PATCH] expand the units that pg_size_pretty supports on output

2021-04-14 Thread David Christensen
Hi folks, Enclosed is a patch that expands the unit output for pg_size_pretty(numeric) going up to Yottabytes; I reworked the existing numeric output code to account for the larger number of units we're using rather than just adding nesting levels. There are also a few other places that could