Re: [PATCHES] CREATE TABLE LIKE x INCLUDING CONSTRAINTS
Patch applied. Thanks. --- Greg Stark wrote: > > Fixed previous patch by calling change_varattnos_of_a_node() to fix up > constraint expressions in case attribute positions don't line up. > > change_varattnos_of_a_node is in tablecmds.c for inherited tables so this > means making it extern. I have a feeling it probably ought to move to some > file of functions for manipulating Nodes but I couldn't really find an > appropriate place. At first I was going to put it in ruleutils.c but then it > seems the other functions in that file go in builtins.h which would be a > strange place for this I think. > > So in the end I left the functions in tablecmds.[ch] at least until someone > more familiar with the source tree suggests another pair of files for them to > be defined in. > [ Attachment, skipping... ] > > -- > greg > > ---(end of broadcast)--- > TIP 6: explain analyze is your friend -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] PQescapeIdentifier
Christopher Kings-Lynne <[EMAIL PROTECTED]> writes: > Yes, phpPgAdmin sure would. I imagine this would be a nightmare to > address properly, so perhaps we should remove the function :( Well, it's fixable, cf PQescapeStringConn, but we should fix it *before* it gets into the field not after. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Patch for BUG #2073: Can't drop sequence when created
Patch applied. Thanks. --- Dhanaraj M wrote: > Hi > > I send the appropriate patch for bug #2073. This fix disallows to change the > default sequence. > I ran the regression test and passed. The bug details are given below. > I am awaiting to answer for any further clarifications. > > === > > Bug reference: 2073 > > Logged by: Aaron Dummer > > Email address: aaron ( at ) dummer ( dot ) info > > PostgreSQL version: 8.0.3 > > Operating system: Debian Linux > > Description:Can't drop sequence when created via SERIAL column > > Details: > > > > If I create a table named foo with a column named bar, column type SERIAL, > > it auto-generates a sequence named foo_bar_seq. Now if I manually create a > > new sequence called custom_seq, and change the default value of foo.bar to > > reference the new sequence, I still can't delete the old sequence > > (foo_bar_seq). > > > > In other words, from a user's point of view, the foo table is no longer > > dependent on the foo_bar_seq, yet the system still sees it as dependent. > > > > I brought this topic up on the #postgresql IRC channel and the behavior was > > confirmed by AndrewSN, scampbell_, and mastermind. > > Right. We have this TODO item: > > * %Disallow changing default expression of a SERIAL column? > > which would prevent you from changing the default expression for a > SERIAL column. So the answer is, don't do that, and in the future, we > might prevent it. > > -- > Bruce Momjian > > == > > > ---(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 2: Don't 'kill -9' the postmaster
Re: [PATCHES] pg_backup_tar.c seems anerror by win32
Hiroshi Saito wrote: > Hi Bruce-san. > > Yesterday patch was inadequate > > pg_backup_tar.c: In function `tarOpen': > pg_backup_tar.c:379: `S_IREAD' undeclared (first use in this function) > pg_backup_tar.c:379: (Each undeclared identifier is reported only once > pg_backup_tar.c:379: for each function it appears in.) > pg_backup_tar.c:379: `S_IWRITE' undeclared (first use in this function) > pg_backup_tar.c: In function `_tarAddFile': > pg_backup_tar.c:1051: warning: comparison is always false due to limited > range of data type > make[3]: *** [pg_backup_tar.o] Error 1 > make[2]: *** [all] Error 2 > make[1]: *** [all] Error 2 > make: *** [all] Error 2 > > This patch helps this. > Thanks. Thanks. Patch applied to HEAD and 8.1.X. -- 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] [HACKERS] PQescapeIdentifier
Tom Lane wrote: > Bruce Momjian <[EMAIL PROTECTED]> writes: > > Tom Lane wrote: > >> Have either of you inquired into the encoding-safety of this code? > >> It certainly looks like no consideration was given for that. > > > I thought of that but I assume we were not accepting user-supplied > > identifiers for this --- that this was only for application use. Am I > > wrong? > > By definition, an escaping routine is not supposed to trust the data it > is handed. We *will* be seeing a CVE report if this function has got > any escaping vulnerability. > > If you insist on a practical example, I can certainly imagine someone > thinking it'd be cool to allow searches on a user-selected column, and > implementing that by passing the user-given column name straight into > the query with only PQescapeIdentifier for safety. OK, does someone want to fix it, or should I revert it? -- 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
[PATCHES] pg_backup_tar.c seems anerror by win32
Hi Bruce-san. Yesterday patch was inadequate pg_backup_tar.c: In function `tarOpen': pg_backup_tar.c:379: `S_IREAD' undeclared (first use in this function) pg_backup_tar.c:379: (Each undeclared identifier is reported only once pg_backup_tar.c:379: for each function it appears in.) pg_backup_tar.c:379: `S_IWRITE' undeclared (first use in this function) pg_backup_tar.c: In function `_tarAddFile': pg_backup_tar.c:1051: warning: comparison is always false due to limited range of data type make[3]: *** [pg_backup_tar.o] Error 1 make[2]: *** [all] Error 2 make[1]: *** [all] Error 2 make: *** [all] Error 2 This patch helps this. Thanks. Regards, Hiroshi Saito pg_backup_tar.c_patch Description: Binary data ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] PQescapeIdentifier
Bruce Momjian <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> Have either of you inquired into the encoding-safety of this code? >> It certainly looks like no consideration was given for that. > I thought of that but I assume we were not accepting user-supplied > identifiers for this --- that this was only for application use. Am I > wrong? By definition, an escaping routine is not supposed to trust the data it is handed. We *will* be seeing a CVE report if this function has got any escaping vulnerability. If you insist on a practical example, I can certainly imagine someone thinking it'd be cool to allow searches on a user-selected column, and implementing that by passing the user-given column name straight into the query with only PQescapeIdentifier for safety. 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
Re: [PATCHES] [HACKERS] PQescapeIdentifier
Tom Lane wrote: > Bruce Momjian <[EMAIL PROTECTED]> writes: > >> * Add PQescapeIdentifier() to libpq > > > Updated patch applied. Thanks. > > Have either of you inquired into the encoding-safety of this code? > It certainly looks like no consideration was given for that. I thought of that but I assume we were not accepting user-supplied identifiers for this --- that this was only for application use. Am I wrong? -- 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] [HACKERS] PQescapeIdentifier
Bruce Momjian <[EMAIL PROTECTED]> writes: >> * Add PQescapeIdentifier() to libpq > Updated patch applied. Thanks. Have either of you inquired into the encoding-safety of this code? It certainly looks like no consideration was given for that. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] pg_dump -Ft failed on Windows XP
Modified patch attached and applied to HEAD and 8.1.X.
I restructured the loop exit, and used the symbols without the leading
underscores. I didn't see any Win32 underscore symbol usage in our
existing code.
Thanks.
-
Zeugswetter Andreas DCP SD wrote:
>
> > >> Apparently it won't work at all if TMP isn't set?
> >
> > > I'm not *too* concerned about that, since TMP is normally set by the
> OS
> > > itself. There's one set in the "system environment" (to
> c:\windows\temp
> > > or whatrever) and then it's overridden by one set by the OS when it
> > > loads a user profile.
> >
> > OK, then maybe not having it would be equivalent to /tmp-not-writable
> > on Unix, ie, admin error.
> >
> > > Also to the point, what would you fall back to?
> >
> > Current directory maybe?
>
> It tries \ (tested on Win 2000), if the dir argument is NULL and TMP is
> not set.
> But TMP is usually set.
>
> Attached is a working version not yet adapted to port/.
> - memoryleak fixed
> - use _tmpname and _fdopen not the compatibility tmpname and fdopen
> (imho only cosmetic)
> - EACCES fixed (Win2000 needs _S_IREAD | _S_IWRITE or fails with EACCES,
> even as Admin)
> - I suggest adding a prefix pg_temp_ (for leftover temp files after
> crash,
> the name I get is then usually pg_temp_2)
>
> Andreas
Content-Description: pg_dump_tempfile.patch.txt
[ Attachment, skipping... ]
>
> ---(end of broadcast)---
> TIP 2: Don't 'kill -9' the postmaster
--
Bruce Momjian [EMAIL PROTECTED]
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: src/bin/pg_dump/pg_backup_tar.c
===
RCS file: /cvsroot/pgsql/src/bin/pg_dump/pg_backup_tar.c,v
retrieving revision 1.52
diff -c -c -r1.52 pg_backup_tar.c
*** src/bin/pg_dump/pg_backup_tar.c 7 Jun 2006 22:24:44 - 1.52
--- src/bin/pg_dump/pg_backup_tar.c 27 Jun 2006 00:32:06 -
***
*** 359,365
--- 359,393
{
tm = calloc(1, sizeof(TAR_MEMBER));
+ #ifndef WIN32
tm->tmpFH = tmpfile();
+ #else
+ /*
+ * On WIN32, tmpfile() generates a filename in the root directory,
+ * which requires administrative permissions on certain systems.
+ * Loop until we find a unique file name we can create.
+ */
+ while (1)
+ {
+ char *name;
+ int fd;
+
+ name = _tempnam(NULL, "pg_temp_");
+ if (name == NULL)
+ break;
+ fd = open(name, O_RDWR | O_CREAT | O_EXCL | O_BINARY |
+ O_TEMPORARY, S_IREAD | S_IWRITE);
+ free(name);
+
+ if (fd != -1) /* created a file */
+ {
+ tm->tmpFH = fdopen(fd, "w+b");
+ break;
+ }
+ else if (errno != EEXIST) /* failure other than file exists */
+ break;
+ }
+ #endif
if (tm->tmpFH == NULL)
die_horribly(AH, modulename, "could not generate temporary file name: %s\n", strerror(errno));
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: [PATCHES] LDAP lookup of connection parameters
I am confused why this patch requires libldap_r. Is there a need for threading? Should this be contingent on whether the threading flag was passed to configure? --- Albe Laurenz wrote: > This patch for libpq allows you to enter an LDAP URL in pg_service.conf. > The URL will be queried and the resulting string(s) parsed for > keyword = value connection options. > > The idea is to have connection information stored centrally on an LDAP > server rather than on the client machine. > > On Windows the native library wldap32.dll is used, else OpenLDAP. > If --enable_thread_safety has been given, -lldap_r is appended to > PTHREAD_LIBS so that libpq will be linked against the tread safe > library. > > There should probably also be a documentation patch for the --with-ldap > option of ./configure, but I didn't write it because it also belongs to > the > "LDAP Auth" patch. > > I have added German translations for the new messages - how can I get > translations into other languages? > > Yours, > Laurenz Albe Content-Description: ldap_service.patch [ Attachment, skipping... ] Content-Description: ldap_service_doc.patch [ Attachment, skipping... ] > > ---(end of broadcast)--- > TIP 6: explain analyze is your friend -- 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] [HACKERS] PQescapeIdentifier
Christopher Kings-Lynne wrote:
> TODO item done for 8.2:
>
> * Add PQescapeIdentifier() to libpq
>
> Someone probably needs to check this :)
Updated patch applied. Thanks.
--
Bruce Momjian [EMAIL PROTECTED]
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: doc/src/sgml/libpq.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.211
diff -c -c -r1.211 libpq.sgml
*** doc/src/sgml/libpq.sgml 23 May 2006 22:13:19 - 1.211
--- doc/src/sgml/libpq.sgml 26 Jun 2006 23:54:12 -
***
*** 2279,2284
--- 2279,2347
+
+ Escaping Identifier for Inclusion in SQL Commands
+
+PQescapeIdentifier
+escaping strings
+
+
+ PQescapeIdentifier escapes a string for use
+ as an identifier name within an SQL command. For example; table names,
+ column names, view names and user names are all identifiers.
+ Double quotes (") must be escaped to prevent them from being interpreted
+ specially by the SQL parser. PQescapeIdentifier performs this
+ operation.
+
+
+
+
+ It is especially important to do proper escaping when handling strings that
+ were received from an untrustworthy source. Otherwise there is a security
+ risk: you are vulnerable to SQL injection attacks wherein unwanted
+ SQL commands are fed to your database.
+
+
+
+
+ Note that it is still necessary to do escaping of identifiers when
+ using functions that support parameterized queries such as PQexecParams or
+ its sibling routines. Only literal values are automatically escaped
+ using these functions, not identifiers.
+
+
+ size_t PQescapeIdentifier (char *to, const char *from, size_t length);
+
+
+
+
+ The parameter from points to the first character of the string
+ that is to be escaped, and the length parameter gives the
+ number of characters in this string. A terminating zero byte is not
+ required, and should not be counted in length. (If
+ a terminating zero byte is found before length bytes are
+ processed, PQescapeIdentifier stops at the zero; the behavior
+ is thus rather like strncpy.)
+ to shall point to a
+ buffer that is able to hold at least one more character than twice
+ the value of length, otherwise the behavior is
+ undefined. A call to PQescapeIdentifier writes an escaped
+ version of the from string to the to
+ buffer, replacing special characters so that they cannot cause any
+ harm, and adding a terminating zero byte. The double quotes that
+ may surround PostgreSQL identifiers are not
+ included in the result string; they should be provided in the SQL
+ command that the result is inserted into.
+
+
+ PQescapeIdentifier returns the number of characters written
+ to to, not including the terminating zero byte.
+
+
+ Behavior is undefined if the to and from
+ strings overlap.
+
+
Escaping Binary Strings for Inclusion in SQL Commands
Index: src/interfaces/libpq/exports.txt
===
RCS file: /cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.11
diff -c -c -r1.11 exports.txt
*** src/interfaces/libpq/exports.txt 28 May 2006 22:42:05 - 1.11
--- src/interfaces/libpq/exports.txt 26 Jun 2006 23:54:20 -
***
*** 130,132
--- 130,134
PQencryptPassword 128
PQisthreadsafe129
enlargePQExpBuffer130
+ PQescapeIdentifier131
+
Index: src/interfaces/libpq/fe-exec.c
===
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v
retrieving revision 1.186
diff -c -c -r1.186 fe-exec.c
*** src/interfaces/libpq/fe-exec.c 28 May 2006 21:13:54 - 1.186
--- src/interfaces/libpq/fe-exec.c 26 Jun 2006 23:54:21 -
***
*** 2516,2521
--- 2516,2557
}
/*
+ * Escaping arbitrary strings to get valid SQL identifier strings.
+ *
+ * Replaces " with "".
+ *
+ * length is the length of the source string. (Note: if a terminating NUL
+ * is encountered sooner, PQescapeIdentifier stops short of "length"; the behavior
+ * is thus rather like strncpy.)
+ *
+ * For safety the buffer at "to" must be at least 2*length + 1 bytes long.
+ * A terminating NUL character is added to the output string, whether the
+ * input is NUL-terminated or not.
+ *
+ * Returns the actual length of the output (not counting the terminating NUL).
+ */
+ size_t
+ PQescapeIdentifier(char *to, const char *from, size_t length)
+ {
+ const char *source = from;
+ char *target = to;
+ size_t remaining = length;
+
+ while (remaining > 0 && *source != '\0')
+ {
+ if (*source == '"')
+ *target++ = *source;
+ *target++ = *source++;
+ remaining--;
+ }
+
+ /* Write the terminating NUL character. */
+ *target = '\0';
+
+ return target - to;
+ }
+
+ /*
* PQescapeBytea - converts from binar
Re: [PATCHES] [HACKERS] Overhead for stats_command_string et al, take
Tom Lane wrote:
> Bruce Momjian <[EMAIL PROTECTED]> writes:
> > If you put a contition test in set_ps_display(), the only clean way to
> > do this is for init_ps_display() to force update_process_title to true
> > before we call set_ps_display(), then reset it to its original value,
> > but that sounds pretty ugly.
>
> No, refactor the code. I was envisioning something called maybe
> transmit_ps_display that would contain the part of set_ps_display
> beginning at "Transmit new setting to kernel". Then both set_ps_display
> and init_ps_display would call that.
I went with a 'force' boolean for set_ps_display().
--
Bruce Momjian [EMAIL PROTECTED]
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: doc/src/sgml/config.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/config.sgml,v
retrieving revision 1.66
diff -c -c -r1.66 config.sgml
*** doc/src/sgml/config.sgml 19 Jun 2006 01:51:21 - 1.66
--- doc/src/sgml/config.sgml 26 Jun 2006 23:20:43 -
***
*** 2888,2893
--- 2888,2908
+
+ update_process_title (boolean)
+
+update_process_title configuration parameter
+
+
+
+ Enables updating of the process title every time a new SQL command
+ is received by the server. The process title is typically viewed
+ by the ps command or in Windows using the Process
+ Explorer. Only superusers can change this setting.
+
+
+
+
stats_start_collector (boolean)
Index: src/backend/bootstrap/bootstrap.c
===
RCS file: /cvsroot/pgsql/src/backend/bootstrap/bootstrap.c,v
retrieving revision 1.217
diff -c -c -r1.217 bootstrap.c
*** src/backend/bootstrap/bootstrap.c 18 Jun 2006 15:38:36 - 1.217
--- src/backend/bootstrap/bootstrap.c 26 Jun 2006 23:20:43 -
***
*** 353,360
statmsg = "??? process";
break;
}
! init_ps_display(statmsg, "", "");
! set_ps_display("");
}
/* Acquire configuration parameters, unless inherited from postmaster */
--- 353,359
statmsg = "??? process";
break;
}
! init_ps_display(statmsg, "", "", "");
}
/* Acquire configuration parameters, unless inherited from postmaster */
Index: src/backend/commands/async.c
===
RCS file: /cvsroot/pgsql/src/backend/commands/async.c,v
retrieving revision 1.131
diff -c -c -r1.131 async.c
*** src/backend/commands/async.c 25 Apr 2006 14:11:54 - 1.131
--- src/backend/commands/async.c 26 Jun 2006 23:20:44 -
***
*** 908,914
if (Trace_notify)
elog(DEBUG1, "ProcessIncomingNotify");
! set_ps_display("notify interrupt");
notifyInterruptOccurred = 0;
--- 908,914
if (Trace_notify)
elog(DEBUG1, "ProcessIncomingNotify");
! set_ps_display("notify interrupt", false);
notifyInterruptOccurred = 0;
***
*** 979,985
*/
pq_flush();
! set_ps_display("idle");
if (Trace_notify)
elog(DEBUG1, "ProcessIncomingNotify: done");
--- 979,985
*/
pq_flush();
! set_ps_display("idle", false);
if (Trace_notify)
elog(DEBUG1, "ProcessIncomingNotify: done");
Index: src/backend/postmaster/autovacuum.c
===
RCS file: /cvsroot/pgsql/src/backend/postmaster/autovacuum.c,v
retrieving revision 1.20
diff -c -c -r1.20 autovacuum.c
*** src/backend/postmaster/autovacuum.c 18 Jun 2006 15:38:37 - 1.20
--- src/backend/postmaster/autovacuum.c 26 Jun 2006 23:20:44 -
***
*** 239,246
MyProcPid = getpid();
/* Identify myself via ps */
! init_ps_display("autovacuum process", "", "");
! set_ps_display("");
SetProcessingMode(InitProcessing);
--- 239,245
MyProcPid = getpid();
/* Identify myself via ps */
! init_ps_display("autovacuum process", "", "", "");
SetProcessingMode(InitProcessing);
***
*** 416,422
*/
InitPostgres(db->name, NULL);
SetProcessingMode(NormalProcessing);
! set_ps_display(db->name);
ereport(DEBUG1,
(errmsg("autovacuum: processing database \"%s\"", db->name)));
--- 415,421
*/
InitPostgres(db->name, NULL);
SetProcessingMode(NormalProcessing);
! set_ps_display(db->name, false);
ereport(DEBUG1,
(errmsg("autovacuum: processing database \"%s\"", db->name)));
Index: src/backend/postmaster/pgarch.c
===
RCS file: /cvsroot/pgsql/src/backend/postmaster/pgarch.c,v
retrieving revision 1.23
diff -c -c -r1.23 pgarch.c
*** src/backend/postmaster/pgarch.c 18 Jun 2006 15:38:37 - 1.23
--- src/backend/postmaster/pgarch.c 26 Jun 2
Re: [PATCHES] [HACKERS] Overhead for stats_command_string et al, take 2
Bruce Momjian <[EMAIL PROTECTED]> writes: > If you put a contition test in set_ps_display(), the only clean way to > do this is for init_ps_display() to force update_process_title to true > before we call set_ps_display(), then reset it to its original value, > but that sounds pretty ugly. No, refactor the code. I was envisioning something called maybe transmit_ps_display that would contain the part of set_ps_display beginning at "Transmit new setting to kernel". Then both set_ps_display and init_ps_display would call that. 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
Re: [PATCHES] [HACKERS] Overhead for stats_command_string et al, take
Tom Lane wrote:
> Bruce Momjian <[EMAIL PROTECTED]> writes:
> > Tom Lane wrote:
> >> This is an ugly patch. Why not *one* test of the GUC variable, inside
> >> set_ps_display(), and no side-effects on callers? You would need to
> >> force an initial update from init_ps_display, but that only requires a
> >> small amount of code refactoring inside ps_status.c.
>
> > Consider all the helper processes that set their process title. The
> > only thing I can think of is to add a boolean to set_ps_display() so say
> > whether this is per-command set or not. Is that your idea?
>
> No, that's not what I said at all. Currently init_ps_display doesn't
> actually force the display to update; it's left to the first
> set_ps_display call to do that. If we made init_ps_display update the
> status unconditionally, then set_ps_display could be a conditional
> no-op, and in the helper process setup code
>
> /* Identify myself via ps */
> init_ps_display("autovacuum process", "", "");
> set_ps_display("");
>
> we could remove the now-unnecessary set_ps_display("") calls, but
> the other set_ps_display() calls would stay exactly like they are.
Yea, I figured that out the merge idea after I replied.
If you put a contition test in set_ps_display(), the only clean way to
do this is for init_ps_display() to force update_process_title to true
before we call set_ps_display(), then reset it to its original value,
but that sounds pretty ugly. Do we create another function that
unconditionally sets the title, and conditionally call that from the
set_ps_display()? These seem uglier than the if() test. Or add a
'force' parameter.
--
Bruce Momjian [EMAIL PROTECTED]
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Overhead for stats_command_string et al, take 2
Bruce Momjian <[EMAIL PROTECTED]> writes:
> Tom Lane wrote:
>> This is an ugly patch. Why not *one* test of the GUC variable, inside
>> set_ps_display(), and no side-effects on callers? You would need to
>> force an initial update from init_ps_display, but that only requires a
>> small amount of code refactoring inside ps_status.c.
> Consider all the helper processes that set their process title. The
> only thing I can think of is to add a boolean to set_ps_display() so say
> whether this is per-command set or not. Is that your idea?
No, that's not what I said at all. Currently init_ps_display doesn't
actually force the display to update; it's left to the first
set_ps_display call to do that. If we made init_ps_display update the
status unconditionally, then set_ps_display could be a conditional
no-op, and in the helper process setup code
/* Identify myself via ps */
init_ps_display("autovacuum process", "", "");
set_ps_display("");
we could remove the now-unnecessary set_ps_display("") calls, but
the other set_ps_display() calls would stay exactly like they are.
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
Re: [PATCHES] [HACKERS] Overhead for stats_command_string et al, take
Tom Lane wrote: > Bruce Momjian <[EMAIL PROTECTED]> writes: > > Attached patch adds GUC 'update_process_title' to control ps display > > updates per SQL command. Default to 'on'. GUC name OK? > > This is an ugly patch. Why not *one* test of the GUC variable, inside > set_ps_display(), and no side-effects on callers? You would need to > force an initial update from init_ps_display, but that only requires a > small amount of code refactoring inside ps_status.c. Consider all the helper processes that set their process title. The only thing I can think of is to add a boolean to set_ps_display() so say whether this is per-command set or not. Is that your idea? > The one place that might be worth having an external test on the GUC is > in lock.c, but then it should bypass the entire business of copying, > modifying, and restoring the title ... not just the two set_ps_display > calls. OK, that makes sense. -- 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] [HACKERS] Overhead for stats_command_string et al, take 2
Bruce Momjian <[EMAIL PROTECTED]> writes: > Attached patch adds GUC 'update_process_title' to control ps display > updates per SQL command. Default to 'on'. GUC name OK? This is an ugly patch. Why not *one* test of the GUC variable, inside set_ps_display(), and no side-effects on callers? You would need to force an initial update from init_ps_display, but that only requires a small amount of code refactoring inside ps_status.c. The one place that might be worth having an external test on the GUC is in lock.c, but then it should bypass the entire business of copying, modifying, and restoring the title ... not just the two set_ps_display calls. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Overhead for stats_command_string et al, take 2
Tom Lane wrote:
> Bruce Momjian <[EMAIL PROTECTED]> writes:
> > Yep, I see 8% here. I will add a patch to allow the ps display to be
> > turned off.
>
> I think we'd still want a backend to set the PS display once with its
> identification data (user/DB name and client address). It's just the
> transient activity updates that should stop.
Attached patch adds GUC 'update_process_title' to control ps display
updates per SQL command. Default to 'on'. GUC name OK?
--
Bruce Momjian [EMAIL PROTECTED]
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: doc/src/sgml/config.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/config.sgml,v
retrieving revision 1.66
diff -c -c -r1.66 config.sgml
*** doc/src/sgml/config.sgml 19 Jun 2006 01:51:21 - 1.66
--- doc/src/sgml/config.sgml 26 Jun 2006 19:59:53 -
***
*** 2888,2893
--- 2888,2908
+
+ update_process_title (boolean)
+
+update_process_title configuration parameter
+
+
+
+ Enables updating of the process title every time a new SQL command
+ is received by the server. The process title is typically viewed
+ by the ps command or in Windows using the Process
+ Explorer. Only superusers can change this setting.
+
+
+
+
stats_start_collector (boolean)
Index: src/backend/commands/async.c
===
RCS file: /cvsroot/pgsql/src/backend/commands/async.c,v
retrieving revision 1.131
diff -c -c -r1.131 async.c
*** src/backend/commands/async.c 25 Apr 2006 14:11:54 - 1.131
--- src/backend/commands/async.c 26 Jun 2006 19:59:56 -
***
*** 908,914
if (Trace_notify)
elog(DEBUG1, "ProcessIncomingNotify");
! set_ps_display("notify interrupt");
notifyInterruptOccurred = 0;
--- 908,915
if (Trace_notify)
elog(DEBUG1, "ProcessIncomingNotify");
! if (update_process_title)
! set_ps_display("notify interrupt");
notifyInterruptOccurred = 0;
***
*** 979,985
*/
pq_flush();
! set_ps_display("idle");
if (Trace_notify)
elog(DEBUG1, "ProcessIncomingNotify: done");
--- 980,987
*/
pq_flush();
! if (update_process_title)
! set_ps_display("idle");
if (Trace_notify)
elog(DEBUG1, "ProcessIncomingNotify: done");
Index: src/backend/postmaster/postmaster.c
===
RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.488
diff -c -c -r1.488 postmaster.c
*** src/backend/postmaster/postmaster.c 20 Jun 2006 22:52:00 - 1.488
--- src/backend/postmaster/postmaster.c 26 Jun 2006 20:00:03 -
***
*** 2714,2721
* title for ps. It's good to do this as early as possible in startup.
*/
init_ps_display(port->user_name, port->database_name, remote_ps_data);
! set_ps_display("authentication");
!
/*
* Now perform authentication exchange.
*/
--- 2714,2724
* title for ps. It's good to do this as early as possible in startup.
*/
init_ps_display(port->user_name, port->database_name, remote_ps_data);
! if (update_process_title)
! set_ps_display("authentication");
! else
! set_ps_display("");
!
/*
* Now perform authentication exchange.
*/
Index: src/backend/storage/lmgr/lock.c
===
RCS file: /cvsroot/pgsql/src/backend/storage/lmgr/lock.c,v
retrieving revision 1.164
diff -c -c -r1.164 lock.c
*** src/backend/storage/lmgr/lock.c 14 Apr 2006 03:38:55 - 1.164
--- src/backend/storage/lmgr/lock.c 26 Jun 2006 20:00:05 -
***
*** 1069,1075
new_status = (char *) palloc(len + 8 + 1);
memcpy(new_status, old_status, len);
strcpy(new_status + len, " waiting");
! set_ps_display(new_status);
new_status[len] = '\0'; /* truncate off " waiting" */
awaitedLock = locallock;
--- 1069,1076
new_status = (char *) palloc(len + 8 + 1);
memcpy(new_status, old_status, len);
strcpy(new_status + len, " waiting");
! if (update_process_title)
! set_ps_display(new_status);
new_status[len] = '\0'; /* truncate off " waiting" */
awaitedLock = locallock;
***
*** 1108,1114
awaitedLock = NULL;
! set_ps_display(new_status);
pfree(new_status);
LOCK_PRINT("WaitOnLock: wakeup on lock",
--- 1109,1116
awaitedLock = NULL;
! if (update_process_title)
! set_ps_display(new_status);
pfree(new_status);
LOCK_PRINT("WaitOnLock: wakeup on lock",
Index: src/backend/tcop/postgres.c
===
RCS file: /cvsroot/pgsql/src/backend/tcop/postgres.c,v
retrieving revision
Re: [PATCHES] Further patch for VS2005
> > *** src/pl/plpython/plpython.c 25 Jun 2006 00:18:24 > - 1.83 > > --- src/pl/plpython/plpython.c 26 Jun 2006 13:58:56 - > > *** > > *** 10,16 > > --- 10,19 > > /* Python uses #pragma to bring in a non-default > libpython on VC++ if > >* _DEBUG is defined */ > > #undef _DEBUG > > + /* Also hide away errcode, since we load Python.h before > postgres.h > > + */ #define errcode __vc_errcode > > #include > > + #undef errcode > > #define _DEBUG > > #else > > #include > > BTW, it strikes me as a fairly bad idea to be including > first; that goes directly against the conventions > we established to be sure that largefile support doesn't > break. Has anyone looked into making plpython.c conform to > project standards by including postgres.h first? Not me. Last time I did something like that it came back and bit me because apparantly python does things significantly different on different platforms. Now that we have the buildfarm it might be worth a try. I don't have a *nix box around with python ATM, but if nobody beats me to it I can try it later... //Magnus ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Further patch for VS2005
"Magnus Hagander" <[EMAIL PROTECTED]> writes: > *** src/pl/plpython/plpython.c25 Jun 2006 00:18:24 - 1.83 > --- src/pl/plpython/plpython.c26 Jun 2006 13:58:56 - > *** > *** 10,16 > --- 10,19 > /* Python uses #pragma to bring in a non-default libpython on VC++ if >* _DEBUG is defined */ > #undef _DEBUG > + /* Also hide away errcode, since we load Python.h before postgres.h */ > + #define errcode __vc_errcode > #include > + #undef errcode > #define _DEBUG > #else > #include BTW, it strikes me as a fairly bad idea to be including first; that goes directly against the conventions we established to be sure that largefile support doesn't break. Has anyone looked into making plpython.c conform to project standards by including postgres.h first? 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] Further patch for VS2005
Hi! Attached patch is required ot build with the CRT that comes with Visual Studio 2005. Basically MS defined errcode in the headers with a typedef, so we have to #define it out of the way. While at it, fix a function declaration in plpython that didn't match the implementation (volatile missing). //Magnus vc.patch Description: vc.patch ---(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] MS-VC build patch
> Hi Bruce-san. > > It does not help me yet. He uses VC2005.:-( It seems that > furthermore, it is still in the middle of work. > One problem is visible to the next.(win32.mak) > + if not exist pg_config_os.h copy port\win32.h pg_config_os.h > If VC6+ is still supported, I will submit the patch again. > What I patch has built both the client and the server by VC6+. I don't think there's anything specific in my patch that should kill VC6. What specifically does not work in VC6? (Just reverting the whole patch doesn't seem right to me...) //Magnus ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Overhead for stats_command_string et al, take 2
Tom Lane wrote: > Bruce Momjian <[EMAIL PROTECTED]> writes: > > Seeing stats_command_string with almost zero overhead is great news! > > Should we remove that setting and just have it enabled all > > the time? > > If you don't need it, you shouldn't have to pay any overhead for it, > I think. One could make an argument now for having stats_command_string > default to ON, though. The attached patch makes stats_command_string default to 'on', and updates the documentation. > Something that might also be interesting is an option to suppress > per-command ps_status reporting. On machines where updating ps status > takes a kernel call, there's now a pretty good argument why you might > want to turn that off and rely on pg_stat_activity instead. OK, can I get a timing report from someone with the title on/off that shows a difference? -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: doc/src/sgml/config.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/config.sgml,v retrieving revision 1.66 diff -c -c -r1.66 config.sgml *** doc/src/sgml/config.sgml 19 Jun 2006 01:51:21 - 1.66 --- doc/src/sgml/config.sgml 26 Jun 2006 17:13:05 - *** *** 2878,2884 Enables the collection of information on the currently executing command of each session, along with the time at ! which that command began execution. This parameter is off by default. Note that even when enabled, this information is not visible to all users, only to superusers and the user owning the session being reported on; so it should not represent a --- 2878,2884 Enables the collection of information on the currently executing command of each session, along with the time at ! which that command began execution. This parameter is on by default. Note that even when enabled, this information is not visible to all users, only to superusers and the user owning the session being reported on; so it should not represent a Index: doc/src/sgml/monitoring.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/monitoring.sgml,v retrieving revision 1.34 diff -c -c -r1.34 monitoring.sgml *** doc/src/sgml/monitoring.sgml 19 Jun 2006 01:51:21 - 1.34 --- doc/src/sgml/monitoring.sgml 26 Jun 2006 17:13:05 - *** *** 170,181 ! Since the parameters stats_command_string, ! stats_block_level, and stats_row_level default to false, very few statistics are collected in the default ! configuration. Enabling one or more of these configuration ! variables will significantly enhance the amount of useful data produced by the statistics facilities, at the expense of additional run-time overhead. --- 170,180 ! Since the parameters stats_block_level, and stats_row_level default to false, very few statistics are collected in the default ! configuration. Enabling either of these configuration ! variables will significantly increase the amount of useful data produced by the statistics facilities, at the expense of additional run-time overhead. *** *** 241,249 process ID, user OID, user name, current query, time at which the current query began execution, time at which the process was started, and client's address and port number. The columns ! that report data on the current query are only available if the parameter stats_command_string has been ! turned on. Furthermore, these columns read as null unless the user examining the view is a superuser or the same as the user owning the process being reported on. --- 240,248 process ID, user OID, user name, current query, time at which the current query began execution, time at which the process was started, and client's address and port number. The columns ! that report data on the current query are available unless the parameter stats_command_string has been ! turned off. Furthermore, these columns are only visible if the user examining the view is a superuser or the same as the user owning the process being reported on. *** *** 635,644 pg_stat_get_backend_activity(integer) text !Active command of the given server process (null if the !current user is not a superuser nor the same user as that of !the session being queried, or !stats_command_string is not on) --- 634,643 pg_stat_get_backend_activity(integer) text !Ac
Re: [PATCHES] [HACKERS] Overhead for stats_command_string et al, take 2
Bruce Momjian <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> Something that might also be interesting is an option to suppress >> per-command ps_status reporting. On machines where updating ps status >> takes a kernel call, there's now a pretty good argument why you might >> want to turn that off and rely on pg_stat_activity instead. > OK, can I get a timing report from someone with the title on/off that > shows a difference? IIRC, newer BSDen use a kernel call for this, so you should be able to measure it on your own machine. Just tweak ps_status.c to force it to select PS_USE_NONE instead of PS_USE_SETPROCTITLE to generate a comparison case. I'll try it on my old HPUX box too. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] MS-VC build patch
Umm, It is strange.. I think that you have another config.h.? I look at much error by the reason for being realistic. I will become on tomorrow night, since I am not in the machine which can work now. Regards, Hiroshi Saito From: "Magnus Hagander" Hi Bruce-san. It does not help me yet. He uses VC2005.:-( It seems that furthermore, it is still in the middle of work. One problem is visible to the next.(win32.mak) + if not exist pg_config_os.h copy port\win32.h pg_config_os.h If VC6+ is still supported, I will submit the patch again. What I patch has built both the client and the server by VC6+. I don't think there's anything specific in my patch that should kill VC6. What specifically does not work in VC6? (Just reverting the whole patch doesn't seem right to me...) //Magnus ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] [HACKERS] Overhead for stats_command_string et al, take 2
I wrote: > IIRC, newer BSDen use a kernel call for this, so you should be able to > measure it on your own machine. Just tweak ps_status.c to force it to > select PS_USE_NONE instead of PS_USE_SETPROCTITLE to generate a > comparison case. I'll try it on my old HPUX box too. On HPUX, I get a median time of 5.59 sec for CVS HEAD vs 5.36 sec with ps_status diked out, for the test case of 1 "SELECT 1;" as separate transactions, assert-disabled build. So, almost 10% overhead. Given that the transactions can't get any more trivial than this, that's about a worst-case number. Not sure if it's worth worrying about or not. However Kris Kennaway's report a couple weeks ago suggested things might be worse on BSD. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] MS-VC build patch
Hi Bruce-san. It does not help me yet. He uses VC2005.:-( It seems that furthermore, it is still in the middle of work. One problem is visible to the next.(win32.mak) + if not exist pg_config_os.h copy port\win32.h pg_config_os.h If VC6+ is still supported, I will submit the patch again. What I patch has built both the client and the server by VC6+. Regards, Hiroshi Saito From: "Bruce Momjian" > > Hiroshi, I applied Magnus's patches. Can you build cleanly now? > > --- > > Hiroshi Saito wrote: > > Hi Bruce-san, and Magnus-san, > > > > I have many problems by client construction of 8.2. > > This patch helps a MS-VC client and server construction. > > However, IPV6 still has the problem. > > Please fully take into consideration. > > > > Thanks. > > > > Regards, > > Hiroshi Saito > > > > [ Attachment, skipping... ] > > > > > ---(end of broadcast)--- > > TIP 6: explain analyze is your friend > > -- > Bruce Momjian [EMAIL PROTECTED] > EnterpriseDBhttp://www.enterprisedb.com > > + If your life is a hard drive, Christ can be your backup. + > > ---(end of broadcast)--- > TIP 2: Don't 'kill -9' the postmaster ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Patch for - Change LIMIT/OFFSET to use int8
Dhanaraj M <[EMAIL PROTECTED]> writes: > I attach the patch for the following TODO item. > SQL COMMAND > * Change LIMIT/OFFSET to use int8 This can't possibly be correct. It doesn't even change the field types in struct LimitState, for example. You've missed half a dozen places in the planner that would need work, too. regards, tom lane ---(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
[PATCHES] Patch for - Change LIMIT/OFFSET to use int8
I attach the patch for the following TODO item.
SQL COMMAND
* Change LIMIT/OFFSET to use int8
It passes all the regression tests and supports int8.
I am waiting for your review.
Thanks
Dhanaraj
*** ./src/backend/executor/nodeLimit.c.orig Sun Jun 25 15:02:46 2006
--- ./src/backend/executor/nodeLimit.c Mon Jun 26 14:31:04 2006
***
*** 23,29
#include "executor/executor.h"
#include "executor/nodeLimit.h"
!
static void recompute_limits(LimitState *node);
--- 23,29
#include "executor/executor.h"
#include "executor/nodeLimit.h"
! #include "catalog/pg_type.h"
static void recompute_limits(LimitState *node);
***
*** 226,239
{
ExprContext *econtext = node->ps.ps_ExprContext;
boolisNull;
if (node->limitOffset)
{
! node->offset =
!
DatumGetInt32(ExecEvalExprSwitchContext(node->limitOffset,
econtext,
&isNull,
NULL));
/* Interpret NULL offset as no offset */
if (isNull)
node->offset = 0;
--- 226,250
{
ExprContext *econtext = node->ps.ps_ExprContext;
boolisNull;
+ Oid type;
if (node->limitOffset)
{
! type = ((Const *) node->limitOffset->expr)->consttype;
!
! if(type == INT8OID)
! node->offset =
!
DatumGetInt64(ExecEvalExprSwitchContext(node->limitOffset,
econtext,
&isNull,
NULL));
+ else
+ node->offset =
+
DatumGetInt32(ExecEvalExprSwitchContext(node->limitOffset,
+
econtext,
+
&isNull,
+
NULL));
+
/* Interpret NULL offset as no offset */
if (isNull)
node->offset = 0;
***
*** 249,259
if (node->limitCount)
{
node->noCount = false;
! node->count =
!
DatumGetInt32(ExecEvalExprSwitchContext(node->limitCount,
econtext,
&isNull,
NULL));
/* Interpret NULL count as no count (LIMIT ALL) */
if (isNull)
node->noCount = true;
--- 260,280
if (node->limitCount)
{
node->noCount = false;
! type = ((Const *) node->limitCount->expr)->consttype;
!
! if(type == INT8OID)
! node->count =
!
DatumGetInt64(ExecEvalExprSwitchContext(node->limitCount,
econtext,
&isNull,
NULL));
+ else
+ node->count =
+
DatumGetInt32(ExecEvalExprSwitchContext(node->limitCount,
+
econtext,
+
&isNull,
+
NULL));
+
/* Interpret NULL count as no count (LIMIT ALL) */
if (isNull)
node->noCount = true;
*** ./src/backend/parser/parse_clause.c.origSun Jun 25 14:55:42 2006
--- ./src/backend/parser/parse_clause.c Mon Jun 26 14:50:28 2006
***
*** 1092,1098
