Thread: Patch for COPY command
I just started playing around with postgresql on cygwin, and hit my first
snag. The COPY command assumes its input/output to have UNIX style line
endings, which is an inconvenience since notepad is my favorite editor
(seriously). Here's an obvious patch to src\backend\commands\copy.c which
all platforms should be happy with (note: PG_BINARY_R,W should probably be
"rb","wb" on all platforms rather than having the current ifdef for cygwin).
I suspect there are other places in the code that should be using explicit
text-mode open calls, but I don't have time to do an exhaustive search.
--Kevin
bash-2.05a$ diff -u copy.c.orig copy.c
--- copy.c.orig Thu Aug 16 12:36:37 2001
+++ copy.c Sun Jan 27 22:43:56 2002
@@ -273,6 +273,7 @@
Relation rel;
const AclMode required_access = from ? ACL_WR : ACL_RD;
int result;
+ char *open_mode = NULL;
/*
* Open and lock the relation, using the appropriate lock type.
@@ -326,7 +327,8 @@
}
else
{
- fp = AllocateFile(filename, PG_BINARY_R);
+ open_mode = binary ? "rb" : "r";
+ fp = AllocateFile(filename, open_mode);
if (fp == NULL)
elog(ERROR, "COPY command, running in
backend with "
"effective uid %d, could not open
file '%s' for "
@@ -370,7 +372,8 @@
" COPY command.");
oumask = umask((mode_t) 022);
- fp = AllocateFile(filename, PG_BINARY_W);
+ open_mode = binary ? "wb" : "w";
+ fp = AllocateFile(filename, open_mode);
umask(oumask);
if (fp == NULL)
_________________________________________________________________
MSN Photos is the easiest way to share and print your photos:
http://photos.msn.com/support/worldwide.aspx
No feedback so far. Should I just forward this to the patches list, or is
it customary for the maintainer to promote/package patches of interest to
the cygwin port?
--Kevin
>From: "Kevin Chase" <kevincha99@hotmail.com>
>To: pgsql-cygwin@postgresql.org
>Subject: [CYGWIN] Patch for COPY command
>Date: Sun, 27 Jan 2002 23:26:06 -0800
>
>I just started playing around with postgresql on cygwin, and hit my first
>snag. The COPY command assumes its input/output to have UNIX style line
>endings, which is an inconvenience since notepad is my favorite editor
>(seriously). Here's an obvious patch to src\backend\commands\copy.c which
>all platforms should be happy with (note: PG_BINARY_R,W should probably be
>"rb","wb" on all platforms rather than having the current ifdef for
>cygwin).
>
>I suspect there are other places in the code that should be using explicit
>text-mode open calls, but I don't have time to do an exhaustive search.
>
>--Kevin
>
>bash-2.05a$ diff -u copy.c.orig copy.c
>--- copy.c.orig Thu Aug 16 12:36:37 2001
>+++ copy.c Sun Jan 27 22:43:56 2002
>@@ -273,6 +273,7 @@
> Relation rel;
> const AclMode required_access = from ? ACL_WR : ACL_RD;
> int result;
>+ char *open_mode = NULL;
>
> /*
> * Open and lock the relation, using the appropriate lock type.
>@@ -326,7 +327,8 @@
> }
> else
> {
>- fp = AllocateFile(filename, PG_BINARY_R);
>+ open_mode = binary ? "rb" : "r";
>+ fp = AllocateFile(filename, open_mode);
> if (fp == NULL)
> elog(ERROR, "COPY command, running in
>backend with "
> "effective uid %d, could not open
>file '%s' for "
>@@ -370,7 +372,8 @@
> " COPY command.");
>
> oumask = umask((mode_t) 022);
>- fp = AllocateFile(filename, PG_BINARY_W);
>+ open_mode = binary ? "wb" : "w";
>+ fp = AllocateFile(filename, open_mode);
> umask(oumask);
>
> if (fp == NULL)
>
>
>
>_________________________________________________________________
>MSN Photos is the easiest way to share and print your photos:
>http://photos.msn.com/support/worldwide.aspx
>
>
>---------------------------(end of broadcast)---------------------------
>TIP 3: if posting/reading through Usenet, please send an appropriate
>subscribe-nomail command to majordomo@postgresql.org so that your
>message can get through to the mailing list cleanly
_________________________________________________________________
MSN Photos is the easiest way to share and print your photos:
http://photos.msn.com/support/worldwide.aspx
There is currently an ongoing thread on the -hackers list discussing ways of
fixing the crlf problem ([HACKERS] Idea for making COPY data
Microsoft-proof). Perhaps you should join in there.
Regards, Dave.
> -----Original Message-----
> From: Kevin Chase [mailto:kevincha99@hotmail.com]
> Sent: 12 February 2002 07:09
> To: pgsql-cygwin@postgresql.org
> Subject: Re: [CYGWIN] Patch for COPY command
>
>
> No feedback so far. Should I just forward this to the
> patches list, or is
> it customary for the maintainer to promote/package patches of
> interest to
> the cygwin port?
>
> --Kevin
>
>
> >From: "Kevin Chase" <kevincha99@hotmail.com>
> >To: pgsql-cygwin@postgresql.org
> >Subject: [CYGWIN] Patch for COPY command
> >Date: Sun, 27 Jan 2002 23:26:06 -0800
> >
> >I just started playing around with postgresql on cygwin, and hit my
> >first snag. The COPY command assumes its input/output to have UNIX
> >style line endings, which is an inconvenience since notepad is my
> >favorite editor (seriously). Here's an obvious patch to
> >src\backend\commands\copy.c which all platforms should be happy with
> >(note: PG_BINARY_R,W should probably be "rb","wb" on all platforms
> >rather than having the current ifdef for cygwin).
> >
> >I suspect there are other places in the code that should be using
> >explicit text-mode open calls, but I don't have time to do an
> >exhaustive search.
> >
> >--Kevin
> >
> >bash-2.05a$ diff -u copy.c.orig copy.c
> >--- copy.c.orig Thu Aug 16 12:36:37 2001
> >+++ copy.c Sun Jan 27 22:43:56 2002
> >@@ -273,6 +273,7 @@
> > Relation rel;
> > const AclMode required_access = from ? ACL_WR : ACL_RD;
> > int result;
> >+ char *open_mode = NULL;
> >
> > /*
> > * Open and lock the relation, using the appropriate
> lock type.
> >@@ -326,7 +327,8 @@
> > }
> > else
> > {
> >- fp = AllocateFile(filename, PG_BINARY_R);
> >+ open_mode = binary ? "rb" : "r";
> >+ fp = AllocateFile(filename, open_mode);
> > if (fp == NULL)
> > elog(ERROR, "COPY command,
> running in
> >backend with "
> > "effective uid %d,
> could not
> >open file '%s' for " @@ -370,7 +372,8 @@
> > " COPY command.");
> >
> > oumask = umask((mode_t) 022);
> >- fp = AllocateFile(filename, PG_BINARY_W);
> >+ open_mode = binary ? "wb" : "w";
> >+ fp = AllocateFile(filename, open_mode);
> > umask(oumask);
> >
> > if (fp == NULL)
> >
> >
> >
> >_________________________________________________________________
> >MSN Photos is the easiest way to share and print your photos:
> >http://photos.msn.com/support/worldwide.aspx
> >
> >
> >---------------------------(end of
> >broadcast)---------------------------
> >TIP 3: if posting/reading through Usenet, please send an appropriate
> >subscribe-nomail command to majordomo@postgresql.org so that your
> >message can get through to the mailing list cleanly
>
>
>
>
> _________________________________________________________________
> MSN Photos is the easiest way to share and print your photos:
> http://photos.msn.com/support/worldwide.aspx
>
>
> ---------------------------(end of
> broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an
> appropriate subscribe-nomail command to
> majordomo@postgresql.org so that your message can get through
> to the mailing list cleanly
>
Kevin, On Mon, Feb 11, 2002 at 11:08:39PM -0800, Kevin Chase wrote: > No feedback so far. Should I just forward this to the patches list, or is > it customary for the maintainer to promote/package patches of interest to > the cygwin port? Huh? Do you mean me? If so, sorry but I'm not inclined on this issue. And don't worry about stepping on toes. Please feel free to submit to patches. The more the merrier. Jason
That discussion may be orthogonal to my peeve, namely the failure of
postgresql to output/input text in textmode so that the mount default for
line endings is observed. However, if the input file is on a textmode
mount, my patch will allow either \r\n or \n endings, which might be the
source of many of the problems. Won't do any good for \r endings though.
--Kevin
>From: Dave Page <dpage@vale-housing.co.uk>
>To: "'Kevin Chase'" <kevincha99@hotmail.com>, pgsql-cygwin@postgresql.org
>Subject: Re: [CYGWIN] Patch for COPY command
>Date: Tue, 12 Feb 2002 08:24:59 -0000
>
>There is currently an ongoing thread on the -hackers list discussing ways
>of
>fixing the crlf problem ([HACKERS] Idea for making COPY data
>Microsoft-proof). Perhaps you should join in there.
>
>Regards, Dave.
>
> > -----Original Message-----
> > From: Kevin Chase [mailto:kevincha99@hotmail.com]
> > Sent: 12 February 2002 07:09
> > To: pgsql-cygwin@postgresql.org
> > Subject: Re: [CYGWIN] Patch for COPY command
> >
> >
> > No feedback so far. Should I just forward this to the
> > patches list, or is
> > it customary for the maintainer to promote/package patches of
> > interest to
> > the cygwin port?
> >
> > --Kevin
> >
> >
> > >From: "Kevin Chase" <kevincha99@hotmail.com>
> > >To: pgsql-cygwin@postgresql.org
> > >Subject: [CYGWIN] Patch for COPY command
> > >Date: Sun, 27 Jan 2002 23:26:06 -0800
> > >
> > >I just started playing around with postgresql on cygwin, and hit my
> > >first snag. The COPY command assumes its input/output to have UNIX
> > >style line endings, which is an inconvenience since notepad is my
> > >favorite editor (seriously). Here's an obvious patch to
> > >src\backend\commands\copy.c which all platforms should be happy with
> > >(note: PG_BINARY_R,W should probably be "rb","wb" on all platforms
> > >rather than having the current ifdef for cygwin).
> > >
> > >I suspect there are other places in the code that should be using
> > >explicit text-mode open calls, but I don't have time to do an
> > >exhaustive search.
> > >
> > >--Kevin
> > >
> > >bash-2.05a$ diff -u copy.c.orig copy.c
> > >--- copy.c.orig Thu Aug 16 12:36:37 2001
> > >+++ copy.c Sun Jan 27 22:43:56 2002
> > >@@ -273,6 +273,7 @@
> > > Relation rel;
> > > const AclMode required_access = from ? ACL_WR : ACL_RD;
> > > int result;
> > >+ char *open_mode = NULL;
> > >
> > > /*
> > > * Open and lock the relation, using the appropriate
> > lock type.
> > >@@ -326,7 +327,8 @@
> > > }
> > > else
> > > {
> > >- fp = AllocateFile(filename, PG_BINARY_R);
> > >+ open_mode = binary ? "rb" : "r";
> > >+ fp = AllocateFile(filename, open_mode);
> > > if (fp == NULL)
> > > elog(ERROR, "COPY command,
> > running in
> > >backend with "
> > > "effective uid %d,
> > could not
> > >open file '%s' for " @@ -370,7 +372,8 @@
> > > " COPY command.");
> > >
> > > oumask = umask((mode_t) 022);
> > >- fp = AllocateFile(filename, PG_BINARY_W);
> > >+ open_mode = binary ? "wb" : "w";
> > >+ fp = AllocateFile(filename, open_mode);
> > > umask(oumask);
> > >
> > > if (fp == NULL)
> > >
> > >
> > >
> > >_________________________________________________________________
> > >MSN Photos is the easiest way to share and print your photos:
> > >http://photos.msn.com/support/worldwide.aspx
> > >
> > >
> > >---------------------------(end of
> > >broadcast)---------------------------
> > >TIP 3: if posting/reading through Usenet, please send an appropriate
> > >subscribe-nomail command to majordomo@postgresql.org so that your
> > >message can get through to the mailing list cleanly
> >
> >
> >
> >
> > _________________________________________________________________
> > MSN Photos is the easiest way to share and print your photos:
> > http://photos.msn.com/support/worldwide.aspx
> >
> >
> > ---------------------------(end of
> > broadcast)---------------------------
> > TIP 3: if posting/reading through Usenet, please send an
> > appropriate subscribe-nomail command to
> > majordomo@postgresql.org so that your message can get through
> > to the mailing list cleanly
> >
>
>---------------------------(end of broadcast)---------------------------
>TIP 3: if posting/reading through Usenet, please send an appropriate
>subscribe-nomail command to majordomo@postgresql.org so that your
>message can get through to the mailing list cleanly
_________________________________________________________________
Chat with friends online, try MSN Messenger: http://messenger.msn.com
"Kevin Chase" <kevincha99@hotmail.com> writes:
> That discussion may be orthogonal to my peeve, namely the failure of
> postgresql to output/input text in textmode so that the mount default for
> line endings is observed. However, if the input file is on a textmode
> mount, my patch will allow either \r\n or \n endings, which might be the
> source of many of the problems. Won't do any good for \r endings though.
If we select text mode I/O on Windows, it will in fact *break* COPY's
current handling of \r and \n embedded in data. Because of that, I do
not believe that your patch is acceptable now.
It'd be possible and perhaps reasonable to use text mode after we make
the changes currently being discussed in the pghackers thread. Once
that happens, exactly which representation is used for newlines will be
irrelevant, so we could allow libc to translate newlines to the local
convention, which is essentially what you're asking for.
regards, tom lane