Re: Allow COPY's 'text' format to output a header

2018-10-01 Thread Michael Paquier
On Fri, Aug 17, 2018 at 01:39:11PM +0900, Michael Paquier wrote: > The point about the header matching mentioned upthread is quite > interesting as it could make the proposed feature way more useful, and > it has not really been discussed. As far as I can see this adds more > sanity checks in

Re: Allow COPY's 'text' format to output a header

2018-08-16 Thread Michael Paquier
On Thu, Aug 09, 2018 at 10:37:28AM -0400, Cynthia Shang wrote: > This patch looks good. I realized I should have changed the status > back while we were discussing all this. It is now (and still is) ready > for committer. I have some comments. -ERROR: COPY HEADER available only in CSV mode

Re: Allow COPY's 'text' format to output a header

2018-08-09 Thread Cynthia Shang
> On Aug 8, 2018, at 2:57 PM, Simon Muller wrote: > > If there's a merge conflict against master, then it'd be good for an > updated patch to be posted. > > Thanks! > > Stephen > > Attached is an updated patch that should directly apply against current > master. > > -- > Simon Muller > >

Re: Allow COPY's 'text' format to output a header

2018-08-08 Thread Simon Muller
On 6 August 2018 at 16:34, Stephen Frost wrote: > Greetings, > > * Cynthia Shang (cynthia.sh...@crunchydata.com) wrote: > > I was able to apply the patch (after resolving a merge conflict which > was expected given an update in master). All looks good. > > If there's a merge conflict against

Re: Allow COPY's 'text' format to output a header

2018-08-06 Thread Stephen Frost
Greetings, * Cynthia Shang (cynthia.sh...@crunchydata.com) wrote: > > On Aug 2, 2018, at 3:30 PM, Simon Muller wrote: > > > > Sure, thanks both for the feedback. Attached is a patch with the error kept > > as ERRCODE_FEATURE_NOT_SUPPORTED. > > I was able to apply the patch (after resolving a

Re: Allow COPY's 'text' format to output a header

2018-08-03 Thread Cynthia Shang
> On Aug 2, 2018, at 3:30 PM, Simon Muller wrote: > > Sure, thanks both for the feedback. Attached is a patch with the error kept > as ERRCODE_FEATURE_NOT_SUPPORTED. > I was able to apply the patch (after resolving a merge conflict which was expected given an update in master). All looks

Re: Allow COPY's 'text' format to output a header

2018-08-02 Thread Simon Muller
On 2 August 2018 at 17:07, Cynthia Shang wrote: > > > On Aug 2, 2018, at 8:11 AM, Daniel Verite > wrote: > > > > That makes sense, thanks for elaborating, although there are also > > a fair number of ERRCODE_FEATURE_NOT_SUPPORTED in copy.c > > that are raised on forbidden/nonsensical

Re: Allow COPY's 'text' format to output a header

2018-08-02 Thread Cynthia Shang
> On Aug 2, 2018, at 8:11 AM, Daniel Verite wrote: > > That makes sense, thanks for elaborating, although there are also > a fair number of ERRCODE_FEATURE_NOT_SUPPORTED in copy.c > that are raised on forbidden/nonsensical combination of features, > so the consistency argument could work both

Re: Allow COPY's 'text' format to output a header

2018-08-02 Thread Daniel Verite
Simon Muller wrote: > I changed the error type and message for consistency with other similar > errors in that file. Whenever options are combined that are incompatible, > it looks like the convention is for a ERRCODE_SYNTAX_ERROR to be thrown. That makes sense, thanks for elaborating,

Re: Allow COPY's 'text' format to output a header

2018-08-01 Thread Simon Muller
On 1 August 2018 at 17:18, Cynthia Shang wrote: > > > On Aug 1, 2018, at 10:20 AM, Daniel Verite > wrote: > > > > /* Check header */ > > - if (!cstate->csv_mode && cstate->header_line) > > + if (cstate->binary && cstate->header_line) > > ereport(ERROR, > > -

Re: Allow COPY's 'text' format to output a header

2018-08-01 Thread Daniel Verite
Simon Muller wrote: > I've incorporated both your suggestions and included the patch you provided > in the attached patch. Hope it's as expected. Still unconvinced about the use case, since COPY's text format is only meant to be consumed by Postgres, and the only way that Postgres will consume

Re: Allow COPY's 'text' format to output a header

2018-07-26 Thread Cynthia Shang
> On Jul 25, 2018, at 6:09 PM, Simon Muller wrote: > > I've incorporated both your suggestions and included the patch you provided > in the attached patch. Hope it's as expected. > -- > Simon Muller > > Reviewed and retested. Changing status to Ready for Committer.

Re: Allow COPY's 'text' format to output a header

2018-07-25 Thread Simon Muller
On 25 July 2018 at 19:24, Cynthia Shang wrote: > > I've reviewed this patch and feel this patch addresses the original ask. I > tested it manually trying to break it and, as mentioned previously, it's > behavior is the same as the CSV copy with regards to it's shortcomings. > However, I feel >

Re: Allow COPY's 'text' format to output a header

2018-07-25 Thread Cynthia Shang
On Wed, Jul 25, 2018 at 1:24 PM, Cynthia Shang wrote: > With regards to #2, the copy.source tests are for things requiring > replacement when running the tests. Given that these copy tests do not, I > have moved the current last set of copy tests to the copy2.sql file and have > provided an

Re: Allow COPY's 'text' format to output a header

2018-07-25 Thread Cynthia Shang
On 4 July 2018 at 22:44, Simon Muller  wrote:I noticed through the patch tester link at http://commitfest.cputube.org/ that my patch caused a file_fdw test to fail (since I previously tested only with "make check" and not with "make check-world").This v2 patch should fix

Re: Allow COPY's 'text' format to output a header

2018-07-10 Thread Simon Muller
On 4 July 2018 at 22:44, Simon Muller wrote: > I noticed through the patch tester link at http://commitfest.cputube.org/ > that my patch caused a file_fdw test to fail (since I previously tested > only with "make check" and not with "make check-world"). > > This v2 patch should fix that. > This

Re: Allow COPY's 'text' format to output a header

2018-07-04 Thread Simon Muller
On 14 May 2018 at 08:35, Simon Muller wrote: > Okay, I've added this to the next commitfest at > https://commitfest.postgresql.org/18/1629/. > > Thanks both Michael and David for the feedback so far. > I noticed through the patch tester link at http://commitfest.cputube.org/ that my patch

Re: Allow COPY's 'text' format to output a header

2018-05-15 Thread Daniel Verite
Isaac Morland wrote: > Just to be clear, we're talking about my "header match" feature, not the > basic idea of allowing a header in text format? For my reply it was on merely allowing it, as does the current patch at https://commitfest.postgresql.org/18/1629 Best regards, -- Daniel

Re: Allow COPY's 'text' format to output a header

2018-05-15 Thread David G. Johnston
On Tuesday, May 15, 2018, Tom Lane wrote: > > > AFAICS, Daniel's just reacting to the basic idea of a header line. > I agree that by itself that's not worth much. However, if we added > your proposed option to insist that the column names match during COPY > IN, I think that

Re: Allow COPY's 'text' format to output a header

2018-05-15 Thread Tom Lane
Isaac Morland writes: > On 15 May 2018 at 10:26, Daniel Verite wrote: >> Andrew Dunstan wrote: >>> I'm not necessarily opposed to this, but I'm not certain about the use >>> case either. >> The downside is that it would create the need, when

Re: Allow COPY's 'text' format to output a header

2018-05-15 Thread Daniel Verite
Andrew Dunstan wrote: > I'm not necessarily opposed to this, but I'm not certain about the use > case either. +1. The downside is that it would create the need, when using COPY TO, to know whether an input file was generated with or without header, and a hazard on mistakes. If you say it

Re: Allow COPY's 'text' format to output a header

2018-05-14 Thread Michael Paquier
On Mon, May 14, 2018 at 04:08:47PM -0400, Isaac Morland wrote: > While we're discussing COPY options, what do people think of an option for > COPY FROM with header to require that the headers match the target column > names? This would help to ensure that the file is actually the right one. I am

Re: Allow COPY's 'text' format to output a header

2018-05-14 Thread Isaac Morland
While we're discussing COPY options, what do people think of an option for COPY FROM with header to require that the headers match the target column names? This would help to ensure that the file is actually the right one. On 14 May 2018 at 14:55, David G. Johnston

Re: Allow COPY's 'text' format to output a header

2018-05-14 Thread David G. Johnston
On Mon, May 14, 2018 at 11:44 AM, Garick Hamlin wrote: > I wonder if there was a way to let COPY FROM detect or ignore headers > as appropriate and rather than cause silently result in headers being > added as data. > ​ ​Not reliably​ > Maybe a blank line after the

Re: Allow COPY's 'text' format to output a header

2018-05-14 Thread Garick Hamlin
On Mon, May 14, 2018 at 09:37:07AM +0900, Michael Paquier wrote: > On Sun, May 13, 2018 at 07:01:00PM -0400, David Steele wrote: > > This patch makes sense to me and looks reasonable. > > One "potential" problem is if a relation has a full set of column which > allows the input of text-like data:

Re: Allow COPY's 'text' format to output a header

2018-05-14 Thread Andrew Dunstan
On 05/14/2018 02:35 AM, Simon Muller wrote: > Okay, I've added this to the next commitfest at > https://commitfest.postgresql.org/18/1629/. > > Thanks both Michael and David for the feedback so far. > > (Please don't top-post on PostgreSQL lists.) I'm not necessarily opposed to this, but I'm

Re: Allow COPY's 'text' format to output a header

2018-05-14 Thread Simon Muller
Okay, I've added this to the next commitfest at https://commitfest.postgresql.org/18/1629/. Thanks both Michael and David for the feedback so far. On 14 May 2018 at 02:37, Michael Paquier wrote: > On Sun, May 13, 2018 at 07:01:00PM -0400, David Steele wrote: > > This

Re: Allow COPY's 'text' format to output a header

2018-05-13 Thread Michael Paquier
On Sun, May 13, 2018 at 07:01:00PM -0400, David Steele wrote: > This patch makes sense to me and looks reasonable. One "potential" problem is if a relation has a full set of column which allows the input of text-like data: if the header has been added with COPY TO, and that the user forgets to

Re: Allow COPY's 'text' format to output a header

2018-05-13 Thread David Steele
Hi Simon, On 5/13/18 6:18 PM, Simon Muller wrote: > This patch adds the capability to use the HEADER feature with the "text" > format of the COPY command. The patch includes the related update to > documentation and an additional regression test for this feature. > > Currently you can only add a

Allow COPY's 'text' format to output a header

2018-05-13 Thread Simon Muller
This patch adds the capability to use the HEADER feature with the "text" format of the COPY command. The patch includes the related update to documentation and an additional regression test for this feature. Currently you can only add a header line (which lists the column names) when exporting