Thanks Michael for working on this. On 2017/09/27 11:28, Michael Paquier wrote: > Hi all, > > I have been looking more closely at the problem in $subject, that I > have mentioned a couple of times, like here: > https://www.postgresql.org/message-id/cab7npqqa37oune_rjzpmwc4exqalx9f27-ma_-rsfl_3mj+...@mail.gmail.com > > As of HEAD, the RangeVar defined in calls of vacuum() is used for > error reporting, only in two cases: for autoanalyze and autovacuum > when reporting to users that a lock cannot be taken on a relation. The > thing is that autovacuum has the good idea to call vacuum() on only > one relation at the same time, with always a relation OID and a > RangeVar defined, so the code currently on HEAD is basically fine with > its error reporting, because get_rel_oids() will always work on the > relation OID with its RangeVar, and because code paths with manual > VACUUMs don't use the RangeVar for any reports.
Yeah, autovacuum_do_vac_analyze will never pass partitioned table OIDs to vacuum, for example, because they're not RELKIND_RELATION relations. > While v10 is safe, I think that this is wrong in the long-term and > that may be a cause of bugs if people begin to generate reports for > manual VACUUMs, which is plausible in my opinion. The patch attached, > is what one solution would look like if we would like to make things > more robust as long as manual VACUUM commands can only specify one > relation at the same time. I have found that tracking the parent OID > by tweaking a bit get_rel_oids() was the most elegant solution. The patch basically looks good to me, FWIW. > Please > note that this range of problems is something that I think is better > solved with the infrastructure that support for VACUUM with multiple > relations includes (last version here > https://www.postgresql.org/message-id/766556dd-aa3c-42f7-acf4-5dc97f41f...@amazon.com). > So I don't think that the patch attached should be integrated, still I > am attaching it to satisfy the curiosity of anybody looking at this > message. Yeah, after looking at the code a bit, it seems that the problem won't really occur for the case we're trying to apply this patch for. > In conclusion, I think that the open item of $subject should be > removed from the list, and we should try to get the multi-VACUUM patch > in to cover any future problems. I'll do so if there are no > objections. +1 to this, taking the previous +1 back [1]. :) > One comment on top of vacuum() is wrong by the way: in the case of a > manual VACUUM or ANALYZE, a NULL RangeVar is used if no relation is > specified in the command. Yep, but it doesn't ever get accessed per our observations. Thanks, Amit [1] https://www.postgresql.org/message-id/8d810dd9-5f64-a5f3-c016-a81f05528fa8%40lab.ntt.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
