Re: [PATCHES] Fix pg_ctl restart bug
Bruce Momjian wrote: Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: However, as of 2004-10-15, this has not worked. :-( The attached patch is the one that caused the bug --- on non-Unix systems, SYSTEMQUOTE is , meaning zero-length string. I should have seen the bug when I applied the contributed patch in 2004. So, shouldn't this fix be back-patched? Well, no one has actually complained about the breakage, and it has been a few years. Also there is always the risk of a new bug being introduced, so I am unsure. Why do we need someone to complain? We know the bug is there. Has the code changed a lot in that area? -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Fix pg_ctl restart bug
Alvaro Herrera wrote: Bruce Momjian wrote: Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: However, as of 2004-10-15, this has not worked. :-( The attached patch is the one that caused the bug --- on non-Unix systems, SYSTEMQUOTE is , meaning zero-length string. I should have seen the bug when I applied the contributed patch in 2004. So, shouldn't this fix be back-patched? Well, no one has actually complained about the breakage, and it has been a few years. Also there is always the risk of a new bug being introduced, so I am unsure. Why do we need someone to complain? We know the bug is there. Has the code changed a lot in that area? Do we have the policy of backpatching every fix? I thought it was only the major bugs we fixed in back branches. If someone wants to backpatch it, feel free to do so. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Fix pg_ctl restart bug
Bruce Momjian wrote:
, meaning zero-length string. I should have seen the bug when I
applied the contributed patch in 2004.
So, shouldn't this fix be back-patched?
Well, no one has actually complained about the breakage, and it has been
a few years. Also there is always the risk of a new bug being
introduced, so I am unsure.
Why do we need someone to complain? We know the bug is there. Has the
code changed a lot in that area?
Do we have the policy of backpatching every fix? I thought it was only
the major bugs we fixed in back branches. If someone wants to backpatch
it, feel free to do so.
OK, I started looking at what it would take to backpatch this and found
another bug I have fixed in CVS HEAD. What back branchs (8.0-8.3.X) are
doing is pretty odd. On non-Win32 systems, it is looking for the null
byte, then putting a null byte before it, and passing a NULL back as the
options and binary location. The test:
if (postgres_path != NULL)
postgres_path = optline;
is backwards, which means that if in 8.3.X you start the server with any
arguments, like:
/usr/var/local/postgres/bin/postgres -i -o -d5
and you use pg_ctl to specify the binary location:
pg_ctl -p /u/pg/bin/postmaster restart
the server actually fails to restart because it chops off the last byte
(a bug) and the test above is wrong (another bug), and it thinks the
binary name is the full string, in quotes:
/usr/var/local/postgres/bin/postgres -i -o -d
and you get this error from pg_ctl:
sh: /usr/var/local/postgres/bin/postgres -i -o -d: not found
This is more than just ignoring the documentation, it is a failure.
I am attaching a minimal patch that will fix the bug in back branches.
Keep in mind that a patched pg_ctl will not be able to restart a backend
that was not patched.
--
Bruce Momjian [EMAIL PROTECTED]http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: src/backend/postmaster/postmaster.c
===
RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.551
diff -c -c -r1.551 postmaster.c
*** src/backend/postmaster/postmaster.c 11 Jan 2008 00:54:09 - 1.551
--- src/backend/postmaster/postmaster.c 26 Jun 2008 18:53:42 -
***
*** 4163,4169
fprintf(fp, %s, fullprogname);
for (i = 1; i argc; i++)
! fprintf(fp, %s%s%s, SYSTEMQUOTE, argv[i], SYSTEMQUOTE);
fputs(\n, fp);
if (fclose(fp))
--- 4163,4169
fprintf(fp, %s, fullprogname);
for (i = 1; i argc; i++)
! fprintf(fp, \%s\, argv[i]);
fputs(\n, fp);
if (fclose(fp))
Index: src/bin/pg_ctl/pg_ctl.c
===
RCS file: /cvsroot/pgsql/src/bin/pg_ctl/pg_ctl.c,v
retrieving revision 1.92.2.3
diff -c -c -r1.92.2.3 pg_ctl.c
*** src/bin/pg_ctl/pg_ctl.c 29 Feb 2008 23:31:42 - 1.92.2.3
--- src/bin/pg_ctl/pg_ctl.c 26 Jun 2008 18:53:43 -
***
*** 613,627
{
char *arg1;
! arg1 = strchr(optline, *SYSTEMQUOTE);
! if (arg1 == NULL || arg1 == optline)
! post_opts = ;
! else
{
! *(arg1 - 1) = '\0'; /* this should be a
space */
! post_opts = arg1;
}
! if (postgres_path != NULL)
postgres_path = optline;
}
else
--- 613,628
{
char *arg1;
! /*
!* Are we at the first option, as defined by
space and
!* double-quote?
!*/
! if ((arg1 = strstr(optline, \)) != NULL)
{
! *arg1 = '\0'; /* terminate so we get
only program name */
! post_opts = arg1 + 1; /* point past
whitespace */
}
! if (postgres_path == NULL)
postgres_path = optline;
}
else
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Fix pg_ctl restart bug
Bruce Momjian wrote:
I am attaching a minimal patch that will fix the bug in back branches.
Keep in mind that a patched pg_ctl will not be able to restart a backend
that was not patched.
I think this patch will work for unpatched backends as well. I am still
uncertain if it should be backpatched.
--
Bruce Momjian [EMAIL PROTECTED]http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: src/backend/postmaster/postmaster.c
===
RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.551
diff -c -c -r1.551 postmaster.c
*** src/backend/postmaster/postmaster.c 11 Jan 2008 00:54:09 - 1.551
--- src/backend/postmaster/postmaster.c 26 Jun 2008 19:11:37 -
***
*** 4163,4169
fprintf(fp, %s, fullprogname);
for (i = 1; i argc; i++)
! fprintf(fp, %s%s%s, SYSTEMQUOTE, argv[i], SYSTEMQUOTE);
fputs(\n, fp);
if (fclose(fp))
--- 4163,4169
fprintf(fp, %s, fullprogname);
for (i = 1; i argc; i++)
! fprintf(fp, \%s\, argv[i]);
fputs(\n, fp);
if (fclose(fp))
Index: src/bin/pg_ctl/pg_ctl.c
===
RCS file: /cvsroot/pgsql/src/bin/pg_ctl/pg_ctl.c,v
retrieving revision 1.92.2.3
diff -c -c -r1.92.2.3 pg_ctl.c
*** src/bin/pg_ctl/pg_ctl.c 29 Feb 2008 23:31:42 - 1.92.2.3
--- src/bin/pg_ctl/pg_ctl.c 26 Jun 2008 19:11:37 -
***
*** 613,627
{
char *arg1;
! arg1 = strchr(optline, *SYSTEMQUOTE);
! if (arg1 == NULL || arg1 == optline)
! post_opts = ;
! else
{
! *(arg1 - 1) = '\0'; /* this should be a space */
! post_opts = arg1;
}
! if (postgres_path != NULL)
postgres_path = optline;
}
else
--- 613,629
{
char *arg1;
! /*
! * Are we at the first option, as defined by space and
! * double-quote?
! */
! if ((arg1 = strstr(optline, \)) != NULL ||
! (arg1 = strstr(optline, -)) != NULL)
{
! *arg1 = '\0'; /* terminate so we get only program name */
! post_opts = arg1 + 1; /* point past whitespace */
}
! if (postgres_path == NULL)
postgres_path = optline;
}
else
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Fix pg_ctl restart bug
Bruce Momjian wrote: Alvaro Herrera wrote: Why do we need someone to complain? We know the bug is there. Has the code changed a lot in that area? Do we have the policy of backpatching every fix? I thought it was only the major bugs we fixed in back branches. If someone wants to backpatch it, feel free to do so. I think the policy is we fix the bugs in supported releases. If you start making exceptions, it becomes needlessly complex. I've always assumed that I'm supposed to backpatch the bugs I fix in HEAD, however far is reasonable. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Fix pg_ctl restart bug
Alvaro Herrera wrote: Bruce Momjian wrote: Alvaro Herrera wrote: Why do we need someone to complain? We know the bug is there. Has the code changed a lot in that area? Do we have the policy of backpatching every fix? I thought it was only the major bugs we fixed in back branches. If someone wants to backpatch it, feel free to do so. I think the policy is we fix the bugs in supported releases. If you start making exceptions, it becomes needlessly complex. I've always assumed that I'm supposed to backpatch the bugs I fix in HEAD, however far is reasonable. I thought we only backatched major bugs to prevent possible instability when fixing minor bugs. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Fix pg_ctl restart bug
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Alvaro Herrera wrote: I've always assumed that I'm supposed to backpatch the bugs I fix in HEAD, however far is reasonable. I thought we only backatched major bugs to prevent possible instability when fixing minor bugs. Actually, Bruce, this *is* a minor bug; if it were major we'd have heard about it from the field. My take on it is that pg_ctl restart must be practically unused. Given that we now know it's completely broken, the only way that patching it could make the situation worse would be if the patch affected some other code path that people actually do use. As long as you're sure it doesn't do that, I see no downside to an attempted fix, even if it fails. OK, done; backpatched from 8.0.X to 8.3.X. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] Fix pg_ctl restart bug
We document 'pg_ctl restart' to preserve any command-line arguments
passed when the server was started:
Restarting the server is almost equivalent to stopping the
server and starting it again except that commandpg_ctl/command
saves and reuses the command line options that were passed
to the previously running instance. To restart
However, as of 2004-10-15, this has not worked. :-( The attached patch
is the one that caused the bug --- on non-Unix systems, SYSTEMQUOTE is
, meaning zero-length string. I should have seen the bug when I
applied the contributed patch in 2004.
The second attached applied patch fixes the problem.
--
Bruce Momjian [EMAIL PROTECTED]http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: src/backend/postmaster/postmaster.c
===
RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.433
retrieving revision 1.434
diff -c -r1.433 -r1.434
*** src/backend/postmaster/postmaster.c 14 Oct 2004 20:23:45 - 1.433
--- src/backend/postmaster/postmaster.c 15 Oct 2004 04:54:31 - 1.434
***
*** 3301,3307
fprintf(fp, %s, fullprogname);
for (i = 1; i argc; i++)
! fprintf(fp, '%s', argv[i]);
fputs(\n, fp);
if (fclose(fp))
--- 3301,3307
fprintf(fp, %s, fullprogname);
for (i = 1; i argc; i++)
! fprintf(fp, %s%s%s, SYSTEMQUOTE, argv[i], SYSTEMQUOTE);
fputs(\n, fp);
if (fclose(fp))
Index: src/bin/pg_ctl/pg_ctl.c
===
RCS file: /cvsroot/pgsql/src/bin/pg_ctl/pg_ctl.c,v
retrieving revision 1.37
retrieving revision 1.38
diff -c -r1.37 -r1.38
*** src/bin/pg_ctl/pg_ctl.c 15 Oct 2004 04:32:14 - 1.37
--- src/bin/pg_ctl/pg_ctl.c 15 Oct 2004 04:54:33 - 1.38
***
*** 501,507
{
char *arg1;
! arg1 = strchr(optline, '\'');
if (arg1 == NULL || arg1 == optline)
post_opts = ;
else
--- 501,507
{
char *arg1;
! arg1 = strchr(optline, *SYSTEMQUOTE);
if (arg1 == NULL || arg1 == optline)
post_opts = ;
else
Index: src/backend/postmaster/postmaster.c
===
RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.560
diff -c -c -r1.560 postmaster.c
*** src/backend/postmaster/postmaster.c 26 Jun 2008 01:35:45 - 1.560
--- src/backend/postmaster/postmaster.c 26 Jun 2008 02:41:04 -
***
*** 4184,4190
fprintf(fp, %s, fullprogname);
for (i = 1; i argc; i++)
! fprintf(fp, SYSTEMQUOTE %s SYSTEMQUOTE, argv[i]);
fputs(\n, fp);
if (fclose(fp))
--- 4184,4190
fprintf(fp, %s, fullprogname);
for (i = 1; i argc; i++)
! fprintf(fp, \%s\, argv[i]);
fputs(\n, fp);
if (fclose(fp))
Index: src/bin/pg_ctl/pg_ctl.c
===
RCS file: /cvsroot/pgsql/src/bin/pg_ctl/pg_ctl.c,v
retrieving revision 1.100
diff -c -c -r1.100 pg_ctl.c
*** src/bin/pg_ctl/pg_ctl.c 26 Jun 2008 01:35:45 - 1.100
--- src/bin/pg_ctl/pg_ctl.c 26 Jun 2008 02:41:04 -
***
*** 573,583
{
if (post_opts == NULL)
{
- char **optlines;
-
post_opts = ; /* defatult */
if (ctl_command == RESTART_COMMAND)
{
optlines = readfile(postopts_file);
if (optlines == NULL)
{
--- 573,583
{
if (post_opts == NULL)
{
post_opts = ; /* defatult */
if (ctl_command == RESTART_COMMAND)
{
+ char **optlines;
+
optlines = readfile(postopts_file);
if (optlines == NULL)
{
***
*** 593,612
else
{
int len;
! char *optline = NULL;
char *arg1;
optline = optlines[0];
len = strcspn(optline, \r\n);
optline[len] = '\0';
! arg1 = strchr(optline, *SYSTEMQUOTE);
! if (arg1 == NULL || arg1 == optline)
! post_opts = ;
! else
{
! *(arg1 - 1) = '\0'; /* this should be a space */
! post_opts = arg1;
}
if (postgres_path != NULL)
postgres_path = optline;
--- 593,618
else
{
int len;
! char *optline;
char *arg1;
optline = optlines[0];
+ /* trim off line endings */
len = strcspn(optline, \r\n);
optline[len] = '\0';
! for (arg1 = optline; *arg1; arg1++)
{
! /*
! * Are we at the first option, as defined by space,
! * double-quote, and a dash?
! */
! if (*arg1 == ' ' *(arg1+1) == '' *(arg1+2) == '-')
! {
! *arg1 = '\0'; /* terminate so we get only program name */
! post_opts = arg1 + 1; /* point past whitespace */
! break;
! }
}
if (postgres_path != NULL)
postgres_path =
Re: [PATCHES] Fix pg_ctl restart bug
Bruce Momjian [EMAIL PROTECTED] writes: However, as of 2004-10-15, this has not worked. :-( The attached patch is the one that caused the bug --- on non-Unix systems, SYSTEMQUOTE is , meaning zero-length string. I should have seen the bug when I applied the contributed patch in 2004. So, shouldn't this fix be back-patched? regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Fix pg_ctl restart bug
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: However, as of 2004-10-15, this has not worked. :-( The attached patch is the one that caused the bug --- on non-Unix systems, SYSTEMQUOTE is , meaning zero-length string. I should have seen the bug when I applied the contributed patch in 2004. So, shouldn't this fix be back-patched? Well, no one has actually complained about the breakage, and it has been a few years. Also there is always the risk of a new bug being introduced, so I am unsure. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Verified fix for Bug 4137
Simon Riggs wrote:
if (restartWALFileName)
{
+ /*
+* Don't do cleanup if the restartWALFileName provided
+* is later than the xlog file requested. This is an error
+* and we must not remove these files from archive.
+* This shouldn't happen, but better safe than sorry.
+*/
+ if (strcmp(restartWALFileName, nextWALFileName) 0)
+ return false;
+
strcpy(exclusiveCleanupFileName, restartWALFileName);
return true;
}
I committed this sanity check into pg_standy, though it really shouldn't
happen, but it just occurred to me that the most likely reason for that
to happen is probably that the user has screwed up his restore_command
line, mixing up the %p and %r arguments. Should we make that an error
instead of just not doing the cleanup?
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Verified fix for Bug 4137
On Fri, 2008-05-09 at 15:37 +0100, Heikki Linnakangas wrote:
Simon Riggs wrote:
if (restartWALFileName)
{
+ /*
+* Don't do cleanup if the restartWALFileName provided
+* is later than the xlog file requested. This is an error
+* and we must not remove these files from archive.
+* This shouldn't happen, but better safe than sorry.
+*/
+ if (strcmp(restartWALFileName, nextWALFileName) 0)
+ return false;
+
strcpy(exclusiveCleanupFileName, restartWALFileName);
return true;
}
I committed this sanity check into pg_standy, though it really shouldn't
happen, but it just occurred to me that the most likely reason for that
to happen is probably that the user has screwed up his restore_command
line, mixing up the %p and %r arguments. Should we make that an error
instead of just not doing the cleanup?
You can't explicitly throw a pgsql error in pg_standby, so the best we
can do is get the file requested if it exists. If the file is the wrong
one then recovery will throw the error. As long as we didn't delete
anything when that happens we can just correct the mistake and retry.
--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Verified fix for Bug 4137
Heikki Linnakangas [EMAIL PROTECTED] writes: Ok, that'll work. Committed, thanks. I changed the sanity check that xlogfname restore point into an Assert, though, because that's a sign that something's wrong. Shouldn't that Assert allow the equality case? regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Verified fix for Bug 4137
Tom Lane wrote: Heikki Linnakangas [EMAIL PROTECTED] writes: Ok, that'll work. Committed, thanks. I changed the sanity check that xlogfname restore point into an Assert, though, because that's a sign that something's wrong. Shouldn't that Assert allow the equality case? Yes. Thanks. Hmm. I can't actually think of a scenario where that would happen, but it was definitely an oversight on my part. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] Verified fix for Bug 4137
The following patch has been independently verified as fixing bug 4137.
The problem was that at the very start of archive recovery the %r
parameter in restore_command could be set to a filename later than the
currently requested filename (%f). This could lead to early truncation
of the archived WAL files and would cause warm standby replication to
fail soon afterwards, in certain specific circumstances.
Fix applied to both core server in generating correct %r filenames and
also to pg_standby to prevent acceptance of out-of-sequence filenames.
Request review and commit. No port specific details.
--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com
Index: contrib/pg_standby/pg_standby.c
===
RCS file: /home/sriggs/pg/REPOSITORY/pgsql/contrib/pg_standby/pg_standby.c,v
retrieving revision 1.10
diff -c -r1.10 pg_standby.c
*** contrib/pg_standby/pg_standby.c 15 Nov 2007 21:14:30 - 1.10
--- contrib/pg_standby/pg_standby.c 3 May 2008 11:27:12 -
***
*** 297,302
--- 297,311
if (restartWALFileName)
{
+ /*
+ * Don't do cleanup if the restartWALFileName provided
+ * is later than the xlog file requested. This is an error
+ * and we must not remove these files from archive.
+ * This shouldn't happen, but better safe than sorry.
+ */
+ if (strcmp(restartWALFileName, nextWALFileName) 0)
+ return false;
+
strcpy(exclusiveCleanupFileName, restartWALFileName);
return true;
}
***
*** 584,590
fprintf(stderr, \nMax wait interval : %d %s,
maxwaittime, (maxwaittime 0 ? seconds : forever));
fprintf(stderr, \nCommand for restore : %s, restoreCommand);
! fprintf(stderr, \nKeep archive history : %s and later, exclusiveCleanupFileName);
fflush(stderr);
}
--- 593,603
fprintf(stderr, \nMax wait interval : %d %s,
maxwaittime, (maxwaittime 0 ? seconds : forever));
fprintf(stderr, \nCommand for restore : %s, restoreCommand);
! fprintf(stderr, \nKeep archive history : );
! if (need_cleanup)
! fprintf(stderr, %s and later, exclusiveCleanupFileName);
! else
! fprintf(stderr, No cleanup required);
fflush(stderr);
}
Index: src/backend/access/transam/xlog.c
===
RCS file: /home/sriggs/pg/REPOSITORY/pgsql/src/backend/access/transam/xlog.c,v
retrieving revision 1.300
diff -c -r1.300 xlog.c
*** src/backend/access/transam/xlog.c 24 Apr 2008 14:23:43 - 1.300
--- src/backend/access/transam/xlog.c 3 May 2008 10:58:08 -
***
*** 2484,2489
--- 2484,2509
}
/*
+ * Calculate the archive file cutoff point. All files
+ * before this file can be deleted from the archive.
+ * The filename is not inclusive, so you must not remove
+ * this filename from the archive; it will definitely be
+ * required. Most of the time this will be the last
+ * restartpoint, but at the start of archive recovery it
+ * will be the file containing the checkpoint written by
+ * pg_start_backup(). We do the calculation now so that
+ * %r is always less than or equal to %f before we start
+ * to construct the command to be executed
+ */
+ XLByteToSeg(ControlFile-checkPointCopy.redo,
+ restartLog, restartSeg);
+ XLogFileName(lastRestartPointFname,
+ ControlFile-checkPointCopy.ThisTimeLineID,
+ restartLog, restartSeg);
+ if (strcmp(lastRestartPointFname, xlogfname) 0)
+ strcpy(lastRestartPointFname, xlogfname);
+
+ /*
* construct the command to be executed
*/
dp = xlogRestoreCmd;
***
*** 2512,2522
case 'r':
/* %r: filename of last restartpoint */
sp++;
- XLByteToSeg(ControlFile-checkPointCopy.redo,
- restartLog, restartSeg);
- XLogFileName(lastRestartPointFname,
- ControlFile-checkPointCopy.ThisTimeLineID,
- restartLog, restartSeg);
StrNCpy(dp, lastRestartPointFname, endp - dp);
dp += strlen(dp);
break;
--- 2532,2537
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Verified fix for Bug 4137
Simon Riggs wrote: The problem was that at the very start of archive recovery the %r parameter in restore_command could be set to a filename later than the currently requested filename (%f). This could lead to early truncation of the archived WAL files and would cause warm standby replication to fail soon afterwards, in certain specific circumstances. Fix applied to both core server in generating correct %r filenames and also to pg_standby to prevent acceptance of out-of-sequence filenames. So the core problem is that we use ControlFile-checkPointCopy.redo in RestoreArchivedFile to determine the safe truncation point, but when there's a backup label file, that's still coming from pg_control file, which is wrong. The patch fixes that by determining the safe truncation point as Min(checkPointCopy.redo, xlogfname), where xlogfname is the xlog file being restored. That depends on the assumption that everything before the first file we (try to) restore is safe to truncate. IOW, we never try to restore file B first, and then A, where A B. I'm not totally convinced that's a safe assumption. As an example, consider doing an archive recovery, but without a backup label, and the latest checkpoint record is broken. We would try to read the latest (broken) checkpoint record first, and call RestoreArchivedFile to get the xlog file containing that. But because that record is broken, we fall back to using the previous checkpoint, and will need the xlog file where the previous checkpoint record is in. That's a pretty contrived example, but the point is that assumption feels fragile. At the very least it should be noted explicitly in the comments. A less fragile approach would be to use something dummy, like as the truncation point until we've successfully read the checkpoint/restartpoint record and started the replay. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Verified fix for Bug 4137
On Tue, 2008-05-06 at 12:02 +0100, Heikki Linnakangas wrote: Simon Riggs wrote: The problem was that at the very start of archive recovery the %r parameter in restore_command could be set to a filename later than the currently requested filename (%f). This could lead to early truncation of the archived WAL files and would cause warm standby replication to fail soon afterwards, in certain specific circumstances. Fix applied to both core server in generating correct %r filenames and also to pg_standby to prevent acceptance of out-of-sequence filenames. So the core problem is that we use ControlFile-checkPointCopy.redo in RestoreArchivedFile to determine the safe truncation point, but when there's a backup label file, that's still coming from pg_control file, which is wrong. The patch fixes that by determining the safe truncation point as Min(checkPointCopy.redo, xlogfname), where xlogfname is the xlog file being restored. That depends on the assumption that everything before the first file we (try to) restore is safe to truncate. IOW, we never try to restore file B first, and then A, where A B. I'm not totally convinced that's a safe assumption. As an example, consider doing an archive recovery, but without a backup label, and the latest checkpoint record is broken. We would try to read the latest (broken) checkpoint record first, and call RestoreArchivedFile to get the xlog file containing that. But because that record is broken, we fall back to using the previous checkpoint, and will need the xlog file where the previous checkpoint record is in. That's a pretty contrived example, but the point is that assumption feels fragile. At the very least it should be noted explicitly in the comments. A less fragile approach would be to use something dummy, like as the truncation point until we've successfully read the checkpoint/restartpoint record and started the replay. Your comments are interesting and I'll think through that some more to see if I can see if that leads to bugs. Will talk more later. The only valid starting place, in all cases, is the checkpoint written by pg_start_backup(). The user doesn't need to have been archiving files prior to that so could legitimately have had a null archive_command. -- Simon Riggs 2ndQuadrant http://www.2ndQuadrant.com -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Verified fix for Bug 4137
Simon Riggs wrote: Falling back to the secondary checkpoint implies we have a corrupted or absent WAL file, so making recovery startup work correctly won't avoid the need to re-run the base backup. We'll end with an unrecoverable error in either case, so it doesn't seem worth attempting to improve this in the way you suggest. That's true whenever you have to fall back to a secondary checkpoint, but we still try to get the database up. One could argue that we shouldn't, of course. Anyway, the point is that the patch relies on a non-obvious assumption. Even if the secondary checkpoint issue is a non-issue, it's not obvious (to me at least) that there isn't other similar scenarios. And someone might inadvertently break the assumption in a future patch, because it's not an obvious one; calling ReadRecord looks very innocent. We shouldn't introduce an assumption like that when we don't have to. I think we should completely prevent access to secondary checkpoints during archive recovery, because if the primary checkpoint isn't present or is corrupt we aren't ever going to get passed it to get to the pg_stop_backup() point. Hence an archive recovery can never be valid in that case. I'll do a separate patch for that because they are unrelated issues. Well, we already don't use the secondary checkpoint if a backup label file is present. And you can take a base backup without pg_start_backup()/pg_stop_backup() if you shut down the system first (a cold backup). -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Verified fix for Bug 4137
On Tue, 2008-05-06 at 17:52 +0100, Heikki Linnakangas wrote: Simon Riggs wrote: We were already assuming archive files were OK to delete, if before. The whole of recovery already relies heavily on the alphabetic sorting property of WAL and associated filenames. Those filenames have been specifically documented as maintaining that sorted order for that reason. If somebody wanted to recover files in non-sorted order, then yes I would expect a few things to break - this aspect wouldn't be the most critical thing though. I didn't suggest that alphabetical sorting property is a new assumption; it sure isn't. The new assumption is that you never call ReadRecord() for record 0002 before you call it for record 0001 (before initializing the checkPointCopy field from the checkpoint record, anyway). I've not done anything with the ordering of calls to ReadRecord(). There is no such assumption introduced here. The patch prevents an erroneous call to delete files. The earlier code just assumed such a call would never occur, which was wrong, hence the patch. There are no new assumptions introduced into the code by this. I very much respect your opinion in all things, most especially on matters of code, more so now you are a committer. I would be willing to make changes to any patch on your say-so, though I can't see how any of your comments improve this patch in this specific case only. I don't doubt that you will find flaws in many of my future patches. I can imagine a future patch to do xlog file prefetching, for example, that breaks that assumption. Maybe, but I think its for such a patch to consider in full the consequences of doing that, not for me to do so in advance. The current assumption is filename ordered recovery, there is definitely more than one part of the code that relies significantly on that point. I think any prefetcher would be advised to stay within one file at a time. Anything else will effect other behaviours in ways that would need careful planning to avoid unintended consequences. Or falling back to the previous checkpoint as discussed. Or maybe you screwed up your recovery.conf, and try to use WAL files that belong to a different installation. The database won't start up, of course, because the checkpoint record isn't in the right place, but the damage has already been done because the recovery command deleted some files it shouldn't have. Granted, none of those are particularly likely, but we should be extra careful when deleting files. With respect, that is exactly what the patch does, though it is certainly better for being tested by your comments. -- Simon Riggs 2ndQuadrant http://www.2ndQuadrant.com -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Verified fix for Bug 4137
Simon Riggs wrote: On Tue, 2008-05-06 at 17:52 +0100, Heikki Linnakangas wrote: I didn't suggest that alphabetical sorting property is a new assumption; it sure isn't. The new assumption is that you never call ReadRecord() for record 0002 before you call it for record 0001 (before initializing the checkPointCopy field from the checkpoint record, anyway). I've not done anything with the ordering of calls to ReadRecord(). There is no such assumption introduced here. Hmm, I see that I was inaccurate when I said the patch introduces that assumption, sorry about the confusion. It's more like the code is currently completely oblivious of the issue, and the patch makes it better but doesn't fully fix it. The code in CVS is broken, as we now know, because it assumes that we can delete all files checkPointCopy.redo, which is not true when checkPointCopy.redo has not yet been initialized from the backup label file, and points to a location that's ahead of the real safe truncation point. The patch makes it better, and the assumption is now that it's safe to truncate everything min(checkPointCopy.redo, xlog file we're reading). That still seems too fragile to me, because we assume that after you've asked for xlog record X, we never need anything earlier then that. In fact, what will happen if the checkpoint record's redo pointer points to an earlier xlog file: 1. The location of the checkpoint record is read by read_backup_label(). Let's say that it's 0005. 2. ReadCheckpointRecord() is called for 0005. The restore command is called because that xlog file is not present. The safe truncation point is determined to be 0005, as that's what we're reading. Everything before that is truncated 3. The redo pointer in the checkpoint record points to 0003. That's where we should start the recovery. Oops :-( I haven't tested this, so, am I missing something that makes that not happen? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Verified fix for Bug 4137
On Tue, 2008-05-06 at 21:51 +0100, Heikki Linnakangas wrote:
In fact, what will happen if the checkpoint record's redo pointer points
to an earlier xlog file:
1. The location of the checkpoint record is read by read_backup_label().
Let's say that it's 0005.
2. ReadCheckpointRecord() is called for 0005. The restore command is
called because that xlog file is not present. The safe truncation point
is determined to be 0005, as that's what we're reading. Everything
before that is truncated
3. The redo pointer in the checkpoint record points to 0003. That's
where we should start the recovery. Oops :-(
Yes, this case could be a problem, if the records are in different
files. It's the files that matter, not the records themselves though.
I've extended the patch without introducing another new status variable,
which was my original concern with what you suggested previously.
--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com
Index: contrib/pg_standby/pg_standby.c
===
RCS file: /home/sriggs/pg/REPOSITORY/pgsql/contrib/pg_standby/pg_standby.c,v
retrieving revision 1.10
diff -c -r1.10 pg_standby.c
*** contrib/pg_standby/pg_standby.c 15 Nov 2007 21:14:30 - 1.10
--- contrib/pg_standby/pg_standby.c 3 May 2008 11:27:12 -
***
*** 297,302
--- 297,311
if (restartWALFileName)
{
+ /*
+ * Don't do cleanup if the restartWALFileName provided
+ * is later than the xlog file requested. This is an error
+ * and we must not remove these files from archive.
+ * This shouldn't happen, but better safe than sorry.
+ */
+ if (strcmp(restartWALFileName, nextWALFileName) 0)
+ return false;
+
strcpy(exclusiveCleanupFileName, restartWALFileName);
return true;
}
***
*** 584,590
fprintf(stderr, \nMax wait interval : %d %s,
maxwaittime, (maxwaittime 0 ? seconds : forever));
fprintf(stderr, \nCommand for restore : %s, restoreCommand);
! fprintf(stderr, \nKeep archive history : %s and later, exclusiveCleanupFileName);
fflush(stderr);
}
--- 593,603
fprintf(stderr, \nMax wait interval : %d %s,
maxwaittime, (maxwaittime 0 ? seconds : forever));
fprintf(stderr, \nCommand for restore : %s, restoreCommand);
! fprintf(stderr, \nKeep archive history : );
! if (need_cleanup)
! fprintf(stderr, %s and later, exclusiveCleanupFileName);
! else
! fprintf(stderr, No cleanup required);
fflush(stderr);
}
Index: src/backend/access/transam/xlog.c
===
RCS file: /home/sriggs/pg/REPOSITORY/pgsql/src/backend/access/transam/xlog.c,v
retrieving revision 1.300
diff -c -r1.300 xlog.c
*** src/backend/access/transam/xlog.c 24 Apr 2008 14:23:43 - 1.300
--- src/backend/access/transam/xlog.c 6 May 2008 23:04:30 -
***
*** 2484,2489
--- 2484,2522
}
/*
+ * Calculate the archive file cutoff point for use during log shipping
+ * replication. All files earlier than this point can be deleted
+ * from the archive, though there is no requirement to do so.
+ *
+ * We initialise this with the filename of an InvalidXLogRecPtr, which
+ * will prevent the deletion of any WAL files from the archive
+ * because of the alphabetic sorting property of WAL filenames.
+ *
+ * Once we have successfully located the redo pointer of the checkpoint
+ * from which we start recovery we never request a file prior to the redo
+ * pointer of the last restartpoint. When redo begins we know that we
+ * have successfully located it, so there is no need for additional
+ * status flags to signify the point when we can begin deleting WAL files
+ * from the archive.
+ *
+ * We do the calculation now so that %r is always equal to or earlier
+ * than %f before we start to construct the command to be executed, as
+ * an additional cross-check on the sanity of our cutoff point.
+ */
+ if (InRedo)
+ {
+ XLByteToSeg(ControlFile-checkPointCopy.redo,
+ restartLog, restartSeg);
+ XLogFileName(lastRestartPointFname,
+ ControlFile-checkPointCopy.ThisTimeLineID,
+ restartLog, restartSeg);
+ if (strcmp(lastRestartPointFname, xlogfname) 0)
+ strcpy(lastRestartPointFname, xlogfname);
+ }
+ else
+ XLogFileName(lastRestartPointFname, 0, 0, 0);
+
+ /*
* construct the command to be executed
*/
dp = xlogRestoreCmd;
***
*** 2512,2522
case 'r':
/* %r: filename of last restartpoint */
sp++;
- XLByteToSeg(ControlFile-checkPointCopy.redo,
- restartLog, restartSeg);
- XLogFileName(lastRestartPointFname,
- ControlFile-checkPointCopy.ThisTimeLineID,
- restartLog, restartSeg);
StrNCpy(dp, lastRestartPointFname, endp - dp);
dp += strlen(dp);
break;
--- 2545,2550
--
Sent via pgsql-patches mailing list
Re: [PATCHES] Proposed patch for bug #3921
Tom Lane [EMAIL PROTECTED] writes: An issue that this patch doesn't address is what happens if the source index is in a non-default tablespace that the current user does not have CREATE permission for. With both current CVS HEAD and this patch, that will result in an error. Is that okay? I was going to say we could add a hint suggesting how to specify the tablespace but as you pointed out earlier there really isn't a way to specify the tablespace. Hm, looking at the grammar we already have something like this for constraints. We could add an OptConsTableSpace after INCLUDING INDEXES. I just tested it and it didn't cause any conflicts which makes sense since like options appear in the column list. So the syntax would be CREATE TABLE foo (..., LIKE bar INCLUDING INDEXES USING INDEX TABLESPACE foo_ts, ...) Kind of verbose but nice that it's the same syntax as constraints. Not sure how easy it would be to shoehorn into t he like processing, I could look at that if you want. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] Proposed patch for bug #3921
Gregory Stark [EMAIL PROTECTED] writes: So the syntax would be CREATE TABLE foo (..., LIKE bar INCLUDING INDEXES USING INDEX TABLESPACE foo_ts, ...) This (presumably) forces all the indexes into the same tablespace, so I don't find it to be a complete solution, just a wart. We could get the same behavior with much less code if we redefined LIKE to not try to copy the source indexes' tablespace(s). Then, the user would set default_tablespace to get the effect of the USING clause. That would also make LIKE entirely free of tablespace permissions hazards, so I'm starting to think more and more that that's really the right definition. regards, tom lane ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
[PATCHES] Proposed patch for bug #3921
Attached is a proposed patch for bug #3921, which complained that CREATE
TABLE LIKE INCLUDING INDEXES fails inappropriately for non-superusers.
There are two parts to the patch: the first follows Greg Starks' opinion
that explicitly specifying the current database's default tablespace
shouldn't be an error at all, and so the permissions checks during
table/index creation are suppressed when tablespaceId ==
MyDatabaseTableSpace. (Note that the assign hooks for
default_tablespace and temp_tablespaces already behaved this way, so we
were not exactly being consistent anyhow.) I couldn't find anyplace in
the documentation that specifies the old behavior, so no docs changes.
The second part fixes the LIKE INCLUDING INDEXES code to default the
index tablespace specification when the source index is in the
database's default tablespace. This would provide an alternative
way of fixing the bug if anyone doesn't like the first part. With the
first part it's redundant, but seems worth doing anyway to avoid the
overhead of looking up the tablespace name and then converting it back
to OID in the typical case.
Also there is a correction of an obsolete comment in parsenodes.h, which
should be applied in any case.
An issue that this patch doesn't address is what happens if the source
index is in a non-default tablespace that the current user does not have
CREATE permission for. With both current CVS HEAD and this patch, that
will result in an error. Is that okay? We could imagine making
parse_utilcmd.c check the permissions and default the tablespace
specification if no good, but that behavior seems a bit peculiar to me.
Command semantics don't normally change based on whether you have
permissions or not.
An entirely different tack we could take is to adopt the position
that LIKE INCLUDING INDEXES shouldn't copy index tablespaces at all,
but I didn't hear anyone favoring that approach in the bug discussion.
regards, tom lane
Index: src/backend/commands/indexcmds.c
===
RCS file: /cvsroot/pgsql/src/backend/commands/indexcmds.c,v
retrieving revision 1.170
diff -c -r1.170 indexcmds.c
*** src/backend/commands/indexcmds.c9 Jan 2008 21:52:36 - 1.170
--- src/backend/commands/indexcmds.c3 Feb 2008 02:32:14 -
***
*** 215,221
}
/* Check permissions except when using database's default */
! if (OidIsValid(tablespaceId))
{
AclResult aclresult;
--- 215,221
}
/* Check permissions except when using database's default */
! if (OidIsValid(tablespaceId) tablespaceId != MyDatabaseTableSpace)
{
AclResult aclresult;
Index: src/backend/commands/tablecmds.c
===
RCS file: /cvsroot/pgsql/src/backend/commands/tablecmds.c,v
retrieving revision 1.241
diff -c -r1.241 tablecmds.c
*** src/backend/commands/tablecmds.c30 Jan 2008 19:46:48 - 1.241
--- src/backend/commands/tablecmds.c3 Feb 2008 02:32:15 -
***
*** 340,346
}
/* Check permissions except when using database's default */
! if (OidIsValid(tablespaceId))
{
AclResult aclresult;
--- 340,346
}
/* Check permissions except when using database's default */
! if (OidIsValid(tablespaceId) tablespaceId != MyDatabaseTableSpace)
{
AclResult aclresult;
Index: src/backend/executor/execMain.c
===
RCS file: /cvsroot/pgsql/src/backend/executor/execMain.c,v
retrieving revision 1.302
diff -c -r1.302 execMain.c
*** src/backend/executor/execMain.c 1 Jan 2008 19:45:49 - 1.302
--- src/backend/executor/execMain.c 3 Feb 2008 02:32:15 -
***
*** 2594,2600
}
/* Check permissions except when using the database's default space */
! if (OidIsValid(tablespaceId))
{
AclResult aclresult;
--- 2594,2600
}
/* Check permissions except when using the database's default space */
! if (OidIsValid(tablespaceId) tablespaceId != MyDatabaseTableSpace)
{
AclResult aclresult;
Index: src/backend/parser/parse_utilcmd.c
===
RCS file: /cvsroot/pgsql/src/backend/parser/parse_utilcmd.c,v
retrieving revision 2.8
diff -c -r2.8 parse_utilcmd.c
*** src/backend/parser/parse_utilcmd.c 1 Jan 2008 19:45:51 - 2.8
--- src/backend/parser/parse_utilcmd.c 3 Feb 2008 02:32:15 -
***
*** 767,773
index = makeNode(IndexStmt);
index-relation = cxt-relation;
index-accessMethod = pstrdup(NameStr(amrec-amname));
! index-tableSpace =
Re: [PATCHES] [HACKERS] Thoughts about bug #3883
I wrote:
No, the problem is merely to get LockWaitCancel to return true so that
StatementCancelHandler will go ahead with the immediate interrupt. No
new cleanup is needed other than resetting the new flag.
Actually there's a better way to do it: we can remove a little bit of
complexity instead of adding more. The basic problem is that
StatementCancelHandler wants to tell the difference between waiting for
client input (which there is no use for it to interrupt) and other
states in which ImmediateInterruptOK is set. ProcWaitForSignal() is
falling on the wrong side of the classification --- and I argue that any
other newly added interruptable wait would too. We should reverse the
sense of the test so that it's not in client input instead of in lock
wait. At the time that code was written, there wasn't any conveniently
cheap way to check for client input state, so we kluged up
LockWaitCancel to detect the other case. But now that we have the
DoingCommandRead flag, it's easy to check that. This lets us remove
LockWaitCancel's return value (which was always a bit untidy, since all
but one of its callers ignored the result), ending up with exactly
parallel code in die() and StatementCancelHandler(). Seems cleaner than
before.
regards, tom lane
Index: src/backend/port/posix_sema.c
===
RCS file: /cvsroot/pgsql/src/backend/port/posix_sema.c,v
retrieving revision 1.19
diff -c -r1.19 posix_sema.c
*** src/backend/port/posix_sema.c 1 Jan 2008 19:45:51 - 1.19
--- src/backend/port/posix_sema.c 25 Jan 2008 22:41:22 -
***
*** 241,277
int errStatus;
/*
!* Note: if errStatus is -1 and errno == EINTR then it means we returned
!* from the operation prematurely because we were sent a signal. So we
!* try and lock the semaphore again.
!*
!* Each time around the loop, we check for a cancel/die interrupt. We
!* assume that if such an interrupt comes in while we are waiting, it
will
!* cause the sem_wait() call to exit with errno == EINTR, so that we
will
!* be able to service the interrupt (if not in a critical section
!* already).
!*
!* Once we acquire the lock, we do NOT check for an interrupt before
!* returning. The caller needs to be able to record ownership of the
lock
!* before any interrupt can be accepted.
!*
!* There is a window of a few instructions between CHECK_FOR_INTERRUPTS
!* and entering the sem_wait() call. If a cancel/die interrupt occurs
in
!* that window, we would fail to notice it until after we acquire the
lock
!* (or get another interrupt to escape the sem_wait()). We can avoid
this
!* problem by temporarily setting ImmediateInterruptOK to true before we
!* do CHECK_FOR_INTERRUPTS; then, a die() interrupt in this interval
will
!* execute directly. However, there is a huge pitfall: there is another
!* window of a few instructions after the sem_wait() before we are able
to
!* reset ImmediateInterruptOK. If an interrupt occurs then, we'll lose
!* control, which means that the lock has been acquired but our caller
did
!* not get a chance to record the fact. Therefore, we only set
!* ImmediateInterruptOK if the caller tells us it's OK to do so, ie, the
!* caller does not need to record acquiring the lock. (This is
currently
!* true for lockmanager locks, since the process that granted us the
lock
!* did all the necessary state updates. It's not true for Posix
semaphores
!* used to implement LW locks or emulate spinlocks --- but the wait time
!* for such locks should not be very long, anyway.)
*/
do
{
--- 241,250
int errStatus;
/*
!* See notes in sysv_sema.c's implementation of PGSemaphoreLock.
!* Just as that code does for semop(), we handle both the case where
!* sem_wait() returns errno == EINTR after a signal, and the case
!* where it just keeps waiting.
*/
do
{
Index: src/backend/port/sysv_sema.c
===
RCS file: /cvsroot/pgsql/src/backend/port/sysv_sema.c,v
retrieving revision 1.22
diff -c -r1.22 sysv_sema.c
*** src/backend/port/sysv_sema.c1 Jan 2008 19:45:51 - 1.22
--- src/backend/port/sysv_sema.c25 Jan 2008 22:41:22 -
***
*** 377,386
* from the operation prematurely because we were sent a signal. So we
* try and lock the semaphore again.
*
!* Each time around the loop, we check for a cancel/die interrupt. We
!* assume that if such an interrupt comes in while we are waiting, it
will
!
[PATCHES] Doc patch for Bug 3877
After reviewing the docs it didn't seem appropriate to put examples in all places. As a result of the review I've added a few other minor points to an earlier section that clearly hasn't been touched in a long time. -- Simon Riggs 2ndQuadrant http://www.2ndQuadrant.com Index: doc/src/sgml/backup.sgml === RCS file: /home/sriggs/pg/REPOSITORY/pgsql/doc/src/sgml/backup.sgml,v retrieving revision 2.112 diff -c -r2.112 backup.sgml *** doc/src/sgml/backup.sgml 17 Dec 2007 09:03:52 - 2.112 --- doc/src/sgml/backup.sgml 17 Jan 2008 17:59:06 - *** *** 323,330 para There are two restrictions, however, which make this method !impractical, or at least inferior to the applicationpg_dump/ !method: orderedlist listitem --- 323,329 para There are two restrictions, however, which make this method !impractical, or at least inferior to other methods. orderedlist listitem *** *** 361,366 --- 360,370 /orderedlist /para + para Continuous archiving provides a mechanism that will +allow you to make a file system backup while the database server +is running. See xref linkend=continuous-archiving. + /para + para An alternative file-system backup approach is to make a quoteconsistent snapshot/quote of the data directory, if the *** *** 407,413 smaller than an SQL dump. On the contrary, it will most likely be larger. (applicationpg_dump/application does not need to dump the contents of indexes for example, just the commands to recreate !them.) /para /sect1 --- 411,417 smaller than an SQL dump. On the contrary, it will most likely be larger. (applicationpg_dump/application does not need to dump the contents of indexes for example, just the commands to recreate !them.) However, taking a file system backup may often be faster. /para /sect1 *** *** 554,562 programlisting archive_command = 'cp -i %p /mnt/server/archivedir/%f lt;/dev/null' /programlisting ! which will copy archivable WAL segments to the directory ! filename/mnt/server/archivedir/. (This is an example, not a ! recommendation, and might not work on all platforms.) /para para --- 558,575 programlisting archive_command = 'cp -i %p /mnt/server/archivedir/%f lt;/dev/null' /programlisting ! Once the parameters have been replaced, the actual command executed ! might look something like the following: ! programlisting ! The above example might be expanded to ! cp -i /var/lib/postgresql/8.2/main/pgdata/pg_xlog/000100A90065 ! /mnt/server/archivedir/000100A90065 /dev/null ! /programlisting ! A similar command is re-generated for each new file to be archived. ! The above command will copy archivable WAL segments directly from the ! data directory into the directory filename/mnt/server/archivedir/. ! (This is an example, not a recommendation, and might not work on all ! platforms.) /para para ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] patch for ECPG (BUG #2956: ECPG does not treat multibyte characters correctly.)
Michael Meskes [EMAIL PROTECTED] wrote: I found bug in ecpg concerning processing of the multi-byte character-code. I reported as bug#2956 before. I'm just committing the changes to CVS but only to HEAD because I cannot check if my changes broke something. The sources work fine on my system and the regression tests pass without a problem. But then I do not have a setup similar to yours. Could you please test this? I tested the change and it worked fine, but I found that this fix should be backported -- it might cause SQL injections depending on the server configurations. The attached patches are backports for the past releases. I hope you will apply them. Thanks. [TEST] 1. initdb --no-locale --encoding=UTF8 2. SET client_encoding = sjis in postgresql.conf 3. ecpg test.pgc 4. gcc test.c 5. test src.sjis.txt [RESULTS] The first charactor is a Japanese kanji. (0x95+0x5c) -- 8.3dev 表'; SELECT ;-- -- 8.2.3 : backslash_quote = safe_encoding sql error 'unsafe use of \' in a string literal' in line 21. -- 8.2.3 : backslash_quote = on (SQL injection!) -- 8.2.3 with patch : backslash_quote = safe_encoding 表'; SELECT ;-- Regards, --- ITAGAKI Takahiro NTT Open Source Software Center test.pgc Description: Binary data src.sjis.txt Description: Binary data ecpg-quote_8.0.11-7.4.10.diff Description: Binary data ecpg-quote_8.1.7.diff Description: Binary data ecpg-quote_8.2.3.diff Description: Binary data ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] patch for ECPG (BUG #2956: ECPG does not treat multibyte characters correctly.)
On Tue, Feb 27, 2007 at 08:51:36PM +0900, ITAGAKI Takahiro wrote: I tested the change and it worked fine, but I found that this fix Good to hear. should be backported -- it might cause SQL injections depending on I absolutely agree, that's why I just committed your patches. :-) Thanks for your effort. Michael -- Michael Meskes Email: Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: [EMAIL PROTECTED] Go SF 49ers! Go Rhein Fire! Use Debian GNU/Linux! Use PostgreSQL! ---(end of broadcast)--- TIP 1: 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] patch for ECPG (BUG #2956: ECPG does not treat multibyte characters correctly.)
On Fri, Feb 09, 2007 at 05:29:21PM +0900, 原田登志 wrote: I found bug in ecpg concerning processing of the multi-byte character-code. I reported as bug#2956 before. Thanks for report and patch. Attached is the patch to fix this problem. This patch is against 8.1, right? I had to apply it manually to HEAD, so I might have made some error here. Also I don't see why we have to add the connection to all thos function calls, just to make sure we get an error message logged in the connection struct that apparently isn't evaluated anyway. Please correct me if I'm wrong on this one. I'm just committing the changes to CVS but only to HEAD because I cannot check if my changes broke something. The sources work fine on my system and the regression tests pass without a problem. But then I do not have a setup similar to yours. Could you please test this? Michael -- Michael Meskes Email: Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: [EMAIL PROTECTED] Go SF 49ers! Go Rhein Fire! Use Debian GNU/Linux! Use PostgreSQL! ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [pgsql-patches] [HACKERS] Fix for bug in plpython bool type conversion
I have had to reverse out this patch because Py_RETURN_TRUE is only
supported in Python versions = 2.3, and we support older versions. I
did add a comment:
* We would like to use Py_RETURN_TRUE and Py_RETURN_FALSE here for
* generating SQL from trigger functions, but those are only
* supported in Python = 2.3, and we support older
* versions. http://docs.python.org/api/boolObjects.html
---
bruce wrote:
Your patch has been added to the PostgreSQL unapplied patches list at:
http://momjian.postgresql.org/cgi-bin/pgpatches
It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.
---
Guido Goldstein wrote:
Hi!
The attached patch fixes a bug in plpython.
This bug was found while creating sql from trigger functions
written in plpython and later running the generated sql.
The problem was that boolean was was silently converted to
integer, which is ok for python but fails when the created
sql is used.
The patch uses the Py_RETURN_xxx macros shown at
http://docs.python.org/api/boolObjects.html .
It would be nice if someone could test and comment
on the patch.
Cheers
Guido
--- postgresql-8.2.1.orig/src/pl/plpython/plpython.c2006-11-21
22:51:05.0 +0100
+++ postgresql-8.2.1/src/pl/plpython/plpython.c 2007-01-17
18:06:58.185497734 +0100
@@ -1580,8 +1580,8 @@
PLyBool_FromString(const char *src)
{
if (src[0] == 't')
- return PyInt_FromLong(1);
- return PyInt_FromLong(0);
+ Py_RETURN_TRUE;
+ Py_RETURN_FALSE;
}
static PyObject *
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
--
Bruce Momjian [EMAIL PROTECTED]
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
--
Bruce Momjian [EMAIL PROTECTED]
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match
Re: [pgsql-patches] [HACKERS] Fix for bug in plpython bool type conversion
Patch applied. Thanks.
---
Guido Goldstein wrote:
Hi!
The attached patch fixes a bug in plpython.
This bug was found while creating sql from trigger functions
written in plpython and later running the generated sql.
The problem was that boolean was was silently converted to
integer, which is ok for python but fails when the created
sql is used.
The patch uses the Py_RETURN_xxx macros shown at
http://docs.python.org/api/boolObjects.html .
It would be nice if someone could test and comment
on the patch.
Cheers
Guido
--- postgresql-8.2.1.orig/src/pl/plpython/plpython.c 2006-11-21
22:51:05.0 +0100
+++ postgresql-8.2.1/src/pl/plpython/plpython.c 2007-01-17
18:06:58.185497734 +0100
@@ -1580,8 +1580,8 @@
PLyBool_FromString(const char *src)
{
if (src[0] == 't')
- return PyInt_FromLong(1);
- return PyInt_FromLong(0);
+ Py_RETURN_TRUE;
+ Py_RETURN_FALSE;
}
static PyObject *
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
--
Bruce Momjian [EMAIL PROTECTED]
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match
Re: [pgsql-patches] [HACKERS] [PATCHES] [BUGS] BUG #2846: inconsistent and
Roman Kononov wrote: On 12/27/2006 01:15 PM, Tom Lane wrote: I'm not convinced that you're fixing things so much as doing your best to destroy IEEE-compliant float arithmetic behavior. I think what we should probably consider is removing CheckFloat4Val and CheckFloat8Val altogether, and just letting the float arithmetic have its head. Most modern hardware gets float arithmetic right per spec, and we shouldn't be second-guessing it. I vote for complete IEEE-compliance. No exceptions with pure floating point math. Float - int conversions should reject overflow, INF and NaN. Float - numeric conversions should reject INF. I think we did that in current CVS. Feel free to download a snapshot from the ftp server and try it out. A slightly less radical proposal is to reject only the case where isinf(result) and neither input isinf(); and perhaps likewise with respect to NaNs. This might look like another possibility for confusion. For example INF-INF=NaN. Yep, that's what we get now. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [Patch] - Fix for bug #2558, InitDB failed to run
Sorry, Im not an expert, and I have the same win 2003 server installation problem, but dont know what to do with the tree .c files downloaded as a patch, can you please direct me on how to use the patch? Thanks ! Andrew Dunstan wrote: 1. a patch is generated by the program diff 2. before we do anything, as Tom Lane says, we need verification of the problem, preferably in writing from Microsoft. cheers andrew dror wrote: 1. When saying: Please submit the changes as patches, instead of the whole files. Do you mean to send each file seperately? or other issues as well? 2. The change was test and design for 8.1.14, but as far as I see it is also true for any other version. Of course merge is needed in case that the files were changed since then, however , due to the fact that it is only few rows it will be easy to do it. 3. Alvaro wrote: it may be useful to lose the redirection only on the cases where it fails, so we still have reasonable behavior on non-broken platforms Nice idea, but it is really doesn't matter: on other platform than win32, the code left unchanged! the fix is only relevant for win32 on which (as I explained) the MSI installer (if used) redirect the output by default to a log file. Regards Dror Date: Mon, 14 Aug 2006 12:40:25 -0400 From: [EMAIL PROTECTED] To: [EMAIL PROTECTED] CC: pgsql-patches@postgresql.org Subject: Re: [PATCHES] [Patch] - Fix for bug #2558, InitDB failed to run on windows 2003 dror wrote: There were two options to solve this issue: Create a new file , grant a write permission for the Postgres user and redirect the output to that file. (EnterpriseDB use this method) Canceling the redirection at all. I choose the second option and omit the redirection in any case that it windows machine and the redirection was sent to DEVNULL. The only files that I changed are: initDB.c, exec.c and pg_ctl.c Please submit the changes as patches, instead of the whole files. Also, please specify which branch do these patches apply -- is this for 8.1, or for the current development code? When checked against the 8.1 pg_ctl.c, the file you sent only contains a regression for a bug fix, and no change related to what you describe above. On the other hand, it may be useful to lose the redirection only on the cases where it fails, so we still have reasonable behavior on non-broken platforms. Or maybe there's a better solution. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support Express yourself instantly with Windows Live Messenger! Windows Live Messenger! http://imagine-msn.com/messenger/launch80/default.aspx?locale=en-ussource=joinmsncom/messenger ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org -- View this message in context: http://www.nabble.com/-PatchFix-for-bug---2558%2C--InitDB-failed-to-run-on-windows-2003-tf2103710.html#a8164273 Sent from the PostgreSQL - patches mailing list archive at Nabble.com. ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [Patch] - Fix for bug #2558, InitDB failed to run
Hi Andrew,Regarding to your comments: 1.apatchisgeneratedbytheprogram"diff"I will do it ,if needed 2.beforewedoanything,asTomLanesays,weneedverificationofthe problem,preferablyinwritingfromMicrosoft.I do understand that, but, de-facto, the current implementation does not work, canceling theredirection (or open a log file) is not a matter of changing the OS behavior,therefore I don't see why a formal verification from Microsoft is needed.Whenthis issue will be revealed in more and more system, it can be harmless to postgress reputation and critical problems for the end users.In addition to the above,as James Hughes have already mention before at item #2268 (http://archives.postgresql.org/pgsql-hackers/2006-03/msg00012.php):He tried to get answers from microsoft butdidn't get any respond.Anyway,For my own version, I solve the issue and built a private version, if you still want me to publish the patch (Just in case) I will do the diff and the effort needed, I don't think that Microsoft respond is needed in this case because it is implementation decision.Regards;Dror Date: Mon, 14 Aug 2006 17:42:58 -0400 From: [EMAIL PROTECTED] To: [EMAIL PROTECTED] CC: [EMAIL PROTECTED]; pgsql-patches@postgresql.org Subject: Re: [PATCHES] [Patch] - Fix for bug #2558, InitDB failed to run1.apatchisgeneratedbytheprogram"diff" 2.beforewedoanything,asTomLanesays,weneedverificationofthe problem,preferablyinwritingfromMicrosoft. cheers andrew drorwrote: 1. Whensaying: "Pleasesubmitthechangesaspatches,insteadofthewholefiles". Doyoumeantosendeachfileseperately?orotherissuesaswell? 2. Thechangewastestanddesignfor8.1.14,butasfarasIsee itisalsotrueforanyotherversion. Ofcoursemergeisneededincasethatthefileswerechanged sincethen,however,duetothefactthatitisonlyfewrows itwillbeeasytodoit. 3. Alvarowrote: "itmaybeusefultolosetheredirectiononlyonthe caseswhereitfails,sowestillhavereasonablebehavioronnon-broken platforms" Niceidea,butitisreallydoesn'tmatter:onotherplatform thanwin32,thecodeleftunchanged!thefixisonlyrelevant forwin32onwhich(asIexplained)theMSIinstaller(ifused) redirecttheoutputbydefaulttoalogfile. Regards Dror Date:Mon,14Aug200612:40:25-0400 From:[EMAIL PROTECTED] To:[EMAIL PROTECTED] CC:pgsql-patches@postgresql.org Subject:Re:[PATCHES][Patch]-Fixforbug#2558,InitDBfailedto runonwindows2003 drorwrote: Thereweretwooptionstosolvethisissue: Createanewfile,grantawritepermissionforthePostgresuser andredirecttheoutputtothatfile.(EnterpriseDBusethismethod) Cancelingtheredirectionatall. Ichoosethesecondoptionandomittheredirectioninanycasethat itwindowsmachineandtheredirectionwassenttoDEVNULL. TheonlyfilesthatIchangedare:initDB.c,exec.candpg_ctl.c Pleasesubmitthechangesaspatches,insteadofthewholefiles.Also, pleasespecifywhichbranchdothesepatchesapply--isthisfor8.1, orforthecurrentdevelopmentcode?Whencheckedagainstthe8.1 pg_ctl.c,thefileyousentonlycontainsaregressionforabugfix, andnochangerelatedtowhatyoudescribeabove. Ontheotherhand,itmaybeusefultolosetheredirectiononlyonthe caseswhereitfails,sowestillhavereasonablebehavioronnon-broken platforms.Ormaybethere'sabettersolution. -- AlvaroHerrerahttp://www.CommandPrompt.com/ PostgreSQLReplication,Consulting,CustomDevelopment,24x7support ExpressyourselfinstantlywithWindowsLiveMessenger!WindowsLive Messenger! http://imagine-msn.com/messenger/launch80/default.aspx?locale=en-ussource=joinmsncom/messenger With MSN Spaces email straight to your blog. Upload jokes, photos and more. It's free! It's free!
Re: [PATCHES] [Patch] - Fix for bug #2558, InitDB failed to run
Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: I am more than somewhat perplexed as to why the NUL device should be a security risk ... what are they thinking?? Frankly, I don't believe it; even Microsoft can't be that stupid. And I can't find any suggestion that they've done this in a google search. I think the OP is misdiagnosing his problem. An older message suggests that a service pack induced this problem, per MS. I just tried it as non-admin on a W2K3 machine with recent hotfixes, and the command dir nul _did_ work for me. Though neglected, it still sounds like a virus scanner issue to me. Regards, Andreas ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] [Patch] - Fix for bug #2558, InitDB failed to run
Bruce Momjian wrote: Andreas Pflug wrote: Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: I am more than somewhat perplexed as to why the NUL device should be a security risk ... what are they thinking?? Frankly, I don't believe it; even Microsoft can't be that stupid. And I can't find any suggestion that they've done this in a google search. I think the OP is misdiagnosing his problem. An older message suggests that a service pack induced this problem, per MS. I just tried it as non-admin on a W2K3 machine with recent hotfixes, and the command dir nul _did_ work for me. Though neglected, it still sounds like a virus scanner issue to me. Yes, it seems we will need more information on this. We need someone at a win32 command prompt to show us a nul failure. OTOH, what issues might arise if the output is redirected to a legal tmp file? Regards, Andreas ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [Patch] - Fix for bug #2558, InitDB failed to run
Andreas Pflug [EMAIL PROTECTED] writes: what issues might arise if the output is redirected to a legal tmp file? Well, (1) finding a place to put the temp file, ie a writable directory; (2) ensuring the file is removed afterwards; (3) not exposing the user to security hazards due to unsafe use of a temp file (ye olde overwrite-a-symlink risk). Perhaps a few more I didn't think of. It's not a trivial change, and the evidence presented so far hasn't convinced me that we need to put in the effort. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [Patch] - Fix for bug #2558, InitDB failed to run on windows 2003
dror wrote: There were two options to solve this issue: Create a new file , grant a write permission for the Postgres user and redirect the output to that file. (EnterpriseDB use this method) Canceling the redirection at all. I choose the second option and omit the redirection in any case that it windows machine and the redirection was sent to DEVNULL. The only files that I changed are: initDB.c, exec.c and pg_ctl.c Please submit the changes as patches, instead of the whole files. Also, please specify which branch do these patches apply -- is this for 8.1, or for the current development code? When checked against the 8.1 pg_ctl.c, the file you sent only contains a regression for a bug fix, and no change related to what you describe above. On the other hand, it may be useful to lose the redirection only on the cases where it fails, so we still have reasonable behavior on non-broken platforms. Or maybe there's a better solution. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [Patch] - Fix for bug #2558, InitDB failed to run
Alvaro Herrera wrote: dror wrote: There were two options to solve this issue: Create a new file , grant a write permission for the Postgres user and redirect the output to that file. (EnterpriseDB use this method) Canceling the redirection at all. I choose the second option and omit the redirection in any case that it windows machine and the redirection was sent to DEVNULL. The only files that I changed are: initDB.c, exec.c and pg_ctl.c Please submit the changes as patches, instead of the whole files. Also, please specify which branch do these patches apply -- is this for 8.1, or for the current development code? When checked against the 8.1 pg_ctl.c, the file you sent only contains a regression for a bug fix, and no change related to what you describe above. On the other hand, it may be useful to lose the redirection only on the cases where it fails, so we still have reasonable behavior on non-broken platforms. Or maybe there's a better solution. I am inclined to say we should make it into a runtime test and use a tmpfile on Windows if the test fails. I am more than somewhat perplexed as to why the NUL device should be a security risk ... what are they thinking?? The case that bothers me more is where input is redirected - will that also work? cheers andrew ---(end of broadcast)--- TIP 1: 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] [Patch] - Fix for bug #2558, InitDB failed to run
When saying:"Pleasesubmitthechangesaspatches,insteadofthewholefiles".Do you mean to send each file seperately? or other issues as well? The change was test and design for 8.1.14, but as far as I see it is also true for any other version.Of course merge is needed in case that the files were changed since then, however , due to the fact that it is only few rows it will be easy to do it. Alvaro wrote: "itmaybeusefultolosetheredirectiononlyonthe caseswhereitfails,sowestillhavereasonablebehavioronnon-brokenplatforms"Nice idea, but it is really doesn't matter: on other platform than win32, the code left unchanged! the fix is only relevant for win32 on which (as I explained) the MSI installer (if used) redirect the output by default to a log file. Regards Dror Date: Mon, 14 Aug 2006 12:40:25 -0400 From: [EMAIL PROTECTED] To: [EMAIL PROTECTED] CC: pgsql-patches@postgresql.org Subject: Re: [PATCHES] [Patch] - Fix for bug #2558, InitDB failed to run on windows 2003 drorwrote: Thereweretwooptionstosolvethisissue: Createanewfile,grantawritepermissionforthePostgresuser andredirecttheoutputtothatfile.(EnterpriseDBusethismethod) Cancelingtheredirectionatall. Ichoosethesecondoptionandomittheredirectioninanycasethat itwindowsmachineandtheredirectionwassenttoDEVNULL. TheonlyfilesthatIchangedare:initDB.c,exec.candpg_ctl.c Pleasesubmitthechangesaspatches,insteadofthewholefiles.Also, pleasespecifywhichbranchdothesepatchesapply--isthisfor8.1, orforthecurrentdevelopmentcode?Whencheckedagainstthe8.1 pg_ctl.c,thefileyousentonlycontainsaregressionforabugfix, andnochangerelatedtowhatyoudescribeabove. Ontheotherhand,itmaybeusefultolosetheredirectiononlyonthe caseswhereitfails,sowestillhavereasonablebehavioronnon-broken platforms.Ormaybethere'sabettersolution. -- AlvaroHerrerahttp://www.CommandPrompt.com/ PostgreSQLReplication,Consulting,CustomDevelopment,24x7supportExpress yourself instantly with Windows Live Messenger! Windows Live Messenger!
Re: [PATCHES] [Patch] - Fix for bug #2558, InitDB failed to run
1. a patch is generated by the program diff 2. before we do anything, as Tom Lane says, we need verification of the problem, preferably in writing from Microsoft. cheers andrew dror wrote: 1. When saying: Please submit the changes as patches, instead of the whole files. Do you mean to send each file seperately? or other issues as well? 2. The change was test and design for 8.1.14, but as far as I see it is also true for any other version. Of course merge is needed in case that the files were changed since then, however , due to the fact that it is only few rows it will be easy to do it. 3. Alvaro wrote: it may be useful to lose the redirection only on the cases where it fails, so we still have reasonable behavior on non-broken platforms Nice idea, but it is really doesn't matter: on other platform than win32, the code left unchanged! the fix is only relevant for win32 on which (as I explained) the MSI installer (if used) redirect the output by default to a log file. Regards Dror Date: Mon, 14 Aug 2006 12:40:25 -0400 From: [EMAIL PROTECTED] To: [EMAIL PROTECTED] CC: pgsql-patches@postgresql.org Subject: Re: [PATCHES] [Patch] - Fix for bug #2558, InitDB failed to run on windows 2003 dror wrote: There were two options to solve this issue: Create a new file , grant a write permission for the Postgres user and redirect the output to that file. (EnterpriseDB use this method) Canceling the redirection at all. I choose the second option and omit the redirection in any case that it windows machine and the redirection was sent to DEVNULL. The only files that I changed are: initDB.c, exec.c and pg_ctl.c Please submit the changes as patches, instead of the whole files. Also, please specify which branch do these patches apply -- is this for 8.1, or for the current development code? When checked against the 8.1 pg_ctl.c, the file you sent only contains a regression for a bug fix, and no change related to what you describe above. On the other hand, it may be useful to lose the redirection only on the cases where it fails, so we still have reasonable behavior on non-broken platforms. Or maybe there's a better solution. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support Express yourself instantly with Windows Live Messenger! Windows Live Messenger! http://imagine-msn.com/messenger/launch80/default.aspx?locale=en-ussource=joinmsncom/messenger ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [Patch] - Fix for bug #2558, InitDB failed to run
Andrew Dunstan [EMAIL PROTECTED] writes: I am more than somewhat perplexed as to why the NUL device should be a security risk ... what are they thinking?? Frankly, I don't believe it; even Microsoft can't be that stupid. And I can't find any suggestion that they've done this in a google search. I think the OP is misdiagnosing his problem. regards, tom lane ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
[PATCHES] Draft patch for bug: ALTER TYPE ... USING(NULL) / NOT NULL violation
Attached is a rather hurried patch for Alexander Pravking's report that ALTER TABLE fails to check pre-existing NOT NULL constraints properly: http://archives.postgresql.org/pgsql-bugs/2006-07/msg00015.php It seems to work but I'm out of time to do more with it, and am leaving for Toronto in the morning. Anyone want to look it over, generate back-patches as appropriate, and apply? regards, tom lane binNYyJ3prBbO.bin Description: notnull.patch ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
[PATCHES] pl/python refcount bug
In PLy_function_build_args(), the code loops repeatedly, constructing one argument at a time and then inserting the argument into a Python list via PyList_SetItem(). This steals the reference to the argument: that is, the reference to the new list member is now held by the Python list itself. This works fine, except if an elog occurs. This causes the function's PG_CATCH() block to be invoked, which decrements the reference counts on both the current argument and the list of arguments. If the elog happens to occur during the second or subsequent iteration of the loop, the reference count on the current argument will be decremented twice. The fix is simple: set the local pointer to the current argument to NULL immediately after adding it to the argument list. This ensures that the Py_XDECREF() in the PG_CATCH() block doesn't double-decrement. I'd like to apply this to HEAD and back branches (as far back as PG_CATCH exists). The broader point is that the current approach to handling reference counting and exceptions in PL/Python seems terribly error-prone. I briefly skimmed the code for other instances of the problem -- while I didn't find any, I don't have a lot of confidence that similar issues don't exist. Any thoughts on how to improve that? (I wonder if we could adapt ResOwner...) -Neil *** src/pl/plpython/plpython.c caab6efbac99de55d61348c6467b72b169c72199 --- src/pl/plpython/plpython.c 29438f318ecb215d1af5aeddd8c4304352d432ac *** *** 895,900 --- 895,901 * FIXME -- error check this */ PyList_SetItem(args, i, arg); + arg = NULL; } } PG_CATCH(); ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] pl/python refcount bug
On Mon, 2006-01-09 at 13:07 -0500, Neil Conway wrote: The fix is simple: set the local pointer to the current argument to NULL immediately after adding it to the argument list. This ensures that the Py_XDECREF() in the PG_CATCH() block doesn't double-decrement. I'd like to apply this to HEAD and back branches (as far back as PG_CATCH exists). Applied to HEAD, REL8_1_STABLE, and REL8_0_STABLE. -Neil ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] CSV consecutive newline bug
Andrew Dunstan wrote: regression patch against 8.0 branch attached. The tiny patch has been applied to REL8_0_STABLE, and the regression test patch has been applied to both REL8_0_STABLE and HEAD. Thanks for the patches. -Neil ---(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
[PATCHES] CSV consecutive newline bug
I have just been alerted to a bug in the 8.0 handling of embedded newlines in CSV data. Basically it barfs on consecutive newlines. The attached patch for 8.0 appears to fix it. The bug isn't present in the HEAD branch, and I'm wondering if we should not backpatch the HEAD multiline patch rather than applying this. OTOH, applying this patch would probably be more in keeping with our conservative approach to changes to stable branches, I guess. cheers andrew Index: src/backend/commands/copy.c === RCS file: /home/cvsmirror/pgsql/src/backend/commands/copy.c,v retrieving revision 1.236 diff -c -r1.236 copy.c *** src/backend/commands/copy.c 31 Dec 2004 21:59:41 - 1.236 --- src/backend/commands/copy.c 11 May 2005 21:12:07 - *** *** 2395,2400 --- 2395,2401 if (done line_buf.len == 0) break; start_cursor = line_buf.cursor; + continue; } end_cursor = line_buf.cursor; ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] CSV consecutive newline bug
Andrew Dunstan wrote: I have just been alerted to a bug in the 8.0 handling of embedded newlines in CSV data. Basically it barfs on consecutive newlines. The attached patch for 8.0 appears to fix it. The bug isn't present in the HEAD branch, and I'm wondering if we should not backpatch the HEAD multiline patch rather than applying this. Is there a particular reason to backport the larger patch? As a general rule I'm inclined to apply minimally-invasive fixes to stable branches, but I don't know the code in question, so perhaps there is some reason to make an exception in this case. Also, a regression test for this bug would be nice :) -Neil ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] CSV consecutive newline bug
Neil Conway wrote: Andrew Dunstan wrote: I have just been alerted to a bug in the 8.0 handling of embedded newlines in CSV data. Basically it barfs on consecutive newlines. The attached patch for 8.0 appears to fix it. The bug isn't present in the HEAD branch, and I'm wondering if we should not backpatch the HEAD multiline patch rather than applying this. Is there a particular reason to backport the larger patch? As a general rule I'm inclined to apply minimally-invasive fixes to stable branches, but I don't know the code in question, so perhaps there is some reason to make an exception in this case. Well, if I'd known we were as far away from a release as we turned out to be at the time the original multiline limitation was discovered, I would have submitted a patch for inclusion in 8.0. Never mind - hindsight doesn't help much. Just go with the tiny patch. If anyone wants the later fix it's very easy to get, because it was the first patch applied after 8.0 branched. Just dropping in the later version of copy.c should work. Also, a regression test for this bug would be nice :) regression patch against 8.0 branch attached. cheers andrew Index: expected/copy2.out === RCS file: /home/cvsmirror/pgsql/src/test/regress/expected/copy2.out,v retrieving revision 1.19.4.1 diff -c -r1.19.4.1 copy2.out *** expected/copy2.out 22 Jan 2005 05:14:19 - 1.19.4.1 --- expected/copy2.out 12 May 2005 01:08:20 - *** *** 191,196 --- 191,199 Jackson, Sam,\\h It is \perfect\., , + --test that we read consecutive LFs properly + CREATE TEMP TABLE testnl (a int, b text, c int); + COPY testnl FROM stdin CSV; DROP TABLE x, y; DROP FUNCTION fn_x_before(); DROP FUNCTION fn_x_after(); Index: sql/copy2.sql === RCS file: /home/cvsmirror/pgsql/src/test/regress/sql/copy2.sql,v retrieving revision 1.10.4.1 diff -c -r1.10.4.1 copy2.sql *** sql/copy2.sql 22 Jan 2005 05:14:25 - 1.10.4.1 --- sql/copy2.sql 12 May 2005 01:05:12 - *** *** 129,134 --- 129,145 COPY y TO stdout WITH CSV QUOTE DELIMITER '|'; COPY y TO stdout WITH CSV FORCE QUOTE col2 ESCAPE '\\'; + --test that we read consecutive LFs properly + + CREATE TEMP TABLE testnl (a int, b text, c int); + + COPY testnl FROM stdin CSV; + 1,a field with two LFs + + inside,2 + \. + + DROP TABLE x, y; DROP FUNCTION fn_x_before(); DROP FUNCTION fn_x_after(); ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] is it a bug ?
if so, why do phppgadmin display data correctly and handle accents and other unicode caracters ? On Thu, 3 Mar 2005 19:28:30 +0100, Peter Eisentraut [EMAIL PROTECTED] wrote: Zouari Fourat wrote: excuse me but i dont understand what should i do ? The locale you specified when you ran initdb needs to match the encoding of the database. The equivalent pairs are usually named like this: Locale Encoding de_DELATIN1 [EMAIL PROTECTED] LATIN9 de_DE.utf8 UNICODE Pick the analogous pairs for you language environment. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(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] is it a bug ?
, 2005-03-03 04:38 -0500, Tom Lane : Zouari Fourat [EMAIL PROTECTED] writes: here's what logs contain : ATTENTION: Laisse de ct les caractres UTF-8 inconvertibles 0xc389 ATTENTION: Laisse de ct les caractres UTF-8 inconvertibles 0xf474e9 ATTENTION: Laisse de ct les caractres UTF-8 inconvertibles 0xf474e9 ATTENTION: Laisse de ct les caractres UTF-8 inconvertibles 0xf474e9 ATTENTION: Laisse de ct les caractres UTF-8 inconvertibles 0xf474e9 PANIQUE: ERRORDATA_STACK_SIZE exceeded [ been expecting someone who knows more than me to step forward, but ... ] What I think is happening here is that PG is expecting the translated messages in your .po files to have the same encoding as your database encoding, but they aren't. Can you convert the .po files to match your preferred encoding? Usually gettext handles this for you. And in fact, postgres.mo for russian is encoded in KOI8-R and messages from the server are displayed correctly UTF-8 database sessions under a UTF-8 locale. Markus -- Markus Bertheau [EMAIL PROTECTED] signature.asc Description: =?koi8-u?Q?=E3=C0?= =?koi8-u?Q?_=DE=C1=D3=D4=C9=CE=D5?= =?koi8-u?Q?_=D0=CF=D7=A6=C4=CF=CD=CC=C5=CE=CE?= =?koi8-u?Q?=D1?= =?koi8-u?Q?_=D0=A6=C4=D0=C9=D3=C1=CE=CF?= =?koi8-u?Q?_=C3=C9=C6=D2=CF=D7=C9=CD?= =?koi8-u?Q?_=D0=A6=C4=D0=C9=D3=CF=CD?=
[PATCHES] is it a bug ?
Hello, When selecting (from php) using this select : SELECT msgid,content,timecreated,soa FROM sms_mo WHERE (LOWER(content) LIKE 'club%') AND (da='87000') AND (timecreated '2005-02-18 17:00:00') ORDER BY msgid ASC I get this error : PANIQUE: ERRORDATA_STACK_SIZE exceeded server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. when applying that select into phppgadmin interface there's no error. but when commenting the line : #client_encoding = LATIN9 in /var/lib/pgsql/data/postgresql.conf i will no longer have an error... but got problems with charsets (i use frensh accents and arab characters) ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] is it a bug ?
here's what logs contain : ATTENTION: Laisse de côté les caractères UTF-8 inconvertibles 0xc389 ATTENTION: Laisse de côté les caractères UTF-8 inconvertibles 0xf474e9 ATTENTION: Laisse de côté les caractères UTF-8 inconvertibles 0xf474e9 ATTENTION: Laisse de côté les caractères UTF-8 inconvertibles 0xf474e9 ATTENTION: Laisse de côté les caractères UTF-8 inconvertibles 0xf474e9 PANIQUE: ERRORDATA_STACK_SIZE exceeded INSTRUCTION : SELECT MsgId as msgid,Content as content,TimeCreated as tc,SOA as oa FROM sms_mo WHERE ((LOWER(Content) LIKE 'club%')) AND ((DA='87000')) AND (TimeCreated '2005-02-18 17:00:00') ORDER BY MsgId ASC; TRACE: processus serveur (PID 4359) a été arrêté par le signal 6 TRACE: Arrêt des autres processus serveur actifs TRACE: Tous les processus serveur se sont arrêtés, réinitialisation TRACE: le système de bases de données a été interrompu à 2005-02-26 16:11:25 CET TRACE: l'enregistrement du point de vérification est à 0/7BF31E0 TRACE: ré-exécution de l'enregistrement à 0/7BF31E0 ; l'annulation de l'enregistrement est à 0/0 ; arrêt TRUE TRACE: prochain identifiant de transaction : 70998 ; prochain OID : 535489 TRACE: le système de bases de données n'a pas été arrêté proprement ; restauration automatique en cours TRACE: la ré-exécution commence à 0/7BF321C TRACE: enregistrement de longueur nulle sur 0/7C05940 TRACE: ré-exécution faite à 0/7C05918 TRACE: le système de bases de données est prêt On Thu, 3 Mar 2005 09:15:20 +0100, Zouari Fourat [EMAIL PROTECTED] wrote: Hello, When selecting (from php) using this select : SELECT msgid,content,timecreated,soa FROM sms_mo WHERE (LOWER(content) LIKE 'club%') AND (da='87000') AND (timecreated '2005-02-18 17:00:00') ORDER BY msgid ASC I get this error : PANIQUE: ERRORDATA_STACK_SIZE exceeded server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. when applying that select into phppgadmin interface there's no error. but when commenting the line : #client_encoding = LATIN9 in /var/lib/pgsql/data/postgresql.conf i will no longer have an error... but got problems with charsets (i use frensh accents and arab characters) ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] is it a bug ?
Zouari Fourat [EMAIL PROTECTED] writes: here's what logs contain : ATTENTION: Laisse de côté les caractères UTF-8 inconvertibles 0xc389 ATTENTION: Laisse de côté les caractères UTF-8 inconvertibles 0xf474e9 ATTENTION: Laisse de côté les caractères UTF-8 inconvertibles 0xf474e9 ATTENTION: Laisse de côté les caractères UTF-8 inconvertibles 0xf474e9 ATTENTION: Laisse de côté les caractères UTF-8 inconvertibles 0xf474e9 PANIQUE: ERRORDATA_STACK_SIZE exceeded [ been expecting someone who knows more than me to step forward, but ... ] What I think is happening here is that PG is expecting the translated messages in your .po files to have the same encoding as your database encoding, but they aren't. Can you convert the .po files to match your preferred encoding? (Obviously this is something we need to improve...) regards, tom lane ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] is it a bug ?
Tom Lane wrote: [ been expecting someone who knows more than me to step forward, but ... ] What I think is happening here is that PG is expecting the translated messages in your .po files to have the same encoding as your database encoding, but they aren't. Can you convert the .po files to match your preferred encoding? No, this again means that you have to have a consistent database encoding and LC_CTYPE. The gettext library will automatically convert between the original encoding in the file and the encoding at run time as declared by LC_CTYPE. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] is it a bug ?
excuse me but i dont understand what should i do ? On Thu, 3 Mar 2005 11:28:50 +0100, Peter Eisentraut [EMAIL PROTECTED] wrote: Tom Lane wrote: [ been expecting someone who knows more than me to step forward, but ... ] What I think is happening here is that PG is expecting the translated messages in your .po files to have the same encoding as your database encoding, but they aren't. Can you convert the .po files to match your preferred encoding? No, this again means that you have to have a consistent database encoding and LC_CTYPE. The gettext library will automatically convert between the original encoding in the file and the encoding at run time as declared by LC_CTYPE. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(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] is it a bug ?
Zouari Fourat wrote: excuse me but i dont understand what should i do ? The locale you specified when you ran initdb needs to match the encoding of the database. The equivalent pairs are usually named like this: Locale Encoding de_DELATIN1 [EMAIL PROTECTED] LATIN9 de_DE.utf8 UNICODE Pick the analogous pairs for you language environment. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
[PATCHES] psql tab completion bug and possible fix
Recently I've been seeing regular but very occasional errors like the
following while using psql:
test= BEGIN ;
BEGIN
test= UPDATE language SET name_native = 'Français' WHERE lang_id='fr';
ERROR: current transaction is aborted, commands ignored until end of
transaction block
where the UPDATE statement itself is entirely correct and is executed
correctly when a new transaction is started. Unfortunately I was never able
to reproduce the error and thought it might be some kind of beta flakiness,
until it turned up in a 7.3 installation too.
The culprit is the following section of psql's tab-complete.c , around line
1248:
/* WHERE */
/* Simple case of the word before the where being the table name */
else if (strcasecmp(prev_wd, WHERE) == 0)
COMPLETE_WITH_ATTR(prev2_wd);
which is correct for SELECT statements. Where the line contains an UPDATE
statement however, and tab is pressed after WHERE, the word before WHERE is
passed to the backend via a sprintf-generated query with the %s between single
quotes, i.e. in the above case
AND c.relname='%s'
is translated to
AND c.relname=''Français''
which is causing a silent error and the transaction failure.
I don't see a simple solution to cater for UPDATE syntax in this context
(you'd need to keep track of whether the statement begins with SELECT
or UPDATE), though it might be a good todo item.
A quick (but not dirty) fix for this and other current or future potential
corner cases would be to ensure any statements executed by the tab completion
functions are quoted correctly, so even if the statement does not produce any
results for tab completion, at least it cannot cause mysterious transaction
errors (and associated doubts about PostgreSQL's stability ;-).
A patch for this using PQescapeString (is there another preferred method?) is
attached as a possible solution.
Ian Barwick
[EMAIL PROTECTED]
Index: tab-complete.c
===
RCS file: /projects/cvsroot/pgsql-server/src/bin/psql/tab-complete.c,v
retrieving revision 1.85
diff -c -r1.85 tab-complete.c
*** tab-complete.c 7 Sep 2003 15:26:54 - 1.85
--- tab-complete.c 14 Oct 2003 21:03:58 -
***
*** 789,801
* list of tables (Also cover the analogous backslash command)
*/
else if (strcasecmp(prev_wd, COPY) == 0 ||
! strcasecmp(prev_wd, \\copy) == 0 ||
(strcasecmp(prev2_wd, COPY) == 0
strcasecmp(prev_wd, BINARY) == 0))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables);
/* If we have COPY|BINARY sth, complete it with TO or FROM */
else if (strcasecmp(prev2_wd, COPY) == 0 ||
! strcasecmp(prev2_wd, \\copy) == 0 ||
strcasecmp(prev2_wd, BINARY) == 0)
{
char *list_FROMTO[] = {FROM, TO, NULL};
--- 789,801
* list of tables (Also cover the analogous backslash command)
*/
else if (strcasecmp(prev_wd, COPY) == 0 ||
! strcasecmp(prev_wd, copy) == 0 ||
(strcasecmp(prev2_wd, COPY) == 0
strcasecmp(prev_wd, BINARY) == 0))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables);
/* If we have COPY|BINARY sth, complete it with TO or FROM */
else if (strcasecmp(prev2_wd, COPY) == 0 ||
! strcasecmp(prev2_wd, copy) == 0 ||
strcasecmp(prev2_wd, BINARY) == 0)
{
char *list_FROMTO[] = {FROM, TO, NULL};
***
*** 1258,1296
/* Backslash commands */
/* TODO: \dc \dd \dl */
! else if (strcmp(prev_wd, \\connect) == 0 || strcmp(prev_wd, \\c) == 0)
COMPLETE_WITH_QUERY(Query_for_list_of_databases);
! else if (strcmp(prev_wd, \\d) == 0 || strcmp(prev_wd, \\d+) == 0)
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tisv);
! else if (strcmp(prev_wd, \\da) == 0)
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_aggregates);
! else if (strcmp(prev_wd, \\dD) == 0)
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_domains);
! else if (strcmp(prev_wd, \\df) == 0 || strcmp(prev_wd, \\df+) == 0)
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_functions);
! else if (strcmp(prev_wd, \\di) == 0 || strcmp(prev_wd, \\di+) == 0)
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_indexes);
! else if (strcmp(prev_wd, \\dn) == 0)
COMPLETE_WITH_QUERY(Query_for_list_of_schemas);
! else if (strcmp(prev_wd, \\dp) == 0 || strcmp(prev_wd, \\z) == 0)
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tsv);
! else if (strcmp(prev_wd, \\ds) == 0 || strcmp(prev_wd, \\ds+) == 0)
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_sequences);
! else if (strcmp(prev_wd, \\dS) == 0 || strcmp(prev_wd, \\dS+) == 0)
COMPLETE_WITH_QUERY(Query_for_list_of_system_relations);
! else if (strcmp(prev_wd, \\dt) == 0 || strcmp(prev_wd, \\dt+) == 0)
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables);
! else if (strcmp(prev_wd, \\dT) == 0 || strcmp(prev_wd, \\dT+) == 0)
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_datatypes);
! else if (strcmp(prev_wd, \\du) == 0)
Re: [PATCHES] psql tab completion bug and possible fix
Ian Barwick [EMAIL PROTECTED] writes: A patch for this using PQescapeString (is there another preferred method?) is attached as a possible solution. Surely all those replacements of \\ with are wrong. I agree that it's insane not to be escaping the user input, though ... 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] [NOVICE] connectby() minor bug in errormessage
Joe Conway [EMAIL PROTECTED] writes: Attached is a patch for the issue reported above by Nabil. Please apply. Applied (with correct spelling ...) 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] [NOVICE] connectby() minor bug in errormessage
Nabil Sayegh wrote: validateConnectbyTupleDesc When the fourth column (tupdesc-attrs[3]) fails the type check, the errormessage should be fourth column must be... and not third column must be ... line 1372 http://www.joeconway.com/tablefunc.tar.gz Attached is a patch for the issue reported above by Nabil. Please apply. Thanks, Joe Index: contrib/tablefunc/tablefunc.c === RCS file: /opt/src/cvs/pgsql-server/contrib/tablefunc/tablefunc.c,v retrieving revision 1.18 diff -c -r1.18 tablefunc.c *** contrib/tablefunc/tablefunc.c 15 Jun 2003 17:59:09 - 1.18 --- contrib/tablefunc/tablefunc.c 25 Jun 2003 02:46:11 - *** *** 1369,1375 /* check that the type of the forth column is TEXT if applicable */ if (show_branch tupdesc-attrs[3]-atttypid != TEXTOID) elog(ERROR, Query-specified return tuple not valid for Connectby: !third column must be type %s, format_type_be(TEXTOID)); /* OK, the tupdesc is valid for our purposes */ } --- 1369,1375 /* check that the type of the forth column is TEXT if applicable */ if (show_branch tupdesc-attrs[3]-atttypid != TEXTOID) elog(ERROR, Query-specified return tuple not valid for Connectby: !forth column must be type %s, format_type_be(TEXTOID)); /* OK, the tupdesc is valid for our purposes */ } ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
