On Mon, Jul 23, 2018 at 11:29:33AM -0400, Robert Haas wrote: > ExecuteTruncate needs to be refactored to use RangeVarGetRelidExtended > with a non-NULL callback rather than heap_openrv, and > expand_vacuum_rel needs to use RangeVarGetRelidExtended with a > callback instead of RangeVarGetRelid. See > cbe24a6dd8fb224b9585f25b882d5ffdb55a0ba5 as an example of what to do. > I fixed a large number of cases of this problem back around that time, > but then ran out of steam and had to move onto other things before I > got them all. Patches welcome.
Thanks for pointing those out, I looked at both code paths recently for some other work... The amount of work does not consist just in using for example RangeVarCallbackOwnsRelation for VACUUM and TRUNCATE. There are a couple of reasons behind that: - While it would make sense, at least to me, to make VACUUM fall into if allow_system_table_mods is allowed, that's not the case of ANALYZE as I think that we should be able to call ANALYZE on a system catalog as well. So we would basically a new flavor of RangeVarCallbackOwnsRelation for VACUUM which makes this difference between vacuum and analyze with an argument in the callback, the options of VacuumStmt would be nice. This would not be used by autovacuum anyway, but adding an assertion and mentioning that in the comments would not hurt. There is an argument for just restricting VACUUM FULL as well and not plain VACUUM, as that's the one hurting here. - TRUNCATE is closer to a solution, as it has its own flavor of relation checks with truncate_check_rel. So the callback would replace truncate_check_rel but CheckTableNotInUse should be moved out of it. TRUNCATE already uses allow_system_table_mods for its checks. Thoughts? -- Michael
signature.asc
Description: PGP signature