Thread: Re: [COMMITTERS] pgsql: Allow units to be specified in relation option setting value.

Michael Paquier <michael.paquier@gmail.com> writes:
> The patch attached fixes pg_upgrade by putting quotes when generating
> reloptions and it passes check-world. I imagine that having quotes by
> default in the value of reloptions in pg_class is the price to pay for
> supporting units. If this is not considered as a correct approach,
> then reverting the patch would be wiser I guess.

Ugh.  I'm not sure what the best solution is, but I don't think I like
that one.

Given that this patch broke both pg_dump and pg_upgrade, I think it should
be reverted for the time being, rather than applying a hasty hack.
Obviously more thought and testing is needed.

            regards, tom lane

On Thu, Aug 28, 2014 at 11:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Michael Paquier <michael.paquier@gmail.com> writes:
>> The patch attached fixes pg_upgrade by putting quotes when generating
>> reloptions and it passes check-world. I imagine that having quotes by
>> default in the value of reloptions in pg_class is the price to pay for
>> supporting units. If this is not considered as a correct approach,
>> then reverting the patch would be wiser I guess.
>
> Ugh.  I'm not sure what the best solution is, but I don't think I like
> that one.

Another approach is to change pg_dump so that it encloses the relopt
values with single quotes. This is the same approach as what
pg_dumpall does for database or role-specific settings. Obvious
drawback of this approach is that it prevents pg_dump with 9.4 or
before from outputting the restorable dump. Maybe we can live with
this because there is no guarantee that older version of pg_dump can
work properly with newer major version of server. But maybe
someone cannot live with that. Not sure.

Further other approach is to change the reloptions code so that it
always stores the plain value without the units (i.e., 1000 is stored
if 1s is specified in autovacuum_vacuum_cost_delay)in pg_class.
This approach doesn't prevent older version of pg_dump from taking
the dump from v9.5 server. One drawback of this approach is that
reloption values are always stored without the units, which might
make it a bit hard for a user to see the reloption values from pg_class.

Regards,

--
Fujii Masao

On Thu, Aug 28, 2014 at 12:22 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
> On Thu, Aug 28, 2014 at 11:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Michael Paquier <michael.paquier@gmail.com> writes:
>>> The patch attached fixes pg_upgrade by putting quotes when generating
>>> reloptions and it passes check-world. I imagine that having quotes by
>>> default in the value of reloptions in pg_class is the price to pay for
>>> supporting units. If this is not considered as a correct approach,
>>> then reverting the patch would be wiser I guess.
>>
>> Ugh.  I'm not sure what the best solution is, but I don't think I like
>> that one.
>
> Another approach is to change pg_dump so that it encloses the relopt
> values with single quotes. This is the same approach as what
> pg_dumpall does for database or role-specific settings. Obvious
> drawback of this approach is that it prevents pg_dump with 9.4 or
> before from outputting the restorable dump. Maybe we can live with
> this because there is no guarantee that older version of pg_dump can
> work properly with newer major version of server. But maybe
> someone cannot live with that. Not sure.

To me, this doesn't seem nearly important enough to justify breaking
pg_dump compatibility.  AAUI, this is just a cosmetic improvement, so
we shouldn't break functional things for that.

> Further other approach is to change the reloptions code so that it
> always stores the plain value without the units (i.e., 1000 is stored
> if 1s is specified in autovacuum_vacuum_cost_delay)in pg_class.
> This approach doesn't prevent older version of pg_dump from taking
> the dump from v9.5 server. One drawback of this approach is that
> reloption values are always stored without the units, which might
> make it a bit hard for a user to see the reloption values from pg_class.

This seems like the way to go.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company