Re: [HACKERS] Explicit psqlrc

2011-02-16 Thread Bruce Momjian
Robert Haas wrote:
 On Wed, Jul 21, 2010 at 11:06 PM, David Christensen da...@endpoint.com 
 wrote:
 
  On Jul 21, 2010, at 12:31 PM, Alvaro Herrera wrote:
 
  Excerpts from Peter Eisentraut's message of mi? jul 21 10:24:26 -0400 2010:
  On tis, 2010-07-20 at 11:48 -0400, Robert Haas wrote:
  It's tempting to propose making .psqlrc apply only in interactive
  mode, period. ?But that would be an incompatibility with previous
  releases, and I'm not sure it's the behavior we want, either.
 
  What is a use case for having .psqlrc be read in noninteractive use?
 
  Even if there weren't one, why does it get applied to -f but not -c?
  They're both noninteractive.
 
 
  So not to let the thread drop, it appears that we're faced with the 
  following situation:
 
 Hmm.  I thought we almost had consensus on changing the historical
 behavior of -c.  If we do that, this all gets much simpler.

Added to TODO:

Consider having psql -c read .psqlrc, for consistency

psql -f already reads .psqlrc 

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

-- 
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] Explicit psqlrc

2010-08-10 Thread Bruce Momjian
Kevin Grittner wrote:
  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.

This brings up the larger issue of whether incomplete/unapplied patches
are recorded on the TODO list or just ignored.  We never really came up
with a plan for that.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

-- 
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] Explicit psqlrc

2010-08-10 Thread Bruce Momjian
Simon Riggs wrote:
 On Tue, 2010-07-20 at 09:05 -0400, Robert Haas wrote:
  On Tue, Jul 20, 2010 at 8:21 AM, Simon Riggs si...@2ndquadrant.com wrote:
   On Tue, 2010-07-20 at 07:49 -0400, Robert Haas wrote:
   A further point is that it's very difficult to
   keep track of progress if the CF page reflects a whole bunch of
   supposedly Waiting on Author patches that are really quite
   thoroughly dead.
  
   True, but the point under discussion is what to do if no reply is
   received from an author. That is something entirely different from a
   patch hitting a brick wall.
  
   We gain nothing by moving early on author-delay situations, so I suggest
   we don't.
  
  No, 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.  If that expectation doesn't
  seem important to you, feel free to try running a CommitFest without
  it.  If you can make it work, I'll happily sign on.
 
 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.
 
 Authors frequently have to wait a long time for a review; why should
 reviewers not be as patient as authors must be?
 
 We should be giving authors as much leeway as possible, or they may not
 come back.

By marking patches as 'returned with feedback' long before the end of
the commit-fest, we show feedback of how close we are to completing the
commit-fest.  If we keep patches in limbo status, it is unclear how
close we are to CF completion.  And, of course, the author can
reactivate the patch just by replying.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

-- 
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] Explicit psqlrc

2010-07-22 Thread Robert Haas
On Wed, Jul 21, 2010 at 11:06 PM, David Christensen da...@endpoint.com wrote:

 On Jul 21, 2010, at 12:31 PM, Alvaro Herrera wrote:

 Excerpts from Peter Eisentraut's message of mié jul 21 10:24:26 -0400 2010:
 On tis, 2010-07-20 at 11:48 -0400, Robert Haas wrote:
 It's tempting to propose making .psqlrc apply only in interactive
 mode, period.  But that would be an incompatibility with previous
 releases, and I'm not sure it's the behavior we want, either.

 What is a use case for having .psqlrc be read in noninteractive use?

 Even if there weren't one, why does it get applied to -f but not -c?
 They're both noninteractive.


 So not to let the thread drop, it appears that we're faced with the following 
 situation:

Hmm.  I thought we almost had consensus on changing the historical
behavior of -c.  If we do that, this all gets much simpler.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

-- 
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] Explicit psqlrc

2010-07-21 Thread Peter Eisentraut
On tis, 2010-07-20 at 11:48 -0400, Robert Haas wrote:
 It's tempting to propose making .psqlrc apply only in interactive
 mode, period.  But that would be an incompatibility with previous
 releases, and I'm not sure it's the behavior we want, either.

What is a use case for having .psqlrc be read in noninteractive use?


-- 
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] Explicit psqlrc

2010-07-21 Thread Robert Haas
On Wed, Jul 21, 2010 at 10:24 AM, Peter Eisentraut pete...@gmx.net wrote:
 On tis, 2010-07-20 at 11:48 -0400, Robert Haas wrote:
 It's tempting to propose making .psqlrc apply only in interactive
 mode, period.  But that would be an incompatibility with previous
 releases, and I'm not sure it's the behavior we want, either.

 What is a use case for having .psqlrc be read in noninteractive use?

Well, for example, if I hate the new ASCII format with a fiery passion
that can never be quenched (and, by the way, I do), then I'd like this
to apply:

\pset linestyle old-ascii

Even when I do this:

psql -c '...whatever...'

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

-- 
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] Explicit psqlrc

2010-07-21 Thread David Christensen

On Jul 21, 2010, at 9:42 AM, Robert Haas wrote:

 On Wed, Jul 21, 2010 at 10:24 AM, Peter Eisentraut pete...@gmx.net wrote:
 On tis, 2010-07-20 at 11:48 -0400, Robert Haas wrote:
 It's tempting to propose making .psqlrc apply only in interactive
 mode, period.  But that would be an incompatibility with previous
 releases, and I'm not sure it's the behavior we want, either.
 
 What is a use case for having .psqlrc be read in noninteractive use?
 
 Well, for example, if I hate the new ASCII format with a fiery passion
 that can never be quenched (and, by the way, I do), then I'd like this
 to apply:
 
 \pset linestyle old-ascii
 
 Even when I do this:
 
 psql -c '...whatever...'


Well, tossing out two possible solutions:

1) .psqlrc + .psql_profile (kinda like how bash separates out the 
interactive/non-interactive parts).  Kinda yucky, but it's a working solution.

2) have a flag which explicitly includes the psqlrc file in non-interactive use 
(perhaps if -x is available, use it for the analogue to -X).

Regards,

David
--
David Christensen
End Point Corporation
da...@endpoint.com





-- 
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] Explicit psqlrc

2010-07-21 Thread Robert Haas
On Wed, Jul 21, 2010 at 11:31 AM, David Christensen da...@endpoint.com wrote:

 On Jul 21, 2010, at 9:42 AM, Robert Haas wrote:

 On Wed, Jul 21, 2010 at 10:24 AM, Peter Eisentraut pete...@gmx.net wrote:
 On tis, 2010-07-20 at 11:48 -0400, Robert Haas wrote:
 It's tempting to propose making .psqlrc apply only in interactive
 mode, period.  But that would be an incompatibility with previous
 releases, and I'm not sure it's the behavior we want, either.

 What is a use case for having .psqlrc be read in noninteractive use?

 Well, for example, if I hate the new ASCII format with a fiery passion
 that can never be quenched (and, by the way, I do), then I'd like this
 to apply:

 \pset linestyle old-ascii

 Even when I do this:

 psql -c '...whatever...'


 Well, tossing out two possible solutions:

 1) .psqlrc + .psql_profile (kinda like how bash separates out the 
 interactive/non-interactive parts).  Kinda yucky, but it's a working solution.

 2) have a flag which explicitly includes the psqlrc file in non-interactive 
 use (perhaps if -x is available, use it for the analogue to -X).

Hmm.  Well, that still doesn't solve the problem that -c and -f do
different things with respect to psqlrc, does it?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

-- 
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] Explicit psqlrc

2010-07-21 Thread Simon Riggs
On Wed, 2010-07-21 at 17:24 +0300, Peter Eisentraut wrote:
 On tis, 2010-07-20 at 11:48 -0400, Robert Haas wrote:
  It's tempting to propose making .psqlrc apply only in interactive
  mode, period.  But that would be an incompatibility with previous
  releases, and I'm not sure it's the behavior we want, either.
 
 What is a use case for having .psqlrc be read in noninteractive use?

Changing the historical defaults, such as error/exit behaviour, ensuring
timing is on etc..

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Development, 24x7 Support, Training and Services


-- 
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] Explicit psqlrc

2010-07-21 Thread Alvaro Herrera
Excerpts from Peter Eisentraut's message of mié jul 21 10:24:26 -0400 2010:
 On tis, 2010-07-20 at 11:48 -0400, Robert Haas wrote:
  It's tempting to propose making .psqlrc apply only in interactive
  mode, period.  But that would be an incompatibility with previous
  releases, and I'm not sure it's the behavior we want, either.
 
 What is a use case for having .psqlrc be read in noninteractive use?

Even if there weren't one, why does it get applied to -f but not -c?
They're both noninteractive.

-- 
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] Explicit psqlrc

2010-07-21 Thread David Christensen

On Jul 21, 2010, at 12:31 PM, Alvaro Herrera wrote:

 Excerpts from Peter Eisentraut's message of mié jul 21 10:24:26 -0400 2010:
 On tis, 2010-07-20 at 11:48 -0400, Robert Haas wrote:
 It's tempting to propose making .psqlrc apply only in interactive
 mode, period.  But that would be an incompatibility with previous
 releases, and I'm not sure it's the behavior we want, either.
 
 What is a use case for having .psqlrc be read in noninteractive use?
 
 Even if there weren't one, why does it get applied to -f but not -c?
 They're both noninteractive.


So not to let the thread drop, it appears that we're faced with the following 
situation:

1) The current behavior is inconsistent with the psqlrc handling of -c and -f.
2) The current behavior is still historical and we presumably want to maintain 
it.

I'm not sure of the cases where we're willing to break backwards compatibility, 
but I do know that it's not done lightly.  So perhaps for this specific patch, 
we'd need/want to punt supporting both -c's in conjunction with -f's, at least 
until we can resolve some of the ambiguities in what the actual behavior should 
be in each of these cases.  This could still be a followup patch for 9.1, if we 
get these issues resolved.

And as long as we're redesigning the bike shed, I think a better use case for 
supporting multiple sql files would be to support them in such a way that you 
wouldn't need to provide explicit -f flags for each.  Many programs utilize the 
'--' token for an end of options flag, with the rest of the arguments then 
becoming something special, such as filenames.  So what about adding the 
interpretation that anything after '--' is interpreted as a filename?  That 
will allow the use of shell wildcards to specify multiple files, and thus allow 
something like:

  $ psql -U myuser mydatabase -- *.sql
  $ psql -- {pre-,,post-}migration.sql

while still being unambiguous with the current convention of having an 
unspecified argument be interpreted as a database name.  This would make it 
possible to actually specify/use multiple files in a fashion that people are 
used to doing, as opposed to having to explicitly type things out or do 
contortions will shell substitutions, etc.

Regards,

David
--
David Christensen
End Point Corporation
da...@endpoint.com





-- 
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] Explicit psqlrc

2010-07-20 Thread Simon Riggs
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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Explicit psqlrc

2010-07-20 Thread Robert Haas
On Tue, Jul 20, 2010 at 3:20 AM, Simon Riggs si...@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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Explicit psqlrc

2010-07-20 Thread Simon Riggs
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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Explicit psqlrc

2010-07-20 Thread Robert Haas
On Tue, Jul 20, 2010 at 7:41 AM, Simon Riggs si...@2ndquadrant.com wrote:
 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?

We don't formally measure that, but yeah, I definitely keep an eye on
it.  I've found that if you don't keep a sharp eye on that, you end up
not done with the CommitFest is supposed to be over.  I'd much rather
boot patches for reasonable justification throughout the CommitFest
than boot everything at the end whether there's a justification or
not.

 Closing early gains us nothing, though might close the door on useful
 work in progress.

IMHO, closing early LOSES us nothing.  People are free to work on
their patches whenever they'd like, and hopefully will.  But
pretending we're going to review them all no matter when they get
resubmitted just makes people grumpy when they find out that we're not
magical and can't.  A further point is that it's very difficult to
keep track of progress if the CF page reflects a whole bunch of
supposedly Waiting on Author patches that are really quite
thoroughly dead.

On the other hand, if this patch was really resubmitted already and I
missed it, as you suggested, that's a whole different situation.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

-- 
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] Explicit psqlrc

2010-07-20 Thread Simon Riggs
On Tue, 2010-07-20 at 07:49 -0400, Robert Haas wrote:
 A further point is that it's very difficult to
 keep track of progress if the CF page reflects a whole bunch of
 supposedly Waiting on Author patches that are really quite
 thoroughly dead. 

True, but the point under discussion is what to do if no reply is
received from an author. That is something entirely different from a
patch hitting a brick wall.

We gain nothing by moving early on author-delay situations, so I suggest
we don't.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Development, 24x7 Support, Training and Services


-- 
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] Explicit psqlrc

2010-07-20 Thread Robert Haas
On Tue, Jul 20, 2010 at 8:21 AM, Simon Riggs si...@2ndquadrant.com wrote:
 On Tue, 2010-07-20 at 07:49 -0400, Robert Haas wrote:
 A further point is that it's very difficult to
 keep track of progress if the CF page reflects a whole bunch of
 supposedly Waiting on Author patches that are really quite
 thoroughly dead.

 True, but the point under discussion is what to do if no reply is
 received from an author. That is something entirely different from a
 patch hitting a brick wall.

 We gain nothing by moving early on author-delay situations, so I suggest
 we don't.

No, 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.  If that expectation doesn't
seem important to you, feel free to try running a CommitFest without
it.  If you can make it work, I'll happily sign on.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

-- 
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] Explicit psqlrc

2010-07-20 Thread Kevin Grittner
Robert Haas robertmh...@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, all.
 
-Kevin

-- 
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] Explicit psqlrc

2010-07-20 Thread Simon Riggs
On Tue, 2010-07-20 at 09:05 -0400, Robert Haas wrote:
 On Tue, Jul 20, 2010 at 8:21 AM, Simon Riggs si...@2ndquadrant.com wrote:
  On Tue, 2010-07-20 at 07:49 -0400, Robert Haas wrote:
  A further point is that it's very difficult to
  keep track of progress if the CF page reflects a whole bunch of
  supposedly Waiting on Author patches that are really quite
  thoroughly dead.
 
  True, but the point under discussion is what to do if no reply is
  received from an author. That is something entirely different from a
  patch hitting a brick wall.
 
  We gain nothing by moving early on author-delay situations, so I suggest
  we don't.
 
 No, 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.  If that expectation doesn't
 seem important to you, feel free to try running a CommitFest without
 it.  If you can make it work, I'll happily sign on.

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.

Authors frequently have to wait a long time for a review; why should
reviewers not be as patient as authors must be?

We should be giving authors as much leeway as possible, or they may not
come back.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Development, 24x7 Support, Training and Services


-- 
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] Explicit psqlrc

2010-07-20 Thread Kevin Grittner
Simon Riggs si...@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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Explicit psqlrc

2010-07-20 Thread Robert Haas
On Tue, Jul 20, 2010 at 9:23 AM, Kevin Grittner
kevin.gritt...@wicourts.gov wrote:
 Robert Haas robertmh...@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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Explicit psqlrc

2010-07-20 Thread David Christensen

On Jul 19, 2010, at 10:40 PM, Robert Haas wrote:

 On Wed, Jun 23, 2010 at 9:22 AM, Robert Haas robertmh...@gmail.com wrote:
 On Wed, Jun 23, 2010 at 9:17 AM, gabrielle gor...@gmail.com wrote:
 On Mon, Jun 21, 2010 at 6:16 PM, Robert Haas robertmh...@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
da...@endpoint.com





-- 
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] Explicit psqlrc

2010-07-20 Thread Robert Haas
On Tue, Jul 20, 2010 at 10:00 AM, David Christensen da...@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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Explicit psqlrc

2010-07-20 Thread David Christensen

On Jul 20, 2010, at 9:05 AM, Robert Haas wrote:

 On Tue, Jul 20, 2010 at 10:00 AM, David Christensen da...@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
da...@endpoint.com





-- 
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] Explicit psqlrc

2010-07-20 Thread Robert Haas
On Tue, Jul 20, 2010 at 11:23 AM, David Christensen da...@endpoint.com wrote:
 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`.

I agree... but then they might also be surprised if psql -c
'something' works differently from psql -c 'something' -f /dev/null

 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).

It's tempting to propose making .psqlrc apply only in interactive
mode, period.  But that would be an incompatibility with previous
releases, and I'm not sure it's the behavior we want, either.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

-- 
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] Explicit psqlrc

2010-07-20 Thread Mark Wong
On Tue, Jul 20, 2010 at 4:06 AM, Robert Haas robertmh...@gmail.com wrote:
 On Tue, Jul 20, 2010 at 3:20 AM, Simon Riggs si...@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?

My bad, I didn't want to appear to hijack the patch from David.  I
have added the link to the commitfest app, and I'll give it here:

http://archives.postgresql.org/message-id/aanlktikfpzrtrl6392ghatqdwlcwqtxfdsmxh5cp5...@mail.gmail.com

Regads,
Mark

-- 
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] Explicit psqlrc

2010-07-20 Thread Robert Haas
On Tue, Jul 20, 2010 at 12:08 PM, Mark Wong mark...@gmail.com wrote:
 On Tue, Jul 20, 2010 at 4:06 AM, Robert Haas robertmh...@gmail.com wrote:
 On Tue, Jul 20, 2010 at 3:20 AM, Simon Riggs si...@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?

 My bad, I didn't want to appear to hijack the patch from David.  I
 have added the link to the commitfest app, and I'll give it here:

 http://archives.postgresql.org/message-id/aanlktikfpzrtrl6392ghatqdwlcwqtxfdsmxh5cp5...@mail.gmail.com

Oh, cool, I missed that, obviously.  So I guess we need someone to
review that version, then.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

-- 
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] Explicit psqlrc

2010-07-20 Thread Alvaro Herrera
Excerpts from Robert Haas's message of mar jul 20 11:48:29 -0400 2010:

  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`.
 
 I agree... but then they might also be surprised if psql -c
 'something' works differently from psql -c 'something' -f /dev/null

I think we should just make sure -X works, and have .psqlrc be read when
it's not specified regardless of -f and -c switches.

Otherwise it's just plain confusing.

-- 
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] Explicit psqlrc

2010-07-20 Thread David Christensen

On Jul 20, 2010, at 2:18 PM, Alvaro Herrera wrote:

 Excerpts from Robert Haas's message of mar jul 20 11:48:29 -0400 2010:
 
 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`.
 
 I agree... but then they might also be surprised if psql -c
 'something' works differently from psql -c 'something' -f /dev/null
 
 I think we should just make sure -X works, and have .psqlrc be read when
 it's not specified regardless of -f and -c switches.
 
 Otherwise it's just plain confusing.


+1.

Regards,

David
--
David Christensen
End Point Corporation
da...@endpoint.com





-- 
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] Explicit psqlrc

2010-07-19 Thread Robert Haas
On Wed, Jun 23, 2010 at 9:22 AM, Robert Haas robertmh...@gmail.com wrote:
 On Wed, Jun 23, 2010 at 9:17 AM, gabrielle gor...@gmail.com wrote:
 On Mon, Jun 21, 2010 at 6:16 PM, Robert Haas robertmh...@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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Explicit psqlrc

2010-06-23 Thread gabrielle
On Mon, Jun 21, 2010 at 6:16 PM, Robert Haas robertmh...@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?

gabrielle

-- 
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] Explicit psqlrc

2010-06-23 Thread Robert Haas
On Wed, Jun 23, 2010 at 9:17 AM, gabrielle gor...@gmail.com wrote:
 On Mon, Jun 21, 2010 at 6:16 PM, Robert Haas robertmh...@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.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

-- 
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] Explicit psqlrc

2010-06-23 Thread Mark Wong
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 gor...@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 si...@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.

Regards,
Mark
-- 
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] Explicit psqlrc

2010-06-23 Thread Mark Wong
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 gor...@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 si...@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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Explicit psqlrc

2010-06-22 Thread Simon Riggs
On Mon, 2010-06-21 at 20:53 -0400, Robert Haas wrote:
 On Mon, Jun 21, 2010 at 7:51 PM, gabrielle gor...@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 si...@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.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Development, 24x7 Support, Training and Services


-- 
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] Explicit psqlrc

2010-06-21 Thread gabrielle
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.

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 si...@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.

 I would like multiple -c commands also, as well as a mix of -f and -c.
 Can we add that at the same time please?

I'll leave this one for someone else to answer. :)

gabrielle

-- 
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] Explicit psqlrc

2010-06-21 Thread Robert Haas
On Mon, Jun 21, 2010 at 7:51 PM, gabrielle gor...@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.

 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 si...@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.

So none of the above sounds like desired behavior to me...  is that just me?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

-- 
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] Explicit psqlrc

2010-06-21 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote:
 So none of the above sounds like desired behavior to me...  is that just me?

Yeah, I'm not really thrilled with this..  I mentioned earlier what I
thought would be a useful feature (basically, a switch which would
ignore the main psqlrc and turn on the various options that make sense
for a script), but that seems to have fallen to the wayside..

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Explicit psqlrc

2010-06-21 Thread Robert Haas
On Mon, Jun 21, 2010 at 9:13 PM, Stephen Frost sfr...@snowman.net wrote:
 * Robert Haas (robertmh...@gmail.com) wrote:
 So none of the above sounds like desired behavior to me...  is that just me?

 Yeah, I'm not really thrilled with this..  I mentioned earlier what I
 thought would be a useful feature (basically, a switch which would
 ignore the main psqlrc and turn on the various options that make sense
 for a script), but that seems to have fallen to the wayside..

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.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

-- 
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] Explicit psqlrc

2010-06-18 Thread Simon Riggs
On Thu, 2010-06-17 at 14:50 -0400, Alvaro Herrera wrote:
 Excerpts from Mark Wong's message of mié jun 16 23:54:52 -0400 2010:
 
  ==Usability review==
  Read what the patch is supposed to do, and consider:
  Does the patch actually implement that?
 
 How does it play with ON_ERROR_STOP/ROLLBACK?

Also, how does it play with --single-transaction.

I would like multiple -c commands also, as well as a mix of -f and -c.
Can we add that at the same time please?

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Development, 24x7 Support, Training and Services


-- 
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] Explicit psqlrc

2010-06-17 Thread Alvaro Herrera
Excerpts from Mark Wong's message of mié jun 16 23:54:52 -0400 2010:

 ==Usability review==
 Read what the patch is supposed to do, and consider:
 Does the patch actually implement that?

How does it play with ON_ERROR_STOP/ROLLBACK?

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] Explicit psqlrc

2010-06-16 Thread Mark Wong
Hi David,

At a pdxpug gathering, we took a look at your patch to psql for
supporting multiple -f's and put together some feedback:

REVIEW:  Patch: support multiple -f options

https://commitfest.postgresql.org/action/patch_view?id=286

==Submission review==
Is the patch in context diff format?
yes
Does it apply cleanly to the current CVS HEAD?
yes
Do all tests pass?
yes
Does it include reasonable tests, necessary doc patches, etc?
- tests: inconclusive
- docs: no:
psql --help does not mention that you can use multiple -f
switches;  do we want it to? also, no doc update was included in the
patch.

==Usability review==
Read what the patch is supposed to do, and consider:
Does the patch actually implement that?
yes
Do we want that?
sure!
Do we already have it?
no
Does it follow SQL spec, or the community-agreed behavior?
NA
Does it include pg_dump support (if applicable)?
NA
Are there dangers?
- subject to the usual Dumb Mistakes (see have all the bases
been covered) Have all the bases been covered?
Scenarios we came up with:
- how does it handle wildcards, eg psql -f *.sql?
Does not - this is a shell issue.  psql is designed to
take the database as the last argument;  giving the -f option a
wildcard expands the list, but does not assign multiple -f
switches...so if you name one of your files the same as one of your
databases, you could accidentally run updates you don't want to do.
This is a known feature of psql, and has already bitten these reviewers
in the butt on other occasions, so nothing to worry about here.
- how does it handle the lack of a file?
as expected:
gabrie...@princess~/tmp/bin/:::-- ./psql -f
./psql: option requires an argument -- 'f'
Try psql --help for more information.
- how does it handle a non-existent file?
as expected:
gabrie...@princess~/tmp/bin/:::-- ./psql -f beer.sql
beer.sql: No such file or directory
- how does it handle files that don't contain valid sql?
as expected, logs an error  carries on with the next
file. 
==Feature test==
Apply the patch, compile it and test:
Does the feature work as advertised?
- Yes;  and 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). Are there corner cases
the author has failed to consider?
- none that we can think of
Are there any assertion failures or crashes?
- Mark got it to segfault due to bug in logic for allocating
pointers for filenames.  It appears the order of operations between '!'
and '%' was not as intended, thus realloc() is never called and a seg
fault may occur after 16 files are passed.  There are a few ways to
fix it, the one we played with to make minimum changes to the patch is
to change:

if (!options-nm_files % FILE_ALLOC_BLOCKS)

to

if (options-num_files  1  !((options-num_files - 1) % FILE_ALLOC_BLOCKS))

==Performance review==
Does the patch slow down simple tests?
- not that we can tell.
If it claims to improve performance, does it?
N/A
Does it slow down other things?
- not that we can tell.

==Coding review==
Read the changes to the code in detail and consider:
Does it follow the project coding guidelines?
- unnecessary whitespace on line 251? 
- inconsistent spacing between operators
Are there portability issues?
- untested
Will it work on Windows/BSD etc?
- untested
Are the comments sufficient and accurate?
Does it do what it says, correctly?
- yes
Does it produce compiler warnings?
- no
Can you make it crash?
- See above about the segfault.

==Architecture review==
Consider the changes to the code in the context of the project as a
whole: Is everything done in a way that fits together coherently with
other features/modules?
- yes
Are there interdependencies that can cause problems?
- not that we are aware of

==Review review==
Did the reviewer cover all the things that kind of reviewer is supposed
to do?
- AFAWK.

Regards,
Mark

-- 
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] Explicit psqlrc

2010-03-08 Thread Robert Haas
On Mon, Mar 8, 2010 at 1:39 AM, David Christensen da...@endpoint.com wrote:

 On Mar 7, 2010, at 9:22 AM, Tom Lane wrote:

 Magnus Hagander mag...@hagander.net writes:

 2010/3/6 Tom Lane t...@sss.pgh.pa.us:

 The analogy I was thinking about was psql -X, but I agree that it's
 not obvious why this shouldn't be thought of as an additional -f file.

 Uh, I don't follow. When we use -f, we'll run the script and then
 exit. The whole point is to run it and *not* exit, since you are
 normally using it to set up the environment in psql.

 If we were going to support multiple -f options, it would be sensible
 to interpret -f - as read from stdin until EOF.  Then you could
 interleave prepared scripts and stdin, which could be pretty handy.
 The default behavior would be equivalent to a single instance of -f -,
 and what you are looking for would be -X -f substitute-psqlrc -f -.

 Here's an initial stab at supporting multiple -f's (not counting the
 interpretation of -f - as STDIN).  There are also a few pieces that are up
 for interpretation, such as the propagation of the return value of the
 MainLoop().  Also, while this patch supports the single-transaction mode, it
 does so in a way that will break if one of the scripts include explicit
 BEGIN/COMMIT statements (although it is no different than the existing code
 in this regard).

I have added to this to the next CommitFest.

...Robert

-- 
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] Explicit psqlrc

2010-03-08 Thread Simon Riggs
On Sun, 2010-03-07 at 16:37 +0100, Magnus Hagander wrote:

 With your interleave, you mean things like psql -f first.sql -f - -f
 second.sql? That does sound like it could be handy - and also really
 dangerous :-)

Multiple -f support would be a good thing.
As would mixed -f and -c options.

What would be even better would be bringing across many of the concepts
that are in pgbench, which has had the multiple file support for some
time.

-- 
 Simon Riggs   www.2ndQuadrant.com


-- 
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] Explicit psqlrc

2010-03-07 Thread Magnus Hagander
2010/3/6 Tom Lane t...@sss.pgh.pa.us:
 Peter Eisentraut pete...@gmx.net writes:
 I can see the environment variable variant as an analogy to BASH_ENV,
 but what is the use case for the --psqlrc option?  Wouldn't it be easier
 and more useful to just be able to process more than one file, say by
 specifying -f more than once?

 The analogy I was thinking about was psql -X, but I agree that it's
 not obvious why this shouldn't be thought of as an additional -f file.

Uh, I don't follow. When we use -f, we'll run the script and then
exit. The whole point is to run it and *not* exit, since you are
normally using it to set up the environment in psql.

That said, it might certainly be useful to be able to process more
than one file with -f, but that's a different usecase, I think.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] Explicit psqlrc

2010-03-07 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 2010/3/6 Tom Lane t...@sss.pgh.pa.us:
 The analogy I was thinking about was psql -X, but I agree that it's
 not obvious why this shouldn't be thought of as an additional -f file.

 Uh, I don't follow. When we use -f, we'll run the script and then
 exit. The whole point is to run it and *not* exit, since you are
 normally using it to set up the environment in psql.

If we were going to support multiple -f options, it would be sensible
to interpret -f - as read from stdin until EOF.  Then you could
interleave prepared scripts and stdin, which could be pretty handy.
The default behavior would be equivalent to a single instance of -f -,
and what you are looking for would be -X -f substitute-psqlrc -f -.

I do think this is potentially cleaner and more general than the
--psqlrc switch.  Maybe that should be reverted and the whole topic
reconsidered during 9.1 devel cycle.

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] Explicit psqlrc

2010-03-07 Thread Magnus Hagander
2010/3/7 Tom Lane t...@sss.pgh.pa.us:
 Magnus Hagander mag...@hagander.net writes:
 2010/3/6 Tom Lane t...@sss.pgh.pa.us:
 The analogy I was thinking about was psql -X, but I agree that it's
 not obvious why this shouldn't be thought of as an additional -f file.

 Uh, I don't follow. When we use -f, we'll run the script and then
 exit. The whole point is to run it and *not* exit, since you are
 normally using it to set up the environment in psql.

 If we were going to support multiple -f options, it would be sensible
 to interpret -f - as read from stdin until EOF.  Then you could
 interleave prepared scripts and stdin, which could be pretty handy.
 The default behavior would be equivalent to a single instance of -f -,
 and what you are looking for would be -X -f substitute-psqlrc -f -.

Right, that would work. Though it would be a lot more user-unfriendly
for such a simple thing, IMHO.

Also, -f - and just psql behaves different today (for example, in
the showing of startup banners). So we couldn't do that without
changing the behaviour of at least one of those. Which may not be a
problem of course, but I'm sure someone will find a place to complain
:)

With your interleave, you mean things like psql -f first.sql -f - -f
second.sql? That does sound like it could be handy - and also really
dangerous :-)


 I do think this is potentially cleaner and more general than the
 --psqlrc switch.  Maybe that should be reverted and the whole topic
 reconsidered during 9.1 devel cycle.

More general, definitely. But cleaner? I'd say rather the opposite.

In the end, I don't see why we can't have both when the implementation
is so trivial.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] Explicit psqlrc

2010-03-07 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 2010/3/7 Tom Lane t...@sss.pgh.pa.us:
 If we were going to support multiple -f options, it would be sensible
 to interpret -f - as read from stdin until EOF.

 Right, that would work. Though it would be a lot more user-unfriendly
 for such a simple thing, IMHO.

If the issue had come up even once before in psql's existence, I might
think that user-friendliness would be a good argument.  As things stand,
I don't believe the average user will care about it in the least.  I'd
be willing to lay long odds that the average user doesn't even have a
.psqlrc file, much less feel the need to override it.  I'd rather see
use a substitute psqlrc be a behavior you can build out of existing
general-purpose switches than still another option that has to be
documented and remembered.

 Also, -f - and just psql behaves different today (for example, in
 the showing of startup banners).

Yes, there would be some things to think about there, which is why it's
a topic for a new devel cycle rather than something to shoehorn in
after the close of the last CF.

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] Explicit psqlrc

2010-03-07 Thread Magnus Hagander
2010/3/7 Tom Lane t...@sss.pgh.pa.us:
 Magnus Hagander mag...@hagander.net writes:
 2010/3/7 Tom Lane t...@sss.pgh.pa.us:
 If we were going to support multiple -f options, it would be sensible
 to interpret -f - as read from stdin until EOF.

 Right, that would work. Though it would be a lot more user-unfriendly
 for such a simple thing, IMHO.

 If the issue had come up even once before in psql's existence, I might
 think that user-friendliness would be a good argument.  As things stand,
 I don't believe the average user will care about it in the least.  I'd
 be willing to lay long odds that the average user doesn't even have a
 .psqlrc file, much less feel the need to override it.  I'd rather see
 use a substitute psqlrc be a behavior you can build out of existing
 general-purpose switches than still another option that has to be
 documented and remembered.

I've heard if a couple of times before, but I agree it's certainly not
a much asked-for one. Most if it has been in off-list scenarios and
people have probabliy just thought it's not a big enough feature to
bother emailing about.


 Also, -f - and just psql behaves different today (for example, in
 the showing of startup banners).

 Yes, there would be some things to think about there, which is why it's
 a topic for a new devel cycle rather than something to shoehorn in
 after the close of the last CF.

Fair enough. I expected it to be a small and noncontroversial thing,
but since there are objections, I'll go revert it.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] Explicit psqlrc

2010-03-07 Thread Bruce Momjian
Magnus Hagander wrote:
  Also, -f - and just psql behaves different today (for example, in
  the showing of startup banners).
 
  Yes, there would be some things to think about there, which is why it's
  a topic for a new devel cycle rather than something to shoehorn in
  after the close of the last CF.
 
 Fair enough. I expected it to be a small and noncontroversial thing,
 but since there are objections, I'll go revert it.

Do you want to create a TODO entry for this?

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  PG East:  http://www.enterprisedb.com/community/nav-pg-east-2010.do

-- 
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] Explicit psqlrc

2010-03-07 Thread David Christensen


On Mar 7, 2010, at 9:22 AM, Tom Lane wrote:


Magnus Hagander mag...@hagander.net writes:

2010/3/6 Tom Lane t...@sss.pgh.pa.us:

The analogy I was thinking about was psql -X, but I agree that it's
not obvious why this shouldn't be thought of as an additional -f  
file.



Uh, I don't follow. When we use -f, we'll run the script and then
exit. The whole point is to run it and *not* exit, since you are
normally using it to set up the environment in psql.


If we were going to support multiple -f options, it would be sensible
to interpret -f - as read from stdin until EOF.  Then you could
interleave prepared scripts and stdin, which could be pretty handy.
The default behavior would be equivalent to a single instance of -f  
-,

and what you are looking for would be -X -f substitute-psqlrc -f -.


Here's an initial stab at supporting multiple -f's (not counting the  
interpretation of -f - as STDIN).  There are also a few pieces that  
are up for interpretation, such as the propagation of the return value  
of the MainLoop().  Also, while this patch supports the single- 
transaction mode, it does so in a way that will break if one of the  
scripts include explicit BEGIN/COMMIT statements (although it is no  
different than the existing code in this regard).


Regards,

David
--
David Christensen
End Point Corporation
da...@endpoint.com





psql_process_multiple_files.patch
Description: Binary data

-- 
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] Explicit psqlrc

2010-03-06 Thread Peter Eisentraut
On fre, 2010-03-05 at 11:30 +0100, Magnus Hagander wrote:
  Do you have a use-case where --psqlrc would be more useful than an
  environment variable, or is it *only* bike-shedding? ;)
 
 Just to be clear, the code difference isn't very large. Attached is a
 patch that does both PSQLRC and --psqlrc.

I can see the environment variable variant as an analogy to BASH_ENV,
but what is the use case for the --psqlrc option?  Wouldn't it be easier
and more useful to just be able to process more than one file, say by
specifying -f more than once?



-- 
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] Explicit psqlrc

2010-03-06 Thread Tom Lane
Peter Eisentraut pete...@gmx.net writes:
 I can see the environment variable variant as an analogy to BASH_ENV,
 but what is the use case for the --psqlrc option?  Wouldn't it be easier
 and more useful to just be able to process more than one file, say by
 specifying -f more than once?

The analogy I was thinking about was psql -X, but I agree that it's
not obvious why this shouldn't be thought of as an additional -f file.

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] Explicit psqlrc

2010-03-05 Thread Magnus Hagander
2010/3/5 David Christensen da...@endpoint.com:

 On Mar 4, 2010, at 4:00 PM, Magnus Hagander wrote:

 I've now for the second time found myself wanting to specify an
 explicit psqlrc file instead of just parsing ~/.psqlrc, so attached is
 a patch that accepts the PSQLRC environment variable to control which
 psqlrc file is used.

 Any objections to this (obviously including documentation - this is
 just the trivial code)


 My bikeshed has a --psqlrc path/to/file, but +1 on the idea.

The main reason I went with environment variable is that it's the
path-of-least-code :-) And it easily fullfills my use-cases for it,
which has me launching interactive psql with completely different
settings from a script.

Do you have a use-case where --psqlrc would be more useful than an
environment variable, or is it *only* bike-shedding? ;)

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] Explicit psqlrc

2010-03-05 Thread Magnus Hagander
2010/3/5 Magnus Hagander mag...@hagander.net:
 2010/3/5 David Christensen da...@endpoint.com:

 On Mar 4, 2010, at 4:00 PM, Magnus Hagander wrote:

 I've now for the second time found myself wanting to specify an
 explicit psqlrc file instead of just parsing ~/.psqlrc, so attached is
 a patch that accepts the PSQLRC environment variable to control which
 psqlrc file is used.

 Any objections to this (obviously including documentation - this is
 just the trivial code)


 My bikeshed has a --psqlrc path/to/file, but +1 on the idea.

 The main reason I went with environment variable is that it's the
 path-of-least-code :-) And it easily fullfills my use-cases for it,
 which has me launching interactive psql with completely different
 settings from a script.

 Do you have a use-case where --psqlrc would be more useful than an
 environment variable, or is it *only* bike-shedding? ;)

Just to be clear, the code difference isn't very large. Attached is a
patch that does both PSQLRC and --psqlrc.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


psqlrc.patch
Description: Binary data

-- 
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] Explicit psqlrc

2010-03-05 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 2010/3/5 David Christensen da...@endpoint.com:
 My bikeshed has a --psqlrc path/to/file, but +1 on the idea.

 Do you have a use-case where --psqlrc would be more useful than an
 environment variable, or is it *only* bike-shedding? ;)

The env variable solution seems a bit surprising to me.  If we were
dealing with a libpq option it would make sense (to avoid having to pass
the info through other layers) but for a psql option it doesn't AFAICS.

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] Explicit psqlrc

2010-03-05 Thread Magnus Hagander
2010/3/5 Tom Lane t...@sss.pgh.pa.us:
 Magnus Hagander mag...@hagander.net writes:
 2010/3/5 David Christensen da...@endpoint.com:
 My bikeshed has a --psqlrc path/to/file, but +1 on the idea.

 Do you have a use-case where --psqlrc would be more useful than an
 environment variable, or is it *only* bike-shedding? ;)

 The env variable solution seems a bit surprising to me.  If we were
 dealing with a libpq option it would make sense (to avoid having to pass
 the info through other layers) but for a psql option it doesn't AFAICS.

Fair enough. It worked in my environment, but I can see your point. So
I'll just strip that part out then :)


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Explicit psqlrc

2010-03-04 Thread Magnus Hagander
I've now for the second time found myself wanting to specify an
explicit psqlrc file instead of just parsing ~/.psqlrc, so attached is
a patch that accepts the PSQLRC environment variable to control which
psqlrc file is used.

Any objections to this (obviously including documentation - this is
just the trivial code)

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


psqlrc.patch
Description: Binary data

-- 
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] Explicit psqlrc

2010-03-04 Thread David Christensen


On Mar 4, 2010, at 4:00 PM, Magnus Hagander wrote:


I've now for the second time found myself wanting to specify an
explicit psqlrc file instead of just parsing ~/.psqlrc, so attached is
a patch that accepts the PSQLRC environment variable to control which
psqlrc file is used.

Any objections to this (obviously including documentation - this is
just the trivial code)



My bikeshed has a --psqlrc path/to/file, but +1 on the idea.

Regards,

David
--
David Christensen
End Point Corporation
da...@endpoint.com





--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers