Re: [HACKERS] Return of the Solaris vacuum polling problem -- anyone remember this?
For a documentation patch should this not be back ported to all relevant versions? On 8/21/10, Bruce Momjian br...@momjian.us wrote: Josh Berkus wrote: On further reflection, though: since we put in the BufferAccessStrategy code, which was in 8.3, the background writer isn't *supposed* to be very much involved in writing pages that are dirtied by VACUUM. VACUUM runs in a small ring of buffers and is supposed to have to clean its own dirt most of the time. So it's wrong to blame this on the bgwriter not holding up its end. Rather, what you need to be thinking about is how come vacuum seems to be making lots of pages dirty on only one of these machines. This is an anti-wraparound vacuum, so it could have something to do with the hint bits. Maybe it's setting the freeze bit on every page, and writing them one page at a time? Still don't understand the call to pollsys, even so, though. We often mention that we do vacuum freeze for anti-wraparound vacuum, but not for pg_clog file removal, which is the primary trigger for autovacuum vacuum freezing. I have added the attached documentation patch for autovacuum_freeze_max_age; back-patched to 9.0. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Rob Wultsch wult...@gmail.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Version Numbering
On Sat, Aug 21, 2010 at 1:00 PM, Greg Stark gsst...@mit.edu wrote: On Sat, Aug 21, 2010 at 4:29 AM, Sergio A. Kessler sergiokess...@gmail.com wrote: on every single planet of the universe, except the one called postgrearth, whose inhabitants breathe sql and eat messages from postgresql mailing lists... :-) most people I know, think 8.1 is just 8.0 with some fixes... Really? is Linux 2.6 just 2.5 with some fixes? Glibc 2.Was Windows 3.5 just 3.4 with some fixes? Gnome 2.28 just 2.27 with some fixes? really !, they don't have *any* idea of the version of the kernel, they know about redhat 4, redhat 5 and so on... glibc ? what is that ? is not something they use... :-) sure, we know better, but the common guy in the computer field, does not read every mailing list on earth... In fact perusing dpkg -l output the *only* software package I find that bumps the major version every single release is Emacs. It stands out as an outlier as soon as you say version 23 -- and that was despite a hiatus when version 18.59 was the newest release for years. imho, emacs does it rigth... regards, /sak -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] More vacuum stats
I noticed that we were already tracking the information about when an autovacuum worker was last started in a database, but this information was not exposed. The attached patch puts this column in pg_stat_database. Was there any particular reason why this wasn't exposed before that I've missed, making this a bad addition? :-) Oh, and this time, I *have* included updates to the regression tests. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ vac_db.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] security hook on authorization
(2010/08/22 0:20), Robert Haas wrote: On Aug 20, 2010, at 8:27 PM, KaiGai Koheikai...@kaigai.gr.jp wrote: (2010/08/20 23:34), Robert Haas wrote: 2010/8/19 KaiGai Koheikai...@ak.jp.nec.com: I think our standard criteria for the inclusion of hooks is that you must demonstrate that the hook can be used to do something interesting that couldn't be done without the hook. So far I'm unconvinced. We cannot handle an error of labeled networking (getpeercon(3)), if we don't have any hook during client authorization stage. If and when a connection came from a host but we don't accept the delivered security label, or labeled networking is misconfigured, getpeercon(3) returns NULL. In this case, server cannot identify what label should be applied on the client, then, we should disconnect this connection due to the error on database login, not any access control decision. In similar case, psm_selinux.so disconnect the connection when it cannot identify what security label shall be assigned on the session, due to some reasons such as misconfigurations. Without any hooks at authorization stage (but it might be different place from this patch, of course), we need to delay the error handling by the time when SE-PostgreSQL module is invoked at first. But it is already connection established and user sends a query. It seems to me quite strange behavior. You mentioned that before. I'm not totally sure I buy it, and I think there are other applications that might benefit from a hook in this area. We need to think about trying to do this in a way that is as general as possible. So I'd like to see some analysis of other possible applications. Yes, I also think this kind of authorization hook should benefit other applications, not only label based mac features. For example, something like 'last' command in operations system which records username and login-time. Stephen mentioned pam_tally that locks down certain accounts who failed authentication too much. Perhaps, PAM modules in operating system give us some hints about other possible applications. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fw: patch for pg_ctl.c to add windows service start-type
Sure, I agree.
New patch attached. How about this?
On Fri, 20 Aug 2010 11:21:18 +0200
Magnus Hagander mag...@hagander.net wrote:
On Fri, Aug 20, 2010 at 01:01, Quan Zongliang quanzongli...@gmail.com wrote:
Because Windows's CreateService has serial start-type:
SERVICE_AUTO_START
SERVICE_BOOT_START
SERVICE_DEMAND_START
SERVICE_DISABLED
SERVICE_SYSTEM_START
Although all of them are not useful for pg service.
I think it is better to use enum.
I don't see us ever using anything other than auto or demand. The
others aren't for regular services, except for disabled. And
adding a disabled service makes no sense :-) So I'm with Alvaro, I
think it's a good idea to simplify that.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
--
Quan Zongliang quanzongli...@gmail.com
*** pg_ctl.c.bak2010-04-07 11:48:51.0 +0800
--- pg_ctl.c2010-08-22 20:15:53.37500 +0800
***
*** 121,126
--- 121,127
static void pgwin32_doRunAsService(void);
static intCreateRestrictedProcess(char *cmd, PROCESS_INFORMATION
*processInfo, bool as_service);
+ static DWORD pgctl_start_type = SERVICE_AUTO_START;
static SERVICE_STATUS status;
static SERVICE_STATUS_HANDLE hStatus = (SERVICE_STATUS_HANDLE) 0;
static HANDLE shutdownHandles[2];
***
*** 1147,1153
if ((hService = CreateService(hSCM, register_servicename,
register_servicename,
SERVICE_ALL_ACCESS,
SERVICE_WIN32_OWN_PROCESS,
!
SERVICE_AUTO_START, SERVICE_ERROR_NORMAL,
pgwin32_CommandLine(true),
NULL, NULL, RPCSS\0, register_username, register_password)) ==
NULL)
{
--- 1148,1154
if ((hService = CreateService(hSCM, register_servicename,
register_servicename,
SERVICE_ALL_ACCESS,
SERVICE_WIN32_OWN_PROCESS,
!
pgctl_start_type, SERVICE_ERROR_NORMAL,
pgwin32_CommandLine(true),
NULL, NULL, RPCSS\0, register_username, register_password)) ==
NULL)
{
***
*** 1586,1592
printf(_( %s killSIGNALNAME PID\n), progname);
#if defined(WIN32) || defined(__CYGWIN__)
printf(_( %s register [-N SERVICENAME] [-U USERNAME] [-P PASSWORD]
[-D DATADIR]\n
![-w] [-t SECS] [-o \OPTIONS\]\n),
progname);
printf(_( %s unregister [-N SERVICENAME]\n), progname);
#endif
--- 1587,1593
printf(_( %s killSIGNALNAME PID\n), progname);
#if defined(WIN32) || defined(__CYGWIN__)
printf(_( %s register [-N SERVICENAME] [-U USERNAME] [-P PASSWORD]
[-D DATADIR]\n
![-S START-TYPE] [-w] [-t SECS] [-o
\OPTIONS\]\n), progname);
printf(_( %s unregister [-N SERVICENAME]\n), progname);
#endif
***
*** 1627,1632
--- 1628,1640
printf(_( -N SERVICENAME service name with which to register
PostgreSQL server\n));
printf(_( -P PASSWORD password of account to register PostgreSQL
server\n));
printf(_( -U USERNAME user name of account to register PostgreSQL
server\n));
+ printf(_( -S START-TYPE service start type to register PostgreSQL
server,\n
+ can be auto or demand\n));
+
+ printf(_(\nStart types are:\n));
+ printf(_( auto service start automatically during system
startup\n));
+ printf(_( demand service start on demand\n));
+
#endif
printf(_(\nReport bugs to pgsql-b...@postgresql.org.\n));
***
*** 1696,1701
--- 1704,1728
+ #if defined(WIN32) || defined(__CYGWIN__)
+ static void
+ set_starttype(char *starttypeopt)
+ {
+ if (strcmp(starttypeopt, a) == 0 || strcmp(starttypeopt, auto) == 0)
+ pgctl_start_type = SERVICE_AUTO_START;
+ else if (strcmp(starttypeopt, d) == 0 || strcmp(starttypeopt,
demand) == 0)
+ pgctl_start_type = SERVICE_DEMAND_START;
+ else
+ {
+ write_stderr(_(%s: unrecognized start type \%s\\n),
progname, starttypeopt);
+ do_advice();
+ exit(1);
+ }
+ }
+ #endif
+
+
+
int
main(int argc, char **argv)
{
***
*** 1772,1778
/* process command-line options */
while (optind argc)
{
! while ((c = getopt_long(argc, argv, cD:l:m:N:o:p:P:st:U:wW,
long_options, option_index)) != -1)
{
switch (c)
{
--- 1799,1805
/* process command-line options */
while
Re: [HACKERS] Return of the Solaris vacuum polling problem -- anyone remember this?
Rob Wultsch wrote: For a documentation patch should this not be back ported to all relevant versions? It is only a minor adjustment and I normally don't backpatch that. --- On 8/21/10, Bruce Momjian br...@momjian.us wrote: Josh Berkus wrote: On further reflection, though: since we put in the BufferAccessStrategy code, which was in 8.3, the background writer isn't *supposed* to be very much involved in writing pages that are dirtied by VACUUM. VACUUM runs in a small ring of buffers and is supposed to have to clean its own dirt most of the time. So it's wrong to blame this on the bgwriter not holding up its end. Rather, what you need to be thinking about is how come vacuum seems to be making lots of pages dirty on only one of these machines. This is an anti-wraparound vacuum, so it could have something to do with the hint bits. Maybe it's setting the freeze bit on every page, and writing them one page at a time? Still don't understand the call to pollsys, even so, though. We often mention that we do vacuum freeze for anti-wraparound vacuum, but not for pg_clog file removal, which is the primary trigger for autovacuum vacuum freezing. I have added the attached documentation patch for autovacuum_freeze_max_age; back-patched to 9.0. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Rob Wultsch wult...@gmail.com -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] More vacuum stats
Magnus Hagander escreveu: Was there any particular reason why this wasn't exposed before that I've missed, making this a bad addition? :-) Not that I know of. Good catch. ;) -- Euler Taveira de Oliveira http://www.timbira.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Return of the Solaris vacuum polling problem -- anyone remember this?
Bruce Momjian br...@momjian.us writes: We often mention that we do vacuum freeze for anti-wraparound vacuum, but not for pg_clog file removal, which is the primary trigger for autovacuum vacuum freezing. I have added the attached documentation patch for autovacuum_freeze_max_age; back-patched to 9.0. This patch does not actually seem like an improvement. The paragraph is all about transaction age, but you've inserted something entirely unrelated, and not only that but chosen to make the unrelated thing seem like the major consequence and anti-wraparound an afterthought. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fw: patch for pg_ctl.c to add windows service start-type
On Sun, Aug 22, 2010 at 10:03:32PM +0800, Quan Zongliang wrote:
Sure, I agree.
New patch attached. How about this?
Docs re-added. Please not to leave these out in future patches. :)
Cheers,
David.
--
David Fetter da...@fetter.org http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
diff --git a/doc/src/sgml/ref/pg_ctl-ref.sgml b/doc/src/sgml/ref/pg_ctl-ref.sgml
index 20d87bb..ac30320 100644
--- a/doc/src/sgml/ref/pg_ctl-ref.sgml
+++ b/doc/src/sgml/ref/pg_ctl-ref.sgml
@@ -97,6 +97,12 @@ PostgreSQL documentation
arg-P replaceablepassword/replaceable/arg
arg-D replaceabledatadir/replaceable/arg
arg-w/arg
+ arg-S
+ group choice=plain
+ arga[uto]/arg
+ argd[emand]/arg
+ /group
+ /arg
arg-t replaceableseconds/replaceable/arg
arg-o replaceableoptions/replaceable/arg
sbr
@@ -377,6 +383,18 @@ PostgreSQL documentation
/para
/listitem
/varlistentry
+
+varlistentry
+ termoption-S replaceable
class=parameterstart-type/replaceable/option/term
+ listitem
+ para
+ Start type of the system service to register. start-type can
+ be literalauto/literal, or literaldemand/literal, or
+ the first letter of one of these two. If this is omitted,
+ literalauto/literal is used.
+ /para
+ /listitem
+/varlistentry
/variablelist
/refsect2
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 1caec12..f7de2a5 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -121,6 +121,7 @@ static void WINAPI pgwin32_ServiceMain(DWORD, LPTSTR *);
static void pgwin32_doRunAsService(void);
static int CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION
*processInfo, bool as_service);
+static DWORD pgctl_start_type = SERVICE_AUTO_START;
static SERVICE_STATUS status;
static SERVICE_STATUS_HANDLE hStatus = (SERVICE_STATUS_HANDLE) 0;
static HANDLE shutdownHandles[2];
@@ -1147,7 +1148,7 @@ pgwin32_doRegister(void)
if ((hService = CreateService(hSCM, register_servicename,
register_servicename,
SERVICE_ALL_ACCESS,
SERVICE_WIN32_OWN_PROCESS,
-
SERVICE_AUTO_START, SERVICE_ERROR_NORMAL,
+
pgctl_start_type, SERVICE_ERROR_NORMAL,
pgwin32_CommandLine(true),
NULL, NULL, RPCSS\0, register_username, register_password)) ==
NULL)
{
@@ -1586,7 +1587,7 @@ do_help(void)
printf(_( %s killSIGNALNAME PID\n), progname);
#if defined(WIN32) || defined(__CYGWIN__)
printf(_( %s register [-N SERVICENAME] [-U USERNAME] [-P PASSWORD]
[-D DATADIR]\n
-[-w] [-t SECS] [-o \OPTIONS\]\n),
progname);
+[-S START-TYPE] [-w] [-t SECS] [-o
\OPTIONS\]\n), progname);
printf(_( %s unregister [-N SERVICENAME]\n), progname);
#endif
@@ -1627,6 +1628,11 @@ do_help(void)
printf(_( -N SERVICENAME service name with which to register
PostgreSQL server\n));
printf(_( -P PASSWORD password of account to register PostgreSQL
server\n));
printf(_( -U USERNAME user name of account to register PostgreSQL
server\n));
+ printf(_( -S START-TYPE service start type to register PostgreSQL
server,\n
+ can be auto or demand\n));
+ printf(_(\nStart types are:\n));
+ printf(_( auto service start automatically during system
startup\n));
+ printf(_( demand service start on demand\n));
#endif
printf(_(\nReport bugs to pgsql-b...@postgresql.org.\n));
@@ -1696,6 +1702,25 @@ set_sig(char *signame)
+#if defined(WIN32) || defined(__CYGWIN__)
+static void
+set_starttype(char *starttypeopt)
+{
+ if (strcmp(starttypeopt, a) == 0 || strcmp(starttypeopt, auto) == 0)
+ pgctl_start_type = SERVICE_AUTO_START;
+ else if (strcmp(starttypeopt, d) == 0 || strcmp(starttypeopt,
demand) == 0)
+ pgctl_start_type = SERVICE_DEMAND_START;
+ else
+ {
+ write_stderr(_(%s: unrecognized start type \%s\\n),
progname, starttypeopt);
+ do_advice();
+ exit(1);
+ }
+}
+#endif
+
+
+
int
main(int argc, char **argv)
{
@@ -1772,7 +1797,7 @@ main(int argc, char **argv)
/* process command-line options */
while (optind argc)
{
- while ((c = getopt_long(argc, argv, cD:l:m:N:o:p:P:st:U:wW,
long_options, option_index)) != -1)
+ while ((c = getopt_long(argc, argv,
Re: [HACKERS] More vacuum stats
Magnus Hagander mag...@hagander.net writes: I noticed that we were already tracking the information about when an autovacuum worker was last started in a database, but this information was not exposed. The attached patch puts this column in pg_stat_database. Was there any particular reason why this wasn't exposed before that I've missed, making this a bad addition? :-) I think that's an implementation detail. If we expose it then we'll be forced to track it forevermore, regardless of whether the AV launcher actually needs it in the future. (In particular, the assumption that this is tracked per-database and not per-something-else seems like an artifact of the current AV launching algorithm.) So I'd like to see a positive argument why this is important for users to know, rather than merely we should expose every conceivable detail by default. Why wouldn't a user care more about last AV time for a specific table, which we already do expose? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_archivecleanup debug message consistency
Erik Rijkers e...@xs4all.nl writes: If only for consistency, this patch adds the path info to that message. Seems reasonable, but speaking of consistency: +#ifdef WIN32 + snprintf(WALFilePath, MAXPGPATH, %s\\%s, archiveLocation, exclusiveCleanupFileName); +#else + snprintf(WALFilePath, MAXPGPATH, %s/%s, archiveLocation, exclusiveCleanupFileName); +#endif I see that you copied-and-pasted this pattern from somewhere else in pg_archivecleanup.c, but I'd like to argue that it's out of place there too. We don't go out of our way to show Windows paths with backslashes anywhere in the core code, so why is pg_archivecleanup doing it? I think we should just drop the ifdef and do %s/%s always. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Replacing the pg_get_expr security hack with a datatype solution
On lör, 2010-08-21 at 15:30 -0400, Tom Lane wrote: The only thing that seems like it might need discussion is the name to give the datatype. My first instinct was pg_expr or pg_expression, but there are some cases where this doesn't exactly fit. In particular, pg_rewrite.ev_action contains a whole Query, in fact a list of them. Perhaps pg_node then. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] More vacuum stats
On Sun, Aug 22, 2010 at 17:29, Tom Lane t...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: I noticed that we were already tracking the information about when an autovacuum worker was last started in a database, but this information was not exposed. The attached patch puts this column in pg_stat_database. Was there any particular reason why this wasn't exposed before that I've missed, making this a bad addition? :-) I think that's an implementation detail. If we expose it then we'll be forced to track it forevermore, regardless of whether the AV launcher actually needs it in the future. (In particular, the assumption that this is tracked per-database and not per-something-else seems like an artifact of the current AV launching algorithm.) That's a good point. OTOH, if we removed the feature, it seems it would be reasonable to remove the column from the statistics view as well. That *could* happen in other stats views as well. So I'd like to see a positive argument why this is important for users to know, rather than merely we should expose every conceivable detail by default. Why wouldn't a user care more about last AV time for a specific table, which we already do expose? You need to connect to every database to do that. If you have many databases, that's a lot of overhead particularly if you're doing tihs for regular monitoring. Plus, those views will only track when autovacuum actually *did* something. Being able to see that autovacuum hasn't even touched a database for too long would be an early-indicator that you have some issues with it. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] More vacuum stats
Magnus Hagander mag...@hagander.net writes: On Sun, Aug 22, 2010 at 17:29, Tom Lane t...@sss.pgh.pa.us wrote: So I'd like to see a positive argument why this is important for users to know, rather than merely we should expose every conceivable detail by default. Why wouldn't a user care more about last AV time for a specific table, which we already do expose? You need to connect to every database to do that. If you have many databases, that's a lot of overhead particularly if you're doing tihs for regular monitoring. Plus, those views will only track when autovacuum actually *did* something. Well, the last-launch-time doesn't prove that autovacuum actually *did* something ;-). Being able to see that autovacuum hasn't even touched a database for too long would be an early-indicator that you have some issues with it. With the current AV launch algorithm, unless you have very serious system-wide issues there will be a worker launched into each database approximately every autovacuum_naptime seconds. AFAICS this does not tell you anything interesting about whether AV is getting its work done. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Return of the Solaris vacuum polling problem -- anyone remember this?
Tom Lane wrote: Bruce Momjian br...@momjian.us writes: We often mention that we do vacuum freeze for anti-wraparound vacuum, but not for pg_clog file removal, which is the primary trigger for autovacuum vacuum freezing. I have added the attached documentation patch for autovacuum_freeze_max_age; back-patched to 9.0. This patch does not actually seem like an improvement. The paragraph is all about transaction age, but you've inserted something entirely unrelated, and not only that but chosen to make the unrelated thing seem like the major consequence and anti-wraparound an afterthought. Well, the reason that value is 200 million is for pg_clog cleanup, not for xid wraparound protection. The next sentence does relate to xid wraparound, but it seems to fit because the previous sentence ends with xid wraparound: Note that the system will launch autovacuum processes to prevent wraparound even when autovacuum is otherwise disabled. If we were worried about just xid wraparound I assume the value would be 2 billion. Do you have a suggestion? Reorder the items? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] security label support, part.2
On tis, 2010-08-17 at 20:04 -0400, Stephen Frost wrote: What I'm thinking of is something like a warning if the permissions on the child don't match those of the parent when the relationship is created, or maybe forcibly setting the permissions on the child (with a NOTICE), so it's at least clear what is going on. Or perhaps, set the permissions on the child only if it doesn't have permissions (with the NOTICE), and issue a WARNING if the child already has permissions set. Perhaps also a WARNING if someone changes the permissions on a child after the relationship has been created too, but let it happen in case someone really wants it.. I think there are perfectly good reasons to have different permissions on parent and child tables. I don't see any reason to monkey around with that. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_archivecleanup debug message consistency
On Sun, August 22, 2010 17:54, Tom Lane wrote:
Erik Rijkers e...@xs4all.nl writes:
If only for consistency, this patch adds the path info to that message.
Seems reasonable, but speaking of consistency:
+#ifdef WIN32
+snprintf(WALFilePath, MAXPGPATH, %s\\%s,
archiveLocation, exclusiveCleanupFileName);
+#else
+snprintf(WALFilePath, MAXPGPATH, %s/%s,
archiveLocation, exclusiveCleanupFileName);
+#endif
I see that you copied-and-pasted this pattern from somewhere else in
pg_archivecleanup.c, but I'd like to argue that it's out of place there
too. We don't go out of our way to show Windows paths with backslashes
anywhere in the core code, so why is pg_archivecleanup doing it? I
think we should just drop the ifdef and do %s/%s always.
yes, I agree that's better; attached is that change.
And it works fine on linux; but I am not in a position to try it on windows.
Erik Rijkers
--- contrib/pg_archivecleanup/pg_archivecleanup.c.orig 2010-08-22 18:31:03.0 +0200
+++ contrib/pg_archivecleanup/pg_archivecleanup.c 2010-08-22 18:33:12.0 +0200
@@ -117,12 +117,7 @@
strspn(xlde-d_name, 0123456789ABCDEF) == XLOG_DATA_FNAME_LEN
strcmp(xlde-d_name + 8, exclusiveCleanupFileName + 8) 0)
{
-#ifdef WIN32
-snprintf(WALFilePath, MAXPGPATH, %s\\%s, archiveLocation, xlde-d_name);
-#else
snprintf(WALFilePath, MAXPGPATH, %s/%s, archiveLocation, xlde-d_name);
-#endif
-
if (debug)
fprintf(stderr, %s: removing file \%s\\n,
progname, WALFilePath);
@@ -308,8 +303,10 @@
SetWALFileNameForCleanup();
if (debug)
- fprintf(stderr, %s: keep WAL file \%s\ and later\n,
-progname, exclusiveCleanupFileName);
+ {
+ snprintf(WALFilePath, MAXPGPATH, %s/%s, archiveLocation, exclusiveCleanupFileName);
+ fprintf(stderr, %s: keep WAL file \%s\ and later\n, progname, WALFilePath);
+ }
/*
* Remove WAL files older than cut-off
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] UTF16 surrogate pairs in UTF8 encoding
I just noticed that we are now advertising the ability to insert UTF16 surrogate pairs in strings and identifiers (see section 4.1.2.2 in current docs, in particular). Is this really wise? I thought that surrogate pairs were specifically prohibited in UTF8 strings, because of the security hazards implicit in having more than one way to represent the same code point. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] security label support, part.2
* Peter Eisentraut (pete...@gmx.net) wrote: I think there are perfectly good reasons to have different permissions on parent and child tables. I don't see any reason to monkey around with that. Even though the permissions on the child table aren't invovled at all if queried through the parent..? The parent implicitly adds to the set of privileges which are granted on the child, but that's not clear at all from the permissions visible on the child. That's principally what I'm complaining about here. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] UTF16 surrogate pairs in UTF8 encoding
On sön, 2010-08-22 at 14:29 -0400, Tom Lane wrote: I just noticed that we are now advertising the ability to insert UTF16 surrogate pairs in strings and identifiers (see section 4.1.2.2 in current docs, in particular). Is this really wise? I thought that surrogate pairs were specifically prohibited in UTF8 strings, because of the security hazards implicit in having more than one way to represent the same code point. We combine the surrogate pair components to a single code point and encode that in UTF-8. We don't encode the components separately; that would be wrong. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] UTF16 surrogate pairs in UTF8 encoding
Peter Eisentraut pete...@gmx.net writes: On sön, 2010-08-22 at 14:29 -0400, Tom Lane wrote: I just noticed that we are now advertising the ability to insert UTF16 surrogate pairs in strings and identifiers (see section 4.1.2.2 in current docs, in particular). Is this really wise? I thought that surrogate pairs were specifically prohibited in UTF8 strings, because of the security hazards implicit in having more than one way to represent the same code point. We combine the surrogate pair components to a single code point and encode that in UTF-8. We don't encode the components separately; that would be wrong. Oh, OK. Should the docs make that a bit clearer? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] security label support, part.2
On sön, 2010-08-22 at 15:08 -0400, Stephen Frost wrote: * Peter Eisentraut (pete...@gmx.net) wrote: I think there are perfectly good reasons to have different permissions on parent and child tables. I don't see any reason to monkey around with that. Even though the permissions on the child table aren't invovled at all if queried through the parent..? The parent implicitly adds to the set of privileges which are granted on the child, but that's not clear at all from the permissions visible on the child. That's principally what I'm complaining about here. Perhaps this is a user interface issue then. Maybe the fact that a table is inherited from another one needs to be shown closer to whereever the permissions are listed. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] security label support, part.2
* Peter Eisentraut (pete...@gmx.net) wrote: On sön, 2010-08-22 at 15:08 -0400, Stephen Frost wrote: Even though the permissions on the child table aren't invovled at all if queried through the parent..? The parent implicitly adds to the set of privileges which are granted on the child, but that's not clear at all from the permissions visible on the child. That's principally what I'm complaining about here. Perhaps this is a user interface issue then. Maybe the fact that a table is inherited from another one needs to be shown closer to whereever the permissions are listed. That's a nice idea, except that we've got a pretty well defined API regarding how to determine what the privileges on a table are, and many different UIs which use it. Fixing it in psql (if it needs to be.. iirc, \d or \d+ may already show it) doesn't really address the problem, in my view. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] pg_archivecleanup debug message consistency
Erik Rijkers e...@xs4all.nl writes: yes, I agree that's better; attached is that change. Looks good, applied to HEAD and 9.0. (I also snuck in a couple of cosmetic cleanups while I was looking at the file.) regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
