Explicit psqlrc
in [PgSql]
|
Prev: [PATCH] elimination of code duplication in DefineOpFamily()
Next: [HACKERS] trace_recovery_messages
From: "Kevin Grittner" on 20 Jul 2010 09:45 Simon Riggs <simon(a)2ndQuadrant.com> wrote: > I don't think so. We can assume people wrote a patch because they > want it included in Postgres. Bumping them doesn't help them or > us, since there is always an issue other than wish-to-complete. > Not everybody is able to commit time in the way we do and we > should respect that better. Sure. If people mail me off-list about needing more time, I'm willing to accommodate, within reason. > Authors frequently have to wait a long time for a review; why > should reviewers not be as patient as authors must be? Let's keep this in perspective. We're talking about pushing review to less than two months away because of lack of author response for over a month. And should a patch appear before then, there's nothing that says an interested member of the community can't review it before the next CF. You, for example, would be free to review it at any time a patch might appear. > We should be giving authors as much leeway as possible, or they > may not come back. One phenomenon I've noticed is that sometimes a patch is submitted because an end user has solved their own problem for themselves, but wishes to share the solution with the community. They're not always motivated to go to the lengths required to polish it up to the standard required for inclusion in core. In such cases, unless someone with the time to do so finds it interesting enough to pick up, it is just going to drop. I hope such authors feel comfortable submitting their next effort, as it might be something which interests a larger audience than the previous effort. We should do what we can to ensure that they understand the dynamics of that. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Robert Haas on 20 Jul 2010 09:59 On Tue, Jul 20, 2010 at 9:23 AM, Kevin Grittner <Kevin.Grittner(a)wicourts.gov> wrote: > Robert Haas <robertmhaas(a)gmail.com> wrote: > >> we gain something quite specific and tangible, namely, the >> expectation that patch authors will stay on top of their patches >> if they want them reviewed by the community. > > Barring some operational emergency here, I'll be reviewing the > status of all the open patches in the CF today. �If I can't find any > new posting by the author for the patch in question, I'll mark it > Returned With Feedback. �I'll probably be cracking the whip on a few > others, one way or another. �If anyone wonders why I don't do so for > certain patches, it will probably be because I've had off-list > messages about needing more time to do a proper review or being in > transit and unable to do more than post short emails at the moment. > > I do request that all authors and reviewers make an effort to keep > the CF app page up-to-date. �If you're not sure all recent patches, > reviews, and significant comment posts are reflected in the app for > a patch for which you're an author or reviewer, please check as soon > as possible and make it right; it's save time for me and will help > keep the process moving smoothly. Thanks, I appreciate it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: David Christensen on 20 Jul 2010 10:00 On Jul 19, 2010, at 10:40 PM, Robert Haas wrote: > On Wed, Jun 23, 2010 at 9:22 AM, Robert Haas <robertmhaas(a)gmail.com> wrote: >> On Wed, Jun 23, 2010 at 9:17 AM, gabrielle <gorthx(a)gmail.com> wrote: >>> On Mon, Jun 21, 2010 at 6:16 PM, Robert Haas <robertmhaas(a)gmail.com> wrote: >>>> Well, that might be a good idea, too, but my expectation is that: >>>> >>>> psql -f one -f two -f three >>>> >>>> ought to behave in a manner fairly similar to: >>>> >>>> cat one two three > all >>>> psql -f all >>>> >>>> and it sounds like with this patch that's far from being the case. >>> >>> Correct. Each is handled individually. >>> >>> Should I continue to check on ON_ERROR_ROLLBACK results, or bounce >>> this back to the author? >> >> It doesn't hurt to continue to review and find other problems so that >> the author can try to fix them all at once, but it certainly seems >> clear that it's not ready to commit at this point, so it definitely >> needs to go back to the author for a rework. > > Since it has been over a month since this review was posted and no new > version of the patch has appeared, I think we should mark this patch > as Returned with Feedback. Sorry for the delays in response. This is fine; I think there are some semantic questions that should still be resolved at this point, particularly if we're moving toward supporting multiple -c and -f lines as expressed (an idea that I agree would be useful). Specifically: 1) Does the -1 flag with multiple files indicate a single transaction for all commands/files or that each command/file is run in its own transaction? I'd implemented this with the intent that all files were run in a single transaction, but it's at least a bit ambiguous, and should probably be documented at the very least. 2) I had a question (expressed in the comments) about how the final error handling status should be reported/handled. I can see this affecting a number of things, particularly ON_ERROR_{STOP,ROLLBACK} behavior; specifically, if there was an error, it sounds like it should just abort processing of any other queued files/commands at this point (in the case of ON_ERROR_STOP, at least). 3) With the switch to multiple intermixed -c and -f flags, the internal representation will essentially have to change to a queue of structs; I think in that case, we'd want to modify the current .psqlrc handling to push a struct representing the .psqlrc file at the front of the queue, depending on the code paths that currently set this up. Are there any gotchas to this approach? (I'm looking essentially for odd code paths where say .psqlrc was not loaded before, but now would be given the proper input of -c, -f file, -f -.) I'll look more in-depth at the posted feedback and Mark's proposed patch. Regards, David -- David Christensen End Point Corporation david(a)endpoint.com -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Robert Haas on 20 Jul 2010 10:05 On Tue, Jul 20, 2010 at 10:00 AM, David Christensen <david(a)endpoint.com> wrote: > Sorry for the delays in response. �This is fine; I think there are some semantic questions that should still be resolved at this point, particularly if we're moving toward supporting multiple -c and -f lines as expressed (an idea that I agree would be useful). �Specifically: > > 1) Does the -1 flag with multiple files indicate a single transaction for all commands/files or that each command/file is run in its own transaction? �I'd implemented this with the intent that all files were run in a single transaction, but it's at least a bit ambiguous, and should probably be documented at the very least. I think your implementation is right. Documentation is always good. > 2) I had a question (expressed in the comments) about how the final error handling status should be reported/handled. �I can see this affecting a number of things, particularly ON_ERROR_{STOP,ROLLBACK} behavior; specifically, if there was an error, it sounds like it should just abort processing of any other queued files/commands at this point (in the case of ON_ERROR_STOP, at least). Right. I think it should behave much as if you concatenated the files and then ran psql on the result. Except with better reporting of error locations, etc. > 3) With the switch to multiple intermixed -c and -f flags, the internal representation will essentially have to change to a queue of structs; I think in that case, we'd want to modify the current .psqlrc handling to push a struct representing the .psqlrc file at the front of the queue, depending on the code paths that currently set this up. �Are there any gotchas to this approach? �(I'm looking essentially for odd code paths where say .psqlrc was not loaded before, but now would be given the proper input of -c, -f file, -f -.) > > I'll look more in-depth at the posted feedback and Mark's proposed patch. Well, IIRC, one of -c and -f suppresses psqlrc, and the other does not. This doesn't seem very consistent to me, but I'm not sure there's much to be done about it at this point. I guess if you use whichever one suppresses psqlrc even once, it's suppressed, and otherwise it's not. :-( -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: David Christensen on 20 Jul 2010 11:23
On Jul 20, 2010, at 9:05 AM, Robert Haas wrote: > On Tue, Jul 20, 2010 at 10:00 AM, David Christensen <david(a)endpoint.com> wrote: >> Sorry for the delays in response. This is fine; I think there are some semantic questions that should still be resolved at this point, particularly if we're moving toward supporting multiple -c and -f lines as expressed (an idea that I agree would be useful). Specifically: >> >> 1) Does the -1 flag with multiple files indicate a single transaction for all commands/files or that each command/file is run in its own transaction? I'd implemented this with the intent that all files were run in a single transaction, but it's at least a bit ambiguous, and should probably be documented at the very least. > > I think your implementation is right. Documentation is always good. > >> 2) I had a question (expressed in the comments) about how the final error handling status should be reported/handled. I can see this affecting a number of things, particularly ON_ERROR_{STOP,ROLLBACK} behavior; specifically, if there was an error, it sounds like it should just abort processing of any other queued files/commands at this point (in the case of ON_ERROR_STOP, at least). > > Right. I think it should behave much as if you concatenated the files > and then ran psql on the result. Except with better reporting of > error locations, etc. > >> 3) With the switch to multiple intermixed -c and -f flags, the internal representation will essentially have to change to a queue of structs; I think in that case, we'd want to modify the current .psqlrc handling to push a struct representing the .psqlrc file at the front of the queue, depending on the code paths that currently set this up. Are there any gotchas to this approach? (I'm looking essentially for odd code paths where say .psqlrc was not loaded before, but now would be given the proper input of -c, -f file, -f -.) >> >> I'll look more in-depth at the posted feedback and Mark's proposed patch. > > Well, IIRC, one of -c and -f suppresses psqlrc, and the other does > not. This doesn't seem very consistent to me, but I'm not sure > there's much to be done about it at this point. I guess if you use > whichever one suppresses psqlrc even once, it's suppressed, and > otherwise it's not. :-( That seems sub-optimal; I can see people wanting to use this feature to do something like: psql -c 'set work_mem = blah' -f script.sql and then being surprised when it works differently than just `psql -f script.sql`. psql -c "select 'starting'" -f file1.sql -c "select 'midway'" -f file2.sql -c "select 'done!'" Although I wonder if the general usecase for .psqlrc is just in interactive mode; i.e., hypothetically if you're running scripts that are sensitive to environment, you're running with -X anyway; so maybe that's not that big of a deal, as it's kind of an implied -X with multiple -c or -f commands. And if you really wanted it, we could add a flag to explicitly include .psqlrc (or the user could just specify -f path/to/psqlrc). Regards, David -- David Christensen End Point Corporation david(a)endpoint.com -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |