Horton
Menu

Thread

  1. [PATCH] Make "psql -1 < file.sql" work as with "-f"

    Fabien COELHO <coelho@cri.ensmp.fr> — 2012-08-01T08:28:09Z

    Dear PostgreSQL developers,

Plese find attached a patch so that:

     Make "psql -1 < file.sql" work as with "-f"

     Make psql --single-transaction option work on a non-interactive
     standard input as well, so that "psql -1 < input.sql" behaves as
     "psql -1 -f input.sql".

This saner/less error-prone behavior was discussed in this thread back in 
June:

 	http://archives.postgresql.org/pgsql-hackers/2012-06/msg00785.php

I have tested it manually and it works for me. I'm not sure this is the 
best possible implementation, but it is a small diff one. I haven't found 
a place in the regression tests where "psql" could be tested with 
different options. Did I miss something?

-- 
Fabien.

  2. Re: [PATCH] Make "psql -1 < file.sql" work as with "-f"

    Fabien COELHO <coelho@cri.ensmp.fr> — 2012-08-03T09:02:51Z

    Here is a new submission with the expected "context diff format".

> Dear PostgreSQL developers,
>
> Plese find attached a patch so that:
>
>    Make "psql -1 < file.sql" work as with "-f"
>
>    Make psql --single-transaction option work on a non-interactive
>    standard input as well, so that "psql -1 < input.sql" behaves as
>    "psql -1 -f input.sql".
>
> This saner/less error-prone behavior was discussed in this thread back in 
> June:
>
> 	http://archives.postgresql.org/pgsql-hackers/2012-06/msg00785.php
>
> I have tested it manually and it works for me. I'm not sure this is the best 
> possible implementation, but it is a small diff one. I haven't found a place 
> in the regression tests where "psql" could be tested with different options. 
> Did I miss something?

-- 
Fabien.

  3. Re: [PATCH] Make "psql -1 < file.sql" work as with "-f"

    Robert Haas <robertmhaas@gmail.com> — 2012-08-08T20:55:43Z

    On Wed, Aug 1, 2012 at 4:28 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
> Dear PostgreSQL developers,
>
> Plese find attached a patch so that:
>
>     Make "psql -1 < file.sql" work as with "-f"
>
>     Make psql --single-transaction option work on a non-interactive
>     standard input as well, so that "psql -1 < input.sql" behaves as
>     "psql -1 -f input.sql".
>
> This saner/less error-prone behavior was discussed in this thread back in
> June:
>
>         http://archives.postgresql.org/pgsql-hackers/2012-06/msg00785.php
>
> I have tested it manually and it works for me. I'm not sure this is the best
> possible implementation, but it is a small diff one. I haven't found a place
> in the regression tests where "psql" could be tested with different options.
> Did I miss something?

I'm wondering if perhaps -- in addition to what you've done here -- we
should make "psql -1" error out if reading from a terminal.

Because accepting options that are intended to cause important
behavior changes and then ignoring those options is Bad.

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

  4. Re: [PATCH] Make "psql -1 < file.sql" work as with "-f"

    David Fetter <david@fetter.org> — 2012-08-08T22:50:22Z

    On Wed, Aug 08, 2012 at 04:55:43PM -0400, Robert Haas wrote:
> On Wed, Aug 1, 2012 at 4:28 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
> > Dear PostgreSQL developers,
> >
> > Plese find attached a patch so that:
> >
> >     Make "psql -1 < file.sql" work as with "-f"
> >
> >     Make psql --single-transaction option work on a non-interactive
> >     standard input as well, so that "psql -1 < input.sql" behaves as
> >     "psql -1 -f input.sql".
> >
> > This saner/less error-prone behavior was discussed in this thread back in
> > June:
> >
> >         http://archives.postgresql.org/pgsql-hackers/2012-06/msg00785.php
> >
> > I have tested it manually and it works for me. I'm not sure this is the best
> > possible implementation, but it is a small diff one. I haven't found a place
> > in the regression tests where "psql" could be tested with different options.
> > Did I miss something?
> 
> I'm wondering if perhaps -- in addition to what you've done here -- we
> should make "psql -1" error out if reading from a terminal.

+1 for this.

> Because accepting options that are intended to cause important
> behavior changes and then ignoring those options is Bad.

Yes, It Is.

Cheers,
David.
-- 
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

  5. Re: [PATCH] Make "psql -1 < file.sql" work as with "-f"

    Robert Haas <robertmhaas@gmail.com> — 2012-08-09T13:08:43Z

    On Wed, Aug 8, 2012 at 6:50 PM, David Fetter <david@fetter.org> wrote:
>> I'm wondering if perhaps -- in addition to what you've done here -- we
>> should make "psql -1" error out if reading from a terminal.
>
> +1 for this.

OK, done.

I had to revise the original patch pretty heavily before committing;
the original patch assumed that it was OK to make psql -1 <file go
through process_file() while having psql -1 <file still go through
MainLoop() directly.  This isn't a good idea, because that means that
any other behavioral differences between process_file() and MainLoop()
will be contingent on whether -1 is used, which is not what we want.
And there is at least one such difference that matters: whether or not
the file and line number get prepended when emitting error messages.

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

  6. Re: [PATCH] Make "psql -1 < file.sql" work as with "-f"

    Fabien COELHO <coelho@cri.ensmp.fr> — 2012-08-09T16:08:21Z

    > OK, done.
>
> I had to revise the original patch pretty heavily before committing;
> the original patch assumed that it was OK to make psql -1 <file go
> through process_file() while having psql -1 <file still go through
> MainLoop() directly.  This isn't a good idea, because that means that
> any other behavioral differences between process_file() and MainLoop()
> will be contingent on whether -1 is used, which is not what we want.

Yep. I did that with a "smallest" and "simplest" change in mind, and 
beside when doing "psql -1 < file", you're hardly going to do anything 
else anyway, so I felt that it was not a big issue.

> And there is at least one such difference that matters: whether or not
> the file and line number get prepended when emitting error messages.

Thanks for the improvements and for the push!

-- 
Fabien.

  7. Re: [PATCH] Make "psql -1 < file.sql" work as with "-f"

    Peter Eisentraut <peter_e@gmx.net> — 2013-05-10T13:50:55Z

    On 8/9/12 9:08 AM, Robert Haas wrote:
> On Wed, Aug 8, 2012 at 6:50 PM, David Fetter <david@fetter.org> wrote:
>>> I'm wondering if perhaps -- in addition to what you've done here -- we
>>> should make "psql -1" error out if reading from a terminal.
>>
>> +1 for this.
> 
> OK, done.
> 
> I had to revise the original patch pretty heavily before committing;

My first use of 9.3beta1 in development failed because of changes
introduced by this patch, specifically because of the newly introduced error

    psql: -1 is incompatible with -c and -l

I'm not convinced this is correct.  -c and -l are single-transaction
actions almost by definition.

This particular aspect of the change wasn't really brought up in the
original thread.  What was your thinking?



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

  8. Re: [PATCH] Make "psql -1 < file.sql" work as with "-f"

    Christopher Browne <cbbrowne@gmail.com> — 2013-05-10T14:50:49Z

    On Fri, May 10, 2013 at 9:50 AM, Peter Eisentraut <peter_e@gmx.net> wrote:

> On 8/9/12 9:08 AM, Robert Haas wrote:
> > On Wed, Aug 8, 2012 at 6:50 PM, David Fetter <david@fetter.org> wrote:
> >>> I'm wondering if perhaps -- in addition to what you've done here -- we
> >>> should make "psql -1" error out if reading from a terminal.
> >>
> >> +1 for this.
> >
> > OK, done.
> >
> > I had to revise the original patch pretty heavily before committing;
>
> My first use of 9.3beta1 in development failed because of changes
> introduced by this patch, specifically because of the newly introduced
> error
>
>     psql: -1 is incompatible with -c and -l
>
> I'm not convinced this is correct.  -c and -l are single-transaction
> actions almost by definition.
>
> This particular aspect of the change wasn't really brought up in the
> original thread.  What was your thinking?
>

FYI, I noticed this issue when building one of our applications against
HEAD;
I'm not sure I agree with you vis-a-vis the -c option, as it is certainly
plausible/meaningful
to do:
  psql -c "begin; update [something]; insert [something]; delete
[something]; commit;"
and for that to be different from
  psql -c "update [something]; insert [something]; delete [something];"

Consider it stipulated that it's pretty plausible to expect things to break
down if, in that
series of requests, the UPDATE fails, and it isn't nearly safe to assume
that the INSERT
and/or DELETE statements would succeed after all that.

I'd be pretty happy for
  psql -1 -c "update [something]; insert [something]; delete [something];"
to implicitly augment the query to:
  psql -c "begin; update [something]; insert [something]; delete
[something]; commit;"

It's a bit annoying (it bit me, giving me a complication, without any
evident benefit) for
"psql -1 -c" to refuse to run.

I'd rather that it behave similarly to "psql -1 -f", and wrap the queries
in a transaction.
For it to behave badly if I try to induce transaction control (e.g. -
embedding BEGIN/END
inside the queries) would not come as a surprise; that would be much the
same as how
"psql -1 -f" works, where the extra BEGIN is warned as redundant and the
extra COMMIT
is considered an error.

As for "psql -1 -l", it seems like a regression for that to fail.  Listing
the databases is
pretty much already a single transaction; adding "-1" is perhaps
overspecifying things,
but it doesn't seem wrong.
-- 
When confronted by a difficult problem, solve it by reducing it to the
question, "How would the Lone Ranger handle this?"

  9. Re: [PATCH] Make "psql -1 < file.sql" work as with "-f"

    Tom Lane <tgl@sss.pgh.pa.us> — 2013-05-10T16:03:19Z

    Christopher Browne <cbbrowne@gmail.com> writes:
> On Fri, May 10, 2013 at 9:50 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
>> My first use of 9.3beta1 in development failed because of changes
>> introduced by this patch, specifically because of the newly introduced
>> error
>> 
>> psql: -1 is incompatible with -c and -l
>> 
>> I'm not convinced this is correct.  -c and -l are single-transaction
>> actions almost by definition.
>> 
>> This particular aspect of the change wasn't really brought up in the
>> original thread.  What was your thinking?

> I'm not sure I agree with you vis-a-vis the -c option, as it is certainly
> plausible/meaningful
> to do:
>   psql -c "begin; update [something]; insert [something]; delete
> [something]; commit;"
> and for that to be different from
>   psql -c "update [something]; insert [something]; delete [something];"

While it might be *plausible* for those to be different, that's not
actually how -c works in practice, because it sends the string as a
single PQexec, which has the effect of making the string a single
transaction even if the string does not contain begin/end explicitly.

I think Peter is right and this error is bogus.  Whatever redeeming
social value it might have for sticklers is not worth breaking existing
apps for.

			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

  10. Re: [PATCH] Make "psql -1 < file.sql" work as with "-f"

    Fabien COELHO <coelho@cri.ensmp.fr> — 2013-05-10T16:40:32Z

    > My first use of 9.3beta1 in development failed because of changes
> introduced by this patch, specifically because of the newly introduced error
>
>    psql: -1 is incompatible with -c and -l
>
> I'm not convinced this is correct.  -c and -l are single-transaction
> actions almost by definition.
>
> This particular aspect of the change wasn't really brought up in the
> original thread.  What was your thinking?

AFAICR, the 3 lines patch I submitted did not include such a check.

Comments by Robert in the source suggest that the -1 option is ignored 
under -c and -l. This is because the "transaction" is handled by 
process_file which is not called in these cases.

However, if they are single transaction nevertheless, the guard may just 
be removed, even if the option does nothing?

ISTM that option -l is readonly, it does not matter much. For -c, I'm not 
that sure.

-- 
Fabien.

  11. Re: [PATCH] Make "psql -1 < file.sql" work as with "-f"

    Robert Haas <robertmhaas@gmail.com> — 2013-05-11T15:55:02Z

    On Fri, May 10, 2013 at 9:50 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
> On 8/9/12 9:08 AM, Robert Haas wrote:
>> On Wed, Aug 8, 2012 at 6:50 PM, David Fetter <david@fetter.org> wrote:
>>>> I'm wondering if perhaps -- in addition to what you've done here -- we
>>>> should make "psql -1" error out if reading from a terminal.
>>>
>>> +1 for this.
>>
>> OK, done.
>>
>> I had to revise the original patch pretty heavily before committing;
>
> My first use of 9.3beta1 in development failed because of changes
> introduced by this patch, specifically because of the newly introduced error
>
>     psql: -1 is incompatible with -c and -l
>
> I'm not convinced this is correct.  -c and -l are single-transaction
> actions almost by definition.
>
> This particular aspect of the change wasn't really brought up in the
> original thread.  What was your thinking?

Well, I think my main thinking was to prevent it in interactive mode,
since it doesn't work in interactive mode, and then it also seemed to
make sense to prevent it in the other cases to which it does not
apply.

I think there are cases where you can detect the fact that -1 -c
wasn't actually wrapping the command in a BEGIN and an END, but I
agree it might be a bit pedantic to worry about them.  There have been
previous proposals to allow multiple -c and -f options, and to allow
those to be intermingled; if we did that, then this would surely
matter.  I agree that's hypothetical though since there's no patch to
do any such thing currently on the table.

Generally, I think we're too lax about detecting and complaining about
conflicting combinations of options.  But I'm not going to stand here
and hold my breath if someone else feels that this particular
combination doesn't merit a complaint.

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