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.
