Re: [PATCHES] [HACKERS] [Fwd: Re: [pgsql-hackers-win32] Import from Linux to
Andrew Dunstan <[EMAIL PROTECTED]> writes: > The patch is not platform-specific. It simply makes psql accept the same > line endings on COPY FROM that the backend will accept - in effect it > makes it line-end agnostic - this is a Good Thing (tm). Strictly speaking it's not there yet --- psql still doesn't cope with line endings that are \r only (original Mac OS style). However, since we do not and probably never will have a port to original Mac OS, I'm satisfied with the code as-is. > We are still months away from releasing 8.0, so I think doing this for > tomorrow's 7.4 bundle makes sense. I agree (and did in fact just commit the back-patch a bit ago). regards, tom lane ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] [Fwd: Re: [pgsql-hackers-win32] Import from Linux to
Andrew Dunstan <[EMAIL PROTECTED]> writes: > Here's the diff with 7.4 line numbers - tests fine for me. Patch applied. Thanks for double-checking it. regards, tom lane ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] [Fwd: Re: [pgsql-hackers-win32] Import from Linux to
Andrew Dunstan wrote: > > > Bruce Momjian wrote: > > >One issue is that pre-8.0, psql files were opened in Win32 text mode, so > >we wouldn't have seen this bug on Win32, but we would on Linux. > >Because we open them on Win32 now in binary mode so we see control-Z it > >will show up on Win32 too. > > > > > > > > true, *BUT* > > The patch is not platform-specific. It simply makes psql accept the same > line endings on COPY FROM that the backend will accept - in effect it > makes it line-end agnostic - this is a Good Thing (tm). > > We are still months away from releasing 8.0, so I think doing this for > tomorrow's 7.4 bundle makes sense. Yes, totally agree. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] [Fwd: Re: [pgsql-hackers-win32] Import from Linux to
Bruce Momjian wrote: One issue is that pre-8.0, psql files were opened in Win32 text mode, so we wouldn't have seen this bug on Win32, but we would on Linux. Because we open them on Win32 now in binary mode so we see control-Z it will show up on Win32 too. true, *BUT* The patch is not platform-specific. It simply makes psql accept the same line endings on COPY FROM that the backend will accept - in effect it makes it line-end agnostic - this is a Good Thing (tm). We are still months away from releasing 8.0, so I think doing this for tomorrow's 7.4 bundle makes sense. cheers andrew ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] [HACKERS] [Fwd: Re: [pgsql-hackers-win32] Import from Linux to
Andrew Dunstan wrote: > > > Tom Lane wrote: > > >"Andrew Dunstan" <[EMAIL PROTECTED]> writes: > > > > > >>No, I think 7.4 should do. 7.3 users will still have the dos2unix workaround > >>available. Are you going to do the 7.4 patch, or do you need me to? I > >>normally only keep a HEAD tree checked out. A quick look at the cvsweb diffs > >>suggests the patch should apply cleanly but with different line offsets. > >> > >> > > > >If you're sure the code in that routine hasn't changed since 7.4, then I > >can just apply the patch to that branch. > > > > > > > > > > It has - the prompt changes in version 1.37. But I don't think that > conflict with this patch. I'll have a look this morning. One issue is that pre-8.0, psql files were opened in Win32 text mode, so we wouldn't have seen this bug on Win32, but we would on Linux. Because we open them on Win32 now in binary mode so we see control-Z it will show up on Win32 too. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] [HACKERS] [Fwd: Re: [pgsql-hackers-win32] Import from Linux to
Andrew Dunstan wrote: Tom Lane wrote: If you're sure the code in that routine hasn't changed since 7.4, then I can just apply the patch to that branch. It has - the prompt changes in version 1.37. But I don't think that conflict with this patch. I'll have a look this morning. As expected: Hunk #1 succeeded at 496 (offset -167 lines). Hunk #2 succeeded at 689 (offset -7 lines). Hunk #3 succeeded at 579 (offset -167 lines). Here's the diff with 7.4 line numbers - tests fine for me. cheers andrew *** copy.c.orig Mon Aug 4 19:59:39 2003 --- copy.c Sat Aug 14 09:12:04 2004 *** *** 496,501 --- 496,502 boolcopydone = false; boolfirstload; boollinedone; + boolsaw_cr = false; charcopybuf[COPYBUFSIZ]; char *s; int bufleft; *** *** 521,550 while (!linedone) { /* for each bufferload in line ... */ s = copybuf; for (bufleft = COPYBUFSIZ - 1; bufleft > 0; bufleft--) { c = getc(copystream); ! if (c == '\n' || c == EOF) { linedone = true; break; } *s++ = c; } *s = '\0'; if (c == EOF && s == copybuf && firstload) { ! PQputline(conn, "\\."); copydone = true; if (pset.cur_cmd_interactive) puts("\\."); break; } PQputline(conn, copybuf); if (firstload) { ! if (!strcmp(copybuf, "\\.")) { copydone = true; break; --- 522,570 while (!linedone) { /* for each bufferload in line ... */ + /* Fetch string until \n, EOF, or buffer full */ s = copybuf; for (bufleft = COPYBUFSIZ - 1; bufleft > 0; bufleft--) { c = getc(copystream); ! if (c == EOF) { linedone = true; break; } *s++ = c; + if (c == '\n') + { + linedone = true; + break; + } + if (c == '\r') + saw_cr = true; } *s = '\0'; + /* EOF with empty line-so-far? */ if (c == EOF && s == copybuf && firstload) { ! /* !* We are guessing a little bit as to the right line-ending !* here... !*/ ! if (saw_cr) ! PQputline(conn, "\\.\r\n"); ! else ! PQputline(conn, "\\.\n"); copydone = true; if (pset.cur_cmd_interactive) puts("\\."); break; } + /* No, so pass the data to the backend */ PQputline(conn, copybuf); + /* Check for line consisting only of \. */ if (firstload) { ! if (strcmp(copybuf, "\\.\n") == 0 || ! strcmp(copybuf, "\\.\r\n") == 0) { copydone = true; break; *** *** 552,558 firstload = false; } } - PQputline(conn, "\n"); linecount++; } ret = !PQendcopy(conn); --- 572,577 ---(end of broadcast)-
Re: [PATCHES] [HACKERS] [Fwd: Re: [pgsql-hackers-win32] Import from Linux to
Tom Lane wrote: "Andrew Dunstan" <[EMAIL PROTECTED]> writes: No, I think 7.4 should do. 7.3 users will still have the dos2unix workaround available. Are you going to do the 7.4 patch, or do you need me to? I normally only keep a HEAD tree checked out. A quick look at the cvsweb diffs suggests the patch should apply cleanly but with different line offsets. If you're sure the code in that routine hasn't changed since 7.4, then I can just apply the patch to that branch. It has - the prompt changes in version 1.37. But I don't think that conflict with this patch. I'll have a look this morning. cheers andrew ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] [Fwd: Re: [pgsql-hackers-win32] Import from Linux to
"Andrew Dunstan" <[EMAIL PROTECTED]> writes: > No, I think 7.4 should do. 7.3 users will still have the dos2unix workaround > available. Are you going to do the 7.4 patch, or do you need me to? I > normally only keep a HEAD tree checked out. A quick look at the cvsweb diffs > suggests the patch should apply cleanly but with different line offsets. If you're sure the code in that routine hasn't changed since 7.4, then I can just apply the patch to that branch. regards, tom lane ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] [HACKERS] [Fwd: Re: [pgsql-hackers-win32] Import from Linux to
Tom Lane said: > Andrew Dunstan <[EMAIL PROTECTED]> writes: >> Should it be backported for the upcoming stable release(s)? Bruce and >> I were discussing this earlier. > > Probably a good idea, since we do support psql on Windows even in the > older releases. > > My personal opinion is to back-port only as far as 7.4, but if you feel > like doing and testing it for 7.3 then I'll apply the patch. I need it > tomorrow (Sat) though, as I'd like to wrap these releases Sunday. > No, I think 7.4 should do. 7.3 users will still have the dos2unix workaround available. Are you going to do the 7.4 patch, or do you need me to? I normally only keep a HEAD tree checked out. A quick look at the cvsweb diffs suggests the patch should apply cleanly but with different line offsets. cheers andrew ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] [Fwd: Re: [pgsql-hackers-win32] Import from Linux to
Andrew Dunstan <[EMAIL PROTECTED]> writes: > Should it be backported for the upcoming stable release(s)? Bruce and I > were discussing this earlier. Probably a good idea, since we do support psql on Windows even in the older releases. My personal opinion is to back-port only as far as 7.4, but if you feel like doing and testing it for 7.3 then I'll apply the patch. I need it tomorrow (Sat) though, as I'd like to wrap these releases Sunday. regards, tom lane ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [HACKERS] [Fwd: Re: [pgsql-hackers-win32] Import from Linux to
Tom Lane wrote: Andrew Dunstan <[EMAIL PROTECTED]> writes: The attached patch appears to solve the problem. However, while it makes us conform to the first sentence below from the docs, it doesn't comply with the second. Not sure what to do about that. Maybe there's a better solution? Attached patch seems much better, I think. I think it is still not quite there. Since as you noted the backend will complain if line endings don't match, if we hit EOF then we have to cons up a \. line with the correct ending. (BTW, this is not actually necessary when talking 3.0 protocol, but it is when talking to an older server.) I modified the patch a little more and applied the attached. It seems to work for me but could use more testing. WorksForMe, and looks good. You're right, I had forgotten the EOF case. Should it be backported for the upcoming stable release(s)? Bruce and I were discussing this earlier. Pro: it's an ugly bug and hard to diagnose - things just seem to die for no apparent reason. Con: there's a workaround - just make sure to run dos2unix on your file if necessary. cheers andrew ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] [Fwd: Re: [pgsql-hackers-win32] Import from Linux to Windows]
Andrew Dunstan <[EMAIL PROTECTED]> writes: >> The attached patch appears to solve the problem. However, while it >> makes us conform to the first sentence below from the docs, it doesn't >> comply with the second. Not sure what to do about that. Maybe there's >> a better solution? > Attached patch seems much better, I think. I think it is still not quite there. Since as you noted the backend will complain if line endings don't match, if we hit EOF then we have to cons up a \. line with the correct ending. (BTW, this is not actually necessary when talking 3.0 protocol, but it is when talking to an older server.) I modified the patch a little more and applied the attached. It seems to work for me but could use more testing. regards, tom lane *** src/bin/psql/copy.c.origFri Aug 13 10:47:23 2004 --- src/bin/psql/copy.c Fri Aug 13 18:51:25 2004 *** *** 663,668 --- 663,669 boolcopydone = false; boolfirstload; boollinedone; + boolsaw_cr = false; charcopybuf[COPYBUFSIZ]; char *s; int bufleft; *** *** 695,724 while (!linedone) { /* for each bufferload in line ... */ s = copybuf; for (bufleft = COPYBUFSIZ - 1; bufleft > 0; bufleft--) { c = getc(copystream); ! if (c == '\n' || c == EOF) { linedone = true; break; } *s++ = c; } *s = '\0'; if (c == EOF && s == copybuf && firstload) { ! PQputline(conn, "\\."); copydone = true; if (pset.cur_cmd_interactive) puts("\\."); break; } PQputline(conn, copybuf); if (firstload) { ! if (!strcmp(copybuf, "\\.")) { copydone = true; break; --- 696,744 while (!linedone) { /* for each bufferload in line ... */ + /* Fetch string until \n, EOF, or buffer full */ s = copybuf; for (bufleft = COPYBUFSIZ - 1; bufleft > 0; bufleft--) { c = getc(copystream); ! if (c == EOF) { linedone = true; break; } *s++ = c; + if (c == '\n') + { + linedone = true; + break; + } + if (c == '\r') + saw_cr = true; } *s = '\0'; + /* EOF with empty line-so-far? */ if (c == EOF && s == copybuf && firstload) { ! /* !* We are guessing a little bit as to the right line-ending !* here... !*/ ! if (saw_cr) ! PQputline(conn, "\\.\r\n"); ! else ! PQputline(conn, "\\.\n"); copydone = true; if (pset.cur_cmd_interactive) puts("\\."); break; } + /* No, so pass the data to the backend */ PQputline(conn, copybuf); + /* Check for line consisting only of \. */ if (firstload) { ! if (strcmp(copybuf, "\\.\n") == 0 || ! strcmp(copybuf, "\\.\r\n") == 0) { copydone = true; break; *