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.000000000 +0530
--- /home/postgres/code/src/test/regress/results/copy.out 2012-09-18
19:57:18.000000000 +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 integer
LANGUAGE plpgsql
AS $function$
DECLARE r int;
BEGIN
COPY tbl1 FROM '/home/kiran/tbl.txt';
get diagnostics r = row_count;
RETURN r;
end;
$function$;
SELECT copy_from_tbl();
DELETE FROM tbl1;
DROP FUNCTION public.copy_from_tbl();
DROP FUNCTION public.copy_to_tbl();
DROP TABLE tbl;
DROP TABLE tbl1;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
