Re: [HACKERS] COPY table FROM STDIN doesn't show count tag

2014-03-13 Thread Rajeev rastogi
On 12 March 2014 23:57, Tom Lane Wrote: 
 Robert Haas robertmh...@gmail.com writes:
  On Wed, Mar 12, 2014 at 12:09 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  My inclination now (see later traffic) is to suppress the status
  report when the COPY destination is the same as pset.queryFout (ie,
 a
  simple test whether the FILE pointers are equal).  This would
  suppress the status report for \copy to stdout and COPY TO
 STDOUT
  cases, and also for \copy to pstdout if you'd not redirected
  queryFout with \o.

Based on my analysis, I observed that just file pointer comparison may not be 
sufficient 
to decide whether to display command tag or not. E.g. imagine below scenario:

psql.exe -d postgres -o 'file.dat' -c  \copy tbl to 'file.dat';

Though both destination files are same but file pointer will be different and 
hence 
printing status in file 'file.dat' will overwrite some part of data copied to 
file.
Also we don't have any direct way of comparison of file name itself.
As I see \copy ... TO.. will print status only in-case of \copy to pstdout if 
-o option is given.

So instead of having so much of confusion and inconsistency that too for one 
very specific case, 
I though not to print status for all case Of STDOUT and \COPY ... TO ...

  This is reasonably similar to what we already do for SELECT, isn't it?
   I mean, the server always sends back a command tag, but psql
  sometimes opts not to print it.
 
 Right, the analogy to SELECT gives some comfort that this is reasonable.

I have modified the patch based on above analysis as:
1. In-case of COPY ... TO STDOUT, command tag will not be displayed.
2. In-case of \COPY ... TO ..., command tag will not be displayed.
3. In all other cases, command tag will be displayed similar as were getting 
displayed earlier. 

I have modified the corresponding documentation.

Please find the attached revised patch.

Thanks and Regards,
Kumar Rajeev Rastogi




psql-copy-count-tag-20140313.patch
Description: psql-copy-count-tag-20140313.patch

-- 
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] COPY table FROM STDIN doesn't show count tag

2014-03-13 Thread Tom Lane
Rajeev rastogi rajeev.rast...@huawei.com writes:
 [ updated patch ]

I've committed this patch with additional revisions.

 Based on my analysis, I observed that just file pointer comparison may not be 
 sufficient 
 to decide whether to display command tag or not. E.g. imagine below scenario:

   psql.exe -d postgres -o 'file.dat' -c  \copy tbl to 'file.dat';

I don't think it's our responsibility to avoid printing both data and
status to the same place in such cases; arguably, in fact, that's exactly
what the user told us to do.  The important thing is to avoid printing
both for the straightforward case of COPY TO STDOUT.  For that, file
pointer comparison is the right thing, since the option-parsing code will
set copysource to match queryFout in exactly the relevant cases.

In any case, this revised patch suppressed the status print in *all*
COPY_OUT cases, which surely seems like throwing the baby out with the
bathwater.

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] COPY table FROM STDIN doesn't show count tag

2014-03-12 Thread David Johnston
Tom Lane-2 wrote
 Unfortunately, while testing it I noticed that there's a potentially
 fatal backwards-compatibility problem, namely that the COPY n status
 gets printed on stdout, which is the same place that COPY OUT data is
 going.  While this isn't such a big problem for interactive use,
 usages like this one are pretty popular:
 
psql -c 'copy mytable to stdout' mydatabase | some-program
 
 With the patch, COPY n gets included in the data sent to some-program,
 which never happened before and is surely not what the user wants.
 The same if the -c string uses \copy.
 
 There are several things we could do about this:
 
 1. Treat this as a non-backwards-compatible change, and document that
 people have to use -q if they don't want the COPY tag in the output.
 I'm not sure this is acceptable.

I've mostly used copy to with files and so wouldn't mind if STDOUT had the
COPY n sent to it as long as the target file is just the copy contents.


 2. Kluge ProcessResult so that it continues to not pass back a PGresult
 for the COPY TO STDOUT case, or does so only in limited circumstances
 (perhaps only if isatty(stdout), for instance).

The main problem with this is that people will test by sending output to a
TTY and see the COPY n.  Although if it can be done consistently then you
minimize backward incompatibility and encourage people to enforce quiet mode
while the command runs...


 3. Modify PrintQueryStatus so that command status goes to stderr not
 stdout.  While this is probably how it should've been done in the first
 place, this would be a far more severe compatibility break than #1.
 (For one thing, there are probably scripts out there that think that any
 output to stderr is an error message.)  I'm afraid this one is definitely
 not acceptable, though it would be by far the cleanest solution were it
 not for compatibility concerns.

Yes, it's a moot point but I'm not sure it would be best anyway.


 4. As #3, but print the command status to stderr only if it's COPY n,
 otherwise to stdout.  This is a smaller compatibility break than #3,
 but still a break since COPY status was formerly issued to stdout
 in non TO STDOUT/FROM STDIN cases.  (Note that PrintQueryStatus can't
 tell whether it was COPY TO STDOUT rather than any other kind of COPY;
 if we want that to factor into the behavior, we need ProcessResult to
 do it.)

Since we are considering stderr my (inexperienced admittedly) gut says that
using stderr for this is generally undesirable and especially given our
existing precedence.  stdout is the seemingly correct target, typically, and
the existing quiet-mode toggle provides sufficient control for typical
needs.


 5. Give up on the print-the-tag aspect of the change, and just fix the
 wrong-line-number issue (so we'd still introduce the copyStream variable,
 but not change how PGresults are passed around).
 
 I'm inclined to think #2 is the best answer if we can't stomach #1.
 But the exact rule for when to print a COPY OUT result probably
 still requires some debate.  Or maybe someone has another idea?
 
 Also, I'm thinking we should back-patch the aspects of the patch
 needed to fix the wrong-line-number issue.  That appears to have been
 introduced in 9.2; older versions of PG get the above example right.
 
 Comments?

I'd like COPY TO to anything but STDOUT to emit a COPY n on STDOUT -
unless suppressed by -q(uiet)

Document that COPY TO STDOUT does not emit COPY n because STDOUT is
already assigned for data and so is not available for notifications.  Since
COPY is more typically used for ETL than a bare-select, in addition to
back-compatibility concerns, this default behavior seems reasonable.

Would it be possible to store the n somewhere and provide a command - like
GET DIAGNOSTICS in pl/pgsql - if the user really wants to know how many rows
were sent to STDOUT?  I'm doubt this is even useful in the typical use-case
for COPY TO STDOUT but figured I'd toss the idea out there.

Is there anything besides a desire for consistency that anyone has or can
put forth as a use-case for COPY TO STDOUT emitting COPY n on STDOUT as
well?  If you are going to view the content inline, and also want a quick
count, ISTM you would be more likely to use SELECT to take advantage of all
its pretty-print features.

If we really need to cater to this use then maybe a --loud-copy-to-stdout
switch can be provided to override its default quiet-mode.

David J.





--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/COPY-table-FROM-STDIN-doesn-t-show-count-tag-tp5775018p5795611.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] COPY table FROM STDIN doesn't show count tag

2014-03-12 Thread Pavel Stehule
2014-03-12 7:10 GMT+01:00 David Johnston pol...@yahoo.com:

 Tom Lane-2 wrote
  Unfortunately, while testing it I noticed that there's a potentially
  fatal backwards-compatibility problem, namely that the COPY n status
  gets printed on stdout, which is the same place that COPY OUT data is
  going.  While this isn't such a big problem for interactive use,
  usages like this one are pretty popular:
 
 psql -c 'copy mytable to stdout' mydatabase | some-program
 
  With the patch, COPY n gets included in the data sent to some-program,
  which never happened before and is surely not what the user wants.
  The same if the -c string uses \copy.
 
  There are several things we could do about this:
 
  1. Treat this as a non-backwards-compatible change, and document that
  people have to use -q if they don't want the COPY tag in the output.
  I'm not sure this is acceptable.

 I've mostly used copy to with files and so wouldn't mind if STDOUT had the
 COPY n sent to it as long as the target file is just the copy contents.


  2. Kluge ProcessResult so that it continues to not pass back a PGresult
  for the COPY TO STDOUT case, or does so only in limited circumstances
  (perhaps only if isatty(stdout), for instance).

 The main problem with this is that people will test by sending output to a
 TTY and see the COPY n.  Although if it can be done consistently then you
 minimize backward incompatibility and encourage people to enforce quiet
 mode
 while the command runs...


  3. Modify PrintQueryStatus so that command status goes to stderr not
  stdout.  While this is probably how it should've been done in the first
  place, this would be a far more severe compatibility break than #1.
  (For one thing, there are probably scripts out there that think that any
  output to stderr is an error message.)  I'm afraid this one is definitely
  not acceptable, though it would be by far the cleanest solution were it
  not for compatibility concerns.

 Yes, it's a moot point but I'm not sure it would be best anyway.


  4. As #3, but print the command status to stderr only if it's COPY n,
  otherwise to stdout.  This is a smaller compatibility break than #3,
  but still a break since COPY status was formerly issued to stdout
  in non TO STDOUT/FROM STDIN cases.  (Note that PrintQueryStatus can't
  tell whether it was COPY TO STDOUT rather than any other kind of COPY;
  if we want that to factor into the behavior, we need ProcessResult to
  do it.)

 Since we are considering stderr my (inexperienced admittedly) gut says that
 using stderr for this is generally undesirable and especially given our
 existing precedence.  stdout is the seemingly correct target, typically,
 and
 the existing quiet-mode toggle provides sufficient control for typical
 needs.


  5. Give up on the print-the-tag aspect of the change, and just fix the
  wrong-line-number issue (so we'd still introduce the copyStream variable,
  but not change how PGresults are passed around).
 
  I'm inclined to think #2 is the best answer if we can't stomach #1.
  But the exact rule for when to print a COPY OUT result probably
  still requires some debate.  Or maybe someone has another idea?
 
  Also, I'm thinking we should back-patch the aspects of the patch
  needed to fix the wrong-line-number issue.  That appears to have been
  introduced in 9.2; older versions of PG get the above example right.
 
  Comments?

 I'd like COPY TO to anything but STDOUT to emit a COPY n on STDOUT -
 unless suppressed by -q(uiet)


+1

This information can be really interesting and sometimes important, when
people has no idea, how they tables are long

Regards

Pavel



 Document that COPY TO STDOUT does not emit COPY n because STDOUT is
 already assigned for data and so is not available for notifications.  Since
 COPY is more typically used for ETL than a bare-select, in addition to
 back-compatibility concerns, this default behavior seems reasonable.

 Would it be possible to store the n somewhere and provide a command -
 like
 GET DIAGNOSTICS in pl/pgsql - if the user really wants to know how many
 rows
 were sent to STDOUT?  I'm doubt this is even useful in the typical use-case
 for COPY TO STDOUT but figured I'd toss the idea out there.

 Is there anything besides a desire for consistency that anyone has or can
 put forth as a use-case for COPY TO STDOUT emitting COPY n on STDOUT as
 well?  If you are going to view the content inline, and also want a quick
 count, ISTM you would be more likely to use SELECT to take advantage of all
 its pretty-print features.

 If we really need to cater to this use then maybe a --loud-copy-to-stdout
 switch can be provided to override its default quiet-mode.

 David J.





 --
 View this message in context:
 http://postgresql.1045698.n5.nabble.com/COPY-table-FROM-STDIN-doesn-t-show-count-tag-tp5775018p5795611.html
 Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.


 --
 Sent via pgsql-hackers mailing list 

Re: [HACKERS] COPY table FROM STDIN doesn't show count tag

2014-03-12 Thread Tom Lane
Rajeev rastogi rajeev.rast...@huawei.com writes:
 On 11 March 2014 19:52, Tom Lane wrote:
 After sleeping on it, I'm inclined to think we should continue to not
 print status for COPY TO STDOUT.  Aside from the risk of breaking
 scripts, there's a decent analogy to be made to SELECT: we don't print
 a status tag for that either.

 It is correct that SELECT does not print conventional way of status tag but 
 still it prints the number
 of rows selected (e.g. (2 rows)) along with rows actual value, which can be 
 very well considered
 as kind of status. User can make out with this result, that how many rows 
 have been selected.

 But in-case of COPY TO STDOUT, if we don't print anything, then user does not 
 have any direct way of finding
 that how many rows were copied from table to STDOUT, which might have been 
 very useful.

Uh, you mean other than the data rows that were just printed?  I fail
to see how this is much different from the SELECT case:

regression=# \copy int8_tbl to stdout
123 456
123 4567890123456789
4567890123456789123
45678901234567894567890123456789
4567890123456789-4567890123456789
regression=# 

(Note that I'm defining TO STDOUT from psql's perspective, ie the rows are
going to the queryFout file, which is the same place the COPY status would
get printed to.)

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] COPY table FROM STDIN doesn't show count tag

2014-03-12 Thread Tom Lane
David Johnston pol...@yahoo.com writes:
 Tom Lane-2 wrote
 1. Treat this as a non-backwards-compatible change, and document that
 people have to use -q if they don't want the COPY tag in the output.
 I'm not sure this is acceptable.

 I've mostly used copy to with files and so wouldn't mind if STDOUT had the
 COPY n sent to it as long as the target file is just the copy contents.

I think you're missing the point: the case I'm concerned about is exactly
that the target file is psql's stdout, or more specifically the same place
that the COPY status would get printed to.

 2. Kluge ProcessResult so that it continues to not pass back a PGresult
 for the COPY TO STDOUT case, or does so only in limited circumstances
 (perhaps only if isatty(stdout), for instance).

 The main problem with this is that people will test by sending output to a
 TTY and see the COPY n.  Although if it can be done consistently then you
 minimize backward incompatibility and encourage people to enforce quiet mode
 while the command runs...

Yeah, the inconsistency of behavior that this solution would cause is not
a good thing.  My inclination now (see later traffic) is to suppress the
status report when the COPY destination is the same as pset.queryFout
(ie, a simple test whether the FILE pointers are equal).  This would
suppress the status report for \copy to stdout and COPY TO STDOUT
cases, and also for \copy to pstdout if you'd not redirected queryFout
with \o.

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] COPY table FROM STDIN doesn't show count tag

2014-03-12 Thread Robert Haas
On Wed, Mar 12, 2014 at 12:09 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 David Johnston pol...@yahoo.com writes:
 Tom Lane-2 wrote
 1. Treat this as a non-backwards-compatible change, and document that
 people have to use -q if they don't want the COPY tag in the output.
 I'm not sure this is acceptable.

 I've mostly used copy to with files and so wouldn't mind if STDOUT had the
 COPY n sent to it as long as the target file is just the copy contents.

 I think you're missing the point: the case I'm concerned about is exactly
 that the target file is psql's stdout, or more specifically the same place
 that the COPY status would get printed to.

 2. Kluge ProcessResult so that it continues to not pass back a PGresult
 for the COPY TO STDOUT case, or does so only in limited circumstances
 (perhaps only if isatty(stdout), for instance).

 The main problem with this is that people will test by sending output to a
 TTY and see the COPY n.  Although if it can be done consistently then you
 minimize backward incompatibility and encourage people to enforce quiet mode
 while the command runs...

 Yeah, the inconsistency of behavior that this solution would cause is not
 a good thing.  My inclination now (see later traffic) is to suppress the
 status report when the COPY destination is the same as pset.queryFout
 (ie, a simple test whether the FILE pointers are equal).  This would
 suppress the status report for \copy to stdout and COPY TO STDOUT
 cases, and also for \copy to pstdout if you'd not redirected queryFout
 with \o.

This is reasonably similar to what we already do for SELECT, isn't it?
 I mean, the server always sends back a command tag, but psql
sometimes opts not to print it.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL 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] COPY table FROM STDIN doesn't show count tag

2014-03-12 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Wed, Mar 12, 2014 at 12:09 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 My inclination now (see later traffic) is to suppress the
 status report when the COPY destination is the same as pset.queryFout
 (ie, a simple test whether the FILE pointers are equal).  This would
 suppress the status report for \copy to stdout and COPY TO STDOUT
 cases, and also for \copy to pstdout if you'd not redirected queryFout
 with \o.

 This is reasonably similar to what we already do for SELECT, isn't it?
  I mean, the server always sends back a command tag, but psql
 sometimes opts not to print it.

Right, the analogy to SELECT gives some comfort that this is reasonable.

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] COPY table FROM STDIN doesn't show count tag

2014-03-11 Thread Rajeev rastogi
On 10 March 2014 23:44, Tom Lane wrote:

 Unfortunately, while testing it I noticed that there's a potentially
 fatal backwards-compatibility problem, namely that the COPY n status
 gets printed on stdout, which is the same place that COPY OUT data is
 going.  While this isn't such a big problem for interactive use, usages
 like this one are pretty popular:
 
psql -c 'copy mytable to stdout' mydatabase | some-program
 
 With the patch, COPY n gets included in the data sent to some-program,
 which never happened before and is surely not what the user wants.
 The same if the -c string uses \copy.
 
 There are several things we could do about this:
 
 1. Treat this as a non-backwards-compatible change, and document that
 people have to use -q if they don't want the COPY tag in the output.
 I'm not sure this is acceptable.
 
 2. Kluge ProcessResult so that it continues to not pass back a PGresult
 for the COPY TO STDOUT case, or does so only in limited circumstances
 (perhaps only if isatty(stdout), for instance).

 I'm inclined to think #2 is the best answer if we can't stomach #1.

Is it OK to have different status output for different flavor of COPY command? 
I am afraid that It will become kind of inconsistent result.

Also not providing the command result status may be inconsistent from 
behavior of any other SQL commands.

I agree that it breaks the backward compatibility but I am not sure if anyone 
is so tightly coupled with this ( or whether they will be effected with 
additional status result).

To me option #1 seems to be more suitable specially since there is an option to 
disable
the status output by giving -q.

Please provide your opinion or let me know If I have missed something.

Thanks and Regards,
Kumar Rajeev Rastogi




-- 
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] COPY table FROM STDIN doesn't show count tag

2014-03-11 Thread Tom Lane
Rajeev rastogi rajeev.rast...@huawei.com writes:
 On 10 March 2014 23:44, Tom Lane wrote:
 Unfortunately, while testing it I noticed that there's a potentially
 fatal backwards-compatibility problem, namely that the COPY n status
 gets printed on stdout, which is the same place that COPY OUT data is
 going.  While this isn't such a big problem for interactive use, usages
 like this one are pretty popular:
 
 psql -c 'copy mytable to stdout' mydatabase | some-program
 
 With the patch, COPY n gets included in the data sent to some-program,
 which never happened before and is surely not what the user wants.
 The same if the -c string uses \copy.

 Is it OK to have different status output for different flavor of COPY 
 command? 
 I am afraid that It will become kind of inconsistent result.

Well, that's the big question here.

We already do have different status output for different kinds of COPY,
ie we don't report it for COPY FROM STDIN/TO STDOUT.  What now emerges
is that there's a good reason for the omission in the case of TO STDOUT.
I certainly hadn't remembered that, and there's no documentation of it
in either code comments or the SGML docs.

After sleeping on it, I'm inclined to think we should continue to not
print status for COPY TO STDOUT.  Aside from the risk of breaking scripts,
there's a decent analogy to be made to SELECT: we don't print a status
tag for that either.

That leaves the question of whether we want to start printing a tag for
the COPY FROM STDIN case.  I don't think that'd create much risk of
breaking anything, and the analogy to SELECT doesn't hold either.
OTOH, Robert opined upthread that FROM STDIN and TO STDOUT shouldn't
behave differently; does that argument still impress anyone?  And given
that different COPY cases are still going to behave differently, maybe
we should just stick with the status quo.  It's been like this for a
mighty long time with few complaints.

In any case, some documentation and code comment changes would be
appropriate ...

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] COPY table FROM STDIN doesn't show count tag

2014-03-11 Thread Rajeev rastogi
On 11 March 2014 19:52, Tom Lane wrote:

 After sleeping on it, I'm inclined to think we should continue to not
 print status for COPY TO STDOUT.  Aside from the risk of breaking
 scripts, there's a decent analogy to be made to SELECT: we don't print
 a status tag for that either.

It is correct that SELECT does not print conventional way of status tag but 
still it prints the number
of rows selected (e.g. (2 rows)) along with rows actual value, which can be 
very well considered
as kind of status. User can make out with this result, that how many rows have 
been selected.

But in-case of COPY TO STDOUT, if we don't print anything, then user does not 
have any direct way of finding
that how many rows were copied from table to STDOUT, which might have been very 
useful.

Please let me know your opinion or if I have missed something.

Thanks and Regards,
Kumar Rajeev Rastogi





-- 
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] COPY table FROM STDIN doesn't show count tag

2014-03-10 Thread Tom Lane
Rajeev rastogi rajeev.rast...@huawei.com writes:
 On 12th December 2013, Rajeev Rastogi Wrote:
 On 9th December, Amit Khandelkar wrote:
 But copystream can be different than pset.cur_cmd_source , right ?

 As per the earlier code, condition result was always true. So pset.lineno 
 was always incremented.
 In the earlier code pset.cur_cmd_source was sent as parameter to function 
 and inside the function same parameter was used with the name copystream. So 
 on entry of this function both will be one and same.

The problem with that argument is you're assuming that the previous
behavior was correct :-(.  It isn't.  If you try a case like this:

$ cat int8data
123 456
123 4567890123456789
4567890123456789123
45678901234567894567890123456789
4567890123456789-4567890123456789
$ cat testcase.sql
select 1+1;

\copy int8_tbl from 'int8data'

select 1/0;

select 2/0;
$ psql -f testcase.sql regression
 ?column? 
--
2
(1 row)

psql:testcase.sql:11: ERROR:  division by zero
psql:testcase.sql:13: ERROR:  division by zero

the script line numbers shown in the error messages are *wrong*,
because handleCopyIn has incorrectly incremented pset.lineno because
it thought it was reading from the current script file.  So the
override_file business is wrong, and getting rid of it with a separate
copyStream variable is a good thing.

However, there wasn't much else that I liked about the patch :-(.
It seemed bizarre to me that the copy source/sink selection logic was
partially in ProcessResult and partially in handleCopyOut/handleCopyIn.
Also you'd created a memory leak because ProcessResult now failed to
PQclear the original PGRES_COPY_OUT/IN PGresult.  I did a bit of work
to clean that up, and the attached updated patch is the result.

Unfortunately, while testing it I noticed that there's a potentially
fatal backwards-compatibility problem, namely that the COPY n status
gets printed on stdout, which is the same place that COPY OUT data is
going.  While this isn't such a big problem for interactive use,
usages like this one are pretty popular:

 psql -c 'copy mytable to stdout' mydatabase | some-program

With the patch, COPY n gets included in the data sent to some-program,
which never happened before and is surely not what the user wants.
The same if the -c string uses \copy.

There are several things we could do about this:

1. Treat this as a non-backwards-compatible change, and document that
people have to use -q if they don't want the COPY tag in the output.
I'm not sure this is acceptable.

2. Kluge ProcessResult so that it continues to not pass back a PGresult
for the COPY TO STDOUT case, or does so only in limited circumstances
(perhaps only if isatty(stdout), for instance).

3. Modify PrintQueryStatus so that command status goes to stderr not
stdout.  While this is probably how it should've been done in the first
place, this would be a far more severe compatibility break than #1.
(For one thing, there are probably scripts out there that think that any
output to stderr is an error message.)  I'm afraid this one is definitely
not acceptable, though it would be by far the cleanest solution were it
not for compatibility concerns.

4. As #3, but print the command status to stderr only if it's COPY n,
otherwise to stdout.  This is a smaller compatibility break than #3,
but still a break since COPY status was formerly issued to stdout
in non TO STDOUT/FROM STDIN cases.  (Note that PrintQueryStatus can't
tell whether it was COPY TO STDOUT rather than any other kind of COPY;
if we want that to factor into the behavior, we need ProcessResult to
do it.)

5. Give up on the print-the-tag aspect of the change, and just fix the
wrong-line-number issue (so we'd still introduce the copyStream variable,
but not change how PGresults are passed around).

I'm inclined to think #2 is the best answer if we can't stomach #1.
But the exact rule for when to print a COPY OUT result probably
still requires some debate.  Or maybe someone has another idea?

Also, I'm thinking we should back-patch the aspects of the patch
needed to fix the wrong-line-number issue.  That appears to have been
introduced in 9.2; older versions of PG get the above example right.

Comments?

regards, tom lane

diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 3a820fa..136eed1 100644
*** a/src/bin/psql/common.c
--- b/src/bin/psql/common.c
*** StoreQueryTuple(const PGresult *result)
*** 628,638 
   * command.  In that event, we'll marshal data for the COPY and then cycle
   * through any subsequent PGresult objects.
   *
!  * When the command string contained no affected COPY command, this function
   * degenerates to an AcceptResult() call.
   *
!  * Changes its argument to point to the last PGresult of the command string,
!  * or NULL if that result was for a COPY FROM STDIN or COPY TO STDOUT.
   *
   * Returns true on complete success, false otherwise.  

Re: [HACKERS] COPY table FROM STDIN doesn't show count tag

2014-03-10 Thread Tom Lane
I wrote:
 Also, I'm thinking we should back-patch the aspects of the patch
 needed to fix the wrong-line-number issue.  That appears to have been
 introduced in 9.2; older versions of PG get the above example right.

I've done that.  For reference' sake, here's an updated patch against
HEAD with just the uncommitted changes.

regards, tom lane

diff -c new/common.c new-wholepatch/common.c
*** new/common.c	Mon Mar 10 14:55:49 2014
--- new-wholepatch/common.c	Mon Mar 10 12:49:02 2014
***
*** 631,638 
   * When the command string contained no such COPY command, this function
   * degenerates to an AcceptResult() call.
   *
!  * Changes its argument to point to the last PGresult of the command string,
!  * or NULL if that result was for a COPY FROM STDIN or COPY TO STDOUT.
   *
   * Returns true on complete success, false otherwise.  Possible failure modes
   * include purely client-side problems; check the transaction status for the
--- 631,637 
   * When the command string contained no such COPY command, this function
   * degenerates to an AcceptResult() call.
   *
!  * Changes its argument to point to the last PGresult of the command string.
   *
   * Returns true on complete success, false otherwise.  Possible failure modes
   * include purely client-side problems; check the transaction status for the
***
*** 641,654 
  static bool
  ProcessResult(PGresult **results)
  {
- 	PGresult   *next_result;
  	bool		success = true;
  	bool		first_cycle = true;
  
! 	do
  	{
  		ExecStatusType result_status;
  		bool		is_copy;
  
  		if (!AcceptResult(*results))
  		{
--- 640,653 
  static bool
  ProcessResult(PGresult **results)
  {
  	bool		success = true;
  	bool		first_cycle = true;
  
! 	for (;;)
  	{
  		ExecStatusType result_status;
  		bool		is_copy;
+ 		PGresult   *next_result;
  
  		if (!AcceptResult(*results))
  		{
***
*** 693,698 
--- 692,698 
  			 * otherwise use queryFout or cur_cmd_source as appropriate.
  			 */
  			FILE	   *copystream = pset.copyStream;
+ 			PGresult   *copy_result;
  
  			SetCancelConn();
  			if (result_status == PGRES_COPY_OUT)
***
*** 700,706 
  if (!copystream)
  	copystream = pset.queryFout;
  success = handleCopyOut(pset.db,
! 		copystream)  success;
  			}
  			else
  			{
--- 700,707 
  if (!copystream)
  	copystream = pset.queryFout;
  success = handleCopyOut(pset.db,
! 		copystream,
! 		copy_result)  success;
  			}
  			else
  			{
***
*** 708,737 
  	copystream = pset.cur_cmd_source;
  success = handleCopyIn(pset.db,
  	   copystream,
! 	   PQbinaryTuples(*results))  success;
  			}
  			ResetCancelConn();
  
! 			/*
! 			 * Call PQgetResult() once more.  In the typical case of a
! 			 * single-command string, it will return NULL.	Otherwise, we'll
! 			 * have other results to process that may include other COPYs.
! 			 */
  			PQclear(*results);
! 			*results = next_result = PQgetResult(pset.db);
  		}
  		else if (first_cycle)
  			/* fast path: no COPY commands; PQexec visited all results */
  			break;
- 		else if ((next_result = PQgetResult(pset.db)))
- 		{
- 			/* non-COPY command(s) after a COPY: keep the last one */
- 			PQclear(*results);
- 			*results = next_result;
  		}
  
  		first_cycle = false;
! 	} while (next_result);
  
  	/* may need this to recover from conn loss during COPY */
  	if (!first_cycle  !CheckConnection())
--- 709,742 
  	copystream = pset.cur_cmd_source;
  success = handleCopyIn(pset.db,
  	   copystream,
! 	   PQbinaryTuples(*results),
! 	   copy_result)  success;
  			}
  			ResetCancelConn();
  
! 			/* replace the COPY_OUT/IN result with COPY command exit status */
  			PQclear(*results);
! 			*results = copy_result;
  		}
  		else if (first_cycle)
+ 		{
  			/* fast path: no COPY commands; PQexec visited all results */
  			break;
  		}
  
+ 		/*
+ 		 * Check PQgetResult() again.  In the typical case of a single-command
+ 		 * string, it will return NULL.  Otherwise, we'll have other results
+ 		 * to process that may include other COPYs.  We keep the last result.
+ 		 */
+ 		next_result = PQgetResult(pset.db);
+ 		if (!next_result)
+ 			break;
+ 
+ 		PQclear(*results);
+ 		*results = next_result;
  		first_cycle = false;
! 	}
  
  	/* may need this to recover from conn loss during COPY */
  	if (!first_cycle  !CheckConnection())
diff -c new/copy.c new-wholepatch/copy.c
*** new/copy.c	Mon Mar 10 14:56:21 2014
--- new-wholepatch/copy.c	Mon Mar 10 12:50:27 2014
***
*** 429,444 
   * conn should be a database connection that you just issued COPY TO on
   * and got back a PGRES_COPY_OUT result.
   * copystream is the file stream for the data to go to.
   *
   * result is true if successful, false if not.
   */
  bool
! handleCopyOut(PGconn *conn, FILE *copystream)
  {
  	bool		OK = true;
  	

Re: [HACKERS] COPY table FROM STDIN doesn't show count tag

2013-12-12 Thread Rajeev rastogi
On 12th December 2013, Rajeev Rastogi Wrote:
On 9th December, Amit Khandelkar wrote:

1.  slashcopyissuev1.patch :- This patch fixes the \COPY issue.
You have removed the if condition in this statement, mentioning that it is 
always true now:
-   if (copystream == pset.cur_cmd_source)
-   pset.lineno++;
+   pset.lineno++;

 But copystream can be different than pset.cur_cmd_source , right ?

As per the earlier code, condition result was always true. So pset.lineno was 
always incremented.
In the earlier code pset.cur_cmd_source was sent as parameter to function and 
inside the function same parameter was used with the name copystream. So on 
entry of this function both will be one and same.
I checked inside the function handleCopyIn, both of these parameters are not 
changing before above check. Also since pset is specific to single session, so 
it cannot change concurrently.
Please let me know, if I am missing something.

+   FILE   *copyStream; /* Stream to read/write for copy 
command */

There is no tab between FILE and *copystream, hence it is not aligned.

OK. I shall change accodingly.

I ran pgindent on settings.h file but found no issue reported. Seems tab is not 
mandatory in between variable declaration.
Even for other parameters also in psqlSetting structure space instead of tab is 
being used.
So seems no change required for this. Please confirm.


2.  initialcopyissuev1_ontopofslashcopy.patch : Fix for COPY table FROM 
STDIN/STDOUT doesn't show count tag.
The following header comments of ProcessResult() need to be modified:
* Changes its argument to point to the last PGresult of the command string,
* or NULL if that result was for a COPY FROM STDIN or COPY TO STDOUT.

OK. I shall change accodingly.
I have changed it in the latest patch.

Regression results show all passed.
Other than this, the patch needs a new regression test.

I had checked the existing regression test cases and observed that it has 
already got all kind of test cases. Like
copystdin,
copystdout,
\copy.stdin
\copy.stdout.

But since as regression framework runs in quite i.e. -q mode, so it does not 
show any message except query output.
So our new code change does not impact regression framework.

Please let me know if you were expecting any other test cases?

Summary of two patches:

1.  slashcopyissuev1.patch :- No change in this patch (same as earlier).

2.  Initialcopyissuev2_ontopofslashcopy.patch : This patch is modified to 
change comment as per above review comments.

Please provide your opinion or let me know if any other changes are required.

Thanks and Regards,
Kumar Rajeev Rastogi




slashcopyissuev1.patch
Description: slashcopyissuev1.patch


initialcopyissuev2_ontopofslashcopy.patch
Description: initialcopyissuev2_ontopofslashcopy.patch

-- 
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] COPY table FROM STDIN doesn't show count tag

2013-12-10 Thread Rajeev rastogi
On 9th December, Amit Khandelkar wrote:

1.  slashcopyissuev1.patch :- This patch fixes the \COPY issue.
You have removed the if condition in this statement, mentioning that it is 
always true now:
-   if (copystream == pset.cur_cmd_source)
-   pset.lineno++;
+   pset.lineno++;

 But copystream can be different than pset.cur_cmd_source , right ?

As per the earlier code, condition result was always true. So pset.lineno was 
always incremented.
In the earlier code pset.cur_cmd_source was sent as parameter to function and 
inside the function same parameter was used with the name copystream. So on 
entry of this function both will be one and same.
I checked inside the function handleCopyIn, both of these parameters are not 
changing before above check. Also since pset is specific to single session, so 
it cannot change concurrently.
Please let me know, if I am missing something.

+   FILE   *copyStream; /* Stream to read/write for copy 
command */

There is no tab between FILE and *copystream, hence it is not aligned.

OK. I shall change accodingly.


2.  initialcopyissuev1_ontopofslashcopy.patch : Fix for COPY table FROM 
STDIN/STDOUT doesn't show count tag.
The following header comments of ProcessResult() need to be modified:
* Changes its argument to point to the last PGresult of the command string,
* or NULL if that result was for a COPY FROM STDIN or COPY TO STDOUT.

OK. I shall change accodingly.

Regression results show all passed.
Other than this, the patch needs a new regression test.

I had checked the existing regression test cases and observed that it has 
already got all kind of test cases. Like
copystdin,
copystdout,
\copy.stdin
\copy.stdout.

But since as regression framework runs in quite i.e. -q mode, so it does not 
show any message except query output.
So our new code change does not impact regression framework.

Please let me know if you were expecting any other test cases?

I don't think we need to do any doc changes, because the doc already mentions 
that COPY should show the COUNT tag, and does not mention anything specific to 
client-side COPY.
 OK.

Please provide you opinion, based on which I shall prepare new patch and share 
the same.

Thanks and Regards,
Kumar Rajeev Rastogi



Re: [HACKERS] COPY table FROM STDIN doesn't show count tag

2013-12-08 Thread Amit Khandekar
On 29 November 2013 19:20, Rajeev rastogi rajeev.rast...@huawei.com wrote:

  On 26 November 2013, Amit Khandelkar wrote:

 Can you please submit the \COPY patch as a separate patch ? Since these
 are two different issues, I would like to have these two fixed and
 committed separately. You can always test the \COPY issue using \COPY TO
 followed by INSERT.



 Please find the attached two separate patches:


Thanks.



 1.  slashcopyissuev1.patch :- This patch fixes the \COPY issue.

You have removed the if condition in this statement, mentioning that it is
always true now:
-   if (copystream == pset.cur_cmd_source)
-   pset.lineno++;
+   pset.lineno++;

 But copystream can be different than pset.cur_cmd_source , right ?


+   FILE   *copyStream; /* Stream to read/write for copy
command */

There is no tab between FILE and *copystream, hence it is not aligned.


 2.  initialcopyissuev1_ontopofslashcopy.patch : Fix for “COPY table
 FROM STDIN/STDOUT doesn't show count tag”.


The following header comments of ProcessResult() need to be modified:
* Changes its argument to point to the last PGresult of the command string,
* or NULL if that result was for a COPY FROM STDIN or COPY TO STDOUT.


Regression results show all passed.

Other than this, the patch needs a new regression test.

I don't think we need to do any doc changes, because the doc already
mentions that COPY should show the COUNT tag, and does not mention anything
specific to client-side COPY.




 Thanks and Regards,

 Kumar Rajeev Rastogi







Re: [HACKERS] COPY table FROM STDIN doesn't show count tag

2013-11-29 Thread Amit Khandekar
On 27 November 2013 09:59, Rajeev rastogi rajeev.rast...@huawei.com wrote:

  On 26 November 2013, Amit Khandelkar wrote:


 On 26 November 2013 18:59, Amit Khandekar amit.khande...@enterprisedb.com
  wrote:




 On 25 November 2013 15:25, Rajeev rastogi rajeev.rast...@huawei.com
  wrote:

 OK. I have revised the patch as per the discussion.

 Could you please submit only the \COPY fix first ? The attached patch
 also contains the fix for the original COPY status fix.

 Can you please submit the \COPY patch as a separate patch ? Since these
are two different issues, I would like to have these two fixed and
committed separately. You can always test the \COPY issue using \COPY TO
followed by INSERT.


Re: [HACKERS] COPY table FROM STDIN doesn't show count tag

2013-11-29 Thread Rajeev rastogi
On 26 November 2013, Amit Khandelkar wrote:
Can you please submit the \COPY patch as a separate patch ? Since these are 
two different issues, I would like to have these two fixed and committed 
separately. You can always test the \COPY issue using \COPY TO followed by 
INSERT.

Please find the attached two separate patches:


1.  slashcopyissuev1.patch :- This patch fixes the \COPY issue.

2.  initialcopyissuev1_ontopofslashcopy.patch : Fix for COPY table FROM 
STDIN/STDOUT doesn't show count tag.

Thanks and Regards,
Kumar Rajeev Rastogi




slashcopyissuev1.patch
Description: slashcopyissuev1.patch


initialcopyissuev1_ontopofslashcopy.patch
Description: initialcopyissuev1_ontopofslashcopy.patch

-- 
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] COPY table FROM STDIN doesn't show count tag

2013-11-26 Thread Amit Khandekar
On 25 November 2013 15:25, Rajeev rastogi rajeev.rast...@huawei.com wrote:

  OK. I have revised the patch as per the discussion.

Could you please submit only the \COPY fix first ? The attached patch also
contains the fix for the original COPY status fix.

Now if \copy command is called then, we are setting the appropriate value
 of _psqlSettings-copystream in do_copy and same is being used inside
 handleCopyIn() and handleCopyOut(). Once the \copy command execution
 finishes, we are resetting the value of _psqlSettings-copystream to NULL.
 And if COPY(No slash) command is used, then in that case
 _psqlSettings-copystream will be NULL. So based on this value being NULL,
 copyStream will be assigned as stdout/stdin depending on TO/FROM
 respectively inside the function handleCopyOut()/handleCopyIn().



 Also in order to address the queries like

 *./psql -d postgres -c \copy tbl to
 '/home/rajeev/9.4gitcode/install/bin/data/temp.txt'; copy tbl from stdin;*

 Inside the function ProcessResult, we check that if it is the second cycle
 and result status is COPY OUT or IN, then we reset the value of
 _psqlSettings-copystream to NULL, so that it can take the value as
 stdout/stdin for further processing.




Yes, that's right, the second cycle should not use pset.copyStream.


handleCopyOut(PGconn *conn, FILE *copystream, PGresult **res)
 {
boolOK = true;
char   *buf;
int ret;
-   PGresult   *res;
+
+   if (!copystream)
+   copystream = stdout;

It should use pset.queryFout if it's NULL. Same in hadleCopyIn().
Otherwise, the result of the following command goes to stdout, when it
should go to the output file :
psql -d postgres -o /tmp/p.out -c copy tab to stdout


+   /*
+* If this is second copy; then it will be
definately not \copy,
+* and also it can not be from any user
given file.
+* So reset the value of copystream to
NULL, so that read/wrie
+* happens from stdin/stdout.
+*/
+   if (!first_cycle)
+   pset.copyStream = NULL;

Let ProcessResult() not change pset.copyStream. Let only do_copy() update
it. Instead of the above location, I suggest, just before calling
handleCopyOut/In(), we decide what to pass them as their copyStream
parameter depending upon whether it is first cycle or not.





  Please provide your opinion.



 Thanks and Regards,

 Kumar Rajeev Rastogi





Re: [HACKERS] COPY table FROM STDIN doesn't show count tag

2013-11-26 Thread Rajeev rastogi
On 26 November 2013, Amit Khandelkar wrote:
Now if \copy command is called then, we are setting the appropriate value of 
_psqlSettings-copystream in do_copy and same is being used inside 
handleCopyIn() and handleCopyOut(). Once the \copy command execution 
finishes, we are resetting
 the value of _psqlSettings-copystream to NULL. And if COPY(No slash) 
 command is used, then in that case _psqlSettings-copystream will be NULL. 
 So based on this value being NULL, copyStream will be assigned as 
 stdout/stdin depending on
TO/FROM respectively inside the function handleCopyOut()/handleCopyIn().
 Also in order to address the queries like
./psql -d postgres -c \copy tbl to 
'/home/rajeev/9.4gitcode/install/bin/data/temp.txt'; copy tbl from stdin;
Inside the function ProcessResult, we check that if it is the second cycle 
and result status is COPY OUT or IN, then we reset the value of 
_psqlSettings-copystream to NULL, so that it can take the value as 
stdout/stdin for further processing.


Yes, that's right, the second cycle should not use pset.copyStream.


handleCopyOut(PGconn *conn, FILE *copystream, PGresult **res)
 {
boolOK = true;
char   *buf;
int ret;
-   PGresult   *res;
+
+   if (!copystream)
+   copystream = stdout;

It should use pset.queryFout if it's NULL. Same in hadleCopyIn(). Otherwise, 
the result of the following command goes to stdout, when it should go to the 
output file :
psql -d postgres -o /tmp/p.out -c copy tab to stdout
Yes you are right, I have changed it accordingly.


+   /*
+* If this is second copy; then it will be 
definately not \copy,
+* and also it can not be from any user given 
file.
+* So reset the value of copystream to NULL, 
so that read/wrie
+* happens from stdin/stdout.
+*/
+   if (!first_cycle)
+   pset.copyStream = NULL;

Let ProcessResult() not change pset.copyStream. Let only do_copy() update it. 
Instead of the above location, I suggest, just before calling 
handleCopyOut/In(), we decide what to pass them as their copyStream parameter 
depending upon whether it is
first cycle or not.
OK. I have changed as per your suggestion.

Also I had removed the below line
if (copystream == pset.cur_cmd_source)
from the function handleCopyIn in my last patch itself. Reason for removal is 
that as per the earlier code the condition result was always true.

Please provide your opinion.

Thanks and Regards,
Kumar Rajeev Rastogi







copyerrorV5.patch
Description: copyerrorV5.patch

-- 
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] COPY table FROM STDIN doesn't show count tag

2013-11-25 Thread Rajeev rastogi
On 25 November 2013, Amit Khandekar 
amit.khande...@enterprisedb.commailto:amit.khande...@enterprisedb.com wrote:
Ok. we will then first fix the \COPY TO issue where it does not revert back 
the overriden psql output file handle. Once this is solved, fix for both 
COPY FROM and COPY TO, like how it is done in the patch earlier 
(copydefectV2.patch).

I analyzed the solution to fix \COPY TO issue but unfortunately I observed 
that do_copy is already resetting the value of cur_cmd_source and queryFout 
but before that itself result status is printed. So we'll have to reset the 
value before result status is
being displayed.
So as other alternative solutions, I have two approaches:

1.  We can store current file destination queryFout in some local 
variable and pass the same to SendQuery function as a parameter. Same can be 
used to reset the value of queryFout after return from ProcessResult

From all other callers of SendQuery , we can pass NULL value for this new 
parameter.
 2.  We can add new structure member variable FILE *prevQueryFout in 
 structure struct _psqlSettings, which hold the value of queryFout before 
 being changed in do_copy. And then same can be used to reset value in 
 SendQuery or ProcessResult.

I think approach #2 is fine. Rather than prevQueryFout, I suggest defining a 
separate FILE * handle for COPY. I don't see any other client-side command 
that uses its own file pointer for reading and writing, like how COPY does. 
And this handle has
 nothing to do with pset stdin and stdout. So we can have this special 
 _psqlSettings-copystream specifically for COPY. Both handleCopyIn() and 
 handleCopyOut() will be passed pset.copystream. In do_copy(),  instead of 
 overriding
pset.queryFout, we can set pset.copystream to copystream, or to stdin/stdout 
if copystream is NULL.

OK. I have revised the patch as per the discussion. Now if \copy command is 
called then, we are setting the appropriate value of _psqlSettings-copystream 
in do_copy and same is being used inside handleCopyIn() and handleCopyOut(). 
Once the \copy command execution finishes, we are resetting the value of 
_psqlSettings-copystream to NULL. And if COPY(No slash) command is used, then 
in that case _psqlSettings-copystream will be NULL. So based on this value 
being NULL, copyStream will be assigned as stdout/stdin depending on TO/FROM 
respectively inside the function handleCopyOut()/handleCopyIn().

Also in order to address the queries like
./psql -d postgres -c \copy tbl to 
'/home/rajeev/9.4gitcode/install/bin/data/temp.txt'; copy tbl from stdin;
Inside the function ProcessResult, we check that if it is the second cycle and 
result status is COPY OUT or IN, then we reset the value of 
_psqlSettings-copystream to NULL, so that it can take the value as 
stdout/stdin for further processing.

Please provide your opinion.

Thanks and Regards,
Kumar Rajeev Rastogi



copyerrorV4.patch
Description: copyerrorV4.patch

-- 
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] COPY table FROM STDIN doesn't show count tag

2013-11-24 Thread Amit Khandekar
On 22 November 2013 16:14, Rajeev rastogi rajeev.rast...@huawei.com wrote:

  On 21 November 2013, Amit Khandekar amit.khande...@enterprisedb.com
 wrote:

 Ok. we will then first fix the \COPY TO issue where it does not revert
 back the overriden psql output file handle. Once this is solved, fix for
 both COPY FROM and COPY TO, like how it is done in the patch earlier (
 copydefectV2.patch).



 I analyzed the solution to fix \COPY TO issue but unfortunately I observed
 that *do_copy* is already resetting the value of *cur_cmd_source and
 queryFout* but before that itself result status is printed. So we’ll have
 to reset the value before result status is being displayed.



 So as other alternative solutions, I have two approaches:

 1.  We can store current file destination *queryFout *in some local
 variable and pass the same to *SendQuery* function as a parameter. Same
 can be used to reset the value of queryFout after return from ProcessResult

 From all other callers of SendQuery , we can pass NULL value for this new
 parameter.



 2.  We can add new structure member variable FILE *prevQueryFout in
 structure “struct _*psqlSettings”, *which hold the value of queryFout
 before being changed in do_copy. And then same can be used to reset value
 in SendQuery or ProcessResult.

I think approach #2 is fine. Rather than prevQueryFout, I suggest defining
a separate FILE * handle for COPY. I don't see any other client-side
command that uses its own file pointer for reading and writing, like how
COPY does. And this handle has nothing to do with pset stdin and stdout. So
we can have this special _psqlSettings-copystream specifically for COPY.
Both handleCopyIn() and handleCopyOut() will be passed pset.copystream. In
do_copy(),  instead of overriding pset.queryFout, we can set
pset.copystream to copystream, or to stdin/stdout if copystream is NULL.



 Please let me know which approach is OK or if any other approach suggested.

 Based on feedback I shall prepare the new patch and share the same.



 Thanks and Regards,

 Kumar Rajeev Rastogi







Re: [HACKERS] COPY table FROM STDIN doesn't show count tag

2013-11-22 Thread Rajeev rastogi
On 19 November 2013, Amit Khandekar wrote:
On 18 November 2013 18:00, Rajeev rastogi 
rajeev.rast...@huawei.commailto:rajeev.rast...@huawei.com wrote:
On 18 November 2013, Amit Khandekar wrote:
 Please find the patch for the same and let me know your suggestions.
In this call :
  success = handleCopyIn(pset.db, 
  pset.cur_cmd_source,
 
 PQbinaryTuples(*results), intres)  success;
  if (success  intres)
  success = 
  PrintQueryResults(intres);
Instead of handling of the result status this way, what if we use the 
ProcessResult()  argument 'result' to pass back the COPY result status to 
the caller ? We already call PrintQueryResults(results) after the 
ProcessResult() call. So we don't have to  have a COPY-specific 
PrintQueryResults() call. Also, if there is a subsequent SQL command in the 
same query string, the consequence of the patch is that the client prints 
both COPY output and the last command output. So my suggestion would also 
allow us to be consistent with the general behaviour that only the last SQL 
command output is printed in case of multiple SQL commands. Here is how it 
gets printed with your patch :
 Thank you for valuable comments. Your suggestion is absolutely correct.
 psql -d postgres -c \copy tab from '/tmp/st.sql' delimiter ' '; insert 
 into tab values ('lll', 3)
COPY 1
INSERT 0 1
This is not harmful, but just a matter of consistency.
I hope you meant to write test case as psql -d postgres -c \copy tab from 
stdin; insert into tab values ('lll', 3), as if we are reading from file, 
then the above issue does not come.
I meant COPY with a slash. \COPY is equivalent to COPY FROM STDIN. So the 
issue can also be reproduced by :
\COPY tab from 'client_filename' ...

 I have modified the patch as per your comment and same is attached with this 
 mail.

Thanks. The COPY FROM looks good.
OK..Thanks

With the patch applied, \COPY TO 'data_file' command outputs the  COPY status 
into the data file, instead of printing it in the psql session.

postgres=# \copy tab to '/tmp/fout';
postgres=#

$ cat /tmp/fout
ee  909
COPY 1
This is probably because client-side COPY overrides the pset.queryFout with 
its own destination file, and while printing the COPY status, the overridden 
file pointer is not yet reverted back.

This looks to be an issue without our new patch also. Like I tried following 
command and output was as follows:
rajeev@linux-ltr9:~/9.4gitcode/install/bin ./psql -d postgres -c \copy tbl to 
'new.txt';insert into tbl values(55);
rajeev@linux-ltr9:~/9.4gitcode/install/bin cat new.txt
5
67
5
67
2
2
99
1
1
INSERT 0 1

I have fixed the same as per your suggestion by resetting the pset.queryFout 
after the function call handleCopyOut.

Please let me know in-case of any other issues.

Thanks and Regards,
Kumar Rajeev Rastogi


copydefectV3.patch
Description: copydefectV3.patch

-- 
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] COPY table FROM STDIN doesn't show count tag

2013-11-22 Thread Rajeev rastogi
On 20 November, Amit Khandekar wrote:
I hope you meant to write test case as psql -d postgres -c \copy tab from 
stdin; insert into tab values ('lll', 3), as if we are reading from file, 
then the above issue does not come.
I meant COPY with a slash. \COPY is equivalent to COPY FROM STDIN. So the 
issue can also be reproduced by :
\COPY tab from 'client_filename' ...

 I have modified the patch as per your comment and same is attached with 
 this mail.

Thanks. The COPY FROM looks good.
OK..Thanks
With the patch applied, \COPY TO 'data_file' command outputs the  COPY 
status into the data file, instead of printing it in the psql session.
postgres=# \copy tab to '/tmp/fout';
postgres=#
 $ cat /tmp/fout
ee  909
COPY 1
This is probably because client-side COPY overrides the pset.queryFout with 
its own destination file, and while printing the COPY status, the overridden 
file pointer is not yet reverted back.
 This looks to be an issue without our new patch also. Like I tried following 
 command and output was as follows:
rajeev@linux-ltr9:~/9.4gitcode/install/binmailto:rajeev@linux-ltr9:~/9.4gitcode/install/bin
 ./psql -d postgres -c \copy tbl to 'new.txt';insert into tbl values(55);
rajeev@linux-ltr9:~/9.4gitcode/install/binmailto:rajeev@linux-ltr9:~/9.4gitcode/install/bin
 cat new.txt
5
67
5
67
2
2
99
1
1
INSERT 0 1

Ok. Yes it is an existing issue. Because we are now printing the COPY status 
even for COPY TO, the existing issue surfaces too easily with the patch. \COPY 
TO is a pretty common scenario. And it does not have to have a subsequent 
another command
to reproduce the issue Just a single \COPY TO command reproduces the issue.
I have fixed the same as per your suggestion by resetting the pset.queryFout 
after the function call handleCopyOut.
!  pset.queryFout = stdout;

The original pset.queryFout may not be stdout. psql -o option can override the 
stdout default.

I think solving the \COPY TO part is going to be a  different (and an 
involved) issue to solve than the COPY FROM. Even if we manage to revert back 
the queryFout, I think ProcessResult() is not the right place to do it. 
ProcessResult() should not
 assume that  somebody else has changed queryFout. Whoever has changed it 
 should revert it. Currently, do_copy() is indeed doing this correctly:

  save_file = *override_file;
  *override_file = copystream;
  success = SendQuery(query.data);
  *override_file = save_file;

But the way SendQuery() itself processes the results and prints them, is 
conflicting with the above.

So I think it is best to solve this as a different issue, and we should , for 
this commitfest,  fix only COPY FROM. Once the \COPY existing issue is solved, 
only then we can start printing the \COPY TO status as well.

You mean to say that I should change the patch to keep only COPY FROM related 
changes and remove changes related to COPY TO.
If yes, then I shall change the patch accordingly  and also mention same in 
documentation also.
Please let me know about this so that I can share the modified patch.

Thanks and Regards,
Kumar Rajeev Rastogi


Re: [HACKERS] COPY table FROM STDIN doesn't show count tag

2013-11-22 Thread Rajeev rastogi
On 21 November 2013, Amit Khandekar 
amit.khande...@enterprisedb.commailto:amit.khande...@enterprisedb.com wrote:
Ok. we will then first fix the \COPY TO issue where it does not revert back 
the overriden psql output file handle. Once this is solved, fix for both COPY 
FROM and COPY TO, like how it is done in the patch earlier 
(copydefectV2.patch).

I analyzed the solution to fix \COPY TO issue but unfortunately I observed that 
do_copy is already resetting the value of cur_cmd_source and queryFout but 
before that itself result status is printed. So we'll have to reset the value 
before result status is being displayed.

So as other alternative solutions, I have two approaches:

1.  We can store current file destination queryFout in some local variable 
and pass the same to SendQuery function as a parameter. Same can be used to 
reset the value of queryFout after return from ProcessResult

From all other callers of SendQuery , we can pass NULL value for this new 
parameter.

2.  We can add new structure member variable FILE *prevQueryFout in 
structure struct _psqlSettings, which hold the value of queryFout before 
being changed in do_copy. And then same can be used to reset value in SendQuery 
or ProcessResult.

Please let me know which approach is OK or if any other approach suggested.
Based on feedback I shall prepare the new patch and share the same.

Thanks and Regards,
Kumar Rajeev Rastogi




Re: [HACKERS] COPY table FROM STDIN doesn't show count tag

2013-11-20 Thread Amit Khandekar
On 19 November 2013 16:05, Rajeev rastogi rajeev.rast...@huawei.com wrote:

  On 19 November 2013, Amit Khandekar wrote:

 On 18 November 2013 18:00, Rajeev rastogi rajeev.rast...@huawei.com
 wrote:

 On 18 November 2013, Amit Khandekar wrote:

  Please find the patch for the same and let me know your suggestions.

 In this call :

   success = handleCopyIn(pset.db,
 pset.cur_cmd_source,

  
   PQbinaryTuples(*results), intres)  success;

   if (success  intres)

   success =
 PrintQueryResults(intres);

 Instead of handling of the result status this way, what if we use the
 ProcessResult()  argument 'result' to pass back the COPY result status to
 the caller ? We already call PrintQueryResults(results) after the
 ProcessResult() call. So we don't have to  have a COPY-specific
 PrintQueryResults() call. Also, if there is a subsequent SQL command in the
 same query string, the consequence of the patch is that the client prints
 both COPY output and the last command output. So my suggestion would also
 allow us to be consistent with the general behaviour that only the last SQL
 command output is printed in case of multiple SQL commands. Here is how it
 gets printed with your patch :

  Thank you for valuable comments. Your suggestion is absolutely correct.

  psql -d postgres -c \copy tab from '/tmp/st.sql' delimiter ' ';
 insert into tab values ('lll', 3)

 COPY 1

 INSERT 0 1

 This is not harmful, but just a matter of consistency.

 I hope you meant to write test case as *psql -d postgres -c \copy tab
 from stdin; insert into tab values ('lll', 3), *as if we are reading
 from file, then the above issue does not come.

 I meant COPY with a slash. \COPY is equivalent to COPY FROM STDIN. So the
 issue can also be reproduced by :

 \COPY tab from 'client_filename' ...



  I have modified the patch as per your comment and same is attached with
 this mail.



 Thanks. The COPY FROM looks good.

 OK..Thanks



 With the patch applied, \COPY TO 'data_file' command outputs the  COPY
 status into the data file, instead of printing it in the psql session.



 postgres=# \copy tab to '/tmp/fout';

 postgres=#



 $ cat /tmp/fout

 ee  909

 COPY 1

 This is probably because client-side COPY overrides the pset.queryFout
 with its own destination file, and while printing the COPY status,
 the overridden file pointer is not yet reverted back.



 This looks to be an issue without our new patch also. Like I tried
 following command and output was as follows:

 rajeev@linux-ltr9:~/9.4gitcode/install/bin ./psql -d postgres -c \copy
 tbl to 'new.txt';insert into tbl values(55);

 rajeev@linux-ltr9:~/9.4gitcode/install/bin cat new.txt

 5

 67

 5

 67

 2

 2

 99

 1

 1

 INSERT 0 1


Ok. Yes it is an existing issue. Because we are now printing the COPY
status even for COPY TO, the existing issue surfaces too easily with the
patch. \COPY TO is a pretty common scenario. And it does not have to have a
subsequent another command to reproduce the issue Just a single \COPY TO
command reproduces the issue.



 I have fixed the same as per your suggestion by resetting the
 pset.queryFout after the function call “handleCopyOut”.


! pset.queryFout = stdout;

The original pset.queryFout may not be stdout. psql -o option can override
the stdout default.

I think solving the \COPY TO part is going to be a  different (and an
involved) issue to solve than the COPY FROM. Even if we manage to revert
back the queryFout, I think ProcessResult() is not the right place to do
it. ProcessResult() should not assume that  somebody else has changed
queryFout. Whoever has changed it should revert it. Currently, do_copy() is
indeed doing this correctly:

save_file = *override_file;
*override_file = copystream;
success = SendQuery(query.data);
 *override_file = save_file;

But the way SendQuery() itself processes the results and prints them, is
conflicting with the above.

So I think it is best to solve this as a different issue, and we should ,
for this commitfest,  fix only COPY FROM. Once the \COPY existing issue is
solved, only then we can start printing the \COPY TO status as well.




 Please let me know in-case of any other issues.



 Thanks and Regards,

 Kumar Rajeev Rastogi



Re: [HACKERS] COPY table FROM STDIN doesn't show count tag

2013-11-20 Thread Amit Khandekar
On 20 November 2013 17:40, Rajeev rastogi rajeev.rast...@huawei.com wrote:

 You mean to say that I should change the patch to keep only COPY FROM
 related changes and remove changes related to COPY TO.

 If yes, then I shall change the patch accordingly  and also mention same
 in documentation also.

 Please let me know about this so that I can share the modified patch.


RIght.




 Thanks and Regards,

 Kumar Rajeev Rastogi



Re: [HACKERS] COPY table FROM STDIN doesn't show count tag

2013-11-20 Thread Robert Haas
On Wed, Nov 20, 2013 at 4:56 AM, Amit Khandekar
amit.khande...@enterprisedb.com wrote:
 So I think it is best to solve this as a different issue, and we should ,
 for this commitfest,  fix only COPY FROM. Once the \COPY existing issue is
 solved, only then we can start printing the \COPY TO status as well.

I actually think that we should probably fix the \COPY issue first.
Otherwise, we may end up (for example) changing COPY FROM in one
release and COPY TO in the next release, and that would be annoying.
It does cause application compatibility problems to some degree when
we change things like this, so it's useful to avoid doing it multiple
times.  And I can't really see a principled reason for COPY FROM and
COPY TO to behave differently, either.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL 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] COPY table FROM STDIN doesn't show count tag

2013-11-20 Thread Amit Khandekar
On 20 November 2013 18:11, Robert Haas robertmh...@gmail.com wrote:

 On Wed, Nov 20, 2013 at 4:56 AM, Amit Khandekar
 amit.khande...@enterprisedb.com wrote:
  So I think it is best to solve this as a different issue, and we should ,
  for this commitfest,  fix only COPY FROM. Once the \COPY existing issue
 is
  solved, only then we can start printing the \COPY TO status as well.

 I actually think that we should probably fix the \COPY issue first.
 Otherwise, we may end up (for example) changing COPY FROM in one
 release and COPY TO in the next release, and that would be annoying.
 It does cause application compatibility problems to some degree when
 we change things like this, so it's useful to avoid doing it multiple
 times.  And I can't really see a principled reason for COPY FROM and
 COPY TO to behave differently, either.


Rather than a behaviour change, it is a bug that we are fixing. User
already expects to see copy status printed, so as per user there would be
no behaviour change.

So the idea is to fix it in two places independently. Whatever fix we are
doing for COPY FROM , we would not revert or change that fix when we fix
the COPY TO issue. The base changes will go in COPY FROM fix, and then we
will be extending (not rewriting) the fix for COPY TO after fixing the
\COPY-TO issue.

Also, we already have a fix ready for COPY FROM, so I thought we better
commit this first.


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



Re: [HACKERS] COPY table FROM STDIN doesn't show count tag

2013-11-20 Thread Tom Lane
Amit Khandekar amit.khande...@enterprisedb.com writes:
 Rather than a behaviour change, it is a bug that we are fixing. User
 already expects to see copy status printed, so as per user there would be
 no behaviour change.

This is arrant nonsense.  It's a behavior change.  You can't make it
not that by claiming something about user expectations.  Especially
since this isn't exactly a corner case that nobody has seen in
the fifteen years or so that it's worked like that.  People do know
how this works.

I don't object to changing it, but I do agree with Robert that it's
important to quantize such changes, ie, try to get a set of related
changes to appear in the same release.  People don't like repeatedly
revising their code for such things.

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] COPY table FROM STDIN doesn't show count tag

2013-11-20 Thread Amit Khandekar
On 21 November 2013 10:13, Tom Lane t...@sss.pgh.pa.us wrote:

 Amit Khandekar amit.khande...@enterprisedb.com writes:
  Rather than a behaviour change, it is a bug that we are fixing. User
  already expects to see copy status printed, so as per user there would be
  no behaviour change.

 This is arrant nonsense.  It's a behavior change.  You can't make it
 not that by claiming something about user expectations.  Especially
 since this isn't exactly a corner case that nobody has seen in
 the fifteen years or so that it's worked like that.  People do know
 how this works.


Yes, I agree that this is not a corner case, so users may already know the
current behaviour.


 I don't object to changing it, but I do agree with Robert that it's
 important to quantize such changes, ie, try to get a set of related
 changes to appear in the same release.  People don't like repeatedly
 revising their code for such things.


Ok. we will then first fix the \COPY TO issue where it does not revert back
the overriden psql output file handle. Once this is solved, fix for both
COPY FROM and COPY TO, like how it is done in the patch earlier (
copydefectV2.patch).


 regards, tom lane



Re: [HACKERS] COPY table FROM STDIN doesn't show count tag

2013-11-18 Thread Amit Khandekar
On 18 October 2013 17:07, Rajeev rastogi rajeev.rast...@huawei.com wrote:

  From the following mail, copy behaviour between stdin and normal file
 having some inconsistency.


 http://www.postgresql.org/message-id/ce85a517.4878e%tim.k...@gmail.com



 The issue was that if copy  execute from stdin, then it goes to the
 server to execute the command and then server request for the input, it
 sends back the control to client to enter the data. So once client sends
 the input to server, server execute the copy command and sends back the
 result to client but client does not print the result instead it just clear
 it out.

 Changes are made to ensure the final result from server get printed before
 clearing the result.



 Please find the patch for the same and let me know your suggestions.


In this call :
success = handleCopyIn(pset.db, pset.cur_cmd_source,
   PQbinaryTuples(*results), intres)  success;

if (success  intres)
 success = PrintQueryResults(intres);

Instead of handling of the result status this way, what if we use the
ProcessResult()  argument 'result' to pass back the COPY result status to
the caller ? We already call PrintQueryResults(results) after the
ProcessResult() call. So we don't have to have a COPY-specific
PrintQueryResults() call. Also, if there is a subsequent SQL command in the
same query string, the consequence of the patch is that the client prints
both COPY output and the last command output. So my suggestion would also
allow us to be consistent with the general behaviour that only the last SQL
command output is printed in case of multiple SQL commands. Here is how it
gets printed with your patch :

psql -d postgres -c \copy tab from '/tmp/st.sql' delimiter ' '; insert
into tab values ('lll', 3)
COPY 1
INSERT 0 1

This is not harmful, but just a matter of consistency.




 *Thanks and Regards,*

 *Kumar Rajeev Rastogi *




 --
 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] COPY table FROM STDIN doesn't show count tag

2013-11-18 Thread Rajeev rastogi
On 18 November 2013, Amit Khandekar wrote:
 On 18 October 2013 17:07, Rajeev rastogi 
 rajeev.rast...@huawei.commailto:rajeev.rast...@huawei.com wrote:
From the following mail, copy behaviour between stdin and normal file having 
some inconsistency.
   
 http://www.postgresql.org/message-id/ce85a517.4878e%tim.k...@gmail.comhttp://www.postgresql.org/message-id/ce85a517.4878e%25tim.k...@gmail.com
The issue was that if copy  execute from stdin, then it goes to the server 
to execute the command and then server request for the input, it sends back 
the control to client to enter the data. So
 once client sends the input to server, server execute the copy command and 
 sends back the result to client but client does not print the result instead 
 it just clear it out.
 Changes are made to ensure the final result from server get printed before 
 clearing the result.
 Please find the patch for the same and let me know your suggestions.
In this call :
  success = handleCopyIn(pset.db, 
 pset.cur_cmd_source,
   
  PQbinaryTuples(*results), intres)  success;

  if (success  intres)
  success = 
 PrintQueryResults(intres);

Instead of handling of the result status this way, what if we use the 
ProcessResult()  argument 'result' to pass back the COPY result status to the 
caller ? We already call PrintQueryResults(results) after the ProcessResult() 
call. So we don't have to have a
 COPY-specific PrintQueryResults() call. Also, if there is a subsequent SQL 
 command in the same query string, the consequence of the patch is that the 
 client prints both COPY output and the last command output. So my suggestion 
 would also allow us
 to be consistent with the general behaviour that only the last SQL command 
 output is printed in case of multiple SQL commands. Here is how it gets 
 printed with your patch :
 Thank you for valuable comments. Your suggestion is absolutely correct.
 psql -d postgres -c \copy tab from '/tmp/st.sql' delimiter ' '; insert into 
 tab values ('lll', 3)
COPY 1
INSERT 0 1

This is not harmful, but just a matter of consistency.
I hope you meant to write test case as psql -d postgres -c \copy tab from 
stdin; insert into tab values ('lll', 3), as if we are reading from file, then 
the above issue does not come.
 I have modified the patch as per your comment and same is attached with this 
mail.
Please let me know in-case of any other issues or suggestions.

Thanks and Regards,
Kumar Rajeev Rastogi



copydefectV2.patch
Description: copydefectV2.patch

-- 
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] COPY table FROM STDIN doesn't show count tag

2013-11-18 Thread Amit Khandekar
On 18 November 2013 18:00, Rajeev rastogi rajeev.rast...@huawei.com wrote:

  On 18 November 2013, Amit Khandekar wrote:

  On 18 October 2013 17:07, Rajeev rastogi rajeev.rast...@huawei.com
 wrote:

 From the following mail, copy behaviour between stdin and normal file
 having some inconsistency.

 
 http://www.postgresql.org/message-id/ce85a517.4878e%tim.k...@gmail.com

 The issue was that if copy  execute from stdin, then it goes to the
 server to execute the command and then server request for the input, it
 sends back the control to client to enter the data. So

  once client sends the input to server, server execute the copy command
 and sends back the result to client but client does not print the result
 instead it just clear it out.

  Changes are made to ensure the final result from server get printed
 before clearing the result.

  Please find the patch for the same and let me know your suggestions.

 In this call :

   success = handleCopyIn(pset.db,
 pset.cur_cmd_source,

 
   PQbinaryTuples(*results), intres)  success;

 

   if (success  intres)

   success =
 PrintQueryResults(intres);

 

 Instead of handling of the result status this way, what if we use the
 ProcessResult()  argument 'result' to pass back the COPY result status to
 the caller ? We already call PrintQueryResults(results) after the
 ProcessResult() call. So we don't have to have a

  COPY-specific PrintQueryResults() call. Also, if there is a subsequent
 SQL command in the same query string, the consequence of the patch is that
 the client prints both COPY output and the last command output. So my
 suggestion would also allow us

  to be consistent with the general behaviour that only the last SQL
 command output is printed in case of multiple SQL commands. Here is how it
 gets printed with your patch :

  Thank you for valuable comments. Your suggestion is absolutely correct.

  psql -d postgres -c \copy tab from '/tmp/st.sql' delimiter ' '; insert
 into tab values ('lll', 3)

 COPY 1

 INSERT 0 1

 

 This is not harmful, but just a matter of consistency.

 I hope you meant to write test case as *psql -d postgres -c \copy tab
 from stdin; insert into tab values ('lll', 3), *as if we are reading
 from file, then the above issue does not come.

I meant COPY with a slash. \COPY is equivalent to COPY FROM STDIN. So the
issue can also be reproduced by :
\COPY tab from 'client_filename' ...


   I have modified the patch as per your comment and same is attached with
 this mail.


Thanks. The COPY FROM looks good.

With the patch applied, \COPY TO 'data_file' command outputs the  COPY
status into the data file, instead of printing it in the psql session.

postgres=# \copy tab to '/tmp/fout';
postgres=#

$ cat /tmp/fout
ee 909
COPY 1

This is probably because client-side COPY overrides the pset.queryFout with
its own destination file, and while printing the COPY status,
the overridden file pointer is not yet reverted back.


 Please let me know in-case of any other issues or suggestions.



 *Thanks and Regards,*

 *Kumar Rajeev Rastogi *





Re: [HACKERS] COPY table FROM STDIN doesn't show count tag

2013-10-22 Thread Rajeev rastogi
On 21 October 2013 20:48,  Robert Haas robertmh...@gmail.com wrote:
On Fri, Oct 18, 2013 at 7:37 AM, Rajeev rastogi rajeev.rast...@huawei.com 
wrote:
 From the following mail, copy behaviour between stdin and normal file 
 having some inconsistency.


 http://www.postgresql.org/message-id/ce85a517.4878e%tim.k...@gmail.com



 The issue was that if copy  execute from stdin, then it goes to the 
 server to execute the command and then server request for the input, 
 it sends back the control to client to enter the data. So once client 
 sends the input to server, server execute the copy command and sends 
 back the result to client but client does not print the result instead it 
 just clear it out.

 Changes are made to ensure the final result from server get printed 
 before clearing the result.



 Please find the patch for the same and let me know your suggestions.



Please add your patch to the currently-open CommitFest so that it does not get 
forgotten:

https://commitfest.postgresql.org/action/commitfest_view/open

Added to the currently-open CommitFest.

Thanks and Regards,
Kumar Rajeev Rastogi


-- 
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] COPY table FROM STDIN doesn't show count tag

2013-10-21 Thread Robert Haas
On Fri, Oct 18, 2013 at 7:37 AM, Rajeev rastogi
rajeev.rast...@huawei.com wrote:
 From the following mail, copy behaviour between stdin and normal file having
 some inconsistency.


 http://www.postgresql.org/message-id/ce85a517.4878e%tim.k...@gmail.com



 The issue was that if copy  execute from stdin, then it goes to the server
 to execute the command and then server request for the input, it sends back
 the control to client to enter the data. So once client sends the input to
 server, server execute the copy command and sends back the result to client
 but client does not print the result instead it just clear it out.

 Changes are made to ensure the final result from server get printed before
 clearing the result.



 Please find the patch for the same and let me know your suggestions.



 Thanks and Regards,

 Kumar Rajeev Rastogi

Please add your patch to the currently-open CommitFest so that it does
not get forgotten:

https://commitfest.postgresql.org/action/commitfest_view/open

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


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