Date: Fri, 2 Sep 2022 17:24:58 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Maciek Sakrejda <m.sakrejda@gmail.com>,
	Robert Haas <robertmhaas@gmail.com>,
	"David G. Johnston" <david.g.johnston@gmail.com>,
	Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>,
	Cary Huang <cary.huang@highgo.ca>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: warn if GUC set to an invalid shared library
Message-ID: <20220902222458.GC31833@telsasoft.com>
References: 
 <CA+TgmoYHTu=Qb+OW3v4qR4KRY=Fc0KDvA0FwXuQ8iKrcXHUpbg@mail.gmail.com>
 <20220210015854.GV31460@telsasoft.com>
 <CAOtHd0CQxJCAF77T+0buj57p5EycwzELfyAWMhJ8oZ3nzCZ6fw@mail.gmail.com>
 <20220722015412.GA7789@telsasoft.com>
 <CAOtHd0Ca3o_YX-2xC+0SOJD242QDHKCS16bG2AX9QQkKsdBF7Q@mail.gmail.com>
 <675610.1658512401@sss.pgh.pa.us>
 <20220722183557.GC7789@telsasoft.com>
 <729940.1658516423@sss.pgh.pa.us>
 <20220722191443.GE7789@telsasoft.com>
 <734432.1658518007@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <734432.1658518007@sss.pgh.pa.us>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20220902222458.GC31833%40telsasoft.com>
Precedence: bulk

On Fri, Jul 22, 2022 at 03:26:47PM -0400, Tom Lane wrote:
> Justin Pryzby <pryzby@telsasoft.com> writes:
> > On Fri, Jul 22, 2022 at 03:00:23PM -0400, Tom Lane wrote:
> >> Shouldn't you be doing this when the source is PGC_S_TEST, instead?
> 
> > That makes sense, but it doesn't work for ALTER SYSTEM, which uses PGC_S_FILE.
> 
> Hmph.  I wonder if we shouldn't change that, because it's a lie.

I think so, and I was going to raise this question some months ago when
I first picked up the patch.

The question is, which behavior do we want ?

postgres=# ALTER SYSTEM SET default_table_access_method=abc;
2022-07-22 15:24:55.445 CDT client backend[27938] psql ERROR:  invalid value for parameter "default_table_access_method": "abc"
2022-07-22 15:24:55.445 CDT client backend[27938] psql DETAIL:  Table access method "abc" does not exist.
2022-07-22 15:24:55.445 CDT client backend[27938] psql STATEMENT:  ALTER SYSTEM SET default_table_access_method=abc;

That behavior differs from ALTER SYSTEM SET shared_preload_libraries,
which supports first seting the GUC and then installing the library.  If
that wasn't supported, I think we'd just throw an error and avoid the
possibility that the server can't start.

It caused no issue when I changed:

                        /* Check that it's acceptable for the indicated parameter */
                        if (!parse_and_validate_value(record, name, value,
-                                                     PGC_S_FILE, ERROR,
+                                                     PGC_S_TEST, ERROR,
                                                      &newval, &newextra))

I'm not sure where to go from here.

-- 
Justin