Horton

Re: Emit a warning if the extension's GUC is set incorrectly

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: peter.eisentraut@enterprisedb.com
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, Shinya11.Kato@oss.nttdata.com, masao.fujii@oss.nttdata.com, bharath.rupireddyforpostgres@gmail.com, daniel@yesql.se, pgsql-hackers@postgresql.org
Date: 2021-12-21T18:33:49Z
Lists: pgsql-hackers

Attachments

I wrote:
> 0003 looks to me like it was superseded by 75d22069e.  I do not
> particularly like that patch though; it seems both inefficient
> and ill-designed.  0003 is adding a check in an equally bizarre
> place.  Why isn't add_placeholder_variable the place to deal
> with this?  Also, once we know that a prefix is reserved,
> why don't we throw an error not just a warning for attempts to
> set some unknown variable under that prefix?  We don't allow
> you to set unknown non-prefixed variables.

Concretely, I think we should do the attached, which removes much of
75d22069e and does the check at the point of placeholder creation.
(After looking closer, add_placeholder_variable's caller is the
function that is responsible for rejecting bad names, not
add_placeholder_variable itself.)

This also fixes an indisputable bug in 75d22069e, namely that it
did prefix comparisons incorrectly and would throw warnings for
cases it shouldn't.  Reserving "plpgsql" shouldn't have the effect
of reserving "pl" as well.

I'm tempted to propose that we also rename EmitWarningsOnPlaceholders
to something like MarkGUCPrefixReserved, to more clearly reflect
what it does now.  (We could provide the old name as a macro alias
to avoid breaking extensions needlessly.)

			regards, tom lane

Commits

  1. Disallow setting bogus GUCs within an extension's reserved namespace.

  2. Revert changes about warnings/errors for placeholders.

  3. Rename EmitWarningsOnPlaceholders() to MarkGUCPrefixReserved().

  4. Rethink handling of settings with a prefix reserved by an extension.

  5. Add missing EmitWarningsOnPlaceholders() calls.

  6. Add EmitWarningsOnPlaceholders calls to contrib modules that are likely to