Explicit psqlrc
in [PgSql]
|
Prev: [PATCH] elimination of code duplication in DefineOpFamily()
Next: [HACKERS] trace_recovery_messages
From: Mark Wong on 23 Jun 2010 23:30 On Jun 23, 2010, at 5:36 PM, Mark Wong wrote: > On Jun 22, 2010, at 1:34 AM, Simon Riggs wrote: > >> On Mon, 2010-06-21 at 20:53 -0400, Robert Haas wrote: >>> On Mon, Jun 21, 2010 at 7:51 PM, gabrielle <gorthx(a)gmail.com> wrote: >>>> On Thu, 2010-06-17 at 14:50 -0400, Alvaro Herrera asked: >>>>> How does it play with ON_ERROR_STOP/ROLLBACK? >>>> >>>> With ON_ERROR_STOP=ON, psql issues an error when it encounters one, >>>> stops processing the file that contains the error, and then continues >>>> to process any remaining files. >> >> That would be undesirable. >> >>>> I'm still investigating ON_ERROR_ROLLBACK. I need to tinker with it >>>> some more before I say anything concrete. >>>> >>>> On Fri, Jun 18, 2010 at 1:48 AM, Simon Riggs <simon(a)2ndquadrant.com> wrote: >>>>> Also, how does it play with --single-transaction. >>>> That was buried in our original report :) "BEGIN-COMMIT statements >>>> within the files cause warnings when the command is wrapped in a >>>> transaction with the -1 switch (as specified in the patch submission)" >>>> >>>> To expand upon that a bit: when psql encounters a file that contains >>>> a BEGIN statement, you get the expected "WARNING: there is already a >>>> transaction in progress" message. The COMMIT at the end of that file >>>> (assuming the user doesn't forget it) generates a COMMIT. Commands >>>> after that commit, or in any remaining files to be processed, are >>>> dealt with according to the user's autocommit settings: >>>> - if autocommit is ON, statements in the remaining files are processed >>>> & committed; the implicit COMMIT at the end of the whole thing then >>>> generates a "WARNING: there is no transaction in progress" message >>>> - if autocommit is OFF, statements in the remaining files generate >>>> "ERROR: current transaction is aborted, commands ignored until end of >>>> transaction block" messages. >> >> This is the existing behaviour. >> >>> So none of the above sounds like desired behavior to me... is that just me? >> >> Single transaction needs some help, but that's not the fault of this >> patch. > > I took a closer look at what was going on and what it would take to meet some of these expectations. In main() the patch adds BEGIN and COMMIT statements outside the call to process_file() in src/bin/psql/command.c. Here lies the previous logic for handling single transaction. Since it remains, it appears that BEGIN can be issued twice before any statements are executed if the single transaction switch is used. Plus there are other a couple of places that call this particular process_file() function, but I think those are straightforward cases to deal with. Heh, not close enough. I was wrong about that scenario. I can see now that the single transaction flag is always set to false in process_file(). I think that turns the single transaction handling in process_file() into dead code with the patch like this. Regards, Mark -- 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 19 Jul 2010 23:40 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. -- 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: Simon Riggs on 20 Jul 2010 03:20 On Mon, 2010-07-19 at 23:40 -0400, Robert Haas wrote: > 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. Mark posted a new patch 6 days ago, AFAICS. Not sure I see any benefit in doing as you propose anyway, or at least not without warning since it just confuses authors who may believe they have time while the commitfest is still on. Commitfests were created to help authors. We must continue to remember that 99% of patch authors are not full time and likely to find smooth responsiveness difficult. We should say "Will there be any further action on this patch?" -- Simon Riggs www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Training and Services -- 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 07:06 On Tue, Jul 20, 2010 at 3:20 AM, Simon Riggs <simon(a)2ndquadrant.com> wrote: > On Mon, 2010-07-19 at 23:40 -0400, Robert Haas wrote: > >> 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. > > Mark posted a new patch 6 days ago, AFAICS. Hmm. I can't find it in my mail, in my archives, or linked off the CommitFest application. Was it posted under a different subject line? Do you have a link to the archives? > Not sure I see any benefit in doing as you propose anyway, or at least > not without warning since it just confuses authors who may believe they > have time while the commitfest is still on. > > Commitfests were created to help authors. We must continue to remember > that 99% of patch authors are not full time and likely to find smooth > responsiveness difficult. > > We should say "Will there be any further action on this patch?" It isn't the purpose of CommitFests to provide patch authors with an unlimited right to have their patches reviewed. They exist, on the one hand, to make sure that patches don't linger forever without getting a review; and on the other hand, to create a defined time for each member of the community to set aside their own work and review/commit other people's patches. It is important that we have them, and it is also important that they end, so that reviews, committers, commitfest managers, etc. can stop working on the CF at some point and get back to their own work. In other words, CommitFests need to represent a balance between the needs of authors and the needs of patch reviewers and committers. Of course, anyone is always welcome to review a patch that has been posted, and a committer can decide to work on a patch at any time also. But if patch authors are entitled to resubmit a previously reviewed patch up until the very last CommitFest are *guaranteed* a possible review and commit even then, then the CommitFest will not end on time. Even if the CommitFest does end on time, more than 50% of the time between now and 9.1 feature freeze is CF-time - that is, time I'm supposed to be putting aside the work I'd like to get done to help other people get the work they'd like to do get done. I'm not really willing to increase that percentage much further. I feel it's important that we give everyone a fair shake, and I have devoted and will continue to devote a LOT of time to making sure that happens - but I want (and if I care to still be employed, need) some time to do my own work, too. To me, the definition of a fair shake is that people get 4-5 days to respond to review comments. This patch has had 33. It's not unfair to anyone to say, you know, since you didn't get around to updating this patch for over a month, you'll need to resubmit the updated version to the next CommitFest. If we have the resources to review and commit a late resubmission of some particular patch, that is great. But as of today, we still have 32 patches that need to be reviewed, many of which do not have a reviewer assigned or which have a reviewer assigned but have not yet had an initial review. Since there are 26 days left in the CommitFest and many of those patches will need multiple rounds of review and much discussion before we decide whether to commit them, send them back for rework, or reject them outright, that's pretty scary. To me, that's where we should be focusing our effort. -- 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: Simon Riggs on 20 Jul 2010 07:41
On Tue, 2010-07-20 at 07:06 -0400, Robert Haas wrote: > To me, the definition of a fair shake is that people get 4-5 days to > respond to review comments. This patch has had 33. It's not unfair > to anyone to say, you know, since you didn't get around to updating > this patch for over a month, you'll need to resubmit the updated > version to the next CommitFest. If we have the resources to review > and commit a late resubmission of some particular patch, that is > great. But as of today, we still have 32 patches that need to be > reviewed, many of which do not have a reviewer assigned or which have > a reviewer assigned but have not yet had an initial review. Since > there are 26 days left in the CommitFest and many of those patches > will need multiple rounds of review and much discussion before we > decide whether to commit them, send them back for rework, or reject > them outright, that's pretty scary. To me, that's where we should be > focusing our effort. So focus your effort by leaving this alone until the end of the CF. Actively terminating things early doesn't help at all with the review work you mention above, but it looks good if we are measuring "cases resolved per day". Are we measuring that? If so, why? Who cares? Just leave them, and if no action at end of CF, boot them then. Saves loads of time on chasing up on people, interpreting what they say, worrying about it and minor admin. Closing early gains us nothing, though might close the door on useful work in progress. Net expected benefit is negative from acting early. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |