On Fri, Jul 22, 2022 at 03:26:47PM -0400, Tom Lane wrote: > Justin Pryzby <pry...@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