> On Thu, Jun 6, 2019 at 9:06 AM Michael Paquier <mich...@paquier.xyz> wrote: > > I was wondering if we actually need at all a catalog lookup at this > stage, simplifying get_table_am_oid() on the way so as we always > throw an error (its missing_ok is here to allow a proper error in the > GUC context).
Just for me to understand, do you suggest to not check default_table_access_method existence in check_default_table_access_method? If yes, then > The table AM lookup happens only when creating a table, so we could just get > a failure when attempting to create a table with this incorrect value. is correct, but doesn't it leave the room for some problems in the future with a wrong assumptions about correctness of default_table_access_method? > Actually, when updating a value and reloading and/or restarting the > server, it is possible to easily get in a state where we have an > invalid table AM parameter stored in the GUC, which is what the > callback is here to avoid. If you attempt to update the parameter > with ALTER SYSTEM, then the command complains. So it seems to me that > the user experience is inconsistent. Right, as far as I see the there is the same for e.g. default_tablespace due to IsTransactionState condition. In fact looks like one can see the very same issue with this option too, so probably it also needs to have MyDatabaseId check.