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

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 Nex

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

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 +ERRO

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

> 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

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 mast

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

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 m

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

> 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 go

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

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 combination

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

> 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 w

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

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, al

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

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, > > - (errcode(ERRCODE_FEATURE_NOT

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

> 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, > - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), > -errmsg("COPY HEADER available only in CSV mod

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

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

> 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

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 > 1)

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

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 attac

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

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.T

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

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

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 caused

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

On Tue, May 15, 2018 at 12:06 PM, Tom Lane wrote: >> One can imagine extensions of the idea: for example, the header could >> actually be used to identify the columns, so the column order in the file >> doesn't matter. There could also be an "AS" syntax to allow the target >> field names to be dif

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

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 Vér

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

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 that could have some

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

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 using COPY TO, >> to know whether an input file was ge

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

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. > > +1. > The downside is that it would create the need, when using COPY TO, > to know whether an input file was generated wi

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

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

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 p

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

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 wrote: > On Mon, May 14, 2018

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

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 header line could prevent th

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

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

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 n

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

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 patch makes sense to me an

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

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 add

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

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

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 wit