Re: [PATCH 4/4] Add tests to dblink covering use of COPY TO FUNCTION

On Tue, Dec 29, 2009 at 8:11 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> It might be possible to do this without introducing a notion of an
> explicit object, but there are a couple of problems with that.  First,
> it requires the user to specify a lot of details on every COPY
> invocation, which is awkward.  Second, there's a security issue to
> think about here.  If we were just copying to a UDF that took a string
> as an argument, that would be fine, but it's not safe to let
> unprivileged users to directly invoke functions that take a
> type-internal argument.  Introducing an explicit object type would
> allow the creation of copy targets to be restricted to super-users but
> then granted out to whom the super-user chooses.
>
> Thoughts?

Agree on the type internal and superuser access -- indeed, if one were
to refactor the two existing COPY modes into external functions, the
stdout behavior would be marked with SECURITY DEFINER and the to-file
functions would only be superuser-accessible. (Interesting note: that
means copy.c could theoretically lose the special check for superuser
privilege for this mode, relying on standard function permissions...).

I was mostly satisfied with a byzantine but otherwise semantically
simple interface until the idea matures some more -- perhaps in
practice -- to inform the most useful kind of convenience to support
it. I don't see a strong reason personally to rush into defining such
an interface just yet, although an important interface we would have
to define is contract a user would have to follow to enable their very
own fully featured COPY output mode.

Still, the patch as-submitted is quite far from achieving one of my
main litmus test goals: subsumption of existing COPY behavior.
Particularly thorny was how to support the copying-to-a-file
semantics, but I believe that the copy-options patch provide a nice
avenue to solve this problem, as one can call a function in the
options list and somehow pass the return value of that initializer --
which may include a file handle -- to the byte-handling function.

Finally, I think a valid point was made that the patch is much more
powerful to end users if it supports byte arrays, and there are some
open questions as to whether this should be the only/primary supported
mode. I personally like the INTERNAL-type interface, as dealing with
the StringInfo buffer used by current COPY code is very convenient
from C and the current implementation is not very complicated yet
avoids unnecessary copying/value coercions, but I agree there is
definitely value in enabling the use of more normal data types.

fdr