Thread: Re: [HACKERS] COPY does not work with regproc and aclitem
Tom Lane wrote:
> Alvaro Herrera <alvherre@commandprompt.com> writes:
>> Hmm, maybe it should be using regprocedure instead?
>
> Not unless you want to break initdb. The only reason regproc still
> exists, really, is to accommodate loading of pg_type during initdb.
> Guess what: we can't do type lookup at that point.
>
I prepared patch which use oid output function instead regproc output.
This change works only for COPY TO command. SELECT behavior is
untouched. I extended copy regression test as well.
Please, look on it if it is acceptable fix.
With regards Zdenek
Index: src/backend/commands/copy.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/commands/copy.c,v
retrieving revision 1.271
diff -c -r1.271 copy.c
*** src/backend/commands/copy.c 31 Aug 2006 03:17:50 -0000 1.271
--- src/backend/commands/copy.c 24 Oct 2006 12:35:45 -0000
***************
*** 1309,1315 ****
&out_func_oid,
&isvarlena);
else
! getTypeOutputInfo(attr[attnum - 1]->atttypid,
&out_func_oid,
&isvarlena);
fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
--- 1309,1317 ----
&out_func_oid,
&isvarlena);
else
! /* For regproc datatype do not lookup proc name, use OID out function instead.
! It avoids problem with COPY FROM. */
! getTypeOutputInfo(attr[attnum - 1]->atttypid == REGPROCOID? OIDOID : attr[attnum - 1]->atttypid,
&out_func_oid,
&isvarlena);
fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
Index: src/test/regress/input/copy.source
===================================================================
RCS file: /projects/cvsroot/pgsql/src/test/regress/input/copy.source,v
retrieving revision 1.14
diff -c -r1.14 copy.source
*** src/test/regress/input/copy.source 2 May 2006 11:28:56 -0000 1.14
--- src/test/regress/input/copy.source 24 Oct 2006 12:35:46 -0000
***************
*** 105,107 ****
--- 105,113 ----
copy copytest3 to stdout csv header;
+ --- test correct handling regproc data type
+ CREATE TEMP TABLE test_regproc (like pg_aggregate);
+ COPY pg_catalog.pg_aggregate TO '@abs_builddir@/results/test_regproc.data';
+ COPY test_regproc FROM '@abs_builddir@/results/test_regproc.data';
+
+ select aggfnoid, cast(aggfnoid as oid) from pg_aggregate where aggfnoid=2147;
Index: src/test/regress/output/copy.source
===================================================================
RCS file: /projects/cvsroot/pgsql/src/test/regress/output/copy.source,v
retrieving revision 1.12
diff -c -r1.12 copy.source
*** src/test/regress/output/copy.source 2 May 2006 11:28:56 -0000 1.12
--- src/test/regress/output/copy.source 24 Oct 2006 12:35:46 -0000
***************
*** 70,72 ****
--- 70,82 ----
c1,"col with , comma","col with "" quote"
1,a,1
2,b,2
+ --- test correct handling regproc data type
+ CREATE TEMP TABLE test_regproc (like pg_aggregate);
+ COPY pg_catalog.pg_aggregate TO '@abs_builddir@/results/test_regproc.data';
+ COPY test_regproc FROM '@abs_builddir@/results/test_regproc.data';
+ select aggfnoid, cast(aggfnoid as oid) from pg_aggregate where aggfnoid=2147;
+ aggfnoid | aggfnoid
+ ------------------+----------
+ pg_catalog.count | 2147
+ (1 row)
+
Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
> I prepared patch which use oid output function instead regproc output.
> This change works only for COPY TO command.
This is not a bug and we're not going to fix it, most especially not
like that.
regards, tom lane
Tom Lane napsal(a):
> Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
>> I prepared patch which use oid output function instead regproc output.
>> This change works only for COPY TO command.
>
> This is not a bug and we're not going to fix it, most especially not
> like that.
>
OK, The behavior of regproc type is described in the documentation, but
if we don't fix it, than Some error message like "Regproc data type is
not supported by COPY TO command" could be useful. Because you find that
something is wrong when you want to restore data back and it should be
too late.
Zdenek
Zdenek Kotala wrote: > Tom Lane napsal(a): > >Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes: > >>I prepared patch which use oid output function instead regproc output. > >>This change works only for COPY TO command. > > > >This is not a bug and we're not going to fix it, most especially not > >like that. > > OK, The behavior of regproc type is described in the documentation, but > if we don't fix it, than Some error message like "Regproc data type is > not supported by COPY TO command" could be useful. Because you find that > something is wrong when you want to restore data back and it should be > too late. But it works as "expected". If the approach you suggest would be one we would take, then it should emit the same error on SELECT as well, shouldn't we? I think the problem is that regproc COPY is not useful to you for your particular use case. But there are workarounds, like the one I suggested and you promptly ignored. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera napsal(a):
> Zdenek Kotala wrote:
>> Tom Lane napsal(a):
>>> Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
>>>> I prepared patch which use oid output function instead regproc output.
>>>> This change works only for COPY TO command.
>>> This is not a bug and we're not going to fix it, most especially not
>>> like that.
>> OK, The behavior of regproc type is described in the documentation, but
>> if we don't fix it, than Some error message like "Regproc data type is
>> not supported by COPY TO command" could be useful. Because you find that
>> something is wrong when you want to restore data back and it should be
>> too late.
>
> But it works as "expected". If the approach you suggest would be one we
> would take, then it should emit the same error on SELECT as well,
> shouldn't we?
It is right.
> I think the problem is that regproc COPY is not useful to you for your
> particular use case. But there are workarounds, like the one I
> suggested and you promptly ignored.
Yes, I read your suggestion It is useful form me thanks for that. But I
thought how to remove that regproc limitation or how to avoid some
confusing. Current mention about regproc limitation/behavior in the
documentation is really best solution.
By the way, If I read carefully your suggestion, Tom's answer and
documentation, correct solution (theoretical) is replace regproc by
regprocedure datatype in the catalog, but there is problem in the
boostrap phase?
Thanks Zdenek