Re: custom function for converting human readable sizes to bytes

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Vitaly Burovoy <vitaly(dot)burovoy(at)gmail(dot)com>
Cc: "Shulgin, Oleksandr" <oleksandr(dot)shulgin(at)zalando(dot)de>, Robert Haas <robertmhaas(at)gmail(dot)com>, Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>, Corey Huinker <corey(dot)huinker(at)gmail(dot)com>, Guillaume Lelarge <guillaume(at)lelarge(dot)info>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: custom function for converting human readable sizes to bytes
Date: 2016-01-26 13:13:09
Message-ID: CAFj8pRBSKoh_Zd+gqzUate9760XWX_4agOin5Yh7Z-WBLpMZVQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi

2016-01-26 13:48 GMT+01:00 Vitaly Burovoy <vitaly(dot)burovoy(at)gmail(dot)com>:

> Hello, Pavel!
>
> That letter was not a complain against you. I'm sorry if it seems like
> that for you.
>

ok.

It was an intermediate review with several points to be clear for _me_
> from experienced hackers, mostly about a code design.
>
> 26.01.2016 07:05, Pavel Stehule пишет:
> >> pg_proc.h has changed, so the CATALOG_VERSION_NO must be also changed.
> > this is not a part of patch - only a commiter knows CATALOG_VERSION_NO
> >
> CATALOG_VERSION_NO is mentioned for a committer, it is not your fault.
>
> >> III. There is no support of 'bytes' unit, it seems such behavior got
> >> majority approval[2].
> >
> > No, because I have to use the supported units by configuration. The
> configuration supports only three chars long units. Support for "bytes" was
> removed, when I removed proprietary unit table.
> >
> Point "III" is the only for the question "d". Also to collect any
> possible features from the thread in one place.
>
> >> V. The documentation lacks a note that the base of the "pg_size_bytes"
> >> is 1024 whereas the base of the "pg_size_pretty" is 1000.
> >
> > It isn't true, base for both are 1024. It was designed when special
> table was used for pg_size_bytes. But when we share same control table,
> then is wrong to use different table. The result can be optically
> different, but semantically same.
> >
> Oops, I was wrong about a base of pg_size_pretty. It was a morning
> after a hard night...

> > negative values is fully supported.
> You have mentioned it at least three times before. It is not my new
> requirement or a point to your fault, it is an argument for
> symmetry/asymmetry of the function.
>
> > support of "bytes" depends on support "bytes" unit by GUC. When "bytes"
> unit will be supported, then it can be used in pg_size_bytes immediately.
> By the way there can be a comparison for a special size unit before
> calling parse_memory_unit.
>

there was more requirements based on original proposal (based in separated
control tables). When I leaved this design, I dropped more requirements and
now is little bit hard to distinguish what is related and with. Some
features can be added later without any possible compatibility issues in
next commit fest or later, and I am searching some good borders for this
patch. Now, this border is a shared memory unit control table. And I am
trying to hold this border.

I am grateful for review - now this patch is better, and I hope near final
stage.

Regards

Pavel

>
> > Regards
> > Pavel
> >
> >> [2]
> http://www.postgresql.org/message-id/CACACo5QW7fFsFfhKsTjtYcP4QF3Oh9zA14SC6Z3DXx2yssJjSw@mail.gmail.com
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2016-01-26 13:21:49 Re: pgbench - allow backslash-continuations in custom scripts
Previous Message Fabien COELHO 2016-01-26 12:53:33 Re: pgbench - allow backslash-continuations in custom scripts