Re: [HACKERS] COPY as a set returning function

> +     /* param 7: escape text default null, -- defaults to whatever quote is */
> +     if (PG_ARGISNULL(7))
> +     {
> +             copy_state.escape = copy_state.quote;
> +     }
> +     else
> +     {
> +             if (copy_state.csv_mode)
> +             {
> +                     copy_state.escape = TextDatumGetCString(PG_GETARG_TEXT_P(7));
> +                     if (strlen(copy_state.escape) != 1)
> +                     {
> +                             ereport(ERROR,
> +                                             (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> +                                              errmsg("COPY escape must be a single one-byte character")));
> +                     }
> +             }
> +             else
> +             {
> +                     ereport(ERROR,
> +                                     (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> +                                      errmsg("COPY escape available only in CSV mode")));
> +             }
> +     }

I don't understand why do we have all these checks.  Can't we just pass
the values to COPY and let it apply the checks?  That way, when COPY is
updated to support multibyte escape chars (for example) we don't need to
touch this code.  Together with removing the unneeded braces that would
make these stanzas about six lines long instead of fifteen.

> +             tuple = heap_form_tuple(tupdesc,values,nulls);
> +             //tuple = BuildTupleFromCStrings(attinmeta, field_strings);
> +             tuplestore_puttuple(tupstore, tuple);

No need to form a tuple; use tuplestore_putvalues here.

> +     }
> +
> +     /* close "file" */
> +     if (copy_state.is_program)
> +     {
> +             int         pclose_rc;
> +
> +             pclose_rc = ClosePipeStream(copy_state.copy_file);
> +             if (pclose_rc == -1)
> +                     ereport(ERROR,
> +                                     (errcode_for_file_access(),
> +                                      errmsg("could not close pipe to external command: %m")));
> +             else if (pclose_rc != 0)
> +                     ereport(ERROR,
> +                                     (errcode(ERRCODE_EXTERNAL_ROUTINE_EXCEPTION),
> +                                      errmsg("program \"%s\" failed",
> +                                                     copy_state.filename),
> +                                      errdetail_internal("%s", wait_result_to_str(pclose_rc))));
> +     }
> +     else
> +     {
> +             if (copy_state.filename != NULL && FreeFile(copy_state.copy_file))
> +                     ereport(ERROR,
> +                                     (errcode_for_file_access(),
> +                                      errmsg("could not close file \"%s\": %m",
> +                                                     copy_state.filename)));
> +     }

I wonder if these should be an auxiliary function in copy.c to do this.
Surely copy.c itself does pretty much the same thing ...