Alvaro Herrera <alvherre@xxxxxxxxxxxxxx> writes: > Here's a quick patchset that makes pg_dump do LOCK TABLE on all > relations it dumps; patch 0001 allows server-side LOCK TABLE to run on > any relkind, and then 0002 makes pg_dump test for that capability at > connection start, and if it exists, then it's used for all relations to > dump. I think the "test for that capability" bit needs more subtlety. In the first place, demanding exclusive lock on a system view has a pretty high probability of failing due to someone else accessing the view at that moment, and/or causing failures in other transations. Since you only need to find out if the command can work at all, you could make this a lot less fragile and unfriendly by just asking for ACCESS SHARE lock. Even with the lower lock level, you might get back a transient can't-lock-it failure rather than the condition you are looking for. So I think there needs to be a check on the errcode here. Should there be a timeout on the LOCK? Or even better, use NOWAIT where available (which actually appears to be all versions of interest). I'd also advise skipping the whole thing if server version >= 14. I see you have a lower-bound check which is good, but not nearly as useful over time as the other will be. (Also, personally I'd put the version check in IsLockTableGeneric, not its caller.) Why is it OK to drop the check for RELKIND_PARTITIONED_TABLE when !hasGenericLockTable? Surely that does the wrong thing on released minor versions? regards, tom lane
