Re: [HACKERS] ToDo: allow to get a number of processed rows by COPY statement [Review of Patch]
Hello
I reduced this patch as just plpgsql (SPI) problem solution.
Correct generic solution needs a discussion about enhancing our libpq
interface - we can take a number of returned rows, but we should to
get number of processed rows too. A users should to parse this number
from completationTag, but this problem is not too hot and usually is
not blocker, because anybody can process completationTag usually.
So this issue is primary for PL/pgSQL, where this information is not
possible. There is precedent - CREATE AS SELECT (thanks for sample
:)), and COPY should to have same impact on ROW_COUNT like this
statement (last patch implements it).
Regards
Pavel
2012/9/21 Amit Kapila amit.kap...@huawei.com:
On Friday, September 21, 2012 1:23 AM Pavel Stehule wrote:
Basic stuff:
- Patch applies OK. but offset difference in line numbers.
- Compiles with errors in contrib [pg_stat_statements, sepgsql] modules
- Regression failed; one test-case in COPY due to incomplete test-case
attached patch. – same as reported by Heikki
fixed patch is in attachment
After modifications:
---
- Patch applies OK
- Compiles cleanly without any errors/warnings
- Regression tests pass.
What it does:
--
Modification to get the number of processed rows evaluated via SPI. The
changes are to add extra parameter in ProcessUtility to get the number of
rows processed by COPY command.
Code Review Comments:
-
1. New parameter is added to ProcessUtility_hook_type function
but the functions which get assigned to these functions like
sepgsql_utility_command, pgss_ProcessUtility, prototype definition is
not modified.
Functionality is not fixed correctly for hook functions, In function
pgss_ProcessUtility
for bellow snippet of code processed parameter is passed NULL, as well as not
initialized.
because of this when pg_stat_statements extention is utilized COPY command
is giving garbage values.
if (prev_ProcessUtility)
prev_ProcessUtility(parsetree, queryString, params,
dest, completionTag,
context, NULL);
else
standard_ProcessUtility(parsetree, queryString, params,
dest, completionTag,
context, NULL);
Testcase is attached.
In this testcase table has only 1000 records but it show garbage value.
postgres=# show shared_preload_libraries ;
shared_preload_libraries
--
pg_stat_statements
(1 row)
postgres=# CREATE TABLE tbl (a int);
CREATE TABLE
postgres=# INSERT INTO tbl VALUES(generate_series(1,1000));
INSERT 0 1000
postgres=# do $$
declare r int;
begin
copy tbl to '/home/kiran/copytest.csv' csv;
get diagnostics r = row_count;
raise notice 'exported % rows', r;
truncate tbl;
copy tbl from '/home/kiran/copytest.csv' csv;
get diagnostics r = row_count;
raise notice 'imported % rows', r;
end;
$$ language plpgsql;
postgres$#
NOTICE: exported 13281616 rows
NOTICE: imported 13281616 rows
DO
2. Why to add the new parameter if completionTag hold the number of
processed tuple information; can be extracted
from it as follows:
_SPI_current-processed = strtoul(completionTag + 7,
NULL, 10);
this is basic question. I prefer a natural type for counter - uint64
instead text. And there are no simply way to get offset (7 in this
case)
I agree with your point, but currently in few other places we are parsing
the completion tag for getting number of tuples processed. So may be in
future we can change those places as well. For example
yes, this step can be done in future - together with libpq enhancing.
Is not adequate change API (and break lot of extensions) for this
isolated problem. So I propose fix plpgsql issue, and add to ToDo -
enhance libpq to return a number of processed rows as number - but
this change breaking compatibility.
Pavel
pgss_ProcessUtility
{
..
/* parse command tag to retrieve the number of affected rows. */
if (completionTag
sscanf(completionTag, COPY UINT64_FORMAT, rows) != 1)
rows = 0;
}
_SPI_execute_plan
{
..
..
if (IsA(stmt, CreateTableAsStmt))
{
Assert(strncmp(completionTag, SELECT , 7) == 0);
_SPI_current-processed = strtoul(completionTag + 7,
NULL, 10);
..
}
With Regards,
Amit Kapila.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
copy-processed-rows-simple.patch
Description:
Re: [HACKERS] ToDo: allow to get a number of processed rows by COPY statement [Review of Patch]
On Friday, September 28, 2012 2:28 AM Pavel Stehule wrote:
Hello
I reduced this patch as just plpgsql (SPI) problem solution.
Correct generic solution needs a discussion about enhancing our libpq
interface - we can take a number of returned rows, but we should to get
number of processed rows too. A users should to parse this number from
completationTag, but this problem is not too hot and usually is not
blocker, because anybody can process completationTag usually.
So this issue is primary for PL/pgSQL, where this information is not
possible. There is precedent - CREATE AS SELECT (thanks for sample :)),
and COPY should to have same impact on ROW_COUNT like this statement
(last patch implements it).
IMO now this patch is okay. I have marked it as Ready For Committer.
With Regards,
Amit Kapila.
2012/9/21 Amit Kapila amit.kap...@huawei.com:
On Friday, September 21, 2012 1:23 AM Pavel Stehule wrote:
Basic stuff:
- Patch applies OK. but offset difference in line numbers.
- Compiles with errors in contrib [pg_stat_statements, sepgsql]
modules
- Regression failed; one test-case in COPY due to incomplete
test-case attached patch. – same as reported by Heikki
fixed patch is in attachment
After modifications:
---
- Patch applies OK
- Compiles cleanly without any errors/warnings
- Regression tests pass.
What it does:
--
Modification to get the number of processed rows evaluated via SPI.
The changes are to add extra parameter in ProcessUtility to get the
number of rows processed by COPY command.
Code Review Comments:
-
1. New parameter is added to ProcessUtility_hook_type function
but the functions which get assigned to these functions like
sepgsql_utility_command, pgss_ProcessUtility, prototype
definition is not modified.
Functionality is not fixed correctly for hook functions, In function
pgss_ProcessUtility for bellow snippet of code processed parameter is
passed NULL, as well as not initialized.
because of this when pg_stat_statements extention is utilized COPY
command is giving garbage values.
if (prev_ProcessUtility)
prev_ProcessUtility(parsetree, queryString, params,
dest,
completionTag, context, NULL);
else
standard_ProcessUtility(parsetree, queryString,
params,
dest,
completionTag, context, NULL);
Testcase is attached.
In this testcase table has only 1000 records but it show garbage
value.
postgres=# show shared_preload_libraries ;
shared_preload_libraries
--
pg_stat_statements
(1 row)
postgres=# CREATE TABLE tbl (a int);
CREATE TABLE
postgres=# INSERT INTO tbl VALUES(generate_series(1,1000));
INSERT 0 1000
postgres=# do $$
declare r int;
begin
copy tbl to '/home/kiran/copytest.csv' csv;
get diagnostics r = row_count;
raise notice 'exported % rows', r;
truncate tbl;
copy tbl from '/home/kiran/copytest.csv' csv;
get diagnostics r = row_count;
raise notice 'imported % rows', r;
end;
$$ language plpgsql;
postgres$#
NOTICE: exported 13281616 rows
NOTICE: imported 13281616 rows
DO
2. Why to add the new parameter if completionTag hold the number of
processed tuple information; can be extracted
from it as follows:
_SPI_current-processed = strtoul(completionTag
+ 7, NULL, 10);
this is basic question. I prefer a natural type for counter - uint64
instead text. And there are no simply way to get offset (7 in this
case)
I agree with your point, but currently in few other places we are
parsing the completion tag for getting number of tuples processed. So
may be in future we can change those places as well. For example
yes, this step can be done in future - together with libpq enhancing.
Is not adequate change API (and break lot of extensions) for this
isolated problem. So I propose fix plpgsql issue, and add to ToDo -
enhance libpq to return a number of processed rows as number - but
this change breaking compatibility.
Pavel
pgss_ProcessUtility
{
..
/* parse command tag to retrieve the number of affected rows. */ if
(completionTag
sscanf(completionTag, COPY UINT64_FORMAT, rows) != 1)
rows = 0;
}
_SPI_execute_plan
{
..
..
if (IsA(stmt, CreateTableAsStmt))
{
Assert(strncmp(completionTag, SELECT , 7) == 0);
_SPI_current-processed = strtoul(completionTag + 7,
NULL, 10);
..
}
With Regards,
Amit Kapila.
--
Re: [HACKERS] ToDo: allow to get a number of processed rows by COPY statement [Review of Patch]
2012/9/28 Amit Kapila amit.kap...@huawei.com:
On Friday, September 28, 2012 2:28 AM Pavel Stehule wrote:
Hello
I reduced this patch as just plpgsql (SPI) problem solution.
Correct generic solution needs a discussion about enhancing our libpq
interface - we can take a number of returned rows, but we should to get
number of processed rows too. A users should to parse this number from
completationTag, but this problem is not too hot and usually is not
blocker, because anybody can process completationTag usually.
So this issue is primary for PL/pgSQL, where this information is not
possible. There is precedent - CREATE AS SELECT (thanks for sample :)),
and COPY should to have same impact on ROW_COUNT like this statement
(last patch implements it).
IMO now this patch is okay. I have marked it as Ready For Committer.
Thank you very much for review
Regards
Pavel
With Regards,
Amit Kapila.
2012/9/21 Amit Kapila amit.kap...@huawei.com:
On Friday, September 21, 2012 1:23 AM Pavel Stehule wrote:
Basic stuff:
- Patch applies OK. but offset difference in line numbers.
- Compiles with errors in contrib [pg_stat_statements, sepgsql]
modules
- Regression failed; one test-case in COPY due to incomplete
test-case attached patch. – same as reported by Heikki
fixed patch is in attachment
After modifications:
---
- Patch applies OK
- Compiles cleanly without any errors/warnings
- Regression tests pass.
What it does:
--
Modification to get the number of processed rows evaluated via SPI.
The changes are to add extra parameter in ProcessUtility to get the
number of rows processed by COPY command.
Code Review Comments:
-
1. New parameter is added to ProcessUtility_hook_type function
but the functions which get assigned to these functions like
sepgsql_utility_command, pgss_ProcessUtility, prototype
definition is not modified.
Functionality is not fixed correctly for hook functions, In function
pgss_ProcessUtility for bellow snippet of code processed parameter is
passed NULL, as well as not initialized.
because of this when pg_stat_statements extention is utilized COPY
command is giving garbage values.
if (prev_ProcessUtility)
prev_ProcessUtility(parsetree, queryString, params,
dest,
completionTag, context, NULL);
else
standard_ProcessUtility(parsetree, queryString,
params,
dest,
completionTag, context, NULL);
Testcase is attached.
In this testcase table has only 1000 records but it show garbage
value.
postgres=# show shared_preload_libraries ;
shared_preload_libraries
--
pg_stat_statements
(1 row)
postgres=# CREATE TABLE tbl (a int);
CREATE TABLE
postgres=# INSERT INTO tbl VALUES(generate_series(1,1000));
INSERT 0 1000
postgres=# do $$
declare r int;
begin
copy tbl to '/home/kiran/copytest.csv' csv;
get diagnostics r = row_count;
raise notice 'exported % rows', r;
truncate tbl;
copy tbl from '/home/kiran/copytest.csv' csv;
get diagnostics r = row_count;
raise notice 'imported % rows', r;
end;
$$ language plpgsql;
postgres$#
NOTICE: exported 13281616 rows
NOTICE: imported 13281616 rows
DO
2. Why to add the new parameter if completionTag hold the number of
processed tuple information; can be extracted
from it as follows:
_SPI_current-processed = strtoul(completionTag
+ 7, NULL, 10);
this is basic question. I prefer a natural type for counter - uint64
instead text. And there are no simply way to get offset (7 in this
case)
I agree with your point, but currently in few other places we are
parsing the completion tag for getting number of tuples processed. So
may be in future we can change those places as well. For example
yes, this step can be done in future - together with libpq enhancing.
Is not adequate change API (and break lot of extensions) for this
isolated problem. So I propose fix plpgsql issue, and add to ToDo -
enhance libpq to return a number of processed rows as number - but
this change breaking compatibility.
Pavel
pgss_ProcessUtility
{
..
/* parse command tag to retrieve the number of affected rows. */ if
(completionTag
sscanf(completionTag, COPY UINT64_FORMAT, rows) != 1)
rows = 0;
}
_SPI_execute_plan
{
..
..
if (IsA(stmt, CreateTableAsStmt))
{
Assert(strncmp(completionTag, SELECT , 7) == 0);
_SPI_current-processed =
Re: [HACKERS] ToDo: allow to get a number of processed rows by COPY statement [Review of Patch]
On Friday, September 21, 2012 1:23 AM Pavel Stehule wrote:
Basic stuff:
- Patch applies OK. but offset difference in line numbers.
- Compiles with errors in contrib [pg_stat_statements, sepgsql] modules
- Regression failed; one test-case in COPY due to incomplete test-case
attached patch. – same as reported by Heikki
fixed patch is in attachment
After modifications:
---
- Patch applies OK
- Compiles cleanly without any errors/warnings
- Regression tests pass.
What it does:
--
Modification to get the number of processed rows evaluated via SPI. The
changes are to add extra parameter in ProcessUtility to get the number of
rows processed by COPY command.
Code Review Comments:
-
1. New parameter is added to ProcessUtility_hook_type function
but the functions which get assigned to these functions like
sepgsql_utility_command, pgss_ProcessUtility, prototype definition is
not modified.
Functionality is not fixed correctly for hook functions, In function
pgss_ProcessUtility
for bellow snippet of code processed parameter is passed NULL, as well as not
initialized.
because of this when pg_stat_statements extention is utilized COPY command is
giving garbage values.
if (prev_ProcessUtility)
prev_ProcessUtility(parsetree, queryString, params,
dest, completionTag,
context, NULL);
else
standard_ProcessUtility(parsetree, queryString, params,
dest, completionTag,
context, NULL);
Testcase is attached.
In this testcase table has only 1000 records but it show garbage value.
postgres=# show shared_preload_libraries ;
shared_preload_libraries
--
pg_stat_statements
(1 row)
postgres=# CREATE TABLE tbl (a int);
CREATE TABLE
postgres=# INSERT INTO tbl VALUES(generate_series(1,1000));
INSERT 0 1000
postgres=# do $$
declare r int;
begin
copy tbl to '/home/kiran/copytest.csv' csv;
get diagnostics r = row_count;
raise notice 'exported % rows', r;
truncate tbl;
copy tbl from '/home/kiran/copytest.csv' csv;
get diagnostics r = row_count;
raise notice 'imported % rows', r;
end;
$$ language plpgsql;
postgres$#
NOTICE: exported 13281616 rows
NOTICE: imported 13281616 rows
DO
2. Why to add the new parameter if completionTag hold the number of
processed tuple information; can be extracted
from it as follows:
_SPI_current-processed = strtoul(completionTag + 7,
NULL, 10);
this is basic question. I prefer a natural type for counter - uint64
instead text. And there are no simply way to get offset (7 in this
case)
I agree with your point, but currently in few other places we are parsing
the completion tag for getting number of tuples processed. So may be in future
we can change those places as well. For example
pgss_ProcessUtility
{
..
/* parse command tag to retrieve the number of affected rows. */
if (completionTag
sscanf(completionTag, COPY UINT64_FORMAT, rows) != 1)
rows = 0;
}
_SPI_execute_plan
{
..
..
if (IsA(stmt, CreateTableAsStmt))
{
Assert(strncmp(completionTag, SELECT , 7) == 0);
_SPI_current-processed = strtoul(completionTag + 7,
NULL,
10);
..
}
With Regards,
Amit Kapila.
--
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] ToDo: allow to get a number of processed rows by COPY statement [Review of Patch]
On 16.08.2012 14:43, Pavel Stehule wrote: Hello here is updated patch [Review of Patch] Basic stuff: - Patch applies OK. but offset difference in line numbers. - Compiles with errors in contrib [pg_stat_statements, sepgsql] modules - Regression failed; one test-case in COPY due to incomplete test-case attached patch. same as reported by Heikki Details of compilation errors and regression failure: -- PATH : postgres/src/contrib/pg_stat_statements gcc -g -ggdb3 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fpic -I. -I. -I../../src/include -D_GNU_SOURCE -c -o pg_stat_statements.o pg_stat_statements.c pg_stat_statements.c: In function â_PG_initâ: pg_stat_statements.c:363: warning: assignment from incompatible pointer type pg_stat_statements.c: In function âpgss_ProcessUtilityâ: pg_stat_statements.c:823: error: too few arguments to function âprev_ProcessUtilityâ pg_stat_statements.c:826: error: too few arguments to function âstandard_ProcessUtilityâ pg_stat_statements.c:884: error: too few arguments to function âprev_ProcessUtilityâ pg_stat_statements.c:887: error: too few arguments to function âstandard_ProcessUtilityâ make: *** [pg_stat_statements.o] Error 1 PATH : postgres/src/contrib/sepgsql gcc -g -ggdb3 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fpic -I. -I. -I../../src/include -D_GNU_SOURCE -c -o hooks.o hooks.c In file included from hooks.c:26: sepgsql.h:17:29: error: selinux/selinux.h: No such file or directory sepgsql.h:18:25: error: selinux/avc.h: No such file or directory In file included from hooks.c:26: sepgsql.h:239: warning: âstruct av_decisionâ declared inside parameter list sepgsql.h:239: warning: its scope is only this definition or declaration, which is probably not what you want hooks.c: In function âsepgsql_utility_commandâ: hooks.c:331: error: too few arguments to function ânext_ProcessUtility_hookâ hooks.c:334: error: too few arguments to function âstandard_ProcessUtilityâ hooks.c: In function â_PG_initâ: hooks.c:365: warning: implicit declaration of function âis_selinux_enabledâ hooks.c:426: warning: assignment from incompatible pointer type make: *** [hooks.o] Error 1 REGRESSION TEST: -- make installcheck test copy ... FAILED 1 of 132 tests failed. ~/postgres/src/test/regress cat regression.diffs *** /home/postgres/code/src/test/regress/expected/copy.out 2012-09-18 19:57:02.0 +0530 --- /home/postgres/code/src/test/regress/results/copy.out 2012-09-18 19:57:18.0 +0530 *** *** 71,73 --- 71,80 c1,col with , comma,col with quote 1,a,1 2,b,2 + -- copy should to return processed rows + do $$ + + ERROR: unterminated dollar-quoted string at or near $$ + + LINE 1: do $$ +^ What it does: -- Modification to get the number of processed rows evaluated via SPI. The changes are to add extra parameter in ProcessUtility to get the number of rows processed by COPY command. Code Review Comments: - 1. New parameter is added to ProcessUtility_hook_type function but the functions which get assigned to these functions like sepgsql_utility_command, pgss_ProcessUtility, prototype definition is not modified. 2. Why to add the new parameter if completionTag hold the number of processed tuple information; can be extracted from it as follows: _SPI_current-processed = strtoul(completionTag + 7, NULL, 10); 3. Why new variable added in portal [portal-processed] this is not used in code ? Testing: The following test is carried out on the patch. 1. Normal SQL command COPY FROM / TO is tested. 2. SQL command COPY FROM / TO is tested from plpgsql. Test cases are attached in Testcases_Copy_Processed.txt With Regards, Amit Kapila. --1) Normal SQL command COPY FROM and TO functionalities. CREATE TABLE tbl (a int); INSERT INTO tbl VALUES(generate_series(1,1000)); COPY tbl TO '/home/kiran/tbl.txt'; CREATE TABLE tbl1 (a int); COPY tbl1 FROM '/home/kiran/tbl.txt'; DELETE FROM tbl1; DROP TABLE tbl; DROP TABLE tbl1; --2) In side SPI [plpgsql function], SQL command COPY FROM and TO functionalities. CREATE TABLE tbl (a int); INSERT INTO tbl VALUES(generate_series(1,1000)); CREATE OR REPLACE FUNCTION public.copy_to_tbl() RETURNS integer LANGUAGE plpgsql AS $function$ DECLARE r int; BEGIN COPY tbl TO '/home/kiran/tbl.txt'; get diagnostics r = row_count; RETURN r; end; $function$; SELECT copy_to_tbl(); CREATE TABLE tbl1 (a int); CREATE OR REPLACE FUNCTION public.copy_from_tbl() RETURNS
Re: [HACKERS] ToDo: allow to get a number of processed rows by COPY statement [Review of Patch]
Hello Basic stuff: - Patch applies OK. but offset difference in line numbers. - Compiles with errors in contrib [pg_stat_statements, sepgsql] modules - Regression failed; one test-case in COPY due to incomplete test-case attached patch. – same as reported by Heikki fixed patch is in attachment Details of compilation errors and regression failure: -- PATH : postgres/src/contrib/pg_stat_statements gcc -g -ggdb3 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fpic -I. -I. -I../../src/include -D_GNU_SOURCE -c -o pg_stat_statements.o pg_stat_statements.c pg_stat_statements.c: In function â_PG_initâ: pg_stat_statements.c:363: warning: assignment from incompatible pointer type pg_stat_statements.c: In function âpgss_ProcessUtilityâ: pg_stat_statements.c:823: error: too few arguments to function âprev_ProcessUtilityâ pg_stat_statements.c:826: error: too few arguments to function âstandard_ProcessUtilityâ pg_stat_statements.c:884: error: too few arguments to function âprev_ProcessUtilityâ pg_stat_statements.c:887: error: too few arguments to function âstandard_ProcessUtilityâ make: *** [pg_stat_statements.o] Error 1 PATH : postgres/src/contrib/sepgsql gcc -g -ggdb3 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fpic -I. -I. -I../../src/include -D_GNU_SOURCE -c -o hooks.o hooks.c In file included from hooks.c:26: sepgsql.h:17:29: error: selinux/selinux.h: No such file or directory sepgsql.h:18:25: error: selinux/avc.h: No such file or directory In file included from hooks.c:26: sepgsql.h:239: warning: âstruct av_decisionâ declared inside parameter list sepgsql.h:239: warning: its scope is only this definition or declaration, which is probably not what you want hooks.c: In function âsepgsql_utility_commandâ: hooks.c:331: error: too few arguments to function ânext_ProcessUtility_hookâ hooks.c:334: error: too few arguments to function âstandard_ProcessUtilityâ hooks.c: In function â_PG_initâ: hooks.c:365: warning: implicit declaration of function âis_selinux_enabledâ hooks.c:426: warning: assignment from incompatible pointer type make: *** [hooks.o] Error 1 REGRESSION TEST: -- make installcheck test copy ... FAILED 1 of 132 tests failed. ~/postgres/src/test/regress cat regression.diffs *** /home/postgres/code/src/test/regress/expected/copy.out 2012-09-18 19:57:02.0 +0530 --- /home/postgres/code/src/test/regress/results/copy.out 2012-09-18 19:57:18.0 +0530 *** *** 71,73 --- 71,80 c1,col with , comma,col with quote 1,a,1 2,b,2 + -- copy should to return processed rows + do $$ + + ERROR: unterminated dollar-quoted string at or near $$ + + LINE 1: do $$ +^ fixed What it does: -- Modification to get the number of processed rows evaluated via SPI. The changes are to add extra parameter in ProcessUtility to get the number of rows processed by COPY command. Code Review Comments: - 1. New parameter is added to ProcessUtility_hook_type function but the functions which get assigned to these functions like sepgsql_utility_command, pgss_ProcessUtility, prototype definition is not modified. 2. Why to add the new parameter if completionTag hold the number of processed tuple information; can be extracted from it as follows: _SPI_current-processed = strtoul(completionTag + 7, NULL, 10); this is basic question. I prefer a natural type for counter - uint64 instead text. And there are no simply way to get offset (7 in this case) 3. Why new variable added in portal [portal-processed] this is not used in code ? This value can be used anywhere instead parsing completionTag to get returned numbers Testing: The following test is carried out on the patch. 1. Normal SQL command COPY FROM / TO is tested. 2. SQL command COPY FROM / TO is tested from plpgsql. Test cases are attached in Testcases_Copy_Processed.txt Regards Pavel With Regards, Amit Kapila. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers copy-processed-rows.diff Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ToDo: allow to get a number of processed rows by COPY statement
On 16.08.2012 14:43, Pavel Stehule wrote: Hello here is updated patch The patch seems to be truncated, it ends with: *** a/src/test/regress/input/copy.source --- b/src/test/regress/input/copy.source *** *** 106,108 this is just a line full of junk that would error out if parsed --- 106,112 \. copy copytest3 to stdout csv header; + + -- copy should to return processed rows + do $$ + I believe the code changes are OK, but regression test changes are missing. Can you resend the full patch, please? - Heikki -- 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] ToDo: allow to get a number of processed rows by COPY statement
Hello
here is updated patch
postgres=# copy omega to '/tmp/xxx';
COPY 60
postgres=# do $$ declare r int;
begin
copy omega from '/tmp/xxx'; get diagnostics r = row_count; raise
notice ' %', r;
end;
$$ language plpgsql;
NOTICE: 60
DO
Regards
Pavel
2012/8/16 Bruce Momjian br...@momjian.us:
What ever happened to this patch? I don't see it on any of the
commit-fests, though someone was asked for it to be added:
http://archives.postgresql.org/pgsql-hackers/2011-10/msg00381.php
---
On Tue, Oct 4, 2011 at 12:22:19PM +0200, Pavel Stehule wrote:
Hello
There is not possible to get a number of processed rows when COPY is
evaluated via SPI. Client can use a tag, but SPI doesn't use a tag.
I propose a small change a ProcessUtility to return a processed rows.
Note: I found a small inconsistency between SPI and Utility interface.
SPI still use a 4 byte unsign int for storing a number of processed
rows. Utility use a 8bytes unsign int.
Motivation:
postgres=# \sf fx
CREATE OR REPLACE FUNCTION public.fx(tablename text, filename text)
RETURNS integer
LANGUAGE plpgsql
AS $function$
declare r int;
begin
execute format('COPY %s FROM %s', quote_ident(tablename),
quote_literal(filename));
get diagnostics r = row_count;
return r;
end;
$function$
Regards
Pavel Stehule
diff --git a/src/backend/executor/functions.c
b/src/backend/executor/functions.c
index 398bc40..a7c2b8f 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -600,6 +600,7 @@ postquel_getnext(execution_state *es,
SQLFunctionCachePtr fcache)
es-qd-params,
false, /* not top level */
es-qd-dest,
+NULL,
NULL);
result = true; /* never stops early */
}
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 688279c..21cabcc 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -1838,6 +1838,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo
paramLI,
{
Node *stmt = (Node *) lfirst(lc2);
boolcanSetTag;
+ boolisCopyStmt = false;
DestReceiver *dest;
_SPI_current-processed = 0;
@@ -1857,6 +1858,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo
paramLI,
{
CopyStmt *cstmt = (CopyStmt *) stmt;
+ isCopyStmt = true;
if (cstmt-filename == NULL)
{
my_res = SPI_ERROR_COPY;
@@ -1911,16 +1913,23 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo
paramLI,
}
else
{
+ uint32 processed;
+
ProcessUtility(stmt,
plansource-query_string,
paramLI,
false, /* not
top level */
dest,
-NULL);
+NULL,
+processed);
/* Update processed if stmt returned tuples
*/
+
if (_SPI_current-tuptable)
_SPI_current-processed =
_SPI_current-tuptable-alloced -
_SPI_current-tuptable-free;
+ else if (canSetTag isCopyStmt)
+ _SPI_current-processed = processed;
+
res = SPI_OK_UTILITY;
}
diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index 466727b..1a861ee 100644
--- a/src/backend/tcop/pquery.c
+++ b/src/backend/tcop/pquery.c
@@ -1184,7 +1184,8 @@ PortalRunUtility(Portal portal, Node *utilityStmt,
bool isTopLevel,
portal-portalParams,
isTopLevel,
dest,
-completionTag);
+completionTag,
+NULL);
/* Some utility statements may change context on us */
MemoryContextSwitchTo(PortalGetHeapMemory(portal));
diff
Re: [HACKERS] ToDo: allow to get a number of processed rows by COPY statement
What ever happened to this patch? I don't see it on any of the
commit-fests, though someone was asked for it to be added:
http://archives.postgresql.org/pgsql-hackers/2011-10/msg00381.php
---
On Tue, Oct 4, 2011 at 12:22:19PM +0200, Pavel Stehule wrote:
Hello
There is not possible to get a number of processed rows when COPY is
evaluated via SPI. Client can use a tag, but SPI doesn't use a tag.
I propose a small change a ProcessUtility to return a processed rows.
Note: I found a small inconsistency between SPI and Utility interface.
SPI still use a 4 byte unsign int for storing a number of processed
rows. Utility use a 8bytes unsign int.
Motivation:
postgres=# \sf fx
CREATE OR REPLACE FUNCTION public.fx(tablename text, filename text)
RETURNS integer
LANGUAGE plpgsql
AS $function$
declare r int;
begin
execute format('COPY %s FROM %s', quote_ident(tablename),
quote_literal(filename));
get diagnostics r = row_count;
return r;
end;
$function$
Regards
Pavel Stehule
diff --git a/src/backend/executor/functions.c
b/src/backend/executor/functions.c
index 398bc40..a7c2b8f 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -600,6 +600,7 @@ postquel_getnext(execution_state *es, SQLFunctionCachePtr
fcache)
es-qd-params,
false, /* not top level */
es-qd-dest,
+NULL,
NULL);
result = true; /* never stops early */
}
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 688279c..21cabcc 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -1838,6 +1838,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo
paramLI,
{
Node *stmt = (Node *) lfirst(lc2);
boolcanSetTag;
+ boolisCopyStmt = false;
DestReceiver *dest;
_SPI_current-processed = 0;
@@ -1857,6 +1858,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo
paramLI,
{
CopyStmt *cstmt = (CopyStmt *) stmt;
+ isCopyStmt = true;
if (cstmt-filename == NULL)
{
my_res = SPI_ERROR_COPY;
@@ -1911,16 +1913,23 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo
paramLI,
}
else
{
+ uint32 processed;
+
ProcessUtility(stmt,
plansource-query_string,
paramLI,
false, /* not
top level */
dest,
-NULL);
+NULL,
+processed);
/* Update processed if stmt returned tuples */
+
if (_SPI_current-tuptable)
_SPI_current-processed =
_SPI_current-tuptable-alloced -
_SPI_current-tuptable-free;
+ else if (canSetTag isCopyStmt)
+ _SPI_current-processed = processed;
+
res = SPI_OK_UTILITY;
}
diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index 466727b..1a861ee 100644
--- a/src/backend/tcop/pquery.c
+++ b/src/backend/tcop/pquery.c
@@ -1184,7 +1184,8 @@ PortalRunUtility(Portal portal, Node *utilityStmt, bool
isTopLevel,
portal-portalParams,
isTopLevel,
dest,
-completionTag);
+completionTag,
+NULL);
/* Some utility statements may change context on us */
MemoryContextSwitchTo(PortalGetHeapMemory(portal));
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 0749227..35db28c 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -319,6 +319,9 @@ CheckRestrictedOperation(const char *cmdname)
* completionTag is only set nonempty if we want to return a
Re: [HACKERS] ToDo: allow to get a number of processed rows by COPY statement
Hello
I can reassign it again
Regards
Pavel
2012/8/16 Bruce Momjian br...@momjian.us:
What ever happened to this patch? I don't see it on any of the
commit-fests, though someone was asked for it to be added:
http://archives.postgresql.org/pgsql-hackers/2011-10/msg00381.php
---
On Tue, Oct 4, 2011 at 12:22:19PM +0200, Pavel Stehule wrote:
Hello
There is not possible to get a number of processed rows when COPY is
evaluated via SPI. Client can use a tag, but SPI doesn't use a tag.
I propose a small change a ProcessUtility to return a processed rows.
Note: I found a small inconsistency between SPI and Utility interface.
SPI still use a 4 byte unsign int for storing a number of processed
rows. Utility use a 8bytes unsign int.
Motivation:
postgres=# \sf fx
CREATE OR REPLACE FUNCTION public.fx(tablename text, filename text)
RETURNS integer
LANGUAGE plpgsql
AS $function$
declare r int;
begin
execute format('COPY %s FROM %s', quote_ident(tablename),
quote_literal(filename));
get diagnostics r = row_count;
return r;
end;
$function$
Regards
Pavel Stehule
diff --git a/src/backend/executor/functions.c
b/src/backend/executor/functions.c
index 398bc40..a7c2b8f 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -600,6 +600,7 @@ postquel_getnext(execution_state *es,
SQLFunctionCachePtr fcache)
es-qd-params,
false, /* not top level */
es-qd-dest,
+NULL,
NULL);
result = true; /* never stops early */
}
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 688279c..21cabcc 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -1838,6 +1838,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo
paramLI,
{
Node *stmt = (Node *) lfirst(lc2);
boolcanSetTag;
+ boolisCopyStmt = false;
DestReceiver *dest;
_SPI_current-processed = 0;
@@ -1857,6 +1858,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo
paramLI,
{
CopyStmt *cstmt = (CopyStmt *) stmt;
+ isCopyStmt = true;
if (cstmt-filename == NULL)
{
my_res = SPI_ERROR_COPY;
@@ -1911,16 +1913,23 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo
paramLI,
}
else
{
+ uint32 processed;
+
ProcessUtility(stmt,
plansource-query_string,
paramLI,
false, /* not
top level */
dest,
-NULL);
+NULL,
+processed);
/* Update processed if stmt returned tuples
*/
+
if (_SPI_current-tuptable)
_SPI_current-processed =
_SPI_current-tuptable-alloced -
_SPI_current-tuptable-free;
+ else if (canSetTag isCopyStmt)
+ _SPI_current-processed = processed;
+
res = SPI_OK_UTILITY;
}
diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index 466727b..1a861ee 100644
--- a/src/backend/tcop/pquery.c
+++ b/src/backend/tcop/pquery.c
@@ -1184,7 +1184,8 @@ PortalRunUtility(Portal portal, Node *utilityStmt,
bool isTopLevel,
portal-portalParams,
isTopLevel,
dest,
-completionTag);
+completionTag,
+NULL);
/* Some utility statements may change context on us */
MemoryContextSwitchTo(PortalGetHeapMemory(portal));
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 0749227..35db28c 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -319,6 +319,9 @@
Re: [HACKERS] ToDo: allow to get a number of processed rows by COPY statement
Pavel Stehule pavel.steh...@gmail.com wrote: There is not possible to get a number of processed rows when COPY is evaluated via SPI. Client can use a tag, but SPI doesn't use a tag. I propose a small change a ProcessUtility to return a processed rows. Please add this to the open CommitFest: https://commitfest.postgresql.org/action/commitfest_view/open -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] ToDo: allow to get a number of processed rows by COPY statement
Hello
There is not possible to get a number of processed rows when COPY is
evaluated via SPI. Client can use a tag, but SPI doesn't use a tag.
I propose a small change a ProcessUtility to return a processed rows.
Note: I found a small inconsistency between SPI and Utility interface.
SPI still use a 4 byte unsign int for storing a number of processed
rows. Utility use a 8bytes unsign int.
Motivation:
postgres=# \sf fx
CREATE OR REPLACE FUNCTION public.fx(tablename text, filename text)
RETURNS integer
LANGUAGE plpgsql
AS $function$
declare r int;
begin
execute format('COPY %s FROM %s', quote_ident(tablename),
quote_literal(filename));
get diagnostics r = row_count;
return r;
end;
$function$
Regards
Pavel Stehule
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index 398bc40..a7c2b8f 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -600,6 +600,7 @@ postquel_getnext(execution_state *es, SQLFunctionCachePtr fcache)
es-qd-params,
false, /* not top level */
es-qd-dest,
+ NULL,
NULL);
result = true; /* never stops early */
}
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 688279c..21cabcc 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -1838,6 +1838,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
{
Node *stmt = (Node *) lfirst(lc2);
bool canSetTag;
+ bool isCopyStmt = false;
DestReceiver *dest;
_SPI_current-processed = 0;
@@ -1857,6 +1858,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
{
CopyStmt *cstmt = (CopyStmt *) stmt;
+ isCopyStmt = true;
if (cstmt-filename == NULL)
{
my_res = SPI_ERROR_COPY;
@@ -1911,16 +1913,23 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
}
else
{
+uint32 processed;
+
ProcessUtility(stmt,
plansource-query_string,
paramLI,
false, /* not top level */
dest,
- NULL);
+ NULL,
+ processed);
/* Update processed if stmt returned tuples */
+
if (_SPI_current-tuptable)
_SPI_current-processed = _SPI_current-tuptable-alloced -
_SPI_current-tuptable-free;
+else if (canSetTag isCopyStmt)
+ _SPI_current-processed = processed;
+
res = SPI_OK_UTILITY;
}
diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index 466727b..1a861ee 100644
--- a/src/backend/tcop/pquery.c
+++ b/src/backend/tcop/pquery.c
@@ -1184,7 +1184,8 @@ PortalRunUtility(Portal portal, Node *utilityStmt, bool isTopLevel,
portal-portalParams,
isTopLevel,
dest,
- completionTag);
+ completionTag,
+ NULL);
/* Some utility statements may change context on us */
MemoryContextSwitchTo(PortalGetHeapMemory(portal));
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 0749227..35db28c 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -319,6 +319,9 @@ CheckRestrictedOperation(const char *cmdname)
* completionTag is only set nonempty if we want to return a nondefault status.
*
* completionTag may be NULL if caller doesn't want a status string.
+ *
+ * processed may be NULL if caller doesn't want a number of processed rows
+ * by COPY statement
*/
void
ProcessUtility(Node *parsetree,
@@ -326,7 +329,8 @@ ProcessUtility(Node *parsetree,
ParamListInfo params,
bool isTopLevel,
DestReceiver *dest,
- char *completionTag)
+ char *completionTag,
+ uint32 *processed)
{
Assert(queryString != NULL); /* required as of 8.4 */
@@ -337,10 +341,10 @@ ProcessUtility(Node *parsetree,
*/
if (ProcessUtility_hook)
(*ProcessUtility_hook) (parsetree, queryString, params,
-isTopLevel, dest, completionTag);
+isTopLevel, dest, completionTag, processed);
else
standard_ProcessUtility(parsetree, queryString, params,
-isTopLevel, dest, completionTag);
+isTopLevel, dest, completionTag, processed);
}
void
@@ -349,7 +353,8 @@ standard_ProcessUtility(Node *parsetree,
ParamListInfo params,
bool isTopLevel,
DestReceiver *dest,
- char *completionTag)
+ char *completionTag,
+ uint32 *processed)
{
check_xact_readonly(parsetree);
@@ -571,6 +576,7 @@ standard_ProcessUtility(Node *parsetree,
params,
false,
None_Receiver,
+ NULL,
NULL);
}
@@ -716,12 +722,14 @@ standard_ProcessUtility(Node *parsetree,
case T_CopyStmt:
{
-uint64 processed;
+uint64 _processed;
-processed = DoCopy((CopyStmt *) parsetree, queryString);
+_processed = DoCopy((CopyStmt *) parsetree, queryString);
if (completionTag)
snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
- COPY UINT64_FORMAT, processed);
+ COPY UINT64_FORMAT,
