Re: [HACKERS] A weird bit in pg_upgrade/exec.c
Alvaro Herrerawrites: > I think odd coding this was introduced recently because of the > pg_resetxlog -> pg_resetwal renaming. Dunno about that, but certainly somebody fat-fingered a refactoring there. The 9.6 code looks quite different but doesn't seem to be doing anything silly. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] A weird bit in pg_upgrade/exec.c
a.akent...@postgrespro.ru writes: > I've came across a weird bit in pg_upgrade/exec.c > We have a function check_bin_dir() which goes like this (old_cluster and > new_cluster are global variables): > void check_bin_dir(ClusterInfo *cluster) > { > ... > get_bin_version(_cluster); > get_bin_version(_cluster); > ... > } > This function has two calls: > check_bin_dir(_cluster); > check_bin_dir(_cluster); > I'd like to substitute these last two lines with this: > get_bin_version(cluster); Yeah, the way it is now seems outright broken. It will try to do get_bin_version on the new cluster before having done validate_exec there, violating its own comment. I think we should change this as a bug fix, independently of whatever else you had in mind to do here. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] A weird bit in pg_upgrade/exec.c
a.akent...@postgrespro.ru wrote: > This function has two calls: > check_bin_dir(_cluster); > check_bin_dir(_cluster); > > I'd like to substitute these last two lines with this: > get_bin_version(cluster); Odd indeed. One would think that if a cluster variable is passed as parameter, the global vars should not be used. +1 for fixing it, and your proposal sounds as good as any. > Doing it would simplify the patch I'm writing, but I'm worried I might break > something that's been there for a long time and has been working fine. I think odd coding this was introduced recently because of the pg_resetxlog -> pg_resetwal renaming. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers