Thread: Re: v12 and pg_restore -f-
[ redirecting to -hackers ] Justin Pryzby <pryzby@telsasoft.com> writes: > I saw this and updated our scripts with pg_restore -f- > https://www.postgresql.org/docs/12/release-12.html > |In pg_restore, require specification of -f - to send the dump contents to standard output (Euler Taveira) > |Previously, this happened by default if no destination was specified, but that was deemed to be unfriendly. > What I didn't realize at first is that -f- has no special meaning in v11 - it > just writes a file called ./- Ugh. I didn't realize that either, or I would have made a stink about this patch. Reducing the risk of getting a dump spewed at you is completely not worth the cost of making it impossible to have cross-version-compatible scripting of pg_restore. Perhaps we could change the back branches so that they interpret "-f -" as "write to stdout", but without enforcing that you use that syntax. Nobody is going to wish that to mean "write to a file named '-'", so I don't think this would be an unacceptable change. Alternatively, we could revert the v12 behavior change. On the whole that might be the wiser course. I do not think the costs and benefits of this change were all that carefully thought through. regards, tom lane
>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes: Tom> Perhaps we could change the back branches so that they interpret Tom> "-f -" as "write to stdout", but without enforcing that you use Tom> that syntax. We should definitely do that. Tom> Alternatively, we could revert the v12 behavior change. On the Tom> whole that might be the wiser course. I do not think the costs and Tom> benefits of this change were all that carefully thought through. Failing to specify -d is a _really fricking common_ mistake for inexperienced users, who may not realize that the fact that they're seeing a ton of SQL on their terminal is not the normal result. Seriously, this comes up on a regular basis on IRC (which is why I suggested initially that we should do something about it). -- Andrew (irc:RhodiumToad)
Andrew Gierth <andrew@tao11.riddles.org.uk> writes: > "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes: > Tom> Perhaps we could change the back branches so that they interpret > Tom> "-f -" as "write to stdout", but without enforcing that you use > Tom> that syntax. > We should definitely do that. > Tom> Alternatively, we could revert the v12 behavior change. On the > Tom> whole that might be the wiser course. I do not think the costs and > Tom> benefits of this change were all that carefully thought through. > Failing to specify -d is a _really fricking common_ mistake for > inexperienced users, who may not realize that the fact that they're > seeing a ton of SQL on their terminal is not the normal result. > Seriously, this comes up on a regular basis on IRC (which is why I > suggested initially that we should do something about it). No doubt, but that seems like a really poor excuse for breaking maintenance scripts in a way that basically can't be fixed. Even with the change suggested above, scripts couldn't rely on "-f -" working anytime soon, because you couldn't be sure whether a back-rev pg_restore had the update or not. The idea I'm leaning to after more thought is that we should change *all* the branches to accept "-f -", but not throw an error if you don't use it. Several years from now, we could put the error back in; but not until there's a plausible argument that nobody is running old versions of pg_restore anymore. regards, tom lane
BTW, the prior discussion is here: https://www.postgresql.org/message-id/24868.1550106683%40sss.pgh.pa.us -- Andrew (irc:RhodiumToad)
Greetings, * Tom Lane (tgl@sss.pgh.pa.us) wrote: > Andrew Gierth <andrew@tao11.riddles.org.uk> writes: > > "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes: > > Tom> Perhaps we could change the back branches so that they interpret > > Tom> "-f -" as "write to stdout", but without enforcing that you use > > Tom> that syntax. > > > We should definitely do that. I agree that this would be a reasonable course of action. Really, it should have always meant that... > > Tom> Alternatively, we could revert the v12 behavior change. On the > > Tom> whole that might be the wiser course. I do not think the costs and > > Tom> benefits of this change were all that carefully thought through. > > > Failing to specify -d is a _really fricking common_ mistake for > > inexperienced users, who may not realize that the fact that they're > > seeing a ton of SQL on their terminal is not the normal result. > > Seriously, this comes up on a regular basis on IRC (which is why I > > suggested initially that we should do something about it). > > No doubt, but that seems like a really poor excuse for breaking > maintenance scripts in a way that basically can't be fixed. Even > with the change suggested above, scripts couldn't rely on "-f -" > working anytime soon, because you couldn't be sure whether a > back-rev pg_restore had the update or not. Maintenance scripts break across major versions. We completely demolished everything around how recovery works, and some idea that you could craft up something easy that would work in a backwards-compatible way is outright ridiculous, so I don't see why we're so concerned about a change to how pg_restore works here. > The idea I'm leaning to after more thought is that we should change > *all* the branches to accept "-f -", but not throw an error if you > don't use it. Several years from now, we could put the error back in; > but not until there's a plausible argument that nobody is running > old versions of pg_restore anymore. No, I don't agree with this, at all. Thanks, Stephen
Attachment
Em ter, 8 de out de 2019 às 15:08, Stephen Frost <sfrost@snowman.net> escreveu: > > Greetings, > > * Tom Lane (tgl@sss.pgh.pa.us) wrote: > > Andrew Gierth <andrew@tao11.riddles.org.uk> writes: > > > "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes: > > > Tom> Perhaps we could change the back branches so that they interpret > > > Tom> "-f -" as "write to stdout", but without enforcing that you use > > > Tom> that syntax. > > > > > We should definitely do that. > > I agree that this would be a reasonable course of action. Really, it > should have always meant that... > Indeed, it was a broken behavior and the idea was to fix it. However, changing pg_restore in back-branches is worse than do nothing because it could break existent scripts. > > > Tom> Alternatively, we could revert the v12 behavior change. On the > > > Tom> whole that might be the wiser course. I do not think the costs and > > > Tom> benefits of this change were all that carefully thought through. > > > > > Failing to specify -d is a _really fricking common_ mistake for > > > inexperienced users, who may not realize that the fact that they're > > > seeing a ton of SQL on their terminal is not the normal result. > > > Seriously, this comes up on a regular basis on IRC (which is why I > > > suggested initially that we should do something about it). > > > > No doubt, but that seems like a really poor excuse for breaking > > maintenance scripts in a way that basically can't be fixed. Even > > with the change suggested above, scripts couldn't rely on "-f -" > > working anytime soon, because you couldn't be sure whether a > > back-rev pg_restore had the update or not. > > Maintenance scripts break across major versions. We completely > demolished everything around how recovery works, and some idea that you > could craft up something easy that would work in a backwards-compatible > way is outright ridiculous, so I don't see why we're so concerned about > a change to how pg_restore works here. > Yeah, if you check pg_restore version, you could use new syntax for 12+. We break scripts every release (mainly with catalog changes) and I don't know why this change is different than the other ones. The pg_restore changes is more user-friendly and less error-prone. Regards, -- Euler Taveira Timbira - http://www.timbira.com.br/ PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
Greetings, * Euler Taveira (euler@timbira.com.br) wrote: > Em ter, 8 de out de 2019 às 15:08, Stephen Frost <sfrost@snowman.net> escreveu: > > * Tom Lane (tgl@sss.pgh.pa.us) wrote: > > > Andrew Gierth <andrew@tao11.riddles.org.uk> writes: > > > > "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes: > > > > Tom> Perhaps we could change the back branches so that they interpret > > > > Tom> "-f -" as "write to stdout", but without enforcing that you use > > > > Tom> that syntax. > > > > > > > We should definitely do that. > > > > I agree that this would be a reasonable course of action. Really, it > > should have always meant that... > > > Indeed, it was a broken behavior and the idea was to fix it. However, > changing pg_restore in back-branches is worse than do nothing because > it could break existent scripts. I can certainly respect that argument, in general, but in this specific case, I've got a really hard time believeing that people wrote scripts which use '-f -' with the expectation that a './-' file was to be created. Thanks, Stephen
Attachment
Hi, On Sun, Oct 6, 2019 at 7:09 PM, Justin Pryzby wrote: > I saw this and updated our scripts with pg_restore -f- > https://www.postgresql.org/docs/12/release-12.html > |In pg_restore, require specification of -f - to send the dump contents to standard output (Euler Taveira) > |Previously, this happened by default if no destination was specified, but that was deemed to be unfriendly. > > What I didn't realize at first is that -f- has no special meaning in v11 - it > just writes a file called ./- And it's considered untennable to change behavior of v11. Ahh... I totally missed thinking about the behavior of "-f -" in v11 when I reviewed this patch. On Wed, Oct 9, 2019 at 0:45 PM, Stephen Frost wrote: > * Euler Taveira (euler@timbira.com.br) wrote: > > Em ter, 8 de out de 2019 às 15:08, Stephen Frost <sfrost@snowman.net> escreveu: > > > * Tom Lane (tgl@sss.pgh.pa.us) wrote: > > > > Andrew Gierth <andrew@tao11.riddles.org.uk> writes: > > > > > "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes: > > > > > Tom> Perhaps we could change the back branches so that they > > > > > interpret Tom> "-f -" as "write to stdout", but without > > > > > enforcing that you use Tom> that syntax. > > > > > > > > > We should definitely do that. > > > > > > I agree that this would be a reasonable course of action. Really, > > > it should have always meant that... > > > > > Indeed, it was a broken behavior and the idea was to fix it. However, > > changing pg_restore in back-branches is worse than do nothing because > > it could break existent scripts. > > I can certainly respect that argument, in general, but in this specific case, I've got a really hard time believeing > that people wrote scripts which use '-f -' with the expectation that a './-' file was to be created. +1. If we only think of the problem that we can't use "-f -" with the meaning "dump to the stdout" in v11 and before ones, itseems a bug and we should fix it. Of course, if we fix it, some people would go into the trouble, but such people are who wrote scripts which use '-f -' withthe expectation that a './-' file. I don't think there are such people a lot. -- Yoshikazu Imai
Greetings, * imai.yoshikazu@fujitsu.com (imai.yoshikazu@fujitsu.com) wrote: > On Sun, Oct 6, 2019 at 7:09 PM, Justin Pryzby wrote: > > I saw this and updated our scripts with pg_restore -f- > > https://www.postgresql.org/docs/12/release-12.html > > |In pg_restore, require specification of -f - to send the dump contents to standard output (Euler Taveira) > > |Previously, this happened by default if no destination was specified, but that was deemed to be unfriendly. > > > > What I didn't realize at first is that -f- has no special meaning in v11 - it > > just writes a file called ./- And it's considered untennable to change > > behavior of v11. > > Ahh... I totally missed thinking about the behavior of "-f -" in v11 when I reviewed this patch. Clearly you weren't the only one. > On Wed, Oct 9, 2019 at 0:45 PM, Stephen Frost wrote: > > * Euler Taveira (euler@timbira.com.br) wrote: > > > Em ter, 8 de out de 2019 às 15:08, Stephen Frost <sfrost@snowman.net> escreveu: > > > > * Tom Lane (tgl@sss.pgh.pa.us) wrote: > > > > > Andrew Gierth <andrew@tao11.riddles.org.uk> writes: > > > > > > "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes: > > > > > > Tom> Perhaps we could change the back branches so that they > > > > > > interpret Tom> "-f -" as "write to stdout", but without > > > > > > enforcing that you use Tom> that syntax. > > > > > > > > > > > We should definitely do that. > > > > > > > > I agree that this would be a reasonable course of action. Really, > > > > it should have always meant that... > > > > > > > Indeed, it was a broken behavior and the idea was to fix it. However, > > > changing pg_restore in back-branches is worse than do nothing because > > > it could break existent scripts. > > > > I can certainly respect that argument, in general, but in this specific case, I've got a really hard time believeing > > that people wrote scripts which use '-f -' with the expectation that a './-' file was to be created. > > +1. > > If we only think of the problem that we can't use "-f -" with the meaning "dump to the stdout" in v11 and before ones,it seems a bug and we should fix it. > Of course, if we fix it, some people would go into the trouble, but such people are who wrote scripts which use '-f -'with the expectation that a './-' file. > I don't think there are such people a lot. This topic, unfortunately, seems a bit stuck right now. Maybe there's a way we can pry it loose and get to a resolution. On the one hand, we have Yoshikazu Imai, Andrew, and I pushing to change back-branches, Eurler argueing that we shouldn't do anything, and Tom argueing to make all versions accept '-f -'. First, I'd like to clarify what I believe Tom's suggestion is, and then talk through that, as his vote sways this topic pretty heavily. Tom, I take it your suggestion is to have '-f -' be accepted to mean 'goes to stdout' in all branches? That goes against the argument that we don't want to break existing scripts, as it's possible that there are existing scripts that depend on '-f -' actually going to a './-' file. Is your argument here that, with the above, existing scripts could be updated to use '-f -' explicitly and work with multiple versions, and that scripts which aren't changed would work as-is? If so, then I don't agree with it- if we really don't want to break existing scripts when moving from pre-v12 to v12, then this patch never should have been accepted at all as that's the only way to avoid breaking anything, but then, we shouldn't be making a lot of other changes between major versions either because there's often a good chance that we'll break things. Instead, the patch was accepted, as a good and forward-moving change, with the understanding that it would require some users to update their scripts when they move to v12, so, for my part at least, that question was answered when we committed the change and released with it. Now, if we wish to adjust back-branches to make it easier for users to have scripts that work with both versions, that seems like a worthwhile change and is very unlikely to to cause breakage- and it's certainly more likely to have users actually change their scripts to use '-f -' explicitly when they start working with v12, instead of depending on stdout being the default, which is ultimately the goal and why the change was made in the first place. If the concern is that we can expect folks to install v12 and then refuse or be unable to upgrade back-branches, then I just don't have any sympathy for that either- minor updates are extremely important, and new major versions are certainly no cake walk to get installed, so that argument just doesn't hold water with me- if they can upgrade to v12, then they can update to the latest minor versions, if they actually need to work with both concurrently (which strikes me as already at least relatively uncommon...). If you meant for all branches to accept '-f -' and have it go to a './-' file then that's just a revert of this entire change, which I can't agree with either- really, folks who are depending on that are depending on buggy behavior in the first place. Thanks, Stephen
Attachment
On Sun, Oct 06, 2019 at 04:43:13PM -0400, Tom Lane wrote: > Nobody is going to wish that to mean "write to a file named '-'", so Probably right, but it occurs to me that someone could make a named pipe called that, possibly to make "dump to stdout" work with scripts that don't support dumping to stdout, but with what's arguably a self-documenting syntax. On Wed, Oct 09, 2019 at 09:07:40AM -0300, Euler Taveira wrote: > > Maintenance scripts break across major versions. ... > Yeah, if you check pg_restore version, you could use new syntax for > 12+. We break scripts every release (mainly with catalog changes) and > I don't know why this change is different than the other ones. The > pg_restore changes is more user-friendly and less error-prone. The issue isn't that scripts broke, but that after I fixed scripts to work with v12, it's impossible (within pg_restore and without relying on shell or linux conventions) to write something that works on both v11 and v12, without conditionalizing on pg_restore --version. On Wed, Oct 16, 2019 at 01:21:48PM -0400, Stephen Frost wrote: > [...] if they actually need to work with both concurrently (which strikes me > as already at least relatively uncommon...). I doubt it's uncommon. In our case, we have customers running centos6 and 7. There's no postgis RPMs provided for centos6 which allow an upgrade path to v12, so we'll end up supporting at least (centos6, pg11) and (centos7,pg12) for months, at least. I have a half dozen maintenance scripts to do things like reindex, vacuum, cluster, alter tblspace. In the immediate case, our backup script runs pg_restore to check if an existing pg_dump backup of an old table is empty when the table is not itself empty - which has happened before due to logic errors and mishandled DST... (We're taking advantage of timeseries partitioning so daily backups exclude tables older than a certain thershold, which are assumed to be unchanged, or at least not have data updated). I'd *like* to be able to deploy our most recent maint scripts during the interval of time our customers are running different major versions. The alternative being to try to remember to avoid deploying updated v12 scripts at customers still running v11. I went to the effort to make our vacuum/analyze script support both versions following the OID change. I worked around the pg_restore change using /dev/stdout ; possibly the documentation should mention that workaround for portability to earlier versions: that would work for maybe 85% of cases. If need be, one could check pg_restore --version. But it's nicer not to need to. Tom's proposed in February to backpatch the -f- behavior, so ISTM that we're right now exactly where we (or at least he) planned to be, except that the backpatch ideally should've been included in the minor releases in August, before v12 was released. https://www.postgresql.org/message-id/24868.1550106683%40sss.pgh.pa.us Justin
Greetings, * Justin Pryzby (pryzby@telsasoft.com) wrote: > On Sun, Oct 06, 2019 at 04:43:13PM -0400, Tom Lane wrote: > > Nobody is going to wish that to mean "write to a file named '-'", so > > Probably right, but it occurs to me that someone could make a named pipe called > that, possibly to make "dump to stdout" work with scripts that don't support > dumping to stdout, but with what's arguably a self-documenting syntax. I'm not super keen to stress a great deal over "someone could" cases. Yes, we can come up with lots of "what ifs" here to justify why someone might think to do this but it still seems extremely rare to me. It'd be nice to get some actual numbers somehow but I haven't got any great ideas about how to do that. Actual research into this would probably be to go digging through source code that's available to try and figure out if such a case exists anywhere public. > On Wed, Oct 09, 2019 at 09:07:40AM -0300, Euler Taveira wrote: > > > Maintenance scripts break across major versions. > ... > > Yeah, if you check pg_restore version, you could use new syntax for > > 12+. We break scripts every release (mainly with catalog changes) and > > I don't know why this change is different than the other ones. The > > pg_restore changes is more user-friendly and less error-prone. > > The issue isn't that scripts broke, but that after I fixed scripts to work with > v12, it's impossible (within pg_restore and without relying on shell or linux > conventions) to write something that works on both v11 and v12, without > conditionalizing on pg_restore --version. Right- you were happy (more-or-less) to update the scripts to deal with the v12 changes, but didn't like that those changes then broke when run against v11, something that would be fixed by correcting those earlier versions. > On Wed, Oct 16, 2019 at 01:21:48PM -0400, Stephen Frost wrote: > > [...] if they actually need to work with both concurrently (which strikes me > > as already at least relatively uncommon...). > > I doubt it's uncommon. In our case, we have customers running centos6 and 7. > There's no postgis RPMs provided for centos6 which allow an upgrade path to > v12, so we'll end up supporting at least (centos6, pg11) and (centos7,pg12) for > months, at least. I suppose the issue here is that you don't want to have different versions of some scripts for centos6/pg11 vs. centos7/pg12? I'm a bit surprised that you don't have to for reasons unrelated to pg_restore. > I have a half dozen maintenance scripts to do things like reindex, vacuum, > cluster, alter tblspace. In the immediate case, our backup script runs > pg_restore to check if an existing pg_dump backup of an old table is empty when > the table is not itself empty - which has happened before due to logic errors > and mishandled DST... (We're taking advantage of timeseries partitioning so > daily backups exclude tables older than a certain thershold, which are assumed > to be unchanged, or at least not have data updated). > > I'd *like* to be able to deploy our most recent maint scripts during the > interval of time our customers are running different major versions. The > alternative being to try to remember to avoid deploying updated v12 scripts at > customers still running v11. I went to the effort to make our vacuum/analyze > script support both versions following the OID change. And I suppose you don't want to install v12 client tools for the v11 systems..? I get that there's an argument for that, but it does also seem like it'd be an alternative solution. > I worked around the pg_restore change using /dev/stdout ; possibly the > documentation should mention that workaround for portability to earlier > versions: that would work for maybe 85% of cases. If need be, one could check > pg_restore --version. But it's nicer not to need to. > > Tom's proposed in February to backpatch the -f- behavior, so ISTM that we're > right now exactly where we (or at least he) planned to be, except that the > backpatch ideally should've been included in the minor releases in August, > before v12 was released. > > https://www.postgresql.org/message-id/24868.1550106683%40sss.pgh.pa.us That continues to strike me as a good way forward, and I'm guessing you agree on that? If so, sorry for not including you in my earlier email. Thanks, Stephen
Attachment
On Wed, Oct 16, 2019 at 03:04:52PM -0400, Stephen Frost wrote: > > On Wed, Oct 16, 2019 at 01:21:48PM -0400, Stephen Frost wrote: > > > [...] if they actually need to work with both concurrently (which strikes me > > > as already at least relatively uncommon...). > > > > I doubt it's uncommon. In our case, we have customers running centos6 and 7. > > There's no postgis RPMs provided for centos6 which allow an upgrade path to > > v12, so we'll end up supporting at least (centos6, pg11) and (centos7,pg12) for > > months, at least. > > I suppose the issue here is that you don't want to have different > versions of some scripts for centos6/pg11 vs. centos7/pg12? I'm a bit > surprised that you don't have to for reasons unrelated to pg_restore. Right, I don't want to "have to" do anything :) If we really need a separate script, or conditional, then we'll do, but it's nicer to be ABLE write something (like this sanity check) in one line and not NEED TO write it in six. So far these maint scripts have significant a bit telsasoft-specific logic, but very little specific to postgres versions. I recall the schema-qualification stuff caused some churn, but it was in a minor release, so everyone was upgraded within 30-40 days, and if they weren't, I probably knew not to deploy updated scripts, either. > > I'd *like* to be able to deploy our most recent maint scripts during the > > interval of time our customers are running different major versions. The > > alternative being to try to remember to avoid deploying updated v12 scripts at > > customers still running v11. I went to the effort to make our vacuum/analyze > > script support both versions following the OID change. > > And I suppose you don't want to install v12 client tools for the v11 > systems..? I get that there's an argument for that, but it does also > seem like it'd be an alternative solution. Hm, I'd be opened to it, except that when I tries this during beta, the PGDG RPMs are compiled with GSSAPI, which creates lots of warnings...but maybe that's just in nagios... > > Tom's proposed in February to backpatch the -f- behavior, so ISTM that we're > > right now exactly where we (or at least he) planned to be, except that the > > backpatch ideally should've been included in the minor releases in August, > > before v12 was released. > > > > https://www.postgresql.org/message-id/24868.1550106683%40sss.pgh.pa.us > > That continues to strike me as a good way forward, and I'm guessing you > agree on that? If so, sorry for not including you in my earlier email. I believe you did include me (?) - I started the thread (on -general). https://www.postgresql.org/message-id/20191016172148.GH6962%40tamriel.snowman.net I think it's a good idea to do some combination of backpatching -f-, and maybe document behavior of pre-12 pg_restore in v12 release notes, and suggest /dev/stdout as a likely workaround. Of course, if backpatched, the behavior of pre-12 will vary, and should be documented as such, which is a kind of alot, but well. |In pg_restore, require specification of -f - to send the dump contents to standard output (Euler Taveira) |Previously, this happened by default if no destination was specified, but that was deemed to be unfriendly. |In the latest minor releases of versions v11 and earlier, pg_restore -f - is updated for |consistency with the new behavior of v12, to allow scripts to be written which |work on both. But note that earlier releases of v9.3 to v11 don't specially |handle "-f -", which will cause them to write to a file called "-" and not |stdout. If called under most unix shells, -f /dev/stdout will write to stdout on all versions of pg_restore. It's not perfect - someone who wants portable behavior has to apply November's minor upgrade before installing any v12 server. And vendors (something like pgadmin) will end up "having to" write to a file to be portable, or else check the full version, not just the major version. Justin
Greetings, * Justin Pryzby (pryzby@telsasoft.com) wrote: > On Wed, Oct 16, 2019 at 03:04:52PM -0400, Stephen Frost wrote: > > > > On Wed, Oct 16, 2019 at 01:21:48PM -0400, Stephen Frost wrote: > > > > [...] if they actually need to work with both concurrently (which strikes me > > > > as already at least relatively uncommon...). > > > > > > I doubt it's uncommon. In our case, we have customers running centos6 and 7. > > > There's no postgis RPMs provided for centos6 which allow an upgrade path to > > > v12, so we'll end up supporting at least (centos6, pg11) and (centos7,pg12) for > > > months, at least. > > > > I suppose the issue here is that you don't want to have different > > versions of some scripts for centos6/pg11 vs. centos7/pg12? I'm a bit > > surprised that you don't have to for reasons unrelated to pg_restore. > > Right, I don't want to "have to" do anything :) Sure, fair enough. > If we really need a separate script, or conditional, then we'll do, but it's > nicer to be ABLE write something (like this sanity check) in one line and not > NEED TO write it in six. So far these maint scripts have significant a bit > telsasoft-specific logic, but very little specific to postgres versions. I > recall the schema-qualification stuff caused some churn, but it was in a minor > release, so everyone was upgraded within 30-40 days, and if they weren't, I > probably knew not to deploy updated scripts, either. Hmm, that's interesting as a data point, at least. > > > I'd *like* to be able to deploy our most recent maint scripts during the > > > interval of time our customers are running different major versions. The > > > alternative being to try to remember to avoid deploying updated v12 scripts at > > > customers still running v11. I went to the effort to make our vacuum/analyze > > > script support both versions following the OID change. > > > > And I suppose you don't want to install v12 client tools for the v11 > > systems..? I get that there's an argument for that, but it does also > > seem like it'd be an alternative solution. > > Hm, I'd be opened to it, except that when I tries this during beta, the PGDG > RPMs are compiled with GSSAPI, which creates lots of warnings...but maybe > that's just in nagios... Warnings in the server log because they attempt to start a GSSAPI encrypted session first, if you are authenticating with GSSAPI? If so then I'm sympathetic, but at least you could address that by setting PGGSSENCMODE to disable, and that'd work for pre-v12 and v12+. > > > Tom's proposed in February to backpatch the -f- behavior, so ISTM that we're > > > right now exactly where we (or at least he) planned to be, except that the > > > backpatch ideally should've been included in the minor releases in August, > > > before v12 was released. > > > > > > https://www.postgresql.org/message-id/24868.1550106683%40sss.pgh.pa.us > > > > That continues to strike me as a good way forward, and I'm guessing you > > agree on that? If so, sorry for not including you in my earlier email. > > I believe you did include me (?) - I started the thread (on -general). > https://www.postgresql.org/message-id/20191016172148.GH6962%40tamriel.snowman.net Ah, no, I mean in the list of who was taking what position- I only named Yoshikazu Imai, Andrew, Eurler, Tom and myself. > I think it's a good idea to do some combination of backpatching -f-, and maybe > document behavior of pre-12 pg_restore in v12 release notes, and suggest > /dev/stdout as a likely workaround. Of course, if backpatched, the behavior of > pre-12 will vary, and should be documented as such, which is a kind of alot, > but well. > > |In pg_restore, require specification of -f - to send the dump contents to standard output (Euler Taveira) > |Previously, this happened by default if no destination was specified, but that was deemed to be unfriendly. > |In the latest minor releases of versions v11 and earlier, pg_restore -f - is updated for > |consistency with the new behavior of v12, to allow scripts to be written which > |work on both. But note that earlier releases of v9.3 to v11 don't specially > |handle "-f -", which will cause them to write to a file called "-" and not > |stdout. If called under most unix shells, -f /dev/stdout will write to stdout on all versions of pg_restore. We'd probably have to list the specific minor versions instead of just saying "latest" and if we're suggesting an alternative course of action then we might want to actually include that in the documentation somewhere.. I'm not really sure that we want to get into such platform-specific recommendations though. > It's not perfect - someone who wants portable behavior has to apply November's > minor upgrade before installing any v12 server. And vendors (something like > pgadmin) will end up "having to" write to a file to be portable, or else check > the full version, not just the major version. See- folks like pgadmin I would expect to have to routinely write custom code for each version since the goal there is to support all of the options available from the utility, so I'm not really sure that this would actually be much of a hardship for them. Of course, I don't really hack on pgAdmin, so I might be wrong there. Thanks, Stephen
Attachment
Stephen Frost <sfrost@snowman.net> writes: > First, I'd like to clarify what I believe Tom's suggestion is, and then > talk through that, as his vote sways this topic pretty heavily. > Tom, I take it your suggestion is to have '-f -' be accepted to mean > 'goes to stdout' in all branches? Yes. > That goes against the argument that > we don't want to break existing scripts, as it's possible that there are > existing scripts that depend on '-f -' actually going to a './-' file. While that's theoretically possible, I think that the number of cases where somebody is actually expecting that is epsilon. It seems more useful to tell people that they can now use "-f -" in all branches, and it's required to use it as of v12. Alternatively, we could revoke the requirement to use "-f -" in 12, and wait a couple releases before enforcing it. The fundamental problem here is that we tried to go from "-f - doesn't work" to "you must use -f -" with no grace period where "-f - is optional". In hindsight that was a bad idea. > If you meant for all branches to accept '-f -' and have it go to a './-' > file then that's just a revert of this entire change, which I can't > agree with either No, I'm not proposing a full revert. But there's certainly room to consider reverting the part that says you *must* write "-f -" to get output to stdout. regards, tom lane
On Thu, Oct 17, 2019 at 12:24:10PM +0200, Tom Lane wrote: > Alternatively, we could revoke the requirement to use "-f -" in 12, > and wait a couple releases before enforcing it. The fundamental > problem here is that we tried to go from "-f - doesn't work" to > "you must use -f -" with no grace period where "-f - is optional". > In hindsight that was a bad idea. I'm going to make an argument in favour of keeping the enforcement of -f- in v12. If there's no enforcement, I don't know if many people would naturally start to use -f-, which means that tools which need to work across a wide range of (minor) versions may never confront this until it's enforced in v14/v15, at which point we probably end up revisiting the whole thing again. Failing pg_restore forces people to confront the new/different behavior. If we defer failing for 2 years, it probably just means it'll be an issue again 2 years from now. However, it's still an issue if (old) back branches (like 11.5) don't support -f-, and I think the differing behavior should be called out in the v12 release notes, as succinctly as possible. Also, I'm taking the opportunity to correct myself, before someone else does: On Wed, Oct 16, 2019 at 02:28:40PM -0500, Justin Pryzby wrote: > And vendors (something like pgadmin) will end up "having to" write to a file > to be portable, or else check the full version, not just the major version. I take back that part .. before v12, they'd get stdout by not specifying -f, and since 12.0 they'd get stdout with -f-. No need to check the minor version, since the "need to" specify -f- wouldn't be backpatched, of course. Justin
Greetings, * Justin Pryzby (pryzby@telsasoft.com) wrote: > On Thu, Oct 17, 2019 at 12:24:10PM +0200, Tom Lane wrote: > > Alternatively, we could revoke the requirement to use "-f -" in 12, > > and wait a couple releases before enforcing it. The fundamental > > problem here is that we tried to go from "-f - doesn't work" to > > "you must use -f -" with no grace period where "-f - is optional". > > In hindsight that was a bad idea. > > I'm going to make an argument in favour of keeping the enforcement of -f- in > v12. > > If there's no enforcement, I don't know if many people would naturally start to > use -f-, which means that tools which need to work across a wide range of > (minor) versions may never confront this until it's enforced in v14/v15, at > which point we probably end up revisiting the whole thing again. > > Failing pg_restore forces people to confront the new/different behavior. If we > defer failing for 2 years, it probably just means it'll be an issue again 2 > years from now. Absolutely agreed on this- deferring the pain doesn't really change things here. > However, it's still an issue if (old) back branches (like 11.5) don't support > -f-, and I think the differing behavior should be called out in the v12 release > notes, as succinctly as possible. I agree that we should call it out in the release notes, of course, and that, in this case, it's alright to fix the '-f-' bug that exists in the back branches as a bug and not something else. > Also, I'm taking the opportunity to correct myself, before someone else does: > > On Wed, Oct 16, 2019 at 02:28:40PM -0500, Justin Pryzby wrote: > > And vendors (something like pgadmin) will end up "having to" write to a file > > to be portable, or else check the full version, not just the major version. > > I take back that part .. before v12, they'd get stdout by not specifying -f, > and since 12.0 they'd get stdout with -f-. No need to check the minor version, > since the "need to" specify -f- wouldn't be backpatched, of course. Ah, yes, that's true. Thanks, Stephen
Attachment
On 2019-Oct-17, Tom Lane wrote: > Stephen Frost <sfrost@snowman.net> writes: > > First, I'd like to clarify what I believe Tom's suggestion is, and then > > talk through that, as his vote sways this topic pretty heavily. > > > Tom, I take it your suggestion is to have '-f -' be accepted to mean > > 'goes to stdout' in all branches? > > Yes. +1 for this, FWIW. Let's get it done before next week minors. Is anybody writing a patch? If not, I can do it. > > If you meant for all branches to accept '-f -' and have it go to a './-' > > file then that's just a revert of this entire change, which I can't > > agree with either > > No, I'm not proposing a full revert. But there's certainly room to > consider reverting the part that says you *must* write "-f -" to get > output to stdout. I don't think this will buy us anything, if we get past branches updated promptly. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes: > On 2019-Oct-17, Tom Lane wrote: >> Stephen Frost <sfrost@snowman.net> writes: >>> Tom, I take it your suggestion is to have '-f -' be accepted to mean >>> 'goes to stdout' in all branches? >> Yes. > +1 for this, FWIW. Let's get it done before next week minors. Is > anybody writing a patch? If not, I can do it. Please do. >> No, I'm not proposing a full revert. But there's certainly room to >> consider reverting the part that says you *must* write "-f -" to get >> output to stdout. > I don't think this will buy us anything, if we get past branches updated > promptly. I'm okay with that approach. regards, tom lane
Em seg., 4 de nov. de 2019 às 11:53, Alvaro Herrera <alvherre@2ndquadrant.com> escreveu: > > On 2019-Oct-17, Tom Lane wrote: > > > Stephen Frost <sfrost@snowman.net> writes: > > > First, I'd like to clarify what I believe Tom's suggestion is, and then > > > talk through that, as his vote sways this topic pretty heavily. > > > > > Tom, I take it your suggestion is to have '-f -' be accepted to mean > > > 'goes to stdout' in all branches? > > > > Yes. > > +1 for this, FWIW. Let's get it done before next week minors. Is > anybody writing a patch? If not, I can do it. > I'm not. > > > If you meant for all branches to accept '-f -' and have it go to a './-' > > > file then that's just a revert of this entire change, which I can't > > > agree with either > > > > No, I'm not proposing a full revert. But there's certainly room to > > consider reverting the part that says you *must* write "-f -" to get > > output to stdout. > > I don't think this will buy us anything, if we get past branches updated > promptly. > +1. -- Euler Taveira Timbira - http://www.timbira.com.br/ PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
Greetings, * Tom Lane (tgl@sss.pgh.pa.us) wrote: > Alvaro Herrera <alvherre@2ndquadrant.com> writes: > > On 2019-Oct-17, Tom Lane wrote: > >> Stephen Frost <sfrost@snowman.net> writes: > >>> Tom, I take it your suggestion is to have '-f -' be accepted to mean > >>> 'goes to stdout' in all branches? > > >> Yes. > > > +1 for this, FWIW. Let's get it done before next week minors. Is > > anybody writing a patch? If not, I can do it. > > Please do. +1 Thanks, Stephen
Attachment
On 2019-Nov-04, Stephen Frost wrote: > > Alvaro Herrera <alvherre@2ndquadrant.com> writes: > > > +1 for this, FWIW. Let's get it done before next week minors. Is > > > anybody writing a patch? If not, I can do it. Turns out that this is a simple partial cherry-pick of the original commit. I'm not sure if we need to call out the incompatibility in the minors' release notes (namely, that people using "-f-" to dump to ./- will need to choose a different file name). -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment
Alvaro Herrera <alvherre@2ndquadrant.com> writes: > Turns out that this is a simple partial cherry-pick of the original > commit. In the back branches, you should keep the statement that stdout is the default output file. Looks sane otherwise (I didn't test it). > I'm not sure if we need to call out the incompatibility in the minors' > release notes (namely, that people using "-f-" to dump to ./- will need > to choose a different file name). Well, we'll have to document the addition of the feature. I think it can be phrased positively though. regards, tom lane
Em seg., 4 de nov. de 2019 às 16:12, Alvaro Herrera <alvherre@2ndquadrant.com> escreveu: > I'm not sure if we need to call out the incompatibility in the minors' > release notes (namely, that people using "-f-" to dump to ./- will need > to choose a different file name). > Should we break translations? I'm -0.5 on changing usage(). If you are using 9.5, you know that it does not work. If you try it by accident (because it works in v12), it will work but it is not that important to inform it in --help (if you are in doubt, checking the docs will answer your question). -- Euler Taveira Timbira - http://www.timbira.com.br/ PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
On 2019-Nov-04, Tom Lane wrote: > Alvaro Herrera <alvherre@2ndquadrant.com> writes: > > Turns out that this is a simple partial cherry-pick of the original > > commit. > > In the back branches, you should keep the statement that stdout > is the default output file. Looks sane otherwise (I didn't test it). I propose this: <para> Specify output file for generated script, or for the listing when used with <option>-l</option>. Use <literal>-</literal> for the standard output, which is also the default. </para> Less invasive formulations sound repetitive (such as "Use - for stdout. The default is stdout"). I'm open to suggestions. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2019-Nov-04, Euler Taveira wrote: > Em seg., 4 de nov. de 2019 às 16:12, Alvaro Herrera > <alvherre@2ndquadrant.com> escreveu: > > I'm not sure if we need to call out the incompatibility in the minors' > > release notes (namely, that people using "-f-" to dump to ./- will need > > to choose a different file name). > > > Should we break translations? I'm -0.5 on changing usage(). If you are > using 9.5, you know that it does not work. If you try it by accident > (because it works in v12), it will work but it is not that important > to inform it in --help (if you are in doubt, checking the docs will > answer your question). I would rather break the translations, and make all users aware if they look at --help. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2019-Nov-04, Alvaro Herrera wrote: > On 2019-Nov-04, Stephen Frost wrote: > > > > Alvaro Herrera <alvherre@2ndquadrant.com> writes: > > > > > +1 for this, FWIW. Let's get it done before next week minors. Is > > > > anybody writing a patch? If not, I can do it. > > Turns out that this is a simple partial cherry-pick of the original > commit. Pushed, with the documentation change suggested downthread. Thanks! -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2019-11-04 15:53, Alvaro Herrera wrote: >> No, I'm not proposing a full revert. But there's certainly room to >> consider reverting the part that says you*must* write "-f -" to get >> output to stdout. > I don't think this will buy us anything, if we get past branches updated > promptly. Users with with hundreds or thousands of servers and various ancient maintenance scripts lying around in hard-to-track ways are not going be able to get everything upgraded to the latest minors *and* new script versions any time soon. Until they do, they are effectively blocked from introducing PG12 into their environment. This is very complicated and risky for them. I think we should revert the part that requires using -f - at least for PG12. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Greetings, * Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote: > On 2019-11-04 15:53, Alvaro Herrera wrote: > >>No, I'm not proposing a full revert. But there's certainly room to > >>consider reverting the part that says you*must* write "-f -" to get > >>output to stdout. > >I don't think this will buy us anything, if we get past branches updated > >promptly. > > Users with with hundreds or thousands of servers and various ancient > maintenance scripts lying around in hard-to-track ways are not going be able > to get everything upgraded to the latest minors *and* new script versions > any time soon. Until they do, they are effectively blocked from introducing > PG12 into their environment. This is very complicated and risky for them. > I think we should revert the part that requires using -f - at least for > PG12. Absolutely not. This argument could be made, with a great deal more justification, against the changes to remove recovery.conf, and I'm sure quite a few other changes that we've made between major versions over the years, but to do so would be to hamstring our ability to make progress and to improve PG. We don't guarantee this kind of compatibility between major versions. Those users have years to address these kinds of changes, that's why we have back-branches and support major versions for 5 years. Thanks, Stephen
Attachment
On 2019-11-05 15:11, Stephen Frost wrote: > We don't guarantee this kind of compatibility between major versions. We do generally ensure compatibility of client side tools across major versions. I don't recall a case where we broke compatibility in a comparable way without a generous transition period. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Stephen Frost <sfrost@snowman.net> writes: > * Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote: >> On 2019-11-04 15:53, Alvaro Herrera wrote: >>>> No, I'm not proposing a full revert. But there's certainly room to >>>> consider reverting the part that says you*must* write "-f -" to get >>>> output to stdout. >>> I don't think this will buy us anything, if we get past branches updated >>> promptly. >> I think we should revert the part that requires using -f - at least for >> PG12. > Absolutely not. This argument could be made, with a great deal more > justification, against the changes to remove recovery.conf, and I'm sure > quite a few other changes that we've made between major versions over > the years, but to do so would be to hamstring our ability to make > progress and to improve PG. In this case, not in the least: we would simply be imposing the sort of *orderly* feature introduction that I thought was the plan from the very beginning [1]. That is, first make "-f -" available, and make it required only in some later version. If we'd back-patched the optional feature back in April, it might've been okay to require it in v12, but we failed to provide any transition period. I'm in favor of making v12 act like the older branches now do, and requiring "-f -" only as of v13. Yeah, the transition will be a little slower, but this feature is not of such huge value that it really justifies breaking scripts with zero notice. regards, tom lane [1] https://www.postgresql.org/message-id/24868.1550106683%40sss.pgh.pa.us
Greetings, * Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote: > On 2019-11-05 15:11, Stephen Frost wrote: > >We don't guarantee this kind of compatibility between major versions. > > We do generally ensure compatibility of client side tools across major > versions. I don't recall a case where we broke compatibility in a > comparable way without a generous transition period. No, we don't. The compatibility of client side tools across major versions that we provide is that newer versions will work with older databases- eg: pg_dump will work back many many years, as will psql, and that's very clear as we have specific code in those client side tools to work with older database versions. However, if you're migrating to the newer version of the tool or the database, you need to test with that new version and should expect to have to make some changes. We routinely make changes, like the removal of recovery.conf, changing the name of pg_xlog to pg_wal, renaming pg_xlogdump to pg_waldump and pg_resetxlog to pg_resetwal, the other xlog -> wal name changes, that will break scripts that people have written (not to mention serious applications like pgAdmin, barman, pgbackrest, postgres_exporter, pg_partman, et al), and certainly we do that without any more lead time than "this is what's in the new release." The XLOG -> WAL changes were even committed quite late in the cycle, February it looks like, with the location -> lsn changes happening in May. I also can't recall off-hand a specific case where we said "this behavior is going to change in release current_release+2, so be prepared", mostly because the point is well made consistently that: a) we don't want to guarantee any such change will actually happen in some kind of timeline like that, meaning people can't actually plan for it, and b) people will either make the change proactively because they track what we're doing closely, or will wait until they actually go to try and use the new version, in which case if it works then they won't bother changing and if it doesn't then they'll put in the effort to make the change, there's no real middle ground there. Thanks, Stephen
Attachment
Greetings, * Tom Lane (tgl@sss.pgh.pa.us) wrote: > In this case, not in the least: we would simply be imposing the sort > of *orderly* feature introduction that I thought was the plan from > the very beginning [1]. That is, first make "-f -" available, and > make it required only in some later version. If we'd back-patched > the optional feature back in April, it might've been okay to require > it in v12, but we failed to provide any transition period. ... just like we didn't provide any transistion period for the recovery.conf changes. > I'm in favor of making v12 act like the older branches now do, and > requiring "-f -" only as of v13. Yeah, the transition will be a > little slower, but this feature is not of such huge value that it > really justifies breaking scripts with zero notice. The recovery.conf changes provided absolutely zero value in this release and breaks a great deal more things. This argument simply doesn't hold with what we've done historically or even in this release, so, no, I don't agree that it makes sense to revert this change any more than it makes sense to revert the recovery.conf changes. Maybe, if this had come up over the summer and this agreement came out that these changes weren't worth the breakage that they cause, we could have reverted them, but that ship has sailed at this point. Thanks, Stephen
Attachment
Stephen Frost <sfrost@snowman.net> writes: > * Tom Lane (tgl@sss.pgh.pa.us) wrote: >> In this case, not in the least: we would simply be imposing the sort >> of *orderly* feature introduction that I thought was the plan from >> the very beginning [1]. That is, first make "-f -" available, and >> make it required only in some later version. If we'd back-patched >> the optional feature back in April, it might've been okay to require >> it in v12, but we failed to provide any transition period. > ... just like we didn't provide any transistion period for the > recovery.conf changes. Sure, because there wasn't any practical way to provide a transition period. I think that case is entirely not comparable to this one, either as to whether a transition period is possible, or as to whether the benefits of the change merit forced breakage. regards, tom lane
Greetings, * Tom Lane (tgl@sss.pgh.pa.us) wrote: > Stephen Frost <sfrost@snowman.net> writes: > > * Tom Lane (tgl@sss.pgh.pa.us) wrote: > >> In this case, not in the least: we would simply be imposing the sort > >> of *orderly* feature introduction that I thought was the plan from > >> the very beginning [1]. That is, first make "-f -" available, and > >> make it required only in some later version. If we'd back-patched > >> the optional feature back in April, it might've been okay to require > >> it in v12, but we failed to provide any transition period. > > > ... just like we didn't provide any transistion period for the > > recovery.conf changes. > > Sure, because there wasn't any practical way to provide a transition > period. I think that case is entirely not comparable to this one, > either as to whether a transition period is possible, or as to whether > the benefits of the change merit forced breakage. We didn't put any effort into trying to provide a transition period, and for good reason- everyone gets 5 years of transition time. I'd be just as happy to not even commit the change to make -f- go to stdout in the back-branches, if I didn't feel that the behavior of it going to a file called ./- was really just an outright bug in the first place. Thanks, Stephen
Attachment
On 2019-Nov-05, Tom Lane wrote: > Sure, because there wasn't any practical way to provide a transition > period. I think that case is entirely not comparable to this one, > either as to whether a transition period is possible, or as to whether > the benefits of the change merit forced breakage. We're not forcing anyone into upgrading. Older versions continue to work, and many people still use those. People who already upgraded and needed a cross-version scriptable mechanism can already use "-f/dev/stdout" as Justin documented in this thread's OP. People upgrading after next week release set can use "-f-". People not upgrading soon can keep their scripts for a while yet. I think this teapot doesn't need the tempest, and nobody's drowning in it anyway. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, Nov 5, 2019 at 10:07 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > I think this teapot doesn't need the tempest, and nobody's drowning in > it anyway. Yeah, I think we're getting awfully worked up over not much. If I had been reviewing this feature initially, I believe I would have voted for making -f- go to stdout first, and requiring it only in a later release. But what's done is done. I don't see this as being such an emergency as to justify whacking around the back-branches or reverting already-release features. We could easily cause more damage by jerking the behavior around than was caused by the original decision. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes: > On Tue, Nov 5, 2019 at 10:07 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote: >> I think this teapot doesn't need the tempest, and nobody's drowning in >> it anyway. > Yeah, I think we're getting awfully worked up over not much. Seems like that's getting to be the consensus opinion. Let's leave things as they stand. I'm happy that we back-patched the ability to use "-f -", and I think that'll probably be enough to satisfy anyone who's really unhappy with the state of affairs as of 12.0. regards, tom lane