Performance testing of COPY (SELECT) TO

Started by Böszörményi Zoltánover 19 years ago44 messages
#1Böszörményi Zoltán
zboszor@dunaweb.hu

Hi,

we have a large export here, I made an in-house benchmark
between Informix, plain PostgreSQL-8.1.4 and
8.2devel+COPY(SELECT) using the same data and query.
Find the results below for the two PostgreSQL versions.
With PostgreSQL 8.1.4, I used this:

begin;
select ... into temp myquery1;
copy myquery1 to stdout csv delimiter '|';
rollback;

With 8.2devel, I simple used
copy (select ...) to stdout csv delimiter '|';

# of clients: 1* 3** 10**
PostgreSQL 1:33 10:58 55:46
PostgreSQL 8.2 1:19 4:55 18:28

* - average of 4 runs, the first was with cold caches after reboot
** - 1 run, average of cliens' runtimes

Performance between 8.1.4 and 8.2devel is interesting:
1 client: 15%
3 clients: 55.2%
10 clients: 66.9%

The same machine was used for testing.

Best regards,
Zolt�n B�sz�rm�nyi

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Böszörményi Zoltán (#1)
Re: Performance testing of COPY (SELECT) TO

=?iso-8859-2?Q?B=F6sz=F6rm=E9nyi_Zolt=E1n?= <zboszor@dunaweb.hu> writes:

With PostgreSQL 8.1.4, I used this:

begin;
select ... into temp myquery1;
copy myquery1 to stdout csv delimiter '|';
rollback;

The performance of this would doubtless vary a lot with the temp_buffers
setting. Did you try different values?

It'd also be interesting to time the same way (with a temp table) in
devel. I don't remember whether we did any performance work on the
COPY CSV data path in this cycle, or whether that was all present in
8.1. In any case it'd be worth proving that the COPY SELECT patch isn't
degrading performance of the copy-a-relation case.

regards, tom lane

#3Böszörményi Zoltán
zboszor@dunaweb.hu
In reply to: Tom Lane (#2)
1 attachment(s)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

B�sz�rm�nyi Zolt�n <zboszor@dunaweb.hu> writes:

With PostgreSQL 8.1.4, I used this:

begin;
select ... into temp myquery1;
copy myquery1 to stdout csv delimiter '|';
rollback;

The performance of this would doubtless vary a lot with the temp_buffers
setting. Did you try different values?

Yes, I did, but now checked back with 8.2CVS.
The previously quoted result was achieved with
temp_buffers = 1000 on both 8.1.4 and 8.2CVS.
On 8.2CVS with temp_buffers = 4096, the 10 client case kills
the machine with swapping, but the 3 client runtime with
COPY(SELECT) went down to 2:41. The SELECT INTO TEMP
case went down to 3:36.

It'd also be interesting to time the same way (with a temp table) in
devel. I don't remember whether we did any performance work on the
COPY CSV data path in this cycle, or whether that was all present in
8.1. In any case it'd be worth proving that the COPY SELECT patch isn't
degrading performance of the copy-a-relation case.

I will report back with that, say on Monday.

In the meantime, I documented the COPY (SELECT) case
and modified parser/analyze.c and tcop/utility.c so neither of them
calls anything from under another directory. I think it's cleaner now.
Also, I tried to implement more closely what printtup() does.
Please, review.

Best regards,
Zolt�n B�sz�rm�nyi

Attachments:

pgsql-copyselect-7.patch.gzapplication/x-gzip; name=pgsql-copyselect-7.patch.gzDownload
#4Böszörményi Zoltán
zboszor@dunaweb.hu
In reply to: Tom Lane (#2)
Re: Performance testing of COPY (SELECT) TO

It'd also be interesting to time the same way (with a temp table) in
devel. I don't remember whether we did any performance work on the
COPY CSV data path in this cycle, or whether that was all present in
8.1. In any case it'd be worth proving that the COPY SELECT patch isn't
degrading performance of the copy-a-relation case.

In the export, there is a largish table, that has both many columns and rows.

With COPY(SELECT) patch applied:

time psql -c "copy (select * from table) to 'file'" dbx
COPY 886046

real 0m13.253s
user 0m0.000s
sys 0m0.000s

time psql -c "copy table to 'file'" dbx
COPY 886046

real 0m13.234s
user 0m0.000s
sys 0m0.000s

time psql -c "copy table to stdout" dbx >file

real 0m15.155s
user 0m0.540s
sys 0m0.450s

time psql -c "copy (select * from table) to stdout" dbx >file

real 0m15.079s
user 0m0.540s
sys 0m0.590s

Surprisingly, without the COPY(SELECT) patch it's slower,
it's the lowest from five runs, e.g. with warm caches:

time psql -c "copy table to 'file'" dbx

real 0m20.464s
user 0m0.000s
sys 0m0.010s

time psql -c "copy table to stdout" dbx >file

real 0m25.753s
user 0m0.570s
sys 0m0.460s

With the original settings, temp_buffers = 1000 on 8.2CVS,
the one client case looks like this: first run 1:44, second run 1:12,
third run 1:04. It seems it's a bit faster both on startup and on
subsequent runs.

Best regards,
Zolt�n B�sz�rm�nyi

#5Zoltan Boszormenyi
zboszor@dunaweb.hu
In reply to: Böszörményi Zoltán (#3)
Re: Performance testing of COPY (SELECT) TO

Hi,

B�sz�rm�nyi Zolt�n �rta:

B�sz�rm�nyi Zolt�n <zboszor@dunaweb.hu> writes:

With PostgreSQL 8.1.4, I used this:

begin;
select ... into temp myquery1;
copy myquery1 to stdout csv delimiter '|';
rollback;

The performance of this would doubtless vary a lot with the temp_buffers
setting. Did you try different values?

Yes, I did, but now checked back with 8.2CVS.
The previously quoted result was achieved with
temp_buffers = 1000 on both 8.1.4 and 8.2CVS.
On 8.2CVS with temp_buffers = 4096, the 10 client case kills
the machine with swapping, but the 3 client runtime with
COPY(SELECT) went down to 2:41. The SELECT INTO TEMP
case went down to 3:36.

It'd also be interesting to time the same way (with a temp table) in
devel. I don't remember whether we did any performance work on the
COPY CSV data path in this cycle, or whether that was all present in
8.1. In any case it'd be worth proving that the COPY SELECT patch isn't
degrading performance of the copy-a-relation case.

I will report back with that, say on Monday.

It seems my previous mail hasn't reached
the hackers list, I answer here.

In the export, there is a largish table,
that has both many columns and rows.

With COPY(SELECT) patch applied:

time psql -c "copy (select * from table) to 'file'" dbx
COPY 886046

real 0m13.253s
user 0m0.000s
sys 0m0.000s

time psql -c "copy table to 'file'" dbx
COPY 886046

real 0m13.234s
user 0m0.000s
sys 0m0.000s

time psql -c "copy table to stdout" dbx >file

real 0m15.155s
user 0m0.540s
sys 0m0.450s

time psql -c "copy (select * from table) to stdout" dbx >file

real 0m15.079s
user 0m0.540s
sys 0m0.590s

Surprisingly, without the COPY(SELECT) patch it's slower,
this is the lowest from five runs, e.g. with warm caches:

time psql -c "copy table to 'file'" dbx

real 0m20.464s
user 0m0.000s
sys 0m0.010s

time psql -c "copy table to stdout" dbx >file

real 0m25.753s
user 0m0.570s
sys 0m0.460s

With the original settings, temp_buffers = 1000 on 8.2CVS,
the export runtime with one client looks like this:
first run 1:44, second run 1:12, third run 1:04.
It seems it's a bit faster both on startup and on
subsequent runs.

Best regards,
Zolt�n B�sz�rm�nyi

#6Bruce Momjian
bruce@momjian.us
In reply to: Böszörményi Zoltán (#3)
Re: Performance testing of COPY (SELECT) TO

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---------------------------------------------------------------------------

B���sz���rm���nyi Zolt���n wrote:

B?sz?rm?nyi Zolt?n <zboszor@dunaweb.hu> writes:

With PostgreSQL 8.1.4, I used this:

begin;
select ... into temp myquery1;
copy myquery1 to stdout csv delimiter '|';
rollback;

The performance of this would doubtless vary a lot with the temp_buffers
setting. Did you try different values?

Yes, I did, but now checked back with 8.2CVS.
The previously quoted result was achieved with
temp_buffers = 1000 on both 8.1.4 and 8.2CVS.
On 8.2CVS with temp_buffers = 4096, the 10 client case kills
the machine with swapping, but the 3 client runtime with
COPY(SELECT) went down to 2:41. The SELECT INTO TEMP
case went down to 3:36.

It'd also be interesting to time the same way (with a temp table) in
devel. I don't remember whether we did any performance work on the
COPY CSV data path in this cycle, or whether that was all present in
8.1. In any case it'd be worth proving that the COPY SELECT patch isn't
degrading performance of the copy-a-relation case.

I will report back with that, say on Monday.

In the meantime, I documented the COPY (SELECT) case
and modified parser/analyze.c and tcop/utility.c so neither of them
calls anything from under another directory. I think it's cleaner now.
Also, I tried to implement more closely what printtup() does.
Please, review.

Best regards,
Zolt?n B?sz?rm?nyi

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

#7Zoltan Boszormenyi
zboszor@dunaweb.hu
In reply to: Bruce Momjian (#6)
1 attachment(s)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Bruce Momjian �rta:

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

Thanks. Would you please add this instead?
psql built-in \copy (select ...) now also work.

Best regards,
Zolt�n B�sz�rm�nyi

Attachments:

pgsql-copyselect-8.patch.gzapplication/x-tar; name=pgsql-copyselect-8.patch.gzDownload
����Dpgsql-copyselect-8.patch�<�w�F�?���{6�0�;~t)�-[��@������bm�D%a������{gF	�����/��i��}�}������y8�c�q��WBw�?��(���������5�&F��c��v���z�����~��~������?V����J��qI	��<8a��7�o��%@��3��Y�~��V��Gv��g;���8s<;�.wgvhOy����������Y��j�k�/�>�����G�=�����W��"�7�^����v���vzN���~0���j������H���< V�S��
!��sQ�c���?�+��{���(�c>�~�|�*4��]�j��GnU������~���H���>t��.�d����]���D�=>F���gOV�'�Nmx��.��76
x��vl�{?r����0��q�zD�8��o/�����1������)6�E,'H�=�e��i(�6vJ<?AE,1[��1>�M������b�
�!�*{��G�M��v	�Ne=g�l/
�c~b��P�avx���.D�}�n
X�R@Y�8���y�*�s�cG����M��8(�$�=���Z����[��,&�����`����@�������C��4/��(z�!����Z������E���@��[W��'��'������1��R�H���F3��#��������H��:#|���_$32��@���X���Fquyw�9��z��xt���{1{��9��(IG�I)�Q���}�n�����g��	��#(Eul�*�G|4�py�?��0F�cm��#���#e�����Y�$Lf��UxH�C�CJ����b��n��H��qt���8~�����A�K��H{�"w;��6�'k���>�a�B���F�#��sa������}�
��
�m]���jD������je�������Q^���i�}�HD'���xp��`���.�v��FF���	��R�HDH�������lk��X��yp������6C�*j;I���7�w������q�p�������	�u�>��{v��1`�C`�����x��*��`�S��g��x��� ��|4�����}��2�������������p������#� �b����&���0������3���3���~���
��a���Nr1���x�HW�t?�����j����|Yv���9�i�[���3~��#7st����0w����0�<�G����#�3�aT����f:;�(����5���%3�0�8���1K^����e�-�����������|����+��
JN��,#M��md ����o� �j�@��3���s�
=rT8��?��sxC���,����L���#k&5�������aE@>r$'���Ax I=�Bx�����PNG�O^��g���u���$e�0!�p#4I�
�I�,�so�3�f�`�����r �xJ���Pcx���@@��ROHI�!t�'7�;[Q#a� ��4h ���5�=��r
�J��1b-��w��7���M����pA��;G��_Cw��)��p�@8� ��O����b.���x%Th���"\�����=
bR���
z#C���li����hl66;�����|E�U�@v*��X�f�{2,�����0?����lr6���k@���PB���?��Cu��_Oh�v���o�O�{�=��`���Nm' f@j{`���C
���li�@>���#
�C��~��#@�F�;88��������"�:���qk�oI8}w�=��������0��S5i����������:@�����J�#S����b�m��b0}�����C�H���i��a�������20@�U<e��5]Y���o`��C�����[����M �8<#�89��!�W@�~4�W@X�4$����p��P6���El�A��?m�y����^���IfG0��ZoX��+�Y��;7��*)=E�\�C���]G+��@���U�S��b��#
<��b�!�_�c����%����� 3b	fX��
X���U�j�O�"���5�6x4=H�=�Us�ul����i�>�����|���T����O�D�$������;���A�	DC��������,��8�K���+�&Q��`%�����<�!��f������%�8Q��>R
:��i���h��un��(-}�t��`�
~��!���s�����f�������{C_d�&!�x��|H��K���F�3���(YE:M�6z�h�0��4fH@��P)=D1���0!�z)�����R�^�V�_�>;�Ox�^u����F(����J���g^����v5��Q�u��;�
*�����P�\^�����Xj�z���IF	2yt�����n[��w���,UN�a�?���]�lq����`��sh����v�e�ad��$lM�Ib�p|N���z��'2Q��:6�!����r1�m[�q��������I��`�~��{��.�B�cv�p�J�zzZ=h@f]?��-4_���qn��s�~9���W�,U��nM����*��$i�P�;��5�������d�P�+�(!MCJ|*����o����^������w���A�Z	?K<�������FF�gR0�LiAn�K�oiXmk������%��	qs�/�y�J�*��F�j�������1���g�C�_�s���@�l-}�e�A#������������vf��bEL,�06�4��I���,N��R���w.��v��h>�!��a�L�#��J!�����:��������s���]��
�iL��|����`�I��>y�Qi��/�S�Q���!��T���5�+f���Y���*b��B%UJN�"Q�'�5��J�E*�%V�rt?�c�t8MZ����t~�\/�{�T����B��!{e����mD^�2"����3{|md])��+E1tE����UA��=��Q����X����*B"W�#k�"`;���
h�PJ Q|C��g������`�~�@�"�2�'�w��<#� }�C��aM�aT��E�(�A��X^�������R���(�00M`�|:���7&%I:)z�Pc���-w�����w����������7[�N��,��������>�ncv��DU���B��o�~��?V��Xg�A`:�J��C���j�6�}���k��_��,����]�da�%~�O��������-��|+�:�_����b��}*?g1'�X�E4�4�v�5`�8�1~-�����������j�4��?�������5���>��)�����(�Ez=�6�������^��U�+�m@��Ua��<H�JD������6�x	����$`�{FUi�l���"5=�DS���)�LH����DP>x�Sk6��#:�N!��P�KaD����3�n�8��C�
B�i$��=�����)B$�FcC	�-��eu��e��U��Y�q���)�}�m�'��(��'���/��B�I �e�*����WhD
o�:��5�h�g�kS?n�j�z}�B 4�x"�`���F��|S�;���"��zK����O��!�������[b��g�%o�?��x���������I�Fs�H��8��f���q�cY;?������$����Q���]�q@�������y(���3����sr����g�(K�z=�k����D<ku���.Xh��<��Q����&�[X	��=8��,�a�#�9���EY�1m��L�U����N��s����"{�"�z�Q�<�����/���`��������V��R2���zt`������W�)�I~�q����qu5"S\��t	�#��A���7����*kI�eu@�b��
��K��TM�+��2��d{eT:�k��D����R��%�}]R���Q��>�UVW��,�N�l����/�X�����P��O.4}�#/��p�m���ML�iSI�hK�����u��f?+p�.���5���a_!��\V����E!����+*aE���r�`�b�E;�!d�z��gaP��"��)v��b�����I��mIaF�S��Yu����;��7�^���V
���R����,�;�;d�v�U�,�CBR�YRl����x)��
�U�/�����wmx���2���~�����f�/Q�') {��9��M�+.������C3j
��&��w�>5�`�l���-1����kF���L9A=��K���<�>Ju����4����k~��}c�;��f�(L��gM��s�d�c,�z?X����Y`L����Sc����h�M�t+���5;��A�kBg���|��� ��}(�[q`���2���zRuee
���}�)�
��������j�M^�2,�wx�KT��*��%a�,s�v�����%U��L���g�.UfWR�����]"�2b�=�h���fr���wl��u��;���Z�T��*�*J��xZb*wX�l&I&�1J<��|b���#^"���m���8t���$&������D���p(�2-z)�������4�Z1�XO���96����?��`6=�������U������&L~@��u$[�a��fv<����0`�yR{4��`��C"1U�P�����vH���p$%���������}�,.YR��4��6|�	�k�M��v��%��I1E��uT�[�U2"Y���RI�������.9��u_R����N��A�0�jA=D�`�f������#��J)'��,W ��JE:0������i:�D?�X
��e��U�RV���>���N��zp���E7!�Z�s�
������~�O�6���w�e�%���^�!$�A����,}��Mz���Zi^d�B�/���T�A��{��q	��l�ct�0�k���"O��&������gTw*��;-!m��LZH�#o�l9���RahSC�CP��T����5����Y�N1I�:@wacN����C0�X��7�d�)�C;A�
�F�h�H7�#�{:#XxtQ�t��S����c�����!�i�}���%�����a���U��'��Gi����~��{��A�#FTL�[�w*_�Z��V�1�"����.B�B���xK�X���H��%A-�+e��L%�������;��f���C�U����I9l���~@\����*�:(b/L(�6w�nl�H�z�vV����V9�����2cV��VM����B����TGHY�/r��L��^�3�����{Y-�6��H����Yy��M�{n�hSt�
���2�s9���>��~�Kcx�M��Y��"��b��N�d�}�,_���5W*U�}�Z���/�}d-�H�����5�w0q�:���w�E�
����!��hR�S?�C��8I���bK���yaV�yE�����K�������K��K�-�2���'��8�~�������V������al�Ck�`��\+����dS��JzwDL�8&���#�����'�mM{e�K�F�e��`����j�h�����+ba��r&��ZU�J���L�$n��)������q�n#�0)�T�����,��6zU�����������lK7�wL|Z���g:���e��i/t�gJ�k>���=�+V���R�s��*S���
����,��=�C��#E��p,���zlR�������0���I�qt"���	�a#`D�����2	@^`�1��>�|<���c�	"e�""�G{Q3J2���'$q\:J4��+���Z9iH��*O�y,=�8�,��a�"���z]��^���D��U�#������V�=���7����m��Fu�:1LH��@H���Q2zI�
�wl�vn��_D�0EqZ��D��?�&��_{�������^}�	��@�$��'�R$�F��W���E!��c �l�����_3;�;�.����{]	����LO�o�{2F(i����o3��!b��i5�A����5�$}�s~��5F� Zx���-L������ �?G��54�-�%��+Q;���E�p/�k3��6��A�V|�������1;ql��U�rL	�	����u	uR�G\H��!F��Z���$��Y�S�Q�>��a����+������gO;�U��+�^
A���%�l8��!_��f`�-���&��$�� o5#��nVy#d����80�,�=�O�Uf$q� �k�|��v�4����bH�0_1�Zn��8�1l�XY�y&rW�"^���f2��+���.�� ��s�
�0%*P�!�e<�>�Z�����z	���V
O�Q�B�K�����{��4= ��j?�R������-y�a�(1���x�`������,5E�en� ~/��66�D�L�,�;�c�4w��;bpKG��V�|�KaxN��Wy>A����
�a���3�.o�d�>*�}_�u-��R�v��n���=$�	�f�J��( q��h���Q���Z��H��Z�����L@�M�X~���2���v���uw����Lg%�8P�h�*�6S�T�`o�r+���
�3oemF��J��"�|e�����|�<enL�����-����������n�9��KT�u��Vd�g�i�6������#�{O�{�@\���{.�U������J�8���;�9���
E��Ul
�����^]�O&5���O�;�Nb�����^&-���Z ��R�h�1�v\�A�='X����H�R"IW���������cr"�������T�<������!U��<����������>�1��m"P�= W������Q���&d�v�>n2u~7iY�����h��+X�(\-P��9�������^�#�t�����y�x^$3�
�)d��G�.���t���/��	YK�ybms�8Fk�C"q`�T<���O���c7n��?�Vg�y5�kE�I��{���j�G��P�G2r�����k6��f[�\��fM����B���UY<���R�3.SR���4yw;5���_"�^�:�g�����9�������-���As.@��F�������Eq��{A����6�1Y��X�=��*C>=dk�*���F����d:�z2@��5$����rT��r�!�n��LI�I�r���M��w��w�{#��\<��Z8�g�g��/���g�qZ����ur(�t�_>��X[/�g����
����>����Il�QX��a(��������u-���[o�jl�����j���	)��t����:�<���3�_��H��bcN��q����S��1g���EK�%�����|X���.<4����X��<I$)1l(�����Sf+�`O�T)�,���5��&,���_��>T L���9�H�?#$����5���7e�I=���1�$��_������������y�@�F3�b��jN[�� �ep,W0�Q%�}2x@� �BLB[,L����JD���^���U,�sO+�%��$Z���i����X�;[�yb��b��d�e�����=���FU/@U�)��\2�LIX�WL�UB#i�������(���R������>7���p^��C�\��$�O���������q��^tC��|o3�|5>,u��.��v�F��1�+�*���L����t�G�����Bc����l;h�z�����'9��7��@d0K��L�� ��/��0ujB�c_d��]�^��H�'�G�u�e(V�1�4-��b���kB
���w�
���Ml_�����;�#~G�W�m�C�aG�=�����H���K\Ku�-�i�J��>Onf�by�dA�T�vc���^x09!+��%�x[R����Q�iJa�I�t��[0��Y�a�������'S�c�ZJ�������)���)���b0�8��B.�tg�O�d���:F� �wo;�c!n��]���L���m@����no��%G�Ye���K}���IeL�D���J~��N���b��K�����%!�@T�T�ju_�n���7��>�s���'M��v�4mr�N[E�a�w�l���'?���y�t?O;s?O�{xV�zK)0z�{I��>}A��n1�_�{oO�n���=b��6J���C3���U^E���K�����
��g�y�#�h�E?��^Q����x�yGN��qc*�%��(:[�a^�$����T ���V�Q���cz�2a�U�����[�i@��C1����?�	z������_`H�
��
�|���l�Q7�/��A�L,A��$9���:]��$ �����v�.����[@h�\[��h�����������nX���	������n�+�mxR���..M6�zqv���R�m`C�V�[�L���A��#�#���:l(�����{���EP�.�hG]q�`����!���y
y��@���/�p��)IH������2�gIf�J�	�zel9�8�	��:�?G1��y���RM���Ww,8���e0�V�miT>u����g��9xH��b-��aO9��Q$�b1a��at7W��	���`��K�xM���N!�f����6�d<bL2���!�+x]W&���e�'d����%�����8i��.2�k����'y���/+�F���	'��7��+:�A��~��e)�N5���(U7��oS}�3�����pdm�8Fc��^�,���fA�Kr���>������������EC�&����x1��US�x�[�rY�44*�XW������_[����<��Tgf��1�e�(|��x/����/��������e�����37z@��Y�m�vd���7��EO�V��i�Y���y��m"(��b�:��[��=���^���W,�����)_���%T
&������|
��ho��k��o��$F&�70I�&�E�9����"���C��(������	2�!B �}R��4?Y`���A}���(c�`-t�`�rr�/+-�����P�� ����)]v���]SP�r�6�r!�`��#|�}�v��7����o�=>�9��t�����������t��������%���Hs9����!>��g����l�
�B*���	����*OW��N�������/�����m�G���^���1�;{���>�w��R;�(���1���8�4�����
A�hO�f������i���x���p�dkAJ����g�$����(����Q�~���������^]W��^[p��Q���u��X��g�ok3��(q�����b��/�w��?>~rM%�5�����.c��a�����n�vJ�i_.��O��� >6^��K�5��BS��]��H�8���n*}������_`P��F�RT��f��6}z�B<1HO�^�Lq�N`�8H����O�y�����|q�
H�D�P��U|g���/S�!����$���h
h!�D���n����"���*���8��VH}�\��e�������o��`����Z����ld��=W��I<�
^��F���Q��i�j�m�x(�
����x2#i�/�WY�$�i>O2�l�Pz��U8�'���Ch��A�N����C^~������Zv����%�r�����fR�-���m�����|����=�@�u3��JD�<mu�8�Y�q�`��.,���\���� O{��{�� Mo��;��Q����4�U��������a$�\���^R���l�n1����.��Y��y��iE��N��Zj��"�Iv�4����N�'���lY�wbp,�-0����>9�������aHO��^�4��6^�{��-�7^6�������vT�q�(���14�C�/Gj��uD���������+�����
^!�g�C_�������?��;�1Q�Y��6$����C����a�H�6��M��V�/�t���\D����3�
J�`�W�E��J����DOz������S|z$FX��N��o�m�t�?������1�i���Gus�t����#0��1��!1#"|!��/f��iu1y'��!�h�@�Bk}�B��@J���
!P)H��F�=/`��]��D"�L��l��"��������]�	�Y��X����Z����'kC�{�g
\r��v*��n5�{3�Q�{�Oi����Tr��C^K�on2�:������F(���(� Y0�Z��L��>RN-aM��n��	�{�P�XU�0��fD���6#�{�aY�s*����by�$�����@qs50������W��B�����w�
�)��eV 0������x���#�s0�
��r�q���lc<��%Z6�Yj���_����hL�v"����u"�$���H�C�6����pE�.��``�]��~*������)�T�;i��UK�X��>���S�m$�����6a�\�U�7Wl�;��������ut�������]����y��s����������gH�����E�E_v�C��?�\�&�>Z���mw��������riH=�N�p8()�� ~<@�5s���{��E��0@J}��s�w
�AK�%	M������}��c�J2s�=��Yi%��iQz���R��� uYj;��m����}�:��Q���n:DK�m��i3���y��(E����&���{����fV�b���`g�~�B�(�rA��l2U��59���:��j-�U�W��n��v�4���\G�
�I{����T)��������O��H?�,�������sa]�	]�3%�/��#0�1��q)s
�����u G\'\��HE��5�(
�&���4��������^R���M>�^���]���]Z�������nU���n(�}�Vg^XV��2���I",��	z%He���v�@�.�c1�j�yg�#O��w��� ���U���m�j��!O�1������/������&����|�>��`�Ejv�O-J��APO�#�=�j��T%���gz��T��/�6y����	�j�2ID{��F��F�(=�V��V�)8k&�w�6�Q����Z"��:���'7bT9w4xa�>)w���jL����h�J� ]\v�O���_�w�T���'g��%y��D�����=�mQ7t��F���'��xP15!��]z���}�Z��]��e�p-�j�1:����DW�&�m����Z1�M�~�A�!e������Y��M��tu�nu��mJ�Z����i�3"�g9���fb�-0���0
zka3�Y[A-�	ka;z��8+7��N~�=����>~���_v�a��T�[�=D*�9d�������;�:[��&����-�&)
/��Y}��[�7���f�w�U����G���$�����")*������������P����EAQ���y9��r�Z-=��c����4�����)����-�������6e�QrI�VR�23cK����
���	��m���!��X
��:�~N����{�J�Y����:ty�sv
k���-����m<Z[�W/}�m	o{�g_K���iVv��z<X��J��:nWx��F<�/���W�*��xyA�.���s��n��oZ|�����p�i��!�9��Vy$�lQ�(C��J���2
G�y��pp�j�B�1����r����Tk�=wX��LE�0������Q<��?NMr�t�u�)*�vN$�]150�>��?\�{��b�-�lmS��t0��w�H�tP`'�_x��b6Pof��)�9�FT������E������Y����&�Z�������S�������O-����+�������CD�����'?P��&��wI�\y�/�)���a�b�X�}���"��/P�w:��
#8Alvaro Herrera
alvherre@commandprompt.com
In reply to: Zoltan Boszormenyi (#7)
1 attachment(s)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Removed Cc: to pgsql-hackers.

Zolt�n,

Zoltan Boszormenyi wrote:

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

Thanks. Would you please add this instead?
psql built-in \copy (select ...) now also work.

Please check this one out. I took the version you posted here and
changed the stuff in the parser that I didn't like, and removed the ugly
"SELECT * FROM" stuff that was bothering me. I also removed the
transformCopyStmt stuff as it seems unnecessary to me. I did all that
stuff in a cleaner way (IMO).

I also cleaned up the grammar -- basically added a separate case from
the regular COPY. I took the opportunity to remove the
backwards-compatible options from there. I didn't check that stuff very
much but it should continue to work ...

I noticed that this works:

alvherre=# copy (values (1, 'uno'), (2, 'dos'), (3, 'tr;es'), (4, NULL)) to stdout with delimiter ';' null 'NUL' csv quote as '"';
1;uno
2;dos
3;"tr;es"
4;NUL

which is nice.

With this patch, the COPY view FROM stdout path now throws an error --
in your version it worked (because of that "COPY * FROM" stuff), and
from previous discussion it seems reasonable to behave differently for
views than for plain tables (i.e. it's reasonable that we fail for
views).

I also broke the check for a FOR UPDATE clause. Not sure where but it
must be easy to fix :-) I'd do it myself but I'm heading to bed right
now.

I also wanted to check these hunks in your patch, which I didn't like
very much:

-ERROR:  column "a" of relation "test" does not exist
+ERROR:  column "a" does not exist

but didn't got around to it.

I also noticed that the new copyselect regression test is not added to
the serial schedule.

I'll repost a reworked version at some point, if no one beats me to it.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Attachments:

copyview-9.patchtext/plain; charset=us-asciiDownload
Index: doc/src/sgml/ref/copy.sgml
===================================================================
RCS file: /home/alvherre/cvs/pgsql/doc/src/sgml/ref/copy.sgml,v
retrieving revision 1.74
diff -c -p -r1.74 copy.sgml
*** doc/src/sgml/ref/copy.sgml	22 Apr 2006 03:03:11 -0000	1.74
--- doc/src/sgml/ref/copy.sgml	27 Aug 2006 04:53:19 -0000
***************
*** 1,5 ****
  <!--
! $PostgreSQL: pgsql/doc/src/sgml/ref/copy.sgml,v 1.73 2006/03/03 19:54:10 tgl Exp $
  PostgreSQL documentation
  -->
  
--- 1,5 ----
  <!--
! $PostgreSQL: pgsql/doc/src/sgml/ref/copy.sgml,v 1.74 2006-04-22 03:03:11 momjian Exp $
  PostgreSQL documentation
  -->
  
*************** COPY <replaceable class="parameter">tabl
*** 33,39 ****
                  [ ESCAPE [ AS ] '<replaceable class="parameter">escape</replaceable>' ]
                  [ FORCE NOT NULL <replaceable class="parameter">column</replaceable> [, ...] ]
  
! COPY <replaceable class="parameter">tablename</replaceable> [ ( <replaceable class="parameter">column</replaceable> [, ...] ) ]
      TO { '<replaceable class="parameter">filename</replaceable>' | STDOUT }
      [ [ WITH ] 
            [ BINARY ]
--- 33,39 ----
                  [ ESCAPE [ AS ] '<replaceable class="parameter">escape</replaceable>' ]
                  [ FORCE NOT NULL <replaceable class="parameter">column</replaceable> [, ...] ]
  
! COPY { <replaceable class="parameter">tablename</replaceable> | <replaceable class="parameter">viewname</replaceable> | ( select_statement ) } [ ( <replaceable class="parameter">column</replaceable> [, ...] ) ]
      TO { '<replaceable class="parameter">filename</replaceable>' | STDOUT }
      [ [ WITH ] 
            [ BINARY ]
*************** COPY <replaceable class="parameter">tabl
*** 55,61 ****
     <command>COPY</command> moves data between
     <productname>PostgreSQL</productname> tables and standard file-system
     files. <command>COPY TO</command> copies the contents of a table
!    <emphasis>to</> a file, while <command>COPY FROM</command> copies
     data <emphasis>from</> a file to a table (appending the data to
     whatever is in the table already).
    </para>
--- 55,63 ----
     <command>COPY</command> moves data between
     <productname>PostgreSQL</productname> tables and standard file-system
     files. <command>COPY TO</command> copies the contents of a table
!    <emphasis>to</> a file, which also work on views and arbitrary
!    SELECT statements. (Internally, the view case is rewitten as
!    <command>SELECT * FROM viewname</command>.) <command>COPY FROM</command> copies
     data <emphasis>from</> a file to a table (appending the data to
     whatever is in the table already).
    </para>
*************** COPY <replaceable class="parameter">tabl
*** 65,71 ****
     only copy the data in the specified columns to or from the file.
     If there are any columns in the table that are not in the column list,
     <command>COPY FROM</command> will insert the default values for
!    those columns.
    </para>
  
    <para>
--- 67,76 ----
     only copy the data in the specified columns to or from the file.
     If there are any columns in the table that are not in the column list,
     <command>COPY FROM</command> will insert the default values for
!    those columns. <command>COPY TO</command> also accepts the list of
!    columns, which can be useful for exporting only a subset of the
!    columns that are in the table, view or select statement, or
!    reordering them in the export.
    </para>
  
    <para>
*************** COPY <replaceable class="parameter">tabl
*** 148,154 ****
       <para>
        Specifies copying the OID for each row.  (An error is raised if
        <literal>OIDS</literal> is specified for a table that does not
!       have OIDs.)
       </para>
      </listitem>
     </varlistentry>
--- 153,159 ----
       <para>
        Specifies copying the OID for each row.  (An error is raised if
        <literal>OIDS</literal> is specified for a table that does not
!       have OIDs, or in the case of COPY (SELECT) TO.)
       </para>
      </listitem>
     </varlistentry>
Index: src/backend/commands/copy.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/backend/commands/copy.c,v
retrieving revision 1.268
diff -c -p -r1.268 copy.c
*** src/backend/commands/copy.c	14 Jul 2006 14:52:18 -0000	1.268
--- src/backend/commands/copy.c	28 Aug 2006 03:12:07 -0000
***************
*** 31,38 ****
  #include "libpq/pqformat.h"
  #include "mb/pg_wchar.h"
  #include "miscadmin.h"
! #include "parser/parse_relation.h"
  #include "rewrite/rewriteHandler.h"
  #include "storage/fd.h"
  #include "tcop/tcopprot.h"
  #include "utils/acl.h"
--- 31,39 ----
  #include "libpq/pqformat.h"
  #include "mb/pg_wchar.h"
  #include "miscadmin.h"
! #include "parser/analyze.h"
  #include "rewrite/rewriteHandler.h"
+ #include "optimizer/planner.h"
  #include "storage/fd.h"
  #include "tcop/tcopprot.h"
  #include "utils/acl.h"
*************** typedef struct CopyStateData
*** 96,104 ****
  	bool		need_transcoding;		/* client encoding diff from server? */
  	bool		encoding_embeds_ascii;	/* ASCII can be non-first byte? */
  	uint64		processed;		/* # of tuples processed */
  
  	/* parameters from the COPY command */
! 	Relation	rel;			/* relation to copy to or from */
  	List	   *attnumlist;		/* integer list of attnums to copy */
  	bool		binary;			/* binary format? */
  	bool		oids;			/* include OIDs? */
--- 97,125 ----
  	bool		need_transcoding;		/* client encoding diff from server? */
  	bool		encoding_embeds_ascii;	/* ASCII can be non-first byte? */
  	uint64		processed;		/* # of tuples processed */
+ 	FmgrInfo   *out_functions;		/* array of output functions (COPY TO) */
+ 	MemoryContext rowcxt;			/* memory for output formatting */
+ 	List	   *force_quote_orig;		/* quote attribute names */
+ 	List	   *force_notnull;
+ 	bool	   *force_quote;		/* quote attribute? */
+ 	char	   *null_print_client;		/* final version of null_print */
+ 	Datum	   *values;			/* row data (COPY TO) */
+ 	bool	   *isnull;			/* row 'isnull' flags (COPY TO) */
+ 	Oid			out_func_oid;
+ 	bool		isvarlena;		/* tuple should be de-toasted */
+ 	TupleDesc	attrinfo;
+ 	int			natts;
  
  	/* parameters from the COPY command */
! 	/* relation to copy to or from, in the "COPY table" case */
! 	Relation	rel;
! 
! 	/* The following are valid in the "COPY (SELECT)" case: */
! 	DestReceiver	*dest;		/* DestReceiver for the COPY (SELECT) case */
! 	QueryDesc	*desc;			/* descriptor for SELECT query */
! 	Plan		*plan;			/* plan for SELECT */
! 
! 	/* These are valid in both cases */
  	List	   *attnumlist;		/* integer list of attnums to copy */
  	bool		binary;			/* binary format? */
  	bool		oids;			/* include OIDs? */
*************** typedef struct CopyStateData
*** 153,158 ****
--- 174,184 ----
  
  typedef CopyStateData *CopyState;
  
+ typedef struct
+ {
+ 	DestReceiver pub;	/* publicly-known function pointers */
+ 	void	*cstate;	/* CopyStateData we are working with */
+ } DR_copy;
  
  /*
   * These macros centralize code used to process line_buf and raw_buf buffers.
*************** static const char BinarySignature[11] = 
*** 223,230 ****
--- 249,259 ----
  
  
  /* non-export function prototypes */
+ static uint64 DoCopyRelation(const CopyStmt *stmt, CopyState cstate);
+ static uint64 DoCopySelect(const CopyStmt *stmt, CopyState cstate);
  static void DoCopyTo(CopyState cstate);
  static void CopyTo(CopyState cstate);
+ static void CopyValuesTo(CopyState cstate, Oid oid, Datum *values, bool *isnull);
  static void CopyFrom(CopyState cstate);
  static bool CopyReadLine(CopyState cstate);
  static bool CopyReadLineText(CopyState cstate);
*************** static Datum CopyReadBinaryAttribute(Cop
*** 239,246 ****
  static void CopyAttributeOutText(CopyState cstate, char *string);
  static void CopyAttributeOutCSV(CopyState cstate, char *string,
  					bool use_quote, bool single_attr);
! static List *CopyGetAttnums(Relation rel, List *attnamelist);
  static char *limit_printout_length(const char *str);
  
  /* Low-level communications functions */
  static void SendCopyBegin(CopyState cstate);
--- 268,276 ----
  static void CopyAttributeOutText(CopyState cstate, char *string);
  static void CopyAttributeOutCSV(CopyState cstate, char *string,
  					bool use_quote, bool single_attr);
! static List *CopyGetAttnums(TupleDesc tupDesc, List *attnamelist);
  static char *limit_printout_length(const char *str);
+ static void prepare_cstate_info(CopyState cstate, TupleDesc typeinfo, int numAttrs);
  
  /* Low-level communications functions */
  static void SendCopyBegin(CopyState cstate);
*************** uint64
*** 697,713 ****
  DoCopy(const CopyStmt *stmt)
  {
  	CopyState	cstate;
- 	RangeVar   *relation = stmt->relation;
- 	char	   *filename = stmt->filename;
- 	bool		is_from = stmt->is_from;
- 	bool		pipe = (stmt->filename == NULL);
- 	List	   *attnamelist = stmt->attlist;
- 	List	   *force_quote = NIL;
- 	List	   *force_notnull = NIL;
- 	AclMode		required_access = (is_from ? ACL_INSERT : ACL_SELECT);
- 	AclResult	aclresult;
  	ListCell   *option;
- 	uint64		processed;
  
  	/* Allocate workspace and zero all fields */
  	cstate = (CopyStateData *) palloc0(sizeof(CopyStateData));
--- 727,733 ----
*************** DoCopy(const CopyStmt *stmt)
*** 783,801 ****
  		}
  		else if (strcmp(defel->defname, "force_quote") == 0)
  		{
! 			if (force_quote)
  				ereport(ERROR,
  						(errcode(ERRCODE_SYNTAX_ERROR),
  						 errmsg("conflicting or redundant options")));
! 			force_quote = (List *) defel->arg;
  		}
  		else if (strcmp(defel->defname, "force_notnull") == 0)
  		{
! 			if (force_notnull)
  				ereport(ERROR,
  						(errcode(ERRCODE_SYNTAX_ERROR),
  						 errmsg("conflicting or redundant options")));
! 			force_notnull = (List *) defel->arg;
  		}
  		else
  			elog(ERROR, "option \"%s\" not recognized",
--- 803,821 ----
  		}
  		else if (strcmp(defel->defname, "force_quote") == 0)
  		{
! 			if (cstate->force_quote_orig)
  				ereport(ERROR,
  						(errcode(ERRCODE_SYNTAX_ERROR),
  						 errmsg("conflicting or redundant options")));
! 			cstate->force_quote_orig = (List *) defel->arg;
  		}
  		else if (strcmp(defel->defname, "force_notnull") == 0)
  		{
! 			if (cstate->force_notnull)
  				ereport(ERROR,
  						(errcode(ERRCODE_SYNTAX_ERROR),
  						 errmsg("conflicting or redundant options")));
! 			cstate->force_notnull = (List *) defel->arg;
  		}
  		else
  			elog(ERROR, "option \"%s\" not recognized",
*************** DoCopy(const CopyStmt *stmt)
*** 888,908 ****
  				 errmsg("COPY escape must be a single character")));
  
  	/* Check force_quote */
! 	if (!cstate->csv_mode && force_quote != NIL)
  		ereport(ERROR,
  				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
  				 errmsg("COPY force quote available only in CSV mode")));
! 	if (force_quote != NIL && is_from)
  		ereport(ERROR,
  				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
  				 errmsg("COPY force quote only available using COPY TO")));
  
  	/* Check force_notnull */
! 	if (!cstate->csv_mode && force_notnull != NIL)
  		ereport(ERROR,
  				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
  				 errmsg("COPY force not null available only in CSV mode")));
! 	if (force_notnull != NIL && !is_from)
  		ereport(ERROR,
  				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
  			  errmsg("COPY force not null only available using COPY FROM")));
--- 908,928 ----
  				 errmsg("COPY escape must be a single character")));
  
  	/* Check force_quote */
! 	if (!cstate->csv_mode && cstate->force_quote != NULL)
  		ereport(ERROR,
  				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
  				 errmsg("COPY force quote available only in CSV mode")));
! 	if (cstate->force_quote != NULL && stmt->is_from)
  		ereport(ERROR,
  				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
  				 errmsg("COPY force quote only available using COPY TO")));
  
  	/* Check force_notnull */
! 	if (!cstate->csv_mode && cstate->force_notnull != NIL)
  		ereport(ERROR,
  				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
  				 errmsg("COPY force not null available only in CSV mode")));
! 	if (cstate->force_notnull != NIL && !stmt->is_from)
  		ereport(ERROR,
  				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
  			  errmsg("COPY force not null only available using COPY FROM")));
*************** DoCopy(const CopyStmt *stmt)
*** 920,931 ****
  				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
  				 errmsg("CSV quote character must not appear in the NULL specification")));
  
  	/* Open and lock the relation, using the appropriate lock type. */
  	cstate->rel = heap_openrv(relation,
! 							  (is_from ? RowExclusiveLock : AccessShareLock));
  
  	/* check read-only transaction */
! 	if (XactReadOnly && is_from &&
  		!isTempNamespace(RelationGetNamespace(cstate->rel)))
  		ereport(ERROR,
  				(errcode(ERRCODE_READ_ONLY_SQL_TRANSACTION),
--- 940,975 ----
  				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
  				 errmsg("CSV quote character must not appear in the NULL specification")));
  
+ 	if (stmt->filename != NULL && !superuser())
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ 				 errmsg("must be superuser to COPY to or from a file"),
+ 				 errhint("Anyone can COPY to stdout or from stdin. "
+ 						 "psql's \\copy command also works for anyone.")));
+ 
+ 	if (stmt->relation)
+ 		return DoCopyRelation(stmt, cstate);
+ 	else
+ 		return DoCopySelect(stmt, cstate);
+ }
+ 
+ static uint64
+ DoCopyRelation(const CopyStmt *stmt, CopyState cstate)
+ {
+ 	RangeVar   *relation = stmt->relation;
+ 	char	   *filename = stmt->filename;
+ 	bool		pipe = (stmt->filename == NULL);
+ 	List	   *attnamelist = stmt->attlist;
+ 	AclMode		required_access = (stmt->is_from ? ACL_INSERT : ACL_SELECT);
+ 	AclResult	aclresult;
+ 	uint64		processed;
+ 
  	/* Open and lock the relation, using the appropriate lock type. */
  	cstate->rel = heap_openrv(relation,
! 							  (stmt->is_from ? RowExclusiveLock : AccessShareLock));
  
  	/* check read-only transaction */
! 	if (XactReadOnly && stmt->is_from &&
  		!isTempNamespace(RelationGetNamespace(cstate->rel)))
  		ereport(ERROR,
  				(errcode(ERRCODE_READ_ONLY_SQL_TRANSACTION),
*************** DoCopy(const CopyStmt *stmt)
*** 937,948 ****
  	if (aclresult != ACLCHECK_OK)
  		aclcheck_error(aclresult, ACL_KIND_CLASS,
  					   RelationGetRelationName(cstate->rel));
- 	if (!pipe && !superuser())
- 		ereport(ERROR,
- 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- 				 errmsg("must be superuser to COPY to or from a file"),
- 				 errhint("Anyone can COPY to stdout or from stdin. "
- 						 "psql's \\copy command also works for anyone.")));
  
  	/* Don't allow COPY w/ OIDs to or from a table without them */
  	if (cstate->oids && !cstate->rel->rd_rel->relhasoids)
--- 981,986 ----
*************** DoCopy(const CopyStmt *stmt)
*** 952,967 ****
  						RelationGetRelationName(cstate->rel))));
  
  	/* Generate or convert list of attributes to process */
! 	cstate->attnumlist = CopyGetAttnums(cstate->rel, attnamelist);
  
  	/* Convert FORCE QUOTE name list to column numbers, check validity */
! 	if (force_quote)
  	{
  		TupleDesc	tupDesc = RelationGetDescr(cstate->rel);
  		Form_pg_attribute *attr = tupDesc->attrs;
  		ListCell   *cur;
  
! 		cstate->force_quote_atts = CopyGetAttnums(cstate->rel, force_quote);
  
  		foreach(cur, cstate->force_quote_atts)
  		{
--- 990,1005 ----
  						RelationGetRelationName(cstate->rel))));
  
  	/* Generate or convert list of attributes to process */
! 	cstate->attnumlist = CopyGetAttnums(RelationGetDescr(cstate->rel), attnamelist);
  
  	/* Convert FORCE QUOTE name list to column numbers, check validity */
! 	if (cstate->force_quote_orig)
  	{
  		TupleDesc	tupDesc = RelationGetDescr(cstate->rel);
  		Form_pg_attribute *attr = tupDesc->attrs;
  		ListCell   *cur;
  
! 		cstate->force_quote_atts = CopyGetAttnums(tupDesc, cstate->force_quote_orig);
  
  		foreach(cur, cstate->force_quote_atts)
  		{
*************** DoCopy(const CopyStmt *stmt)
*** 976,989 ****
  	}
  
  	/* Convert FORCE NOT NULL name list to column numbers, check validity */
! 	if (force_notnull)
  	{
  		TupleDesc	tupDesc = RelationGetDescr(cstate->rel);
  		Form_pg_attribute *attr = tupDesc->attrs;
  		ListCell   *cur;
  
! 		cstate->force_notnull_atts = CopyGetAttnums(cstate->rel,
! 													force_notnull);
  
  		foreach(cur, cstate->force_notnull_atts)
  		{
--- 1014,1026 ----
  	}
  
  	/* Convert FORCE NOT NULL name list to column numbers, check validity */
! 	if (cstate->force_notnull)
  	{
  		TupleDesc	tupDesc = RelationGetDescr(cstate->rel);
  		Form_pg_attribute *attr = tupDesc->attrs;
  		ListCell   *cur;
  
! 		cstate->force_notnull_atts = CopyGetAttnums(tupDesc, cstate->force_notnull);
  
  		foreach(cur, cstate->force_notnull_atts)
  		{
*************** DoCopy(const CopyStmt *stmt)
*** 1019,1025 ****
  
  	cstate->copy_dest = COPY_FILE;		/* default */
  
! 	if (is_from)
  	{							/* copy from file to database */
  		if (cstate->rel->rd_rel->relkind != RELKIND_RELATION)
  		{
--- 1056,1062 ----
  
  	cstate->copy_dest = COPY_FILE;		/* default */
  
! 	if (stmt->is_from)
  	{							/* copy from file to database */
  		if (cstate->rel->rd_rel->relkind != RELKIND_RELATION)
  		{
*************** DoCopy(const CopyStmt *stmt)
*** 1149,1159 ****
  	 * got; if writing, we should hold the lock until end of transaction to
  	 * ensure that updates will be committed before lock is released.
  	 */
! 	heap_close(cstate->rel, (is_from ? NoLock : AccessShareLock));
  
  	/* Clean up storage (probably not really necessary) */
  	processed = cstate->processed;
  
  	pfree(cstate->attribute_buf.data);
  	pfree(cstate->line_buf.data);
  	pfree(cstate->raw_buf);
--- 1186,1372 ----
  	 * got; if writing, we should hold the lock until end of transaction to
  	 * ensure that updates will be committed before lock is released.
  	 */
! 	heap_close(cstate->rel, (stmt->is_from ? NoLock : AccessShareLock));
! 
! 	/* Clean up storage (probably not really necessary) */
! 	processed = cstate->processed;
! 
! 	pfree(cstate->attribute_buf.data);
! 	pfree(cstate->line_buf.data);
! 	pfree(cstate->raw_buf);
! 	pfree(cstate);
! 
! 	return processed;
! }
! 
! static uint64
! DoCopySelect(const CopyStmt *stmt, CopyState cstate)
! {
! 	char	   *filename = stmt->filename;
! 	bool		pipe = (stmt->filename == NULL);
! 	List	   *attnamelist = stmt->attlist;
! 	uint64		processed = 0;
! 	List	   *queries;
! 	Query	   *query;
! 
! 	/* Don't allow COPY w/ OIDs from a select */
! 	if (cstate->oids)
! 		ereport(ERROR,
! 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
! 				 errmsg("COPY (SELECT) WITH OIDS is not supported")));
! 
! 	queries = parse_analyze((Node *) stmt->selectstmt, NULL, NULL, 0);
! 
! 	/* shouldn't happen */
! 	if (list_length(queries) != 1)
! 		elog(ERROR, "COPY (SELECT) returns more than one Query");
! 	query = linitial(queries);
! 
! 	/* pass the query through the rewriter */
! 	queries = QueryRewrite(query);
! 
! 	/* shouldn't happen, or could it? */
! 	if (list_length(queries) != 1)
! 		elog(ERROR, "COPY (SELECT) returns more than one Query after rewrite");
! 	query = linitial(queries);
! 
! 	cstate->plan = planner(query, true, 0, NULL);
! 
! 	cstate->dest = CreateDestReceiver(DestCopyDR, NULL);
! 	((DR_copy *) cstate->dest)->cstate = cstate;
! 	cstate->desc = CreateQueryDesc(query, cstate->plan,
! 								   ActiveSnapshot, InvalidSnapshot,
! 								   cstate->dest, NULL, false);
! 	/* Execute query */
! 	ExecutorStart(cstate->desc, false);
! 
! 	/* Generate or convert list of attributes to process */
! 	cstate->attnumlist = CopyGetAttnums(cstate->desc->tupDesc, attnamelist);
! 
! 	/* Convert FORCE QUOTE name list to column numbers, check validity */
! 	if (cstate->force_quote_orig)
! 	{
! 		TupleDesc	tupDesc = cstate->desc->tupDesc;
! 		Form_pg_attribute *attr = tupDesc->attrs;
! 		ListCell   *cur;
! 
! 		cstate->force_quote_atts = CopyGetAttnums(tupDesc, cstate->force_quote_orig);
! 
! 		foreach(cur, cstate->force_quote_atts)
! 		{
! 			int			attnum = lfirst_int(cur);
! 
! 			if (!list_member_int(cstate->attnumlist, attnum))
! 				ereport(ERROR,
! 						(errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
! 				   errmsg("FORCE QUOTE column \"%s\" not referenced by COPY",
! 						  NameStr(attr[attnum - 1]->attname))));
! 		}
! 	}
! 
! 	/* Convert FORCE NOT NULL name list to column numbers, check validity */
! 	if (cstate->force_notnull)
! 	{
! 		TupleDesc	tupDesc = cstate->desc->tupDesc;
! 		Form_pg_attribute *attr = tupDesc->attrs;
! 		ListCell   *cur;
! 
! 		cstate->force_notnull_atts = CopyGetAttnums(tupDesc, cstate->force_notnull);
! 
! 		foreach(cur, cstate->force_notnull_atts)
! 		{
! 			int			attnum = lfirst_int(cur);
! 
! 			if (!list_member_int(cstate->attnumlist, attnum))
! 				ereport(ERROR,
! 						(errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
! 				errmsg("FORCE NOT NULL column \"%s\" not referenced by COPY",
! 					   NameStr(attr[attnum - 1]->attname))));
! 		}
! 	}
! 
! 	/* Set up variables to avoid per-attribute overhead. */
! 	initStringInfo(&cstate->attribute_buf);
! 	initStringInfo(&cstate->line_buf);
! 	cstate->line_buf_converted = false;
! 	cstate->raw_buf = (char *) palloc(RAW_BUF_SIZE + 1);
! 	cstate->raw_buf_index = cstate->raw_buf_len = 0;
! 	cstate->processed = 0;
! 
! 	/*
! 	 * Set up encoding conversion info.  Even if the client and server
! 	 * encodings are the same, we must apply pg_client_to_server() to
! 	 * validate data in multibyte encodings.
! 	 */
! 	cstate->client_encoding = pg_get_client_encoding();
! 	cstate->need_transcoding =
! 		(cstate->client_encoding != GetDatabaseEncoding() ||
! 		 pg_database_encoding_max_length() > 1);
! 	/* See Multibyte encoding comment above */
! 	cstate->encoding_embeds_ascii = PG_ENCODING_IS_CLIENT_ONLY(cstate->client_encoding);
! 
! 	cstate->copy_dest = COPY_FILE;		/* default */
! 
! 	/* copy from database to file */
! 	if (pipe)
! 	{
! 		if (whereToSendOutput == DestRemote)
! 			cstate->fe_copy = true;
! 		else
! 			cstate->copy_file = stdout;
! 	}
! 	else
! 	{
! 		mode_t		oumask; /* Pre-existing umask value */
! 		struct stat st;
! 
! 		/*
! 		 * Prevent write to relative path ... too easy to shoot oneself in
! 		 * the foot by overwriting a database file ...
! 		 */
! 		if (!is_absolute_path(filename))
! 			ereport(ERROR,
! 					(errcode(ERRCODE_INVALID_NAME),
! 					  errmsg("relative path not allowed for COPY to file")));
! 
! 		oumask = umask((mode_t) 022);
! 		cstate->copy_file = AllocateFile(filename, PG_BINARY_W);
! 		umask(oumask);
! 
! 		if (cstate->copy_file == NULL)
! 			ereport(ERROR,
! 					(errcode_for_file_access(),
! 					 errmsg("could not open file \"%s\" for writing: %m",
! 							filename)));
! 
! 		fstat(fileno(cstate->copy_file), &st);
! 		if (S_ISDIR(st.st_mode))
! 		{
! 			FreeFile(cstate->copy_file);
! 			ereport(ERROR,
! 					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
! 					 errmsg("\"%s\" is a directory", filename)));
! 		}
! 	}
! 
! 	DoCopyTo(cstate);
! 
! 	if (!pipe)
! 	{
! 		/* we assume only the write case could fail here */
! 		if (FreeFile(cstate->copy_file))
! 			ereport(ERROR,
! 					(errcode_for_file_access(),
! 					 errmsg("could not write to file \"%s\": %m",
! 							filename)));
! 	}
  
  	/* Clean up storage (probably not really necessary) */
  	processed = cstate->processed;
  
+ 	ExecutorEnd(cstate->desc);
+ 	FreeQueryDesc(cstate->desc);
+ 
  	pfree(cstate->attribute_buf.data);
  	pfree(cstate->line_buf.data);
  	pfree(cstate->raw_buf);
*************** DoCopyTo(CopyState cstate)
*** 1193,1250 ****
  	PG_END_TRY();
  }
  
  /*
!  * Copy from relation TO file.
   */
  static void
  CopyTo(CopyState cstate)
  {
- 	HeapTuple	tuple;
  	TupleDesc	tupDesc;
- 	HeapScanDesc scandesc;
  	int			num_phys_attrs;
- 	int			attr_count;
  	Form_pg_attribute *attr;
- 	FmgrInfo   *out_functions;
- 	bool	   *force_quote;
- 	char	   *string;
- 	char	   *null_print_client;
  	ListCell   *cur;
- 	MemoryContext oldcontext;
- 	MemoryContext mycontext;
  
! 	tupDesc = cstate->rel->rd_att;
  	attr = tupDesc->attrs;
  	num_phys_attrs = tupDesc->natts;
! 	attr_count = list_length(cstate->attnumlist);
! 	null_print_client = cstate->null_print;		/* default */
  
  	/* We use fe_msgbuf as a per-row buffer regardless of copy_dest */
  	cstate->fe_msgbuf = makeStringInfo();
  
  	/* Get info about the columns we need to process. */
! 	out_functions = (FmgrInfo *) palloc(num_phys_attrs * sizeof(FmgrInfo));
! 	force_quote = (bool *) palloc(num_phys_attrs * sizeof(bool));
  	foreach(cur, cstate->attnumlist)
  	{
  		int			attnum = lfirst_int(cur);
- 		Oid			out_func_oid;
- 		bool		isvarlena;
- 
- 		if (cstate->binary)
- 			getTypeBinaryOutputInfo(attr[attnum - 1]->atttypid,
- 									&out_func_oid,
- 									&isvarlena);
- 		else
- 			getTypeOutputInfo(attr[attnum - 1]->atttypid,
- 							  &out_func_oid,
- 							  &isvarlena);
- 		fmgr_info(out_func_oid, &out_functions[attnum - 1]);
  
  		if (list_member_int(cstate->force_quote_atts, attnum))
! 			force_quote[attnum - 1] = true;
  		else
! 			force_quote[attnum - 1] = false;
  	}
  
  	/*
--- 1406,1479 ----
  	PG_END_TRY();
  }
  
+ static void
+ prepare_cstate_info(CopyState cstate, TupleDesc typeinfo, int numAttrs)
+ {
+ 	Form_pg_attribute *attr;
+ 	ListCell   *cur;
+ 
+ 	/* get rid of any old data */
+ 	attr = typeinfo->attrs;
+ 	cstate->attrinfo = typeinfo;
+ 	cstate->natts = numAttrs;
+ 	if (numAttrs <= 0)
+ 		return;
+ 
+ 	foreach(cur, cstate->attnumlist)
+ 	{
+ 		int			attnum = lfirst_int(cur);
+ 
+ 		if (cstate->binary)
+ 			getTypeBinaryOutputInfo(attr[attnum - 1]->atttypid,
+ 									&cstate->out_func_oid,
+ 									&cstate->isvarlena);
+ 		else
+ 			getTypeOutputInfo(attr[attnum - 1]->atttypid,
+ 							  &cstate->out_func_oid,
+ 							  &cstate->isvarlena);
+ 		fmgr_info(cstate->out_func_oid, &cstate->out_functions[attnum - 1]);
+ 	}
+ }
+ 
  /*
!  * Copy from select or relation TO file.
   */
  static void
  CopyTo(CopyState cstate)
  {
  	TupleDesc	tupDesc;
  	int			num_phys_attrs;
  	Form_pg_attribute *attr;
  	ListCell   *cur;
  
! 	if (cstate->rel)
! 		tupDesc = cstate->rel->rd_att;
! 	else
! 		tupDesc = cstate->desc->tupDesc;
  	attr = tupDesc->attrs;
  	num_phys_attrs = tupDesc->natts;
! 	cstate->null_print_client = cstate->null_print;		/* default */
  
  	/* We use fe_msgbuf as a per-row buffer regardless of copy_dest */
  	cstate->fe_msgbuf = makeStringInfo();
  
  	/* Get info about the columns we need to process. */
! 	cstate->out_functions = (FmgrInfo *) palloc(num_phys_attrs * sizeof(FmgrInfo));
! 	cstate->force_quote = (bool *) palloc(num_phys_attrs * sizeof(bool));
! 
! 	cstate->isnull = (bool *) palloc(num_phys_attrs * sizeof(bool));
! 	cstate->values = (Datum *) palloc(num_phys_attrs * sizeof(Datum));
! 
! 	prepare_cstate_info(cstate, tupDesc, tupDesc->natts);
! 
  	foreach(cur, cstate->attnumlist)
  	{
  		int			attnum = lfirst_int(cur);
  
  		if (list_member_int(cstate->force_quote_atts, attnum))
! 			cstate->force_quote[attnum - 1] = true;
  		else
! 			cstate->force_quote[attnum - 1] = false;
  	}
  
  	/*
*************** CopyTo(CopyState cstate)
*** 1253,1259 ****
  	 * datatype output routines, and should be faster than retail pfree's
  	 * anyway.	(We don't need a whole econtext as CopyFrom does.)
  	 */
! 	mycontext = AllocSetContextCreate(CurrentMemoryContext,
  									  "COPY TO",
  									  ALLOCSET_DEFAULT_MINSIZE,
  									  ALLOCSET_DEFAULT_INITSIZE,
--- 1482,1488 ----
  	 * datatype output routines, and should be faster than retail pfree's
  	 * anyway.	(We don't need a whole econtext as CopyFrom does.)
  	 */
! 	cstate->rowcxt = AllocSetContextCreate(CurrentMemoryContext,
  									  "COPY TO",
  									  ALLOCSET_DEFAULT_MINSIZE,
  									  ALLOCSET_DEFAULT_INITSIZE,
*************** CopyTo(CopyState cstate)
*** 1282,1288 ****
  		 * encoding, because it will be sent directly with CopySendString.
  		 */
  		if (cstate->need_transcoding)
! 			null_print_client = pg_server_to_client(cstate->null_print,
  													cstate->null_print_len);
  
  		/* if a header has been requested send the line */
--- 1511,1517 ----
  		 * encoding, because it will be sent directly with CopySendString.
  		 */
  		if (cstate->need_transcoding)
! 			cstate->null_print_client = pg_server_to_client(cstate->null_print,
  													cstate->null_print_len);
  
  		/* if a header has been requested send the line */
*************** CopyTo(CopyState cstate)
*** 1309,1408 ****
  		}
  	}
  
! 	scandesc = heap_beginscan(cstate->rel, ActiveSnapshot, 0, NULL);
! 
! 	while ((tuple = heap_getnext(scandesc, ForwardScanDirection)) != NULL)
  	{
! 		bool		need_delim = false;
! 
! 		CHECK_FOR_INTERRUPTS();
  
! 		MemoryContextReset(mycontext);
! 		oldcontext = MemoryContextSwitchTo(mycontext);
! 
! 		if (cstate->binary)
  		{
! 			/* Binary per-tuple header */
! 			CopySendInt16(cstate, attr_count);
! 			/* Send OID if wanted --- note attr_count doesn't include it */
! 			if (cstate->oids)
! 			{
! 				Oid			oid = HeapTupleGetOid(tuple);
  
! 				/* Hack --- assume Oid is same size as int32 */
! 				CopySendInt32(cstate, sizeof(int32));
! 				CopySendInt32(cstate, oid);
! 			}
! 		}
! 		else
! 		{
! 			/* Text format has no per-tuple header, but send OID if wanted */
! 			/* Assume digits don't need any quoting or encoding conversion */
  			if (cstate->oids)
! 			{
! 				string = DatumGetCString(DirectFunctionCall1(oidout,
! 								  ObjectIdGetDatum(HeapTupleGetOid(tuple))));
! 				CopySendString(cstate, string);
! 				need_delim = true;
! 			}
! 		}
! 
! 		foreach(cur, cstate->attnumlist)
! 		{
! 			int			attnum = lfirst_int(cur);
! 			Datum		value;
! 			bool		isnull;
! 
! 			value = heap_getattr(tuple, attnum, tupDesc, &isnull);
! 
! 			if (!cstate->binary)
! 			{
! 				if (need_delim)
! 					CopySendChar(cstate, cstate->delim[0]);
! 				need_delim = true;
! 			}
  
! 			if (isnull)
! 			{
! 				if (!cstate->binary)
! 					CopySendString(cstate, null_print_client);
! 				else
! 					CopySendInt32(cstate, -1);
! 			}
! 			else
! 			{
! 				if (!cstate->binary)
! 				{
! 					string = OutputFunctionCall(&out_functions[attnum - 1],
! 												value);
! 					if (cstate->csv_mode)
! 						CopyAttributeOutCSV(cstate, string,
! 											force_quote[attnum - 1],
! 											list_length(cstate->attnumlist) == 1);
! 					else
! 						CopyAttributeOutText(cstate, string);
! 				}
! 				else
! 				{
! 					bytea	   *outputbytes;
! 
! 					outputbytes = SendFunctionCall(&out_functions[attnum - 1],
! 												   value);
! 					CopySendInt32(cstate, VARSIZE(outputbytes) - VARHDRSZ);
! 					CopySendData(cstate, VARDATA(outputbytes),
! 								 VARSIZE(outputbytes) - VARHDRSZ);
! 				}
! 			}
  		}
  
! 		CopySendEndOfRow(cstate);
! 
! 		MemoryContextSwitchTo(oldcontext);
! 		
! 		cstate->processed++;
  	}
! 
! 	heap_endscan(scandesc);
  
  	if (cstate->binary)
  	{
--- 1538,1567 ----
  		}
  	}
  
! 	if (cstate->rel)
  	{
! 		HeapScanDesc	scandesc;
! 		HeapTuple	tuple;
! 	
! 		scandesc = heap_beginscan(cstate->rel, ActiveSnapshot, 0, NULL);
  
! 		while ((tuple = heap_getnext(scandesc, ForwardScanDirection)) != NULL)
  		{
! 			Oid oid = InvalidOid;
  
! 			CHECK_FOR_INTERRUPTS();
! 		
  			if (cstate->oids)
! 				oid = HeapTupleGetOid(tuple);
  
! 			heap_deform_tuple(tuple, tupDesc, cstate->values, cstate->isnull);
! 			CopyValuesTo(cstate, oid, cstate->values, cstate->isnull);
  		}
  
! 		heap_endscan(scandesc);
  	}
! 	else
! 		ExecutorRun(cstate->desc, ForwardScanDirection, 0);
  
  	if (cstate->binary)
  	{
*************** CopyTo(CopyState cstate)
*** 1412,1421 ****
  		CopySendEndOfRow(cstate);
  	}
  
! 	MemoryContextDelete(mycontext);
  
! 	pfree(out_functions);
! 	pfree(force_quote);
  }
  
  
--- 1571,1582 ----
  		CopySendEndOfRow(cstate);
  	}
  
! 	MemoryContextDelete(cstate->rowcxt);
  
! 	pfree(cstate->out_functions);
! 	pfree(cstate->force_quote);
! 	pfree(cstate->values);
! 	pfree(cstate->isnull);
  }
  
  
*************** CopyAttributeOutCSV(CopyState cstate, ch
*** 3057,3070 ****
   * columns).
   */
  static List *
! CopyGetAttnums(Relation rel, List *attnamelist)
  {
  	List	   *attnums = NIL;
  
  	if (attnamelist == NIL)
  	{
  		/* Generate default column list */
- 		TupleDesc	tupDesc = RelationGetDescr(rel);
  		Form_pg_attribute *attr = tupDesc->attrs;
  		int			attr_count = tupDesc->natts;
  		int			i;
--- 3218,3230 ----
   * columns).
   */
  static List *
! CopyGetAttnums(TupleDesc tupDesc, List *attnamelist)
  {
  	List	   *attnums = NIL;
  
  	if (attnamelist == NIL)
  	{
  		/* Generate default column list */
  		Form_pg_attribute *attr = tupDesc->attrs;
  		int			attr_count = tupDesc->natts;
  		int			i;
*************** CopyGetAttnums(Relation rel, List *attna
*** 3085,3099 ****
  		{
  			char	   *name = strVal(lfirst(l));
  			int			attnum;
  
  			/* Lookup column name */
! 			/* Note we disallow system columns here */
! 			attnum = attnameAttNum(rel, name, false);
  			if (attnum == InvalidAttrNumber)
  				ereport(ERROR,
  						(errcode(ERRCODE_UNDEFINED_COLUMN),
! 						 errmsg("column \"%s\" of relation \"%s\" does not exist",
! 								name, RelationGetRelationName(rel))));
  			/* Check for duplicates */
  			if (list_member_int(attnums, attnum))
  				ereport(ERROR,
--- 3245,3269 ----
  		{
  			char	   *name = strVal(lfirst(l));
  			int			attnum;
+ 			int			i;
  
  			/* Lookup column name */
! 			attnum = InvalidAttrNumber;
! 			for (i = 0; i < tupDesc->natts; i++)
! 			{
! 				if (tupDesc->attrs[i]->attisdropped)
! 					continue;
! 				if (namestrcmp(&(tupDesc->attrs[i]->attname), name) == 0)
! 				{
! 					attnum = tupDesc->attrs[i]->attnum;
! 					break;
! 				}
! 			}
  			if (attnum == InvalidAttrNumber)
  				ereport(ERROR,
  						(errcode(ERRCODE_UNDEFINED_COLUMN),
! 						 errmsg("column \"%s\" does not exist",
! 								name)));
  			/* Check for duplicates */
  			if (list_member_int(attnums, attnum))
  				ereport(ERROR,
*************** CopyGetAttnums(Relation rel, List *attna
*** 3106,3108 ****
--- 3276,3438 ----
  
  	return attnums;
  }
+ 
+ /*
+  * Copies data from 'values' array
+  */
+ static void 
+ CopyValuesTo(CopyState cstate, Oid oid, Datum *values, bool *isnull)
+ {
+ 	int attr_count;
+ 	char *string; 
+ 	ListCell *cur;
+ 	MemoryContext oldcontext;
+ 	bool need_delim = false;
+ 
+ 	Assert(cstate);
+ 	Assert(values);
+ 	Assert(isnull);
+ 
+ 	attr_count = list_length(cstate->attnumlist);
+ 
+ 	MemoryContextReset(cstate->rowcxt);
+ 	oldcontext = MemoryContextSwitchTo(cstate->rowcxt);
+ 
+ 	if (cstate->binary)
+ 	{
+ 		/* Binary per-tuple header */
+ 		CopySendInt16(cstate, attr_count);
+ 		/* Send OID if wanted --- note attr_count doesn't include it */
+ 		if (cstate->oids)
+                 {
+                 	Assert(oid);
+                 	/* Hack --- assume Oid is same size as int32 */
+                 	CopySendInt32(cstate, sizeof(int32));
+                 	CopySendInt32(cstate, oid);
+ 		}
+ 	}
+ 	else
+ 	{   
+ 		/* Text format has no per-tuple header, but send OID if wanted */
+ 		/* Assume digits don't need any quoting or encoding conversion */
+ 		if (cstate->oids)
+ 		{
+ 			Assert(oid);
+ 			string = DatumGetCString(DirectFunctionCall1(oidout,
+ 						 ObjectIdGetDatum(oid)));
+ 			CopySendString(cstate, string);
+ 			need_delim = true;
+ 		}
+ 	}
+ 
+ 	foreach(cur, cstate->attnumlist)
+ 	{
+ 		int attnum = lfirst_int(cur);
+ 
+ 		if (!cstate->binary)
+ 		{
+ 			if (need_delim)
+ 				CopySendChar(cstate, cstate->delim[0]);
+ 			need_delim = true;
+ 		}
+ 		if (isnull[attnum-1])
+ 		{
+ 			if (!cstate->binary)
+ 				CopySendString(cstate, cstate->null_print_client);
+ 			else
+ 				CopySendInt32(cstate, -1);
+ 		}
+ 		else
+ 		{
+ 			if (!cstate->binary)
+ 			{
+ 				string = DatumGetCString(FunctionCall1(
+ 							&cstate->out_functions[attnum - 1],
+ 							values[attnum - 1]));
+ 				if (cstate->csv_mode)
+ 					CopyAttributeOutCSV(cstate, string, 
+ 							cstate->force_quote[attnum - 1],
+ 							list_length(cstate->attnumlist) == 1);
+ 				else
+ 					CopyAttributeOutText(cstate, string);
+ 			}
+ 			else
+ 			{   
+ 				bytea      *outputbytes;
+ 
+ 				outputbytes = SendFunctionCall(&cstate->out_functions[attnum - 1],
+ 									values[attnum - 1]);
+ 
+ 				/* We assume the result will not have been toasted */
+ 				CopySendInt32(cstate, VARSIZE(outputbytes) - VARHDRSZ);
+ 				CopySendData(cstate, VARDATA(outputbytes),
+ 							VARSIZE(outputbytes) - VARHDRSZ);
+ 			}
+ 		}
+ 	}
+ 	CopySendEndOfRow(cstate);
+ 	MemoryContextSwitchTo(oldcontext);
+ 
+ 	cstate->processed++;
+ }
+ 
+ 
+ /*
+  * Callback for executor destination receiver (COPY view TO)
+  */
+ static void
+ copy_dest_printtup(TupleTableSlot *slot, DestReceiver *self)
+ {
+ 	TupleDesc	typeinfo = slot->tts_tupleDescriptor;
+ 	DR_copy	   *copyDR = (DR_copy *)self;
+ 	CopyState	cstate = copyDR->cstate;
+ 	int		natts = typeinfo->natts;
+ 
+ 	if (cstate->attrinfo != typeinfo ||cstate->natts != natts)
+ 		prepare_cstate_info(cstate, typeinfo, natts);
+ 
+ 	/* Make sure the tuple is fully deconstructed */
+ 	slot_getallattrs(slot);
+ 
+ 	if (slot->tts_tuple)
+ 	{
+ 		/* Executor returns standard tuple */
+ 		heap_deform_tuple(slot->tts_tuple, typeinfo, cstate->values, cstate->isnull);
+ 		CopyValuesTo(cstate, InvalidOid, cstate->values, cstate->isnull);
+ 	}
+ 	else
+ 		/* Executor returns "virtual" tuple */
+ 		CopyValuesTo(cstate, InvalidOid, slot->tts_values, slot->tts_isnull);
+ 
+ }
+ 
+ static void
+ copy_startup(DestReceiver *self, int operation, TupleDesc typeinfo)
+ {
+ 	/* no-op */
+ }
+ 
+ static void
+ copy_shutdown(DestReceiver *self)
+ {
+ 	/* no-op */
+ }
+ 
+ static void
+ copy_destroy(DestReceiver *self)
+ {
+ 	/* no-op */
+ }
+ 
+ DestReceiver *
+ CreateCopyDestReceiver(void)
+ {
+ 	DR_copy *self = (DR_copy *) palloc(sizeof(DR_copy));
+ 
+ 	self->pub.receiveSlot = copy_dest_printtup;
+ 	self->pub.rStartup = copy_startup;
+ 	self->pub.rShutdown = copy_shutdown;
+ 	self->pub.rDestroy = copy_destroy;
+ 	self->pub.mydest = DestCopyDR;
+ 	return (DestReceiver *)self;
+ }
Index: src/backend/parser/gram.y
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.558
diff -c -p -r2.558 gram.y
*** src/backend/parser/gram.y	25 Aug 2006 04:06:51 -0000	2.558
--- src/backend/parser/gram.y	27 Aug 2006 05:33:07 -0000
*************** ClosePortalStmt:
*** 1614,1624 ****
  /*****************************************************************************
   *
   *		QUERY :
!  *				COPY <relname> ['(' columnList ')'] FROM/TO [WITH options]
   *
   *				BINARY, OIDS, and DELIMITERS kept in old locations
   *				for backward compatibility.  2002-06-18
   *
   *****************************************************************************/
  
  CopyStmt:	COPY opt_binary qualified_name opt_column_list opt_oids
--- 1614,1627 ----
  /*****************************************************************************
   *
   *		QUERY :
!  *				COPY <relname> ['(' columnList ')'] FROM/TO <file> [WITH options]
   *
   *				BINARY, OIDS, and DELIMITERS kept in old locations
   *				for backward compatibility.  2002-06-18
   *
+  *				COPY ( SELECT ... ) TO <file> [WITH options]
+  *				This form doesn't have the backwards-compatible locations for options.
+  *
   *****************************************************************************/
  
  CopyStmt:	COPY opt_binary qualified_name opt_column_list opt_oids
*************** CopyStmt:	COPY opt_binary qualified_name
*** 1626,1631 ****
--- 1629,1635 ----
  				{
  					CopyStmt *n = makeNode(CopyStmt);
  					n->relation = $3;
+ 					n->selectstmt = NULL;
  					n->attlist = $4;
  					n->is_from = $6;
  					n->filename = $7;
*************** CopyStmt:	COPY opt_binary qualified_name
*** 1642,1647 ****
--- 1646,1664 ----
  						n->options = list_concat(n->options, $10);
  					$$ = (Node *)n;
  				}
+ 			| COPY select_with_parens TO copy_file_name opt_with
+ 			  copy_opt_list
+ 				{
+ 					CopyStmt *n = makeNode(CopyStmt);
+ 					n->relation = NULL;
+ 					n->selectstmt = (SelectStmt *) $2;
+ 					n->attlist = NIL;
+ 					n->is_from = false;
+ 					n->filename = $4;
+ 
+ 					n->options = $6;
+ 					$$ = (Node *)n;
+ 				}
  		;
  
  copy_from:
*************** copy_from:
*** 1652,1658 ****
  /*
   * copy_file_name NULL indicates stdio is used. Whether stdin or stdout is
   * used depends on the direction. (It really doesn't make sense to copy from
!  * stdout. We silently correct the "typo".		 - AY 9/94
   */
  copy_file_name:
  			Sconst									{ $$ = $1; }
--- 1669,1675 ----
  /*
   * copy_file_name NULL indicates stdio is used. Whether stdin or stdout is
   * used depends on the direction. (It really doesn't make sense to copy from
!  * stdout. We silently correct the "typo".)		 - AY 9/94
   */
  copy_file_name:
  			Sconst									{ $$ = $1; }
Index: src/backend/tcop/dest.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/backend/tcop/dest.c,v
retrieving revision 1.69
diff -c -p -r1.69 dest.c
*** src/backend/tcop/dest.c	12 Aug 2006 02:52:05 -0000	1.69
--- src/backend/tcop/dest.c	27 Aug 2006 04:53:19 -0000
***************
*** 30,35 ****
--- 30,36 ----
  
  #include "access/printtup.h"
  #include "access/xact.h"
+ #include "commands/copy.h"
  #include "executor/executor.h"
  #include "executor/tstoreReceiver.h"
  #include "libpq/libpq.h"
*************** CreateDestReceiver(CommandDest dest, Por
*** 117,122 ****
--- 118,126 ----
  		case DestSPI:
  			return &spi_printtupDR;
  
+ 		case DestCopyDR:
+ 			return CreateCopyDestReceiver();
+ 
  		case DestTuplestore:
  			if (portal == NULL)
  				elog(ERROR, "no portal specified for DestTuplestore receiver");
*************** EndCommand(const char *commandTag, Comma
*** 153,158 ****
--- 157,163 ----
  		case DestSPI:
  		case DestTuplestore:
  		case DestIntoRel:
+ 		case DestCopyDR:
  			break;
  	}
  }
*************** NullCommand(CommandDest dest)
*** 192,197 ****
--- 197,203 ----
  		case DestSPI:
  		case DestTuplestore:
  		case DestIntoRel:
+ 		case DestCopyDR:
  			break;
  	}
  }
*************** ReadyForQuery(CommandDest dest)
*** 233,238 ****
--- 239,245 ----
  		case DestSPI:
  		case DestTuplestore:
  		case DestIntoRel:
+ 		case DestCopyDR:
  			break;
  	}
  }
Index: src/bin/psql/copy.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/bin/psql/copy.c,v
retrieving revision 1.66
diff -c -p -r1.66 copy.c
*** src/bin/psql/copy.c	14 Jun 2006 16:49:02 -0000	1.66
--- src/bin/psql/copy.c	27 Aug 2006 04:53:19 -0000
***************
*** 36,42 ****
   * -- parses \copy command line
   *
   * The documented preferred syntax is:
!  *	\copy tablename [(columnlist)] from|to filename
   *	  [ with ] [ binary ] [ oids ] [ delimiter [as] char ] [ null [as] string ]
   *
   * The pre-7.3 syntax was:
--- 36,42 ----
   * -- parses \copy command line
   *
   * The documented preferred syntax is:
!  *	\copy { tablename | viewname | ( select stmt ) } [(columnlist)] from|to filename
   *	  [ with ] [ binary ] [ oids ] [ delimiter [as] char ] [ null [as] string ]
   *
   * The pre-7.3 syntax was:
*************** parse_slash_copy(const char *args)
*** 142,147 ****
--- 142,168 ----
  
  	result->table = pg_strdup(token);
  
+ 	/* Handle COPY (SELECT) case */
+ 	if (token[0] == '(')
+ 	{
+ 		char	*selectstmt;
+ 		int	brackets = 1;
+ 
+ 		while (brackets > 0)
+ 		{
+ 			token = strtokx(NULL, whitespace, ".,()", "\"'",
+ 						0, false, false, pset.encoding);
+ 			if (!token)
+ 				goto error;
+ 			if (token[0] == '(')
+ 				brackets++;
+ 			else if (token[0] == ')')
+ 				brackets--;
+ 			xstrcat(&result->table, " ");
+ 			xstrcat(&result->table, token);
+ 		}
+ 	}
+ 
  	token = strtokx(NULL, whitespace, ".,()", "\"",
  					0, false, false, pset.encoding);
  	if (!token)
Index: src/include/commands/copy.h
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/include/commands/copy.h,v
retrieving revision 1.27
diff -c -p -r1.27 copy.h
*** src/include/commands/copy.h	5 Mar 2006 15:58:55 -0000	1.27
--- src/include/commands/copy.h	27 Aug 2006 04:53:19 -0000
***************
*** 7,13 ****
   * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
   * Portions Copyright (c) 1994, Regents of the University of California
   *
!  * $PostgreSQL: pgsql/src/include/commands/copy.h,v 1.26 2006/03/03 19:54:10 tgl Exp $
   *
   *-------------------------------------------------------------------------
   */
--- 7,13 ----
   * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
   * Portions Copyright (c) 1994, Regents of the University of California
   *
!  * $PostgreSQL: pgsql/src/include/commands/copy.h,v 1.27 2006-03-05 15:58:55 momjian Exp $
   *
   *-------------------------------------------------------------------------
   */
***************
*** 15,22 ****
--- 15,24 ----
  #define COPY_H
  
  #include "nodes/parsenodes.h"
+ #include "tcop/dest.h"
  
  
+ extern DestReceiver *CreateCopyDestReceiver(void);
  extern uint64 DoCopy(const CopyStmt *stmt);
  
  #endif   /* COPY_H */
Index: src/include/nodes/parsenodes.h
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/include/nodes/parsenodes.h,v
retrieving revision 1.325
diff -c -p -r1.325 parsenodes.h
*** src/include/nodes/parsenodes.h	25 Aug 2006 04:06:56 -0000	1.325
--- src/include/nodes/parsenodes.h	27 Aug 2006 05:26:28 -0000
*************** typedef struct GrantRoleStmt
*** 1017,1023 ****
--- 1017,1025 ----
  typedef struct CopyStmt
  {
  	NodeTag		type;
+ 	/* one of the two following must be NULL and the other must not be: */
  	RangeVar   *relation;		/* the relation to copy */
+ 	SelectStmt *selectstmt;		/* the SELECT to copy */
  	List	   *attlist;		/* List of column names (as Strings), or NIL
  								 * for all columns */
  	bool		is_from;		/* TO or FROM */
Index: src/include/tcop/dest.h
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/include/tcop/dest.h,v
retrieving revision 1.51
diff -c -p -r1.51 dest.h
*** src/include/tcop/dest.h	12 Aug 2006 02:52:06 -0000	1.51
--- src/include/tcop/dest.h	27 Aug 2006 04:53:19 -0000
*************** typedef enum
*** 85,91 ****
  	DestRemoteExecute,			/* sent to frontend, in Execute command */
  	DestSPI,					/* results sent to SPI manager */
  	DestTuplestore,				/* results sent to Tuplestore */
! 	DestIntoRel					/* results sent to relation (SELECT INTO) */
  } CommandDest;
  
  /* ----------------
--- 85,92 ----
  	DestRemoteExecute,			/* sent to frontend, in Execute command */
  	DestSPI,					/* results sent to SPI manager */
  	DestTuplestore,				/* results sent to Tuplestore */
! 	DestIntoRel,					/* results sent to relation (SELECT INTO) */
! 	DestCopyDR					/* results sent to file */
  } CommandDest;
  
  /* ----------------
Index: src/interfaces/ecpg/test/Makefile.regress
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/interfaces/ecpg/test/Makefile.regress,v
retrieving revision 1.3
diff -c -p -r1.3 Makefile.regress
*** src/interfaces/ecpg/test/Makefile.regress	9 Aug 2006 22:48:17 -0000	1.3
--- src/interfaces/ecpg/test/Makefile.regress	28 Aug 2006 02:50:19 -0000
***************
*** 1,4 ****
! override CPPFLAGS := -I$(srcdir)/../../include -I$(libpq_srcdir) $(CPPFLAGS) 
  override CFLAGS += $(PTHREAD_CFLAGS) 
  
  override LDFLAGS := -L../../ecpglib -L../../pgtypeslib -L../../../libpq $(LDFLAGS)
--- 1,4 ----
! override CPPFLAGS := -I$(srcdir)/../../include -I$(top_builddir)/$(subdir)/../../include -I$(libpq_srcdir) $(CPPFLAGS) 
  override CFLAGS += $(PTHREAD_CFLAGS) 
  
  override LDFLAGS := -L../../ecpglib -L../../pgtypeslib -L../../../libpq $(LDFLAGS)
Index: src/test/regress/parallel_schedule
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/test/regress/parallel_schedule,v
retrieving revision 1.34
diff -c -p -r1.34 parallel_schedule
*** src/test/regress/parallel_schedule	12 Aug 2006 02:52:06 -0000	1.34
--- src/test/regress/parallel_schedule	27 Aug 2006 04:53:19 -0000
*************** test: create_function_2
*** 34,40 ****
  # execute two copy tests parallel, to check that copy itself
  # is concurrent safe.
  # ----------
! test: copy
  
  # ----------
  # The third group of parallel test
--- 34,40 ----
  # execute two copy tests parallel, to check that copy itself
  # is concurrent safe.
  # ----------
! test: copy copyselect
  
  # ----------
  # The third group of parallel test
Index: src/test/regress/expected/alter_table.out
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/test/regress/expected/alter_table.out,v
retrieving revision 1.97
diff -c -p -r1.97 alter_table.out
*** src/test/regress/expected/alter_table.out	21 Aug 2006 00:57:26 -0000	1.97
--- src/test/regress/expected/alter_table.out	27 Aug 2006 04:53:19 -0000
*************** alter table test drop a;
*** 947,955 ****
  copy test to stdout;
  2	3
  copy test(a) to stdout;
! ERROR:  column "a" of relation "test" does not exist
  copy test("........pg.dropped.1........") to stdout;
! ERROR:  column "........pg.dropped.1........" of relation "test" does not exist
  copy test from stdin;
  ERROR:  extra data after last expected column
  CONTEXT:  COPY test, line 1: "10	11	12"
--- 947,955 ----
  copy test to stdout;
  2	3
  copy test(a) to stdout;
! ERROR:  column "a" does not exist
  copy test("........pg.dropped.1........") to stdout;
! ERROR:  column "........pg.dropped.1........" does not exist
  copy test from stdin;
  ERROR:  extra data after last expected column
  CONTEXT:  COPY test, line 1: "10	11	12"
*************** select * from test;
*** 968,976 ****
  (2 rows)
  
  copy test(a) from stdin;
! ERROR:  column "a" of relation "test" does not exist
  copy test("........pg.dropped.1........") from stdin;
! ERROR:  column "........pg.dropped.1........" of relation "test" does not exist
  copy test(b,c) from stdin;
  select * from test;
   b  | c  
--- 968,976 ----
  (2 rows)
  
  copy test(a) from stdin;
! ERROR:  column "a" does not exist
  copy test("........pg.dropped.1........") from stdin;
! ERROR:  column "........pg.dropped.1........" does not exist
  copy test(b,c) from stdin;
  select * from test;
   b  | c  
Index: src/test/regress/expected/copy2.out
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/test/regress/expected/copy2.out,v
retrieving revision 1.25
diff -c -p -r1.25 copy2.out
*** src/test/regress/expected/copy2.out	27 Feb 2006 16:09:50 -0000	1.25
--- src/test/regress/expected/copy2.out	27 Aug 2006 04:53:19 -0000
*************** COPY x (b, d) from stdin;
*** 28,34 ****
  COPY x (a, b, c, d, e) from stdin;
  -- non-existent column in column list: should fail
  COPY x (xyz) from stdin;
! ERROR:  column "xyz" of relation "x" does not exist
  -- too many columns in column list: should fail
  COPY x (a, b, c, d, e, d, c) from stdin;
  ERROR:  column "d" specified more than once
--- 28,34 ----
  COPY x (a, b, c, d, e) from stdin;
  -- non-existent column in column list: should fail
  COPY x (xyz) from stdin;
! ERROR:  column "xyz" does not exist
  -- too many columns in column list: should fail
  COPY x (a, b, c, d, e, d, c) from stdin;
  ERROR:  column "d" specified more than once
#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#8)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Alvaro Herrera <alvherre@commandprompt.com> writes:

... I'd do it myself but I'm heading to bed right now.
...
I'll repost a reworked version at some point, if no one beats me to it.

I was planning to start looking at this patch tomorrow (unless Gavin
produces a new bitmap-index patch by then). I'll work from this one
unless somebody produces a better version meanwhile.

regards, tom lane

#10Böszörményi Zoltán
zboszor@dunaweb.hu
In reply to: Tom Lane (#9)
1 attachment(s)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Hi,

what's the problem with COPY view TO, other than you don't like it? :-)
That was the beginning and is used in production
according to the original authors.

I also broke the check for a FOR UPDATE clause. Not sure where but it
must be easy to fix :-) I'd do it myself but I'm heading to bed right
now.

Fixed.

I also wanted to check these hunks in your patch, which I didn't like
very much:

-ERROR:  column "a" of relation "test" does not exist
+ERROR:  column "a" does not exist

It was because of too much code sharing. I fixed it by passing
the relation name to CopyGetAttnums() in the relation case,
so the other regression tests aren't bothered now.

The docs and the regression test is modified according to your version.

Best regards,
Zolt�n B�sz�rm�nyi

Attachments:

pgsql-copyselect-10.patch.gzapplication/x-gzip; name=pgsql-copyselect-10.patch.gzDownload
#11Alvaro Herrera
alvherre@commandprompt.com
In reply to: Böszörményi Zoltán (#10)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

B�sz�rm�nyi Zolt�n wrote:

Hi,

what's the problem with COPY view TO, other than you don't like it? :-)

The problem is that it required a ugly piece of code. Not supporting it
means we can keep the code nice. The previous discussion led to this
conclusion anyway so I don't know why we are debating it again.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

#12Andrew Dunstan
andrew@dunslane.net
In reply to: Alvaro Herrera (#11)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Alvaro Herrera wrote:

B�sz�rm�nyi Zolt�n wrote:

Hi,

what's the problem with COPY view TO, other than you don't like it? :-)

The problem is that it required a ugly piece of code. Not supporting it
means we can keep the code nice. The previous discussion led to this
conclusion anyway so I don't know why we are debating it again.

What is so ugly about it? I haven't looked at the code, but I am curious
to know.

I also don't recall the consensus being quite so clear cut. I guess
there is a case for saying that if it's not allowed then you know that
"COPY relname TO" is going to be fast. But, code aesthetics aside, the
reasons for disallowing it seem a bit thin, to me.

cheers

andrew

#13Zoltan Boszormenyi
zboszor@dunaweb.hu
In reply to: Andrew Dunstan (#12)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Andrew Dunstan �rta:

Alvaro Herrera wrote:

B�sz�rm�nyi Zolt�n wrote:

Hi,

what's the problem with COPY view TO, other than you don't like it? :-)

The problem is that it required a ugly piece of code. Not supporting it
means we can keep the code nice. The previous discussion led to this
conclusion anyway so I don't know why we are debating it again.

What is so ugly about it? I haven't looked at the code, but I am
curious to know.

I also don't recall the consensus being quite so clear cut. I guess
there is a case for saying that if it's not allowed then you know that
"COPY relname TO" is going to be fast. But, code aesthetics aside, the
reasons for disallowing it seem a bit thin, to me.

cheers

andrew

I would say the timing difference between
"COPY table TO" and "COPY (SELECT * FROM table) TO"
was noise, so it's not even faster.

And an updatable VIEW *may* allow COPY view FROM...

Best regards,
Zolt�n B�sz�rm�nyi

#14Alvaro Herrera
alvherre@commandprompt.com
In reply to: Zoltan Boszormenyi (#13)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Zoltan Boszormenyi wrote:

Andrew Dunstan �rta:

Alvaro Herrera wrote:

B�sz�rm�nyi Zolt�n wrote:

what's the problem with COPY view TO, other than you don't like it? :-)

The problem is that it required a ugly piece of code. Not supporting it
means we can keep the code nice. The previous discussion led to this
conclusion anyway so I don't know why we are debating it again.

What is so ugly about it? I haven't looked at the code, but I am
curious to know.

It used a "SELECT * FROM %s" string that was passed back to the parser.

I also don't recall the consensus being quite so clear cut. I guess
there is a case for saying that if it's not allowed then you know that
"COPY relname TO" is going to be fast. But, code aesthetics aside, the
reasons for disallowing it seem a bit thin, to me.

I would say the timing difference between
"COPY table TO" and "COPY (SELECT * FROM table) TO"
was noise, so it's not even faster.

Remember that we were talking about supporting views, not tables. And
if a view uses a slow query then you are in immediate danger of having a
slow COPY. This may not be a problem but it needs to be discussed and
an agreement must be reached before introducing such a change (and not
during feature freeze).

And an updatable VIEW *may* allow COPY view FROM...

May I remind you that we've been in feature freeze for four weeks
already? Now it's *not* the time to be drooling over cool features that
would be nice to have.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

#15Hans-Juergen Schoenig
postgres@cybertec.at
In reply to: Alvaro Herrera (#14)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Remember that we were talking about supporting views, not tables. And
if a view uses a slow query then you are in immediate danger of having a
slow COPY. This may not be a problem but it needs to be discussed and
an agreement must be reached before introducing such a change (and not
during feature freeze).

this will definitely be the case - however, this is not what it was made
for. it has not been made to be fast but it has been made to fulfill
some other task. the reason why this has been implemented is: consider a
large scale database containing hundreds of gigs of data. in our special
case we have to export in a flexible way. the data which has to be
exported comes from multiple tables (between 3 and 7 depending on the
data we are looking at in this project. the export has to be performed
in a flexible way and it needs certain parameters. defining tmp tables
and store the data in there is simply not "nice" at all. in most cases
exports want to transform data on the fly - speed is not as important as
flexibility here.

so in my view the speed argument does not matter. if somebody passes a
stupid query to copy he will get stupid runtimes - just like on ordinary
sql. however, we can use COPY's capabilities to format / escape data to
make exports more flexible. so basically it is a win.

best regards,

hans

--
Cybertec Geschwinde & Sch�nig GmbH
Sch�ngrabern 134; A-2020 Hollabrunn
Tel: +43/1/205 10 35 / 340
www.postgresql.at, www.cybertec.at

#16Alvaro Herrera
alvherre@commandprompt.com
In reply to: Hans-Juergen Schoenig (#15)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Hans-Juergen Schoenig wrote:

Remember that we were talking about supporting views, not tables. And
if a view uses a slow query then you are in immediate danger of having a
slow COPY. This may not be a problem but it needs to be discussed and
an agreement must be reached before introducing such a change (and not
during feature freeze).

this will definitely be the case - however, this is not what it was made
for. it has not been made to be fast but it has been made to fulfill
some other task. the reason why this has been implemented is: consider a
large scale database containing hundreds of gigs of data. in our special
case we have to export in a flexible way. the data which has to be
exported comes from multiple tables (between 3 and 7 depending on the
data we are looking at in this project. the export has to be performed
in a flexible way and it needs certain parameters. defining tmp tables
and store the data in there is simply not "nice" at all. in most cases
exports want to transform data on the fly - speed is not as important as
flexibility here.

My question is, if we allow this:

copy (select * from view) to stdout;

(or to a file, whatever), is it enough for you? Or would you insist on
also having

copy view to stdout;
?

We can, and the posted patch does, support the first form, but not the
second. In fact I deliberately removed support for the second form for
Zolt�n's patch because it uglifies the surrounding code.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

#17Zoltan Boszormenyi
zboszor@dunaweb.hu
In reply to: Alvaro Herrera (#14)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Alvaro Herrera �rta:

Zoltan Boszormenyi wrote:

Andrew Dunstan �rta:

Alvaro Herrera wrote:

B�sz�rm�nyi Zolt�n wrote:

what's the problem with COPY view TO, other than you don't like it? :-)

The problem is that it required a ugly piece of code. Not supporting it
means we can keep the code nice. The previous discussion led to this
conclusion anyway so I don't know why we are debating it again.

What is so ugly about it? I haven't looked at the code, but I am
curious to know.

It used a "SELECT * FROM %s" string that was passed back to the parser.

I also don't recall the consensus being quite so clear cut. I guess
there is a case for saying that if it's not allowed then you know that
"COPY relname TO" is going to be fast. But, code aesthetics aside, the
reasons for disallowing it seem a bit thin, to me.

I would say the timing difference between
"COPY table TO" and "COPY (SELECT * FROM table) TO"
was noise, so it's not even faster.

Remember that we were talking about supporting views, not tables. And
if a view uses a slow query then you are in immediate danger of having a
slow COPY. This may not be a problem but it needs to be discussed and
an agreement must be reached before introducing such a change (and not
during feature freeze).

COPY relname TO meant tables _and_ views to me.
My previous tsting showed no difference between
COPY table TO and COPY (SELECT * FROM table) TO.
Similarly a slow query defined in the view should show
no difference between COPY view TO and
COPY (SELECT * FROM view) TO.

And remember, Bruce put the original COPY view TO
patch into the unapplied queue, without the SELECT
feature.

Rewriting COPY view TO internally to
COPY (SELECT * FROM view) TO is very
straightforward, even if you think it's ugly.
BTW, why is it ugly if I call raw_parser()
from under src/backend/parser/*.c ?
It is on a query distinct to the query the parser
is currently running. Or is it the recursion
that bothers you? It's not a possible infinite
recursion.

And an updatable VIEW *may* allow COPY view FROM...

May I remind you that we've been in feature freeze for four weeks
already? Now it's *not* the time to be drooling over cool features that
would be nice to have

Noted. However, as the COPY view TO is
a straight internal rewrite, a COPY view FROM
could also be. Even if it's a long term development.
I wasn't proposing delaying beta.

Best regards,
Zolt�n B�sz�rm�nyi

#18Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#16)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Alvaro Herrera <alvherre@commandprompt.com> writes:

My question is, if we allow this:
copy (select * from view) to stdout;
(or to a file, whatever), is it enough for you? Or would you insist on
also having
copy view to stdout;
?

We can, and the posted patch does, support the first form, but not the
second. In fact I deliberately removed support for the second form for
Zolt�n's patch because it uglifies the surrounding code.

Personally, I have no moral objection to supporting the second form
as a special case of the general COPY-from-select feature, but if it
can't be done without uglifying the code then I'd agree with dropping
it. I guess the question is whether the uglification is intrinsic or
just a result of being descended from a poor original implementation.

The feature-freeze argument seems not relevant, given that the code
we had on the feature-freeze date did both things.

Has this patch settled to the point where I can review it, or is it
still in motion?

regards, tom lane

#19Hans-Juergen Schoenig
postgres@cybertec.at
In reply to: Alvaro Herrera (#16)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Alvaro Herrera wrote:

Hans-Juergen Schoenig wrote:

Remember that we were talking about supporting views, not tables. And
if a view uses a slow query then you are in immediate danger of having a
slow COPY. This may not be a problem but it needs to be discussed and
an agreement must be reached before introducing such a change (and not
during feature freeze).

this will definitely be the case - however, this is not what it was made
for. it has not been made to be fast but it has been made to fulfill
some other task. the reason why this has been implemented is: consider a
large scale database containing hundreds of gigs of data. in our special
case we have to export in a flexible way. the data which has to be
exported comes from multiple tables (between 3 and 7 depending on the
data we are looking at in this project. the export has to be performed
in a flexible way and it needs certain parameters. defining tmp tables
and store the data in there is simply not "nice" at all. in most cases
exports want to transform data on the fly - speed is not as important as
flexibility here.

My question is, if we allow this:

copy (select * from view) to stdout;

(or to a file, whatever), is it enough for you? Or would you insist on
also having

copy view to stdout;
?

i would say that "copy view to stdout" is just some syntactic sugar (to
me at least). the important thing is that we add the flexibility of
SELECT to it. a view is nothing else than a rule on SELECT anyway. to be
honest i never thought about views when creating this copy idea.
however, i think it is not bad to have it because i have seen a couple
of times already that tables turn into views when new features are added
to an existing data structure . if we support copy on views this means
that exports can stay as they are even if the data structure is changed
in that way.
however, if people think that views are not needed that way it is still
a good solution as views are not the basic reason why this new
functionality is a good thing to have.

many thanks,

hans

--
Cybertec Geschwinde & Sch�nig GmbH
Sch�ngrabern 134; A-2020 Hollabrunn
Tel: +43/1/205 10 35 / 340
www.postgresql.at, www.cybertec.at

#20Alvaro Herrera
alvherre@commandprompt.com
In reply to: Zoltan Boszormenyi (#17)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Zoltan Boszormenyi wrote:

Alvaro Herrera �rta:

Remember that we were talking about supporting views, not tables. And
if a view uses a slow query then you are in immediate danger of having a
slow COPY. This may not be a problem but it needs to be discussed and
an agreement must be reached before introducing such a change (and not
during feature freeze).

COPY relname TO meant tables _and_ views to me.
My previous tsting showed no difference between
COPY table TO and COPY (SELECT * FROM table) TO.
Similarly a slow query defined in the view should show
no difference between COPY view TO and
COPY (SELECT * FROM view) TO.

The difference is that we are giving a very clear distinction between a
table and a view. If we don't support the view in the direct COPY, but
instead insist that it be passed via a SELECT query, then the user will
be aware that it may be slow.

"relname" at this point may mean anything -- are you supporting
sequences and toast tables as well?

And remember, Bruce put the original COPY view TO
patch into the unapplied queue, without the SELECT
feature.

All sort of junk enters that queue so that's not an argument. (Not
meant to insult Bruce -- I'm just saying that he doesn't filter stuff.
We've had patches rejected from the queue before plenty of times.)

Rewriting COPY view TO internally to
COPY (SELECT * FROM view) TO is very
straightforward, even if you think it's ugly.
BTW, why is it ugly if I call raw_parser()
from under src/backend/parser/*.c ?
It is on a query distinct to the query the parser
is currently running. Or is it the recursion
that bothers you? It's not a possible infinite
recursion.

It's ugly because you are forcing the system to parse something that
was already parsed.

On the other hand I don't see why you are arguing in favor of a useless
feature whose coding is dubious; you can have _the same thing_ with nice
code and no discussion.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

#21Alvaro Herrera
alvherre@commandprompt.com
In reply to: Tom Lane (#18)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Tom Lane wrote:

Alvaro Herrera <alvherre@commandprompt.com> writes:

My question is, if we allow this:
copy (select * from view) to stdout;
(or to a file, whatever), is it enough for you? Or would you insist on
also having
copy view to stdout;
?

We can, and the posted patch does, support the first form, but not the
second. In fact I deliberately removed support for the second form for
Zolt�n's patch because it uglifies the surrounding code.

Personally, I have no moral objection to supporting the second form
as a special case of the general COPY-from-select feature, but if it
can't be done without uglifying the code then I'd agree with dropping
it. I guess the question is whether the uglification is intrinsic or
just a result of being descended from a poor original implementation.

I'm quite sure you could refactor things as needed to support the "COPY
view" case reasonably. It's just beyond what I'd do during the current
freeze.

It seems I'm alone on the "view may be slow" camp. If I lost that
argument I have no problem accepting that.

The feature-freeze argument seems not relevant, given that the code
we had on the feature-freeze date did both things.

Actually IIRC the patch on the queue only did the "COPY view" stuff, not
the COPY select. (Thanks go to Zoltan for properly morphing the patch).

Has this patch settled to the point where I can review it, or is it
still in motion?

Personally I'm finished doing the cleanup I wanted to do.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

#22Hans-Juergen Schoenig
postgres@cybertec.at
In reply to: Alvaro Herrera (#20)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Remember that we were talking about supporting views, not tables. And
if a view uses a slow query then you are in immediate danger of having a
slow COPY. This may not be a problem but it needs to be discussed and
an agreement must be reached before introducing such a change (and not
during feature freeze).

COPY relname TO meant tables _and_ views to me.
My previous tsting showed no difference between
COPY table TO and COPY (SELECT * FROM table) TO.
Similarly a slow query defined in the view should show
no difference between COPY view TO and
COPY (SELECT * FROM view) TO.

The difference is that we are giving a very clear distinction between a
table and a view. If we don't support the view in the direct COPY, but
instead insist that it be passed via a SELECT query, then the user will
be aware that it may be slow.

what kind of clever customers do you have in the US? ;) i would never
say something like that here :).
i see your point and i think it is not a too bad idea. at least some
folks might see that there is no voodoo going on ...

"relname" at this point may mean anything -- are you supporting
sequences and toast tables as well?

good point ...

It's ugly because you are forcing the system to parse something that
was already parsed.

definitely an argument for dropping the view stuff ...

On the other hand I don't see why you are arguing in favor of a useless
feature whose coding is dubious; you can have _the same thing_ with nice
code and no discussion.

what are you referring to?

hans

--
Cybertec Geschwinde & Sch�nig GmbH
Sch�ngrabern 134; A-2020 Hollabrunn
Tel: +43/1/205 10 35 / 340
www.postgresql.at, www.cybertec.at

#23Zoltan Boszormenyi
zboszor@dunaweb.hu
In reply to: Alvaro Herrera (#20)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Alvaro Herrera �rta:

Zoltan Boszormenyi wrote:

Alvaro Herrera �rta:

Remember that we were talking about supporting views, not tables. And
if a view uses a slow query then you are in immediate danger of having a
slow COPY. This may not be a problem but it needs to be discussed and
an agreement must be reached before introducing such a change (and not
during feature freeze).

COPY relname TO meant tables _and_ views to me.
My previous tsting showed no difference between
COPY table TO and COPY (SELECT * FROM table) TO.
Similarly a slow query defined in the view should show
no difference between COPY view TO and
COPY (SELECT * FROM view) TO.

The difference is that we are giving a very clear distinction between a
table and a view. If we don't support the view in the direct COPY, but
instead insist that it be passed via a SELECT query, then the user will
be aware that it may be slow.

It still can be documented with supporting
the COPY view TO syntax.

But COPY view (col1, col2, ...) TO may still be
useful even if the COPY (SELECT ...) (col1, col2, ...) TO
is pointless. [1]

"relname" at this point may mean anything -- are you supporting
sequences and toast tables as well?

Well, not really. :-)

And remember, Bruce put the original COPY view TO
patch into the unapplied queue, without the SELECT
feature.

All sort of junk enters that queue so that's not an argument. (Not
meant to insult Bruce -- I'm just saying that he doesn't filter stuff.
We've had patches rejected from the queue before plenty of times.)

OK. :-)

Rewriting COPY view TO internally to
COPY (SELECT * FROM view) TO is very
straightforward, even if you think it's ugly.
BTW, why is it ugly if I call raw_parser()
from under src/backend/parser/*.c ?
It is on a query distinct to the query the parser
is currently running. Or is it the recursion
that bothers you? It's not a possible infinite
recursion.

It's ugly because you are forcing the system to parse something that
was already parsed.

Well, to be true to the word, during parsing COPY view TO
the parser never saw SELECT * FROM view.

On the other hand I don't see why you are arguing in favor of a useless
feature whose coding is dubious; you can have _the same thing_ with nice
code and no discussion.

Because of [1] and because Mr. Schoenig's arguments
about changing schemas.

Best regards,
Zolt�n B�sz�rm�nyi

#24Alvaro Herrera
alvherre@commandprompt.com
In reply to: Hans-Juergen Schoenig (#22)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Hans-Juergen Schoenig wrote:

It's ugly because you are forcing the system to parse something that
was already parsed.

definitely an argument for dropping the view stuff ...

On the other hand, it's quite possible that this could be made to work
_without_ doing black magic (which would be OK by me).

On the other hand I don't see why you are arguing in favor of a useless
feature whose coding is dubious; you can have _the same thing_ with nice
code and no discussion.

what are you referring to?

The fact that the direct "copy view" feature is just syntactic sugar
over "copy (select * from view)". The latter we can have without
discussion -- from me, that is :-)

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

#25Alvaro Herrera
alvherre@commandprompt.com
In reply to: Zoltan Boszormenyi (#23)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Zoltan Boszormenyi wrote:

Alvaro Herrera �rta:

But COPY view (col1, col2, ...) TO may still be
useful even if the COPY (SELECT ...) (col1, col2, ...) TO
is pointless. [1]

Hum, I don't understand what you're saying here -- are you saying that
you can't do something with the first form, that you cannot do with the
second?

It's ugly because you are forcing the system to parse something that
was already parsed.

Well, to be true to the word, during parsing COPY view TO
the parser never saw SELECT * FROM view.

Hmm!

The COPY view stuff stopped working when I changed back the "relation"
case. Your patch changed it so that instead of flowing as RangeVar all
the way to the copy.c code, the parser changed it into a "select * from
%s" query, and then stashed the resulting Query node into the "query"
%case. (So what was happening was that the Relation case was never
%used). I reverted this.

On the other hand I don't see why you are arguing in favor of a useless
feature whose coding is dubious; you can have _the same thing_ with nice
code and no discussion.

Because of [1] and because Mr. Schoenig's arguments
about changing schemas.

Yeah, that argument makes sense to me as well.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

#26Hans-Juergen Schoenig
postgres@cybertec.at
In reply to: Zoltan Boszormenyi (#23)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

On the other hand I don't see why you are arguing in favor of a useless
feature whose coding is dubious; you can have _the same thing_ with nice
code and no discussion.

Because of [1] and because Mr. Schoenig's arguments
about changing schemas.

first of all; hans is enough - skip the mr ;)
i think changing schema is a good argument but we could sacrifice that
for the sake of clarity and clean code. i am not against keeping it but
i can understand the argument against views. i always preferred select.

mr hans ;)

--
Cybertec Geschwinde & Sch�nig GmbH
Sch�ngrabern 134; A-2020 Hollabrunn
Tel: +43/1/205 10 35 / 340
www.postgresql.at, www.cybertec.at

#27Zoltan Boszormenyi
zboszor@dunaweb.hu
In reply to: Alvaro Herrera (#25)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Alvaro Herrera �rta:

Zoltan Boszormenyi wrote:

Alvaro Herrera �rta:

But COPY view (col1, col2, ...) TO may still be
useful even if the COPY (SELECT ...) (col1, col2, ...) TO
is pointless. [1]

Hum, I don't understand what you're saying here -- are you saying that
you can't do something with the first form, that you cannot do with the
second?

Say you have a large often used query.
Would you like to retype it every time
or just create a view? Later you may want to
export only a subset of the fields...

My v8 had the syntax support for

COPY (SELECT ...) (col1, col2, ...) TO
and it was actually working. In your v9
you rewrote the syntax parsing so that
feature was lost in translation.

It's ugly because you are forcing the system to parse something that
was already parsed.

Well, to be true to the word, during parsing COPY view TO
the parser never saw SELECT * FROM view.

Hmm!

The COPY view stuff stopped working when I changed back the "relation"
case. Your patch changed it so that instead of flowing as RangeVar all
the way to the copy.c code, the parser changed it into a "select * from
%s" query, and then stashed the resulting Query node into the "query"
%case. (So what was happening was that the Relation case was never
%used). I reverted this.

Well, the VIEW case wasn't supported before
so I took the opportunity to transform it in
analyze.c which you deleted as being ugly.

On the other hand I don't see why you are arguing in favor of a useless
feature whose coding is dubious; you can have _the same thing_ with nice
code and no discussion.

Because of [1] and because Mr. Schoenig's arguments
about changing schemas.

Yeah, that argument makes sense to me as well.

So, may I put it back? :-)
Also, can you suggest anything cleaner than
calling raw_parser("SELECT * FROM view")?

#28Alvaro Herrera
alvherre@commandprompt.com
In reply to: Zoltan Boszormenyi (#27)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Zoltan Boszormenyi wrote:

Alvaro Herrera �rta:

Zoltan Boszormenyi wrote:

Alvaro Herrera �rta:

But COPY view (col1, col2, ...) TO may still be
useful even if the COPY (SELECT ...) (col1, col2, ...) TO
is pointless. [1]

Hum, I don't understand what you're saying here -- are you saying that
you can't do something with the first form, that you cannot do with the
second?

Say you have a large often used query.
Would you like to retype it every time
or just create a view? Later you may want to
export only a subset of the fields...

My v8 had the syntax support for

COPY (SELECT ...) (col1, col2, ...) TO
and it was actually working. In your v9
you rewrote the syntax parsing so that
feature was lost in translation.

Interesting. I didn't realize this was possible -- obviously I didn't
test it (did you have a test for it in the regression tests? I may have
missed it). In fact, I deliberately removed the column list from the
grammar, because it can certainly be controlled inside the SELECT, so I
thought there was no reason the support controlling it in the COPY
column list.

I don't think it's difficult to put it back. But this has nothing to do
with COPY view, does it?

On the other hand I don't see why you are arguing in favor of a useless
feature whose coding is dubious; you can have _the same thing_ with nice
code and no discussion.

Because of [1] and because Mr. Schoenig's arguments
about changing schemas.

Yeah, that argument makes sense to me as well.

So, may I put it back? :-)
Also, can you suggest anything cleaner than
calling raw_parser("SELECT * FROM view")?

I think at this point is someone else's judgement whether you can put it
back or not. Tom already said that he doesn't object to the feature per
se; no one else seems opposed to the feature per se, in fact.

Now, I don't really see _how_ to do it in nice code, so no, I don't have
any suggestion for you. You may want to give the pumpkin to Tom so that
he gives the patch the finishing touches (hopefully making it support
the "COPY view" feature as well).

If it were up to me, I'd just commit it as is (feature-wise -- more
thorough review is still needed) and revisit the COPY view stuff in 8.3
if there is demand.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

#29Zoltan Boszormenyi
zboszor@dunaweb.hu
In reply to: Alvaro Herrera (#28)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Alvaro Herrera �rta:

Zoltan Boszormenyi wrote:

Alvaro Herrera �rta:

Zoltan Boszormenyi wrote:

Alvaro Herrera �rta:

But COPY view (col1, col2, ...) TO may still be
useful even if the COPY (SELECT ...) (col1, col2, ...) TO
is pointless. [1]

Hum, I don't understand what you're saying here -- are you saying that
you can't do something with the first form, that you cannot do with the
second?

Say you have a large often used query.
Would you like to retype it every time
or just create a view? Later you may want to
export only a subset of the fields...

My v8 had the syntax support for

COPY (SELECT ...) (col1, col2, ...) TO
and it was actually working. In your v9
you rewrote the syntax parsing so that
feature was lost in translation.

Interesting. I didn't realize this was possible -- obviously I didn't
test it (did you have a test for it in the regression tests? I may have
missed it). In fact, I deliberately removed the column list from the
grammar, because it can certainly be controlled inside the SELECT, so I
thought there was no reason the support controlling it in the COPY
column list.

Yes, it was even documented. I thought about having
queries stored statically somewhere (not in views) and
being able to use only part of the result.

I don't think it's difficult to put it back. But this has nothing to do
with COPY view, does it?

No, but it may be confusing seeing
COPY (SELECT ) (col1, col2, ...) TO
instead of COPY (SELECT col1, col2, ...) TO.
With the COPY VIEW (col1, col2, ...) TO syntax
it may be cleaner from the user's point of view.
Together with the changing schemas argument
it gets more and more tempting.

On the other hand I don't see why you are arguing in favor of a useless
feature whose coding is dubious; you can have _the same thing_ with nice
code and no discussion.

Because of [1] and because Mr. Schoenig's arguments
about changing schemas.

Yeah, that argument makes sense to me as well.

So, may I put it back? :-)
Also, can you suggest anything cleaner than
calling raw_parser("SELECT * FROM view")?

I think at this point is someone else's judgement whether you can put it
back or not. Tom already said that he doesn't object to the feature per
se; no one else seems opposed to the feature per se, in fact.

Now, I don't really see _how_ to do it in nice code, so no, I don't have
any suggestion for you. You may want to give the pumpkin to Tom so that
he gives the patch the finishing touches (hopefully making it support
the "COPY view" feature as well).

If it were up to me, I'd just commit it as is (feature-wise -- more
thorough review is still needed) and revisit the COPY view stuff in 8.3
if there is demand.

OK, I will put it back as it was in v8
keeping all your other cleanup and
let Bruce and Tom decide.

(BTW, is there anyone as high-ranking as them,
or the "committee" is a duumvirate? :-) )

#30Alvaro Herrera
alvherre@commandprompt.com
In reply to: Zoltan Boszormenyi (#29)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Zoltan Boszormenyi wrote:

I think at this point is someone else's judgement whether you can put it
back or not. Tom already said that he doesn't object to the feature per
se; no one else seems opposed to the feature per se, in fact.

Now, I don't really see _how_ to do it in nice code, so no, I don't have
any suggestion for you. You may want to give the pumpkin to Tom so that
he gives the patch the finishing touches (hopefully making it support
the "COPY view" feature as well).

If it were up to me, I'd just commit it as is (feature-wise -- more
thorough review is still needed) and revisit the COPY view stuff in 8.3
if there is demand.

OK, I will put it back as it was in v8
keeping all your other cleanup and
let Bruce and Tom decide.

Hum, are you going to put back the original cruft to support copy view?
I suggest you don't do that.

(BTW, is there anyone as high-ranking as them,
or the "committee" is a duumvirate? :-) )

There is a "core", there are committers, there are "major developers",
and there are "contributors". This is documented in the developer's
page on the website, though the committers group is not documented
anywhere. (Most, but not all, of Core are also committers. Some Major
Developers are committers as well).

There is no committee. The closer you get to that, is people vocal
enough on pgsql-hackers.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

#31Zoltan Boszormenyi
zboszor@dunaweb.hu
In reply to: Alvaro Herrera (#30)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Alvaro Herrera �rta:

Zoltan Boszormenyi wrote:

I think at this point is someone else's judgement whether you can put it
back or not. Tom already said that he doesn't object to the feature per
se; no one else seems opposed to the feature per se, in fact.

Now, I don't really see _how_ to do it in nice code, so no, I don't have
any suggestion for you. You may want to give the pumpkin to Tom so that
he gives the patch the finishing touches (hopefully making it support
the "COPY view" feature as well).

If it were up to me, I'd just commit it as is (feature-wise -- more
thorough review is still needed) and revisit the COPY view stuff in 8.3
if there is demand.

OK, I will put it back as it was in v8
keeping all your other cleanup and
let Bruce and Tom decide.

Hum, are you going to put back the original cruft to support copy view?
I suggest you don't do that.

Well, the other way around is to teach heap_open()
to use views. Brrr. Would it be any cleaner?

#32Alvaro Herrera
alvherre@commandprompt.com
In reply to: Zoltan Boszormenyi (#31)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Zoltan Boszormenyi wrote:

Alvaro Herrera �rta:

Zoltan Boszormenyi wrote:

I think at this point is someone else's judgement whether you can put it
back or not. Tom already said that he doesn't object to the feature per
se; no one else seems opposed to the feature per se, in fact.

Now, I don't really see _how_ to do it in nice code, so no, I don't have
any suggestion for you. You may want to give the pumpkin to Tom so that
he gives the patch the finishing touches (hopefully making it support
the "COPY view" feature as well).

If it were up to me, I'd just commit it as is (feature-wise -- more
thorough review is still needed) and revisit the COPY view stuff in 8.3
if there is demand.

OK, I will put it back as it was in v8
keeping all your other cleanup and
let Bruce and Tom decide.

Hum, are you going to put back the original cruft to support copy view?
I suggest you don't do that.

Well, the other way around is to teach heap_open()
to use views. Brrr. Would it be any cleaner?

Certainly not.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

#33Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#28)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Alvaro Herrera <alvherre@commandprompt.com> writes:

Zoltan Boszormenyi wrote:

My v8 had the syntax support for
COPY (SELECT ...) (col1, col2, ...) TO
and it was actually working. In your v9
you rewrote the syntax parsing so that
feature was lost in translation.

Interesting. I didn't realize this was possible -- obviously I didn't
test it (did you have a test for it in the regression tests? I may have
missed it). In fact, I deliberately removed the column list from the
grammar, because it can certainly be controlled inside the SELECT, so I
thought there was no reason the support controlling it in the COPY
column list.

I would vote against allowing a column list here, because it's useless
and it strikes me as likely to result in strange syntax error messages
if the user makes any little mistake. What worries me is that the
above looks way too nearly like a function call, which means that for
instance if you omit a right paren somewhere in the SELECT part, you're
likely to get a syntax error that points far to the right of the actual
mistake. The parser could also mistake the column list for a table-alias
column list.

Specifying a column list with a view name is useful, of course, but
what is the point when you are writing out a SELECT anyway?

regards, tom lane

#34Tom Lane
tgl@sss.pgh.pa.us
In reply to: Zoltan Boszormenyi (#31)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Zoltan Boszormenyi <zboszor@dunaweb.hu> writes:

Alvaro Herrera �rta:

Hum, are you going to put back the original cruft to support copy view?
I suggest you don't do that.

Well, the other way around is to teach heap_open()
to use views. Brrr. Would it be any cleaner?

Don't even think of going there ;-)

regards, tom lane

#35Zoltan Boszormenyi
zboszor@dunaweb.hu
In reply to: Tom Lane (#34)
1 attachment(s)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Tom Lane �rta:

Zoltan Boszormenyi <zboszor@dunaweb.hu> writes:

Alvaro Herrera �rta:

Hum, are you going to put back the original cruft to support copy view?
I suggest you don't do that.

Well, the other way around is to teach heap_open()
to use views. Brrr. Would it be any cleaner?

Don't even think of going there ;-)

regards, tom lane

I didn't. :-)

Here's my last, the "cruft" (i.e. COPY view TO support
by rewriting to a SELECT) put back. Tested and
docs modified accordingly.

You can find the previous one (v10) on the list
without it if you need it.

Best regards,
Zolt�n B�sz�rm�nyi

Attachments:

pgsql-copyselect-11.patch.gzapplication/x-tar; name=pgsql-copyselect-11.patch.gzDownload
#36Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#33)
Re: [HACKERS] Performance testing of COPY (SELECT)

Tom Lane wrote:

Alvaro Herrera <alvherre@commandprompt.com> writes:

Zoltan Boszormenyi wrote:

My v8 had the syntax support for
COPY (SELECT ...) (col1, col2, ...) TO
and it was actually working. In your v9
you rewrote the syntax parsing so that
feature was lost in translation.

Interesting. I didn't realize this was possible -- obviously I didn't
test it (did you have a test for it in the regression tests? I may have
missed it). In fact, I deliberately removed the column list from the
grammar, because it can certainly be controlled inside the SELECT, so I
thought there was no reason the support controlling it in the COPY
column list.

I would vote against allowing a column list here, because it's useless
and it strikes me as likely to result in strange syntax error messages
if the user makes any little mistake. What worries me is that the
above looks way too nearly like a function call, which means that for
instance if you omit a right paren somewhere in the SELECT part, you're
likely to get a syntax error that points far to the right of the actual
mistake. The parser could also mistake the column list for a table-alias
column list.

Specifying a column list with a view name is useful, of course, but
what is the point when you are writing out a SELECT anyway?

If you don't support COPY view TO, at least return an error messsage
that suggests using COPY (SELECT * FROM view). And if you support COPY
VIEW, you are going to have to support a column list for that. Is that
additional complexity in COPY? If so, it might be a reason to just
throw an error on views and do use COPY SELECT.

Seeing that COPY VIEW only supports TO, not FROM, and COPY SELECT
support only TO, not FROM, it seems logical for COPY to just support
relations, and COPY SELECT to be used for views, if we can throw an
error on COPY VIEW to tell people to use COPY SELECT.

--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

#37Bruno Wolff III
bruno@wolff.to
In reply to: Zoltan Boszormenyi (#29)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

On Mon, Aug 28, 2006 at 19:35:11 +0200,
Zoltan Boszormenyi <zboszor@dunaweb.hu> wrote:

(BTW, is there anyone as high-ranking as them,
or the "committee" is a duumvirate? :-) )

There is a group referred to as "core" that is the final arbitrator of things.
Tom and Bruce are both members of this group.
Tom and Bruce tend to be the most visibly active "committers" for getting
patches committed for people that can't do it themselves. So you will see them
speak up more than others on the patches list.

#38Bruce Momjian
bruce@momjian.us
In reply to: Bruno Wolff III (#37)
Re: [HACKERS] Performance testing of COPY (SELECT)

Bruno Wolff III wrote:

On Mon, Aug 28, 2006 at 19:35:11 +0200,
Zoltan Boszormenyi <zboszor@dunaweb.hu> wrote:

(BTW, is there anyone as high-ranking as them,
or the "committee" is a duumvirate? :-) )

There is a group referred to as "core" that is the final arbitrator of things.
Tom and Bruce are both members of this group.
Tom and Bruce tend to be the most visibly active "committers" for getting
patches committed for people that can't do it themselves. So you will see them
speak up more than others on the patches list.

We do try not to be decision-makers, but rather give our opinions and
see how the group decides.

--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

#39Zoltan Boszormenyi
zboszor@dunaweb.hu
In reply to: Bruce Momjian (#36)
Re: [HACKERS] Performance testing of COPY (SELECT)

Bruce Momjian �rta:

Tom Lane wrote:

Alvaro Herrera <alvherre@commandprompt.com> writes:

Zoltan Boszormenyi wrote:

My v8 had the syntax support for
COPY (SELECT ...) (col1, col2, ...) TO
and it was actually working. In your v9
you rewrote the syntax parsing so that
feature was lost in translation.

Interesting. I didn't realize this was possible -- obviously I didn't
test it (did you have a test for it in the regression tests? I may have
missed it). In fact, I deliberately removed the column list from the
grammar, because it can certainly be controlled inside the SELECT, so I
thought there was no reason the support controlling it in the COPY
column list.

I would vote against allowing a column list here, because it's useless
and it strikes me as likely to result in strange syntax error messages
if the user makes any little mistake. What worries me is that the
above looks way too nearly like a function call, which means that for
instance if you omit a right paren somewhere in the SELECT part, you're
likely to get a syntax error that points far to the right of the actual
mistake. The parser could also mistake the column list for a table-alias
column list.

Specifying a column list with a view name is useful, of course, but
what is the point when you are writing out a SELECT anyway?

If you don't support COPY view TO, at least return an error messsage
that suggests using COPY (SELECT * FROM view). And if you support COPY
VIEW, you are going to have to support a column list for that. Is that
additional complexity in COPY? If so, it might be a reason to just
throw an error on views and do use COPY SELECT.

No, it oes not have any additional complexity,
it uses the same code COPY tablename TO uses.

Seeing that COPY VIEW only supports TO, not FROM, and COPY SELECT
support only TO, not FROM, it seems logical for COPY to just support
relations, and COPY SELECT to be used for views, if we can throw an
error on COPY VIEW to tell people to use COPY SELECT.

The additional hint would be enough if the VIEW case is
not supported.

#40Böszörményi Zoltán
zboszor@dunaweb.hu
In reply to: Böszörményi Zoltán (#10)
1 attachment(s)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Hi,

as per your suggestion, the COPY view TO support was cut and
a hint was added. Please, review.

Best regards,
Zolt�n B�sz�rm�nyi

Attachments:

pgsql-copyselect-12.patch.gzapplication/x-gzip; name=pgsql-copyselect-12.patch.gzDownload
#41Jim C. Nasby
jnasby@pervasive.com
In reply to: Zoltan Boszormenyi (#29)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

On Mon, Aug 28, 2006 at 07:35:11PM +0200, Zoltan Boszormenyi wrote:

COPY (SELECT ...) (col1, col2, ...) TO
and it was actually working. In your v9
you rewrote the syntax parsing so that
feature was lost in translation.

Interesting. I didn't realize this was possible -- obviously I didn't
test it (did you have a test for it in the regression tests? I may have
missed it). In fact, I deliberately removed the column list from the
grammar, because it can certainly be controlled inside the SELECT, so I
thought there was no reason the support controlling it in the COPY
column list.

Yes, it was even documented. I thought about having
queries stored statically somewhere (not in views) and
being able to use only part of the result.

ISTM that there should have been a regression test that tried that
capability out. That would have made it obvious when the functionality
was lost, at least.
--
Jim C. Nasby, Sr. Engineering Consultant jnasby@pervasive.com
Pervasive Software http://pervasive.com work: 512-231-6117
vcard: http://jim.nasby.net/pervasive.vcf cell: 512-569-9461

#42Tom Lane
tgl@sss.pgh.pa.us
In reply to: Böszörményi Zoltán (#40)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

=?iso-8859-2?Q?B=F6sz=F6rm=E9nyi_Zolt=E1n?= <zboszor@dunaweb.hu> writes:

as per your suggestion, the COPY view TO support was cut and
a hint was added. Please, review.

Committed after some refactoring to avoid code duplication.

Unfortunately, in a moment of pure brain fade, I looked at the wrong
item in my inbox and wrote Bernd Helmle's name instead of yours in the
commit message :-(. My sincere apologies. Bruce, would you make a note
to be sure the right person gets credit in the release notes?

regards, tom lane

#43Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#42)
Re: [HACKERS] Performance testing of COPY (SELECT)

Tom Lane wrote:

=?iso-8859-2?Q?B=F6sz=F6rm=E9nyi_Zolt=E1n?= <zboszor@dunaweb.hu> writes:

as per your suggestion, the COPY view TO support was cut and
a hint was added. Please, review.

Committed after some refactoring to avoid code duplication.

Unfortunately, in a moment of pure brain fade, I looked at the wrong
item in my inbox and wrote Bernd Helmle's name instead of yours in the
commit message :-(. My sincere apologies. Bruce, would you make a note
to be sure the right person gets credit in the release notes?

Fixed with new commit message to copy.c.

--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

#44Zoltan Boszormenyi
zboszor@dunaweb.hu
In reply to: Tom Lane (#42)
Re: [HACKERS] Performance testing of COPY (SELECT) TO

Thanks!!!

Tom Lane �rta:

Show quoted text

=?iso-8859-2?Q?B=F6sz=F6rm=E9nyi_Zolt=E1n?= <zboszor@dunaweb.hu> writes:

as per your suggestion, the COPY view TO support was cut and
a hint was added. Please, review.

Committed after some refactoring to avoid code duplication.

Unfortunately, in a moment of pure brain fade, I looked at the wrong
item in my inbox and wrote Bernd Helmle's name instead of yours in the
commit message :-(. My sincere apologies. Bruce, would you make a note
to be sure the right person gets credit in the release notes?

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq