Re: pg_get_functiondef forgets about most GUC_LIST_INPUT GUCs

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
Cc: michael(at)paquier(dot)xyz, pavel(dot)stehule(at)gmail(dot)com, a(dot)zakirov(at)postgrespro(dot)ru, pgsql-hackers(at)postgresql(dot)org
Subject: Re: pg_get_functiondef forgets about most GUC_LIST_INPUT GUCs
Date: 2018-03-20 17:27:35
Message-ID: 32403.1521566855@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

So here's what I think we should do about this:

1. Only GUC_LIST_QUOTE variables need to be special-cased. It works
fine to treat plain LIST variables (e.g. datestyle) with the regular
code path. This is good because there are so few of them.

2. We should make an effort to minimize the number of places that know
explicitly which variables are GUC_LIST_QUOTE. In the backend, the
right thing to do is ask guc.c. In pg_dump, there's no convenient
way to ask the backend (and certainly nothing that would work against
old servers), but we should at least make sure there's only one place
to fix not two.

3. We need to prevent extensions from trying to create GUC_LIST_QUOTE
variables, because those are not going to work correctly if they're
set before the extension is loaded, nor is there any hope of pg_dump
dumping them correctly.

The attached 0001 patch does the first two things, and could be
back-patched. The 0002 patch, which I'd only propose for HEAD,
is one way we could address point 3. A different way would be
to throw a WARNING rather than ERROR and then just mask off the
problematic bit. Or we could decide that either of these cures
is worse than the disease, but I don't really agree with that.

Comments?

regards, tom lane

Attachment Content-Type Size
0001-handle-core-quoted-variables-correctly.patch text/x-diff 10.8 KB
0002-reject-extension-quoted-variables.patch text/x-diff 971 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2018-03-20 17:28:27 Re: [PATCH] session_replication_role = replica with TRUNCATE
Previous Message Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?= 2018-03-20 17:23:26 Re: configure's checks for --enable-tap-tests are insufficient