Re: [HACKERS] Simple postgresql.conf wizard
Jonah H. Harris wrote: On Thu, Nov 13, 2008 at 3:20 PM, Grzegorz Jaskiewicz <[EMAIL PROTECTED]> wrote: If that's the situation, me thinks you guys have to start thinking about some sort of automated way to increase this param per column as needed. Is there any way planner could actually tell, that it would do better job with more stats for certain column ? Other systems do it. For example, Oracle tracks column usage and attempts to determine the optimal statistics for that column (based on the queries that used it) on an iterative basis. We don't track column usage at all, so that option wouldn't be quite that easy to implement. Though, there are certain things ANALYZE would be able to determine with a little help, such as knowing to collect more samples for columns it finds extremely skewed data in. That kind of feedback loops are a bit dangerous. For starters, it would mean that your test system would behave differently than your production system, just because you run different queries on it. There's also all kinds of weird dynamic behaviors that could kick in. For example, a query could run fine for the first few hundred times, but then the analyzer notices that a certain column is being accessed frequently and decides to increase the stats target for it, which changes the plan, for worse. Usually the new plan would be better, but the planner isn't perfect. There are other things that could be done as well... so the answer is, yes. Yes, just have to be careful.. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] WIP: Column-level Privileges
Stephen Frost wrote: > Markus, > > * Markus Wanner ([EMAIL PROTECTED]) wrote: > > Stephen Frost wrote: > > > Attached patch has this fixed and still passes all regression tests, > > > etc. > > > > Do you have an up-to-date patch laying around? The current one conflicts > > with some CVS tip changes. > > No, not yet. I suspect the array_agg patch is conflicting (which is a > good thing, really) and the addition of array_agg by my patch can now be > removed. Testing should be done to ensure nothing changed wrt output, > of course, but I'm not too worried. Yes, it has a conflict with the array_agg patch. I had some time on my hands today so I stole the part of the patch that dealt with pg_attribute tuples, tinkered with it a bit, and committed it. So now your patch conflicts with more stuff :-( I'll probably fix both things and submit an updated version tomorrow. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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] DirectFunctionCall3 and array_in
Ashish Kamra <[EMAIL PROTECTED]> writes: > I was trying to call the array_in() function using the > DirectFunctionCall3() interface. It fails as the code in array_in() > tries to refer to fcinfo->flinfo->fnextra where flinfo is set to NULL by > the DirectFunctionCall3() interface. I am not sure if this is a bug or > that we are not supposed to use DirectFunctionCall3 to call array_in. You should be using InputFunctionCall to invoke any datatype input function. There are plenty of examples to follow in the standard PLs. 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] Simple postgresql.conf wizard
>> In any case, saying that somebody is certifiably insane in a public >> forum is at best questionable. I would like to see the comment >> withdrawn. > > I'm not too nervous that Josh might have actually thought I thought he was > really insane. (Or for that matter that anyone else reading it might have > thought so.) > > On the other hand what does occur to me in retrospect is that I regret that I > didn't think about how I was disparaging the importance of mental illness and > hope nobody took offense for that reason. I hope so too, but I think we're taking this way too seriously. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] CREATE AGGREGATE disallows STYPE = internal
So I went off to convert contrib/intagg to a wrapper around the new core functions, along this line: CREATE OR REPLACE FUNCTION int_agg_state (internal, int4) RETURNS internal AS 'array_agg_transfn' LANGUAGE INTERNAL; CREATE OR REPLACE FUNCTION int_agg_final_array (internal) RETURNS int4[] AS 'array_agg_finalfn' LANGUAGE INTERNAL; CREATE AGGREGATE int_array_aggregate ( BASETYPE = int4, SFUNC = int_agg_state, STYPE = internal, FINALFUNC = int_agg_final_array ); But it doesn't work: psql:int_aggregate.sql:27: ERROR: aggregate transition data type cannot be internal I thought about declaring the transition functions with some other datatype, but that seemed entirely unsafe. Now CREATE AGGREGATE has fairly good reason to reject internal as the transition type, since nodeAgg.c doesn't really know how to copy that type around --- but we're effectively *exploiting* that fact in the new array_agg stuff, as per the comment I added: /* * We cheat quite a lot here by assuming that a pointer datum will be * preserved intact when nodeAgg.c thinks it is a value of type "internal". * This will in fact work because internal is stated to be pass-by-value * in pg_type.h, and nodeAgg will never do anything with a pass-by-value * transvalue except pass it around in Datum form. But it's mighty * shaky seeing that internal is also stated to be 4 bytes wide in * pg_type.h. If nodeAgg did put the value into a tuple this would * crash and burn on 64-bit machines. */ So it seems like maybe we should open up the same technique to writers of add-on modules. You can certainly screw yourself up by connecting some incompatible internal-accepting functions together this way. So what I propose is that we allow STYPE = internal to be specified in CREATE AGGREGATE, but only by superusers, who are trusted not to create security holes anyway. Comments? 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] array_agg and array_accum (patch)
It looks to me like section 34.10 of the docs might benefit from some sort of update in light of this patch, since the builtin array_agg now does the same thing as the proposed user-defined array_accum, only better. Presumably we should either pick a different example, or add a note that a builtin is available that does the same thing more efficiently. ...Robert On Thu, Nov 13, 2008 at 11:07 AM, Peter Eisentraut <[EMAIL PROTECTED]> wrote: > Jeff Davis wrote: >> >> Here's an updated patch for just array_accum() with some simple docs. > > I have committed a "best of Robert Haas and Jeff Davis" array_agg() function > with standard SQL semantics. I believe this gives the best consistency with > other aggregate functions for the no-input-rows case. If some other behavior > is wanted, it is a coalesce() away, as the documentation states. > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > -- 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] Simple postgresql.conf wizard
Simon Riggs <[EMAIL PROTECTED]> writes: > Your factual comments are accurate, but for Josh's stated target of Data > Warehousing, a stats target of 400 is not unreasonable in some cases. > What you forget to mention is that sample size is also determined by > stats target and for large databases this can be a more important > consideration than the points you mention. Even for data warehousing I would not recommend setting it as a *default* statistics target, at least not without verifying that it doesn't cause any problems. I would certainly consider 400 reasonable for specific columns. But the default statistics target controls how large a histogram to store for *every* column. Even columns never used by any clauses or used by clauses which do not have any indexes on them. Actually a plausible argument could be made that for data warehousing databases in particular large values of default_statistics_target are especially damaging. Queries on these databases are likely to have a large number of clauses which are not indexed and a large number of joins with complex join clauses. Not every data warehouse query runs for hours, what I'm afraid of is potentially the first time someone pops up complaining how Postgres sucks because it randomly takes minutes to plan their queries. Only to find it's retrieving kilobytes of data from toasted statistics arrays and performing n^2 comparisons of that data. > In any case, saying that somebody is certifiably insane in a public > forum is at best questionable. I would like to see the comment > withdrawn. I'm not too nervous that Josh might have actually thought I thought he was really insane. (Or for that matter that anyone else reading it might have thought so.) On the other hand what does occur to me in retrospect is that I regret that I didn't think about how I was disparaging the importance of mental illness and hope nobody took offense for that reason. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! -- 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] Updated posix fadvise patch v19
I was pretty leery about reviewing this one due to the feeling that I might well be in over my head, but they talked me into it, so here goes nothin'. Apologies in advance for any deficiencies in this review. - Overall, this looks pretty clean. The style appears to be consistent with the surrounding code, the patch applies with only minor fuzz, there is not much cruft in the diff (I found a few very minor unnecessary hunks, see department of nitpicking below) and the whole thing compiles and passes regression tests. Also, while I'm not totally qualified to judge the success of your efforts, it appears that you've attempted to make the changes in a way that preserves the existing abstractions, which is good. - However, having said that, it looks as if there is still a bit of experimentation going on in terms of what you actually want the patch to do. There are a couple of things that say FIXME or XXX, and at least one diff hunk that adds code surrounded by #if 0 (in FileRead). Maybe committing FIXME is OK, but certainly #if 0 isn't, so there's at least a bit of work needed here to put this in its final form, though I think perhaps not that much. As you mentioned when posting this version of the patch, it does two unrelated things only one of which you are confident is beneficial. I suggest splitting this into two patches and trying to get the prefetch part committed first. I think that would make for easier review, too. - I dislike cluttering up EXPLAIN ANALYZE with even more crap. On the flip side, I am loathe to take away any sort of instrumentation that might be helpful. I think we need to revamp the syntax of EXPLAIN [ANALYZE] to support some kind of option list, so that users can request the information they care about and not be overwhelmed by what they don't care about. (ISTR that a similar proposal was made with regard to VACUUM some time ago... perhaps the same ideas could be applied to both.) That would need to be done as a separate patch, so maybe we shouldn't worry about it here. - It's not clear to me whether there is a reason why we are only adding instrumentation for bitmap heap scan; would not other scan types benefit from something similar? If we're not going to add instrumentation for all the cases that can benefit from it, then we should just rip it out. - StrategyFileStrategy doesn't handle the recently added BAS_BULKWRITE strategy. I'm not sure whether it needs to, but it seems to me that this a trap for the unwary: we should probably add a comment where the BAS_* constants are defined warning that any changes here may/will also necessitate changes there. I think a detailed comment on the function itself explaining why it does what it does and how to decide what to do for a new type of BufferAccessStrategy would be a good idea. - I am mildly concerned that we are overusing the word Strategy. The purpose of StrategyFileStrategy is, of course, to take a BufferAccessStrategy (which is an object) and return a FILE_STRATEGY_* constant. But someone might not find that entirely evident from the name. If we're going to have multiple things floating around that are different from each other but all called strategies, ISTM we should name functions etc. to make clear which one we're talking about. Or else pick a different word. Maybe for now it's sufficient to rename this function to AccessStrategyGetFileStrategy, or something. - The WARNING at the top of PrefetchBuffer() seems strange. Is that really only a WARNING? We just continue anyway? Department of nitpicking: - The very first comment change in src/backend/commands/explain.c is apparently extraneous. - guc.c misspells the work "concurrent" as "concurrenct". - The first diff hunk in each of fd.h and smgr.h is an unnecessary whitespace change. - nodeBitmapHeapscan.c adds an ELOG at DEBUG2 - do we really want this, or is it leftover debugging code? ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] DirectFunctionCall3 and array_in
I was trying to call the array_in() function using the DirectFunctionCall3() interface. It fails as the code in array_in() tries to refer to fcinfo->flinfo->fnextra where flinfo is set to NULL by the DirectFunctionCall3() interface. I am not sure if this is a bug or that we are not supposed to use DirectFunctionCall3 to call array_in. Anyway, I debugged some array related to queries to find that the following function sequence is used to call array_in ... OidInputFunctionCall InputFunctionCall ... For the time being I will use this, but can someone clarify if what I stated above is a problem? Thanks, Ashish -- 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] Updated posix fadvise patch v19
> - StrategyFileStrategy doesn't handle the recently added BAS_BULKWRITE > strategy. I'm not sure whether it needs to, but it seems to me that > this a trap for the unwary: we should probably add a comment where the > BAS_* constants are defined warning that any changes here may/will > also necessitate changes there. I think a detailed comment on the > function itself explaining why it does what it does and how to decide > what to do for a new type of BufferAccessStrategy would be a good > idea. In fact, now that I look at this a little further, I see that in general you've not added comments at the beginnings of functions - for example, the other functions in the files that contain smgrprefetch, mdprefetch, PrefetchBuffer seem to have a description of the purpose of those functions; the ones you've added do not. Good luck, I'd like to see this one get in - the performance results you've reported sound very impressive. ...Robert -- 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] array_agg and array_accum (patch)
> It seems that it would be an easy evening's work to implement unnest(), > at least in the simple form >function unnest(anyarray) returns setof anyelement > > without the WITH ORDINALITY syntax proposed by the SQL spec. Then > we could eliminate intagg's C code altogether, and just write it > as a couple of wrapper functions. > > Does anyone have an objection to doing that? I think it would be great. ...Robert -- 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] Simple postgresql.conf wizard
On Thu, 13 Nov 2008, Robert Haas wrote: listen_addresses = '*' This doesn't seem like a good thing to autogenerate from a security perspective. I think we should not attempt to guess the user's requirements in this area. Yeah, I don't want to be the guy who flips the switch for being less secure by default. Particularly because it's unlikely to do anything by itself--need some changes to pg_hba.conf in most cases. However, not setting listen_addresses to something useful is a common newbie problem. I was thinking of producing a warning to standard error with some suggestions if listen_addresses isn't set to the usual '*', but not actually changing the setting. max_fsm_pages = DBsize / PageSize / 8 Isn't this moot for 8.4? At some point this is going to target earlier versions as well so it's good to have that intelligence in the app, even if it ends up not being a setting that is altered. -- * Greg Smith [EMAIL PROTECTED] http://www.gregsmith.com Baltimore, MD -- 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] Simple postgresql.conf wizard
On Thu, 2008-11-13 at 18:07 -0500, Greg Smith wrote: > On Thu, 13 Nov 2008, Josh Berkus wrote: > Since Josh's latest parameter model takes a database size as an input, > perhaps a reasonable way to proceed here is to put the DW model into size > tiers. Something like this: > > DW default_statistics_target: > > db size setting > <1GB 10 > 1GB-10GB 50 > 10GB-100GB100 > 100GB-1TB 200 > >1TB 400 > > Going along with my idea that this tool should produce a reasonable result > with minimal data, I was thinking of making the database size default to > 10GB if there isn't any input given there. That would give someone who > specified DW but nothing else a result of 100, which seems a less > controversial setting. > Why are we building wizards for settings that will be configured by experts? I thought the idea here was: Simple postgresql.conf wizard If you are running a DW you are beyond the point of this tool are you not? Joshua D. Drake > -- > * Greg Smith [EMAIL PROTECTED] http://www.gregsmith.com Baltimore, MD > -- -- 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] Simple postgresql.conf wizard
On Thu, 2008-11-13 at 17:21 -0500, Greg Smith wrote: > On Thu, 13 Nov 2008, Josh Berkus wrote: > > BTW, I think this is still in enough flux that we really ought to make > > it a pgfoundry project. I don't think we'll have anything ready for 8.4 > > contrib. > > I find your lack of faith disturbing. I'll have another rev that > incorporates all your feedback done within a week. There are some pretty > hairy patches still active in this final 'Fest. I think I'll have the > simple version feature complete, documented, and have already done a round > of polishing at least a week or two before all that work wraps up. > I agree if we stick to the actual point (simple postgresql.conf wizard) then we shouldn't have any worry of getting it in. Joshua D. Drake > -- > * Greg Smith [EMAIL PROTECTED] http://www.gregsmith.com Baltimore, MD > -- -- 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] array_agg and array_accum (patch)
Tom Lane wrote: > The original reason for doing this work, I think, was to let us > deprecate contrib/intagg, or at least turn it into a thin wrapper > around core-provided functionality. We now have the means to do that > for int_array_aggregate, but what about int_array_enum? And what about the patch to add sorted-array versions of some routines? -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] Simple postgresql.conf wizard
Josh Berkus wrote: > Thanks for defending me. I think Greg threw that at me because he knows > I'm very difficult to offend, though. I assume that Greg wouldn't make a > post like that to other members of the community. I would print it and frame it to hang somewhere in the office ... or maybe get some business cards with it. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] Block-level CRC checks
Tom Lane wrote: > Still, I agree that the whole thing looks too Rube Goldbergian to count > as a reliability enhancer, which is what the point is after all. Agreed. > I think the argument is about whether we increase our vulnerability to > torn-page problems if we just add a CRC and don't do anything else to > the overall writing process. Right now, a partial write on a > hint-bit-only update merely results in some hint bits getting lost > (as long as you discount the scenario where the disk fails to read a > partially-written sector at all --- maybe we're fooling ourselves to > ignore that?). With a CRC added, that suddenly becomes a corrupted-page > situation, and it's not easy to tell that no real harm was done. The first idea that comes to mind is skipping hint bits in the CRC too. That does away with a lot of the trouble (PD_UNLOGGED_CHANGE, the necessity of WAL-logging hint bits, etc). The problem, again, is that the checksumming process becomes page type-specific; but then maybe that's the only workable approach. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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] WIP: Column-level Privileges
Markus, * Markus Wanner ([EMAIL PROTECTED]) wrote: > Stephen Frost wrote: > > Attached patch has this fixed and still passes all regression tests, > > etc. > > Do you have an up-to-date patch laying around? The current one conflicts > with some CVS tip changes. No, not yet. I suspect the array_agg patch is conflicting (which is a good thing, really) and the addition of array_agg by my patch can now be removed. Testing should be done to ensure nothing changed wrt output, of course, but I'm not too worried. I can probably update it this weekend, depending on how things are going with the newborn (she's only 4 days old at this point, after all.. :). Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] pg_dump roles support
On 2008-11-08 09:25, Benedek László wrote:
Does this work if the role name contains a ' ?
Right, this one fails with ' in the role name. An update coming soon
closing this issue.
Here is an updated patch, which deals with 's in the rolename.
Please review.
doc/src/sgml/ref/pg_dump.sgml| 16 +
doc/src/sgml/ref/pg_dumpall.sgml | 15
src/bin/pg_dump/pg_backup.h |2 +
src/bin/pg_dump/pg_backup_archiver.c | 35 ++-
src/bin/pg_dump/pg_dump.c| 60
+-
src/bin/pg_dump/pg_dumpall.c | 23 +
6 files changed, 148 insertions(+), 3 deletions(-)
Thank you, regards
Benedek Laszlo
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 2e30906..5e4c3e0 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -698,6 +698,22 @@ PostgreSQL documentation
+
+
+ --role=rolename
+
+
+Specifies the user identifier used by the dump session. This cause
+pg_dump to issue a
+SET role = rolename
+command just after a successful database connection. It is useful in cases when
+the logged in user specified by the -U option has not enough privileges needed by
+pg_dump but can switch to a role with the needed rights.
+The SET ROLE command is reserved in the archive because most of the time this
+user identifier also needed for the restore to succeed.
+
+
+
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index ec40890..640723d 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -417,6 +417,21 @@ PostgreSQL documentation
+
+
+ --role=rolename
+
+
+Specifies the user identifier used by the dump session. This option is passed
+to pg_dump too and cause these applications to issue a
+SET role = rolename
+command just after a successful database connection. It is useful in cases when
+the logged in user specified by the -U option has not enough privileges needed by
+pg_dumpall but can switch to a role with the needed rights.
+The SET ROLE command is reserved in the archive by pg_dump.
+
+
+
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index c57bb22..c9e7e72 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -70,6 +70,8 @@ typedef struct _Archive
int encoding; /* libpq code for client_encoding */
bool std_strings; /* standard_conforming_strings */
+ char *rolename; /* role name in escaped form */
+
/* error handling */
bool exit_on_error; /* whether to exit on SQL errors... */
int n_errors; /* number of errors (if no die) */
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 7bd44f2..53bbfdf 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -56,6 +56,7 @@ static void _selectOutputSchema(ArchiveHandle *AH, const char *schemaName);
static void _selectTablespace(ArchiveHandle *AH, const char *tablespace);
static void processEncodingEntry(ArchiveHandle *AH, TocEntry *te);
static void processStdStringsEntry(ArchiveHandle *AH, TocEntry *te);
+static void processRolenameEntry(ArchiveHandle *AH, TocEntry *te);
static teReqs _tocEntryRequired(TocEntry *te, RestoreOptions *ropt, bool include_acls);
static void _disableTriggersIfNecessary(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt);
static void _enableTriggersIfNecessary(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt);
@@ -1979,6 +1980,8 @@ ReadToc(ArchiveHandle *AH)
processEncodingEntry(AH, te);
else if (strcmp(te->desc, "STDSTRINGS") == 0)
processStdStringsEntry(AH, te);
+ else if (strcmp(te->desc, "ROLENAME") == 0)
+ processRolenameEntry(AH, te);
}
}
@@ -2026,14 +2029,38 @@ processStdStringsEntry(ArchiveHandle *AH, TocEntry *te)
te->defn);
}
+static void
+processRolenameEntry(ArchiveHandle *AH, TocEntry *te)
+{
+ /* te->defn should have the form SET role = "foo"; */
+ char *defn = strdup(te->defn);
+ char *ptr1;
+ char *ptr2 = NULL;
+
+ ptr1 = strchr(defn, '"');
+ if (ptr1)
+ ptr2 = strrchr(ptr1+1, '"');
+ if (ptr2)
+ {
+ *++ptr2 = '\0';
+ AH->public.rolename = strdup(ptr1);
+ }
+ else
+ die_horribly(AH, modulename, "invalid ROLENAME item: %s\n",
+ te->defn);
+
+ free(defn);
+}
+
static teReqs
_tocEntryRequired(TocEntry *te, RestoreOptions *ropt, bool include_acls)
{
teReqs res = REQ_ALL;
- /* ENCODING and STDSTRINGS items are dumped specially, so always reject */
+ /* ENCODING, STDSTRINGS and ROLENAME items are dumped specially, so always reject */
if (strcmp(te->desc, "ENCODING") == 0 ||
- strcmp(te->desc, "STDSTRINGS") == 0)
+
Re: [HACKERS] array_agg and array_accum (patch)
Peter Eisentraut <[EMAIL PROTECTED]> writes: > Jeff Davis wrote: >> Here's an updated patch for just array_accum() with some simple docs. > I have committed a "best of Robert Haas and Jeff Davis" array_agg() > function with standard SQL semantics. I believe this gives the best > consistency with other aggregate functions for the no-input-rows case. > If some other behavior is wanted, it is a coalesce() away, as the > documentation states. The original reason for doing this work, I think, was to let us deprecate contrib/intagg, or at least turn it into a thin wrapper around core-provided functionality. We now have the means to do that for int_array_aggregate, but what about int_array_enum? It seems that it would be an easy evening's work to implement unnest(), at least in the simple form function unnest(anyarray) returns setof anyelement without the WITH ORDINALITY syntax proposed by the SQL spec. Then we could eliminate intagg's C code altogether, and just write it as a couple of wrapper functions. Does anyone have an objection to doing that? 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] Simple postgresql.conf wizard
>listen_addresses = '*' This doesn't seem like a good thing to autogenerate from a security perspective. I think we should not attempt to guess the user's requirements in this area. >max_fsm_pages = DBsize / PageSize / 8 Isn't this moot for 8.4? ...Robert -- 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] Simple postgresql.conf wizard
On Thu, 13 Nov 2008, Josh Berkus wrote: Even though we all agree default_statistics_target = 10 is too low, proposing a 40X increase in the default value requires more evidence than this. In particular, the prospect of a 1600-fold increase in the typical cost of eqjoinsel() is a mite scary. It's a *completely* acceptable tradeoff for a *data warehousing* database, where queries take multiple seconds to execute even under the best plans ... and minutes or hours for the worst. And that's what I'm proposing a value of 400 for The idea that planning time is trivial compared with query runtime in a data warehouse application is certainly true. I remain a bit concerned about making the target so large for everyone just because they picked that option though. I'd hate to see somebody who doesn't quite understand what that term implies get their plan times exploding. Since Josh's latest parameter model takes a database size as an input, perhaps a reasonable way to proceed here is to put the DW model into size tiers. Something like this: DW default_statistics_target: db size setting <1GB 10 1GB-10GB50 10GB-100GB 100 100GB-1TB 200 1TB 400 Going along with my idea that this tool should produce a reasonable result with minimal data, I was thinking of making the database size default to 10GB if there isn't any input given there. That would give someone who specified DW but nothing else a result of 100, which seems a less controversial setting. -- * Greg Smith [EMAIL PROTECTED] http://www.gregsmith.com Baltimore, MD -- 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] Simple postgresql.conf wizard
Simon, In any case, saying that somebody is certifiably insane in a public forum is at best questionable. I would like to see the comment withdrawn. Thanks for defending me. I think Greg threw that at me because he knows I'm very difficult to offend, though. I assume that Greg wouldn't make a post like that to other members of the community. --Josh -- 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] Simple postgresql.conf wizard
Tom, Even though we all agree default_statistics_target = 10 is too low, proposing a 40X increase in the default value requires more evidence than this. In particular, the prospect of a 1600-fold increase in the typical cost of eqjoinsel() is a mite scary. It's a *completely* acceptable tradeoff for a *data warehousing* database, where queries take multiple seconds to execute even under the best plans ... and minutes or hours for the worst. And that's what I'm proposing a value of 400 for, if you read my posting rather than just Greg's outraged response. (and yes, I've done this for multiple *production* data warehouses and the results have been good) For a web database, keep it at 10. It might turn out that an increase to 25 or 50 would benefit even web applications, but we don't yet have the testing resources to determine that. Of *course* it would be better for the DBA to go through and set statistics column-by-column. But few will. --Josh -- 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] Block-level CRC checks
Martijn van Oosterhout <[EMAIL PROTECTED]> writes: > Actually, the real problem to me seems to be that to check the checksum > when you read the page in, you need to look at the contents of the page > and "assume" some of the values in there are correct, before you can > even calculate the checksum. If the page really is corrupted, chances > are the item pointers are going to be bogus, but you need to read them > to calculate the checksum... Hmm. You could verify the values closely enough to ensure you don't crash while redoing the CRC calculation, which ought to be sufficient. Still, I agree that the whole thing looks too Rube Goldbergian to count as a reliability enhancer, which is what the point is after all. > Double-buffering allows you to simply checksum the whole page, so > creating a COMP_CRC32_WITH_COPY() macro would do it. Just allocate a > block on the stack, copy/checksum it there, do the write() syscall and > forget it. I think the argument is about whether we increase our vulnerability to torn-page problems if we just add a CRC and don't do anything else to the overall writing process. Right now, a partial write on a hint-bit-only update merely results in some hint bits getting lost (as long as you discount the scenario where the disk fails to read a partially-written sector at all --- maybe we're fooling ourselves to ignore that?). With a CRC added, that suddenly becomes a corrupted-page situation, and it's not easy to tell that no real harm was done. Again, the real bottom line here is whether there will be a *net* gain in reliability. If a CRC adds too many false-positive reports of bad data, it's not going to be a win. 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] Simple postgresql.conf wizard
On Thu, 2008-11-13 at 20:33 +0100, Greg Stark wrote: > A statistic target of 400 fir a specific column may make sense but > even then I would recommend monitoring performance to ensure it > doesn't cause problems. As a global setting it's, IMHO, ridiculous. > > Even for the smaller data types (except boolean and "char") and array > of 400 will be large enough to be toasted. Planning queries will > involve many more disk I/Os than some of those queries end up taking > themselves. Even for stats which are already cached there are some > algorithms in the planner known to be inefficient for large arrays. > > It may make sense for specific skewed columns with indexes on them, > but keep in mind postgres needs to consult the statistics on any > column referenced in a qual even if there are no indexes and for most > data distributions do fine with a target of 10. Your factual comments are accurate, but for Josh's stated target of Data Warehousing, a stats target of 400 is not unreasonable in some cases. What you forget to mention is that sample size is also determined by stats target and for large databases this can be a more important consideration than the points you mention. In any case, saying that somebody is certifiably insane in a public forum is at best questionable. I would like to see the comment withdrawn. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- 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] Simple postgresql.conf wizard
On Thu, 13 Nov 2008, Josh Berkus wrote: don't bother with os.sysconf, or make it optional and error-trap it. Right, I've moved in that direction in the updated rev I already sent--memory is an input value, but if you leave it out it tries to guess. Just need a bit more error trapping around that I think, then I can add Windows support too. A goal I don't think is unreachable here is to give a useful middle of the road tuning on most platforms if you just run it but don't tell it anything. In the advanced version, we'll also want to ask... I'm pretty focused right now only on getting a simple version solidified, to keep the scope under control. Thanks for the more focused parameter suggestions. The spreadsheet version I had from you was a bit too complicated to work with, this reformulation is much easier to automate. Because this comes up so often, we should output to a seperate file a set of sysctl.conf lines to support SysV memory, depending on OS. Agreed, will add that to the required feature list. BTW, I think this is still in enough flux that we really ought to make it a pgfoundry project. I don't think we'll have anything ready for 8.4 contrib. I find your lack of faith disturbing. I'll have another rev that incorporates all your feedback done within a week. There are some pretty hairy patches still active in this final 'Fest. I think I'll have the simple version feature complete, documented, and have already done a round of polishing at least a week or two before all that work wraps up. -- * Greg Smith [EMAIL PROTECTED] http://www.gregsmith.com Baltimore, MD -- 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] Block-level CRC checks
On Thu, Nov 13, 2008 at 01:45:52PM -0500, Tom Lane wrote: > Alvaro Herrera <[EMAIL PROTECTED]> writes: > > I'm still testing this; please beware that this likely has an even > > higher bug density than my regular patches (and some debugging printouts > > as well). > > This seems impossibly fragile ... and the non-modular assumptions about > what is in a disk page aren't even the worst part :-(. The worst part > is the race conditions. Actually, the real problem to me seems to be that to check the checksum when you read the page in, you need to look at the contents of the page and "assume" some of the values in there are correct, before you can even calculate the checksum. If the page really is corrupted, chances are the item pointers are going to be bogus, but you need to read them to calculate the checksum... Double-buffering allows you to simply checksum the whole page, so creating a COMP_CRC32_WITH_COPY() macro would do it. Just allocate a block on the stack, copy/checksum it there, do the write() syscall and forget it. Have a nice day, -- Martijn van Oosterhout <[EMAIL PROTECTED]> http://svana.org/kleptog/ > Please line up in a tree and maintain the heap invariant while > boarding. Thank you for flying nlogn airlines. signature.asc Description: Digital signature
Re: [HACKERS] auto_explain contrib moudle
On Thu, 2008-11-13 at 14:31 -0500, Tom Lane wrote: > Jeff Davis <[EMAIL PROTECTED]> writes: > > Thanks! This patch is ready to go, as far as I'm concerned. > > This patch seems to contain a subset of the "contrib infrastructure" > patch that's listed separately on the commitfest page. While I have > no strong objection to what's here, I'm wondering what sort of process > we want to follow. Is the infrastructure stuff getting separately > reviewed or not? > I can review it, but not until this weekend. It looks like someone already added me to the list of reviewers on that patch. I'm not sure if Matthew Wetmore has already started reviewing it or not. Regards, Jeff Davis -- 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] WIP: Automatic view update rules
>> - "make check" fails 16 of 118 tests for me with this patch applied. > Most of them are caused by additional NOTICE messages or unexpected > additional rules in the rewriter regression tests. I don't see anything > critical here. Possible; in that case you should patch the expected regression output appropriately. But I seem to remember thinking that \d was producing the wrong column list on one of the system catalogs you modified, so you might want to double check that part... maybe I'm all wet. ...Robert -- 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] Block-level CRC checks
* Tom Lane <[EMAIL PROTECTED]> [081113 14:43]: > Well, if we adopt the double buffering approach then the ex-lock would > only need to be held for long enough to copy the page contents to local > memory. So maybe this would be acceptable. It would certainly be a > heck of a lot simpler than any workable variant of the current patch > is likely to be; and we could simplify some existing code too (no more > need for the BM_JUST_DIRTIED flag for instance). Well, can we get rid of the PD_UNLOGGED_CHANGE completely? I think that if the buffer is dirty (FlushBuffer was called, and you've gotten through the StartBufferIO and gotten the lock), you can just WAL log the hint bits from the *local double-buffered* "page" (don't know if the current code allows it easily) If I understand tom's objections its that with the shared lock, other hint bits may still change... But we don't relly care if we get all the hint bits to WAL in our write, what we care about is that we get the hint-bits *that we checksummed* to WAL. You'll need throw the CRC in the WAL as well for the really paranoid. That way, if the write is torn, on recovery, the correct hint bits and matching CRC will be available. This means your chewing up more WAL. You get the WAL record for all the hint bits on every page write. For that you get: 1) Simplified locking (and maybe with releasing the lock before the write shorter lock hold-times) 2) Simplified CRC/checksum (don't have to try and skip hint-bits) 3) HINT bits WAL logged even for blocks written that aren't hint-bit only You trade WAL and simplicity for verifiable integrety. a. -- Aidan Van Dyk Create like a god, [EMAIL PROTECTED] command like a king, http://www.highrise.ca/ work like a slave. signature.asc Description: Digital signature
Re: [HACKERS] Simple postgresql.conf wizard
On Thu, Nov 13, 2008 at 3:20 PM, Grzegorz Jaskiewicz <[EMAIL PROTECTED]> wrote: > If that's the situation, me thinks you guys have to start thinking about > some sort of automated way to increase this param per column as needed. > Is there any way planner could actually tell, that it would do better job > with more stats for certain column ? Other systems do it. For example, Oracle tracks column usage and attempts to determine the optimal statistics for that column (based on the queries that used it) on an iterative basis. We don't track column usage at all, so that option wouldn't be quite that easy to implement. Though, there are certain things ANALYZE would be able to determine with a little help, such as knowing to collect more samples for columns it finds extremely skewed data in. There are other things that could be done as well... so the answer is, yes. -- Jonah H. Harris, Senior DBA myYearbook.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] gram.y => preproc.y
On Thu, Nov 13, 2008 at 03:10:04PM -0500, Tom Lane wrote: > clean distclean: > ! rm -f keywords.c *.o ecpg$(X) preproc.y > > Actually, we want to fix it so that preproc.y is treated like preproc.c, > ie, it's part of the shipped tarballs even though it's no longer in CVS. That's what I did first, but Magnus had a good reasoning to not keep preproc.y if we keep preproc.c in our tarball. And I agreed that there doesn't seem to be an advantage. Michael -- Michael Meskes Email: Michael at Fam-Meskes dot De ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: [EMAIL PROTECTED] Go SF 49ers! Go Rhein Fire! Use Debian GNU/Linux! Use PostgreSQL! -- 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] gram.y => preproc.y
Michael Meskes <[EMAIL PROTECTED]> writes: > That's what I did first, but Magnus had a good reasoning to not keep preproc.y > if we keep preproc.c in our tarball. And I agreed that there doesn't seem to > be > an advantage. Other than whether it *works*, you mean? make will not be happy if it has a dependency from preproc.c to preproc.y and preproc.y is not there. Please don't mess with this. 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] libpq-events windows gotcha
Andrew Chernow <[EMAIL PROTECTED]> writes: >> Here are the options we see: >> >> 1. PQregisterEventProc returns a handle, synchronized counter >> incremented by libpq. A small table could map handle value to proc >> address, so register always returns the same handle for a provided >> eventproc. Only register would take an eventproc, instanceData >> functions would take this magical handle. >> >> 2. string key, has collision issues (already been ruled out) >> >> 3. have implementors return a static variable address (doesn't seem all >> that different from returning a static funcaddr) >> >> 4. what we do now, but document loudly that PGEventProc must be static. >> If it can't be referenced outside the DLL directly then the issue can't >> arise. If you need the function address for calls to PQinstanceData, an >> implementor can create a public function that returns their private >> PGEventProc address. > Do you have a preference form the list above, or an another idea? If > not, we would probably implement #1. Although, the simplest thing is #4 > which leaves everything as is and updates the sgml docs with a strong > warning to implementors. I think #1 is mostly going to complicate life for both libpq and callers --- libpq has to deal with generating the handles (threading issues here) and callers have to remember them somewhere. And it's not even clear to me that it fixes the problem: wouldn't you get two different handles if you supplied the internal and external addresses of an eventproc? On the whole I vote for #4 out of these. I was just wondering whether there were any better alternatives, and it seems there's not. 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] Simple postgresql.conf wizard
On 2008-11-13, at 19:33, Greg Stark wrote: A statistic target of 400 fir a specific column may make sense but even then I would recommend monitoring performance to ensure it doesn't cause problems. As a global setting it's, IMHO, ridiculous. If that's the situation, me thinks you guys have to start thinking about some sort of automated way to increase this param per column as needed. Is there any way planner could actually tell, that it would do better job with more stats for certain column ? -- 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] Simple postgresql.conf wizard
Heikki Linnakangas <[EMAIL PROTECTED]> writes: > Another idea would be to take a large sample in ANALYZE, but if the > distribution looks "regular enough", store less samples in the > Most-Common-Values list and fewer histograms, to make the planning faster. Yeah, some flexibility at that end might not be a bad idea either. 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] gram.y => preproc.y
Michael Meskes <[EMAIL PROTECTED]> writes:
> since my last email seems to have disappeared, here we go again. Here's my
> current patch that includes the changes to the build system. Thanks to Magnus
> for the Windows part.
> Comments anyone?
+ $(srcdir)/preproc.y: $(top_srcdir)/src/backend/parser/gram.y
+ perl parse.pl < $< > $@
Use $(PERL) here.
(I'm not sure about the equivalent in the Windows world; it looks like
there are already places in Solution.pm that invoke perl via just
system("perl ..."), but is that really a good idea? Anyway, not
directly your problem.)
clean distclean:
! rm -f keywords.c *.o ecpg$(X) preproc.y
Actually, we want to fix it so that preproc.y is treated like preproc.c,
ie, it's part of the shipped tarballs even though it's no longer in CVS.
For the same reason, you want to generate it in $(srcdir) even in a
VPATH build. (Parts of this patch have that right and part don't.
You might want to test in a VPATH build before committing.)
Can't comment on the MSVC change.
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] Block-level CRC checks
Heikki Linnakangas wrote: > Alvaro Herrera wrote: >> XFS, for example, zeroes out during recovery any block >> that was written to but not fsync'ed before a crash. This means that if >> we change a hint bit after a checkpoing and mark the page dirty, the >> system can write the page. Suppose we crash at this point. On >> recovery, XFS will zero out the block, but there will be nothing with >> which to recovery it, because there's no backup block ... > > Really? That would mean that you're prone to lose data if you run > PostgreSQL on XFS, even without the CRC patch. > > I doubt that's true, though. Google found this: > > http://marc.info/?l=linux-xfs&m=122549156102504&w=2 Ah, there's no problem here then. This email mentions another one by "Eric" which is this one: http://marc.info/?l=linux-xfs&m=122546510218150&w=2 It contains more information about the problem. > Although, Florian Weimer suggested earlier in this thread that IBM DTLA > disks have exactly that problem; a sector could be zero-filled if the > write is interrupted. Hmm. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] Simple postgresql.conf wizard
Tom Lane wrote: Heikki Linnakangas <[EMAIL PROTECTED]> writes: A lot of people have suggested raising our default_statistics target, and it has been rejected because there's some O(n^2) behavior in the planner, and it makes ANALYZE slower, but it's not that crazy. I think everyone agrees it ought to be raised. Where the rubber meets the road is deciding just *what* to raise it to. We've got no convincing evidence in favor of any particular value. If someone actually wanted to put some effort into this, I'd suggest taking some reasonably complex benchmark (maybe TPCH or one of the DBT series) and plotting planner runtime for each query as a function of statistics_target, taking care to mark the breakpoints where it shifted to a better (or worse?) plan due to having better stats. Yeah, that would be a good starting point. After we have some data to work with, we could also look into making the planner faster with large samples. Another idea would be to take a large sample in ANALYZE, but if the distribution looks "regular enough", store less samples in the Most-Common-Values list and fewer histograms, to make the planning faster. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] gram.y => preproc.y
Hi, since my last email seems to have disappeared, here we go again. Here's my current patch that includes the changes to the build system. Thanks to Magnus for the Windows part. Comments anyone? Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: [EMAIL PROTECTED] Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL! ecpg.diff.gz 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] Block-level CRC checks
Alvaro Herrera wrote: XFS, for example, zeroes out during recovery any block that was written to but not fsync'ed before a crash. This means that if we change a hint bit after a checkpoing and mark the page dirty, the system can write the page. Suppose we crash at this point. On recovery, XFS will zero out the block, but there will be nothing with which to recovery it, because there's no backup block ... Really? That would mean that you're prone to lose data if you run PostgreSQL on XFS, even without the CRC patch. I doubt that's true, though. Google found this: http://marc.info/?l=linux-xfs&m=122549156102504&w=2 See the bottom of that mail. Although, Florian Weimer suggested earlier in this thread that IBM DTLA disks have exactly that problem; a sector could be zero-filled if the write is interrupted. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] Simple postgresql.conf wizard
Heikki Linnakangas <[EMAIL PROTECTED]> writes: > A lot of people have suggested raising our default_statistics target, > and it has been rejected because there's some O(n^2) behavior in the > planner, and it makes ANALYZE slower, but it's not that crazy. I think everyone agrees it ought to be raised. Where the rubber meets the road is deciding just *what* to raise it to. We've got no convincing evidence in favor of any particular value. If someone actually wanted to put some effort into this, I'd suggest taking some reasonably complex benchmark (maybe TPCH or one of the DBT series) and plotting planner runtime for each query as a function of statistics_target, taking care to mark the breakpoints where it shifted to a better (or worse?) plan due to having better stats. 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] Block-level CRC checks
Alvaro Herrera wrote: > Tom Lane wrote: > > > Basically, you can't make any critical changes to a shared buffer > > if you haven't got exclusive lock on it. But that's exactly what > > this patch is assuming it can do. > > It seems to me that the only possible way to close this hole is to > acquire an exclusive lock before calling FlushBuffers, not shared. > This lock would be held until the flag has been examined and reset; the > actual WAL record and write would continue with a shared lock, as now. We don't seem to have an API for reducing LWLock strength though ... -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] Block-level CRC checks
Alvaro Herrera <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> Basically, you can't make any critical changes to a shared buffer >> if you haven't got exclusive lock on it. But that's exactly what >> this patch is assuming it can do. > It seems to me that the only possible way to close this hole is to > acquire an exclusive lock before calling FlushBuffers, not shared. > This lock would be held until the flag has been examined and reset; the > actual WAL record and write would continue with a shared lock, as now. Well, if we adopt the double buffering approach then the ex-lock would only need to be held for long enough to copy the page contents to local memory. So maybe this would be acceptable. It would certainly be a heck of a lot simpler than any workable variant of the current patch is likely to be; and we could simplify some existing code too (no more need for the BM_JUST_DIRTIED flag for instance). > (The alternative seems to be to abandon this idea for hint bit logging; > we'll need something else.) I'm feeling dissatisfied too --- seems like we're one idea short of a good solution. In the larger scheme of things, this patch shouldn't go in anyway as long as there is some chance that we could have upgrade-in-place for 8.4 at the price of not increasing the page header size. So I think there's time to keep thinking about it. 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] Simple postgresql.conf wizard
Gregory Stark wrote: Josh Berkus <[EMAIL PROTECTED]> writes: DW: default_statistics_target = 400 Mixed: default_statistics_target = 100 You, my friend, are certifiably insane. I almost fell off the chair because of that comment, but after I stopped laughing and actually looked at those values, it doesn't seem that unreasonable. Arbitrary, sure, but not insane. Or do I need stronger glasses? A lot of people have suggested raising our default_statistics target, and it has been rejected because there's some O(n^2) behavior in the planner, and it makes ANALYZE slower, but it's not that crazy. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] Simple postgresql.conf wizard
A statistic target of 400 fir a specific column may make sense but even then I would recommend monitoring performance to ensure it doesn't cause problems. As a global setting it's, IMHO, ridiculous. Even for the smaller data types (except boolean and "char") and array of 400 will be large enough to be toasted. Planning queries will involve many more disk I/Os than some of those queries end up taking themselves. Even for stats which are already cached there are some algorithms in the planner known to be inefficient for large arrays. It may make sense for specific skewed columns with indexes on them, but keep in mind postgres needs to consult the statistics on any column referenced in a qual even if there are no indexes and for most data distributions do fine with a target of 10. I think we all agree the default may need to be raised but until there is some data we have little basis to recommend anything specific. I would suggest starting from the basis that "mixed" (with a conservative memory setting) is the same as "Postgres default". Perhaps (probably) the defaults should be changed but we shouldn't have two different tools with different (drastically different!) ideas for the same situation. greg On 13 Nov 2008, at 07:46 PM, Josh Berkus <[EMAIL PROTECTED]> wrote: Gregory Stark wrote: Josh Berkus <[EMAIL PROTECTED]> writes: DW: default_statistics_target = 400 Mixed: default_statistics_target = 100 You, my friend, are certifiably insane. Hmmm? Why? I've used those settings in the field, fairly frequently. I was actually wondering if we should raise the default for web as well, but decided to let it alone. Actually, I think a DW should begin at 400; often it needs to go up to 1000, but I don't think a script should do that. --Josh -- 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] auto_explain contrib moudle
Jeff Davis <[EMAIL PROTECTED]> writes: > Thanks! This patch is ready to go, as far as I'm concerned. This patch seems to contain a subset of the "contrib infrastructure" patch that's listed separately on the commitfest page. While I have no strong objection to what's here, I'm wondering what sort of process we want to follow. Is the infrastructure stuff getting separately reviewed or not? 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] Block-level CRC checks
Aidan Van Dyk wrote: > > I think I'm missing something... > > In this patch, I see you writing WAL records for hint-bits (bufmgr.c > FlushBuffer). But doesn't XLogInsert then make a "backup block" record > (unless > it's already got one since last checkpoint)? I'm not causing a backup block to be written with that WAL record. The rationale is that it's not needed -- if there was a critical write to the page, then there's already a backup block. If the only write was a hint bit being set, then the page cannot possibly be torn. Now that I think about this, I wonder if this can cause problems in some filesystems. XFS, for example, zeroes out during recovery any block that was written to but not fsync'ed before a crash. This means that if we change a hint bit after a checkpoing and mark the page dirty, the system can write the page. Suppose we crash at this point. On recovery, XFS will zero out the block, but there will be nothing with which to recovery it, because there's no backup block ... -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] Block-level CRC checks
Tom Lane wrote: > Basically, you can't make any critical changes to a shared buffer > if you haven't got exclusive lock on it. But that's exactly what > this patch is assuming it can do. It seems to me that the only possible way to close this hole is to acquire an exclusive lock before calling FlushBuffers, not shared. This lock would be held until the flag has been examined and reset; the actual WAL record and write would continue with a shared lock, as now. I'm wary of this "solution" because it's likely to reduce concurrency tremendously ... thoughts? (The alternative seems to be to abandon this idea for hint bit logging; we'll need something else.) -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] Block-level CRC checks
I think I'm missing something... In this patch, I see you writing WAL records for hint-bits (bufmgr.c FlushBuffer). But doesn't XLogInsert then make a "backup block" record (unless it's already got one since last checkpoint)? Once there's a backup block record, the torn-page problem that causes the whole CRCs to not validate, isn't it? On crash/recovery, you won't read this torn block because the WAL log will have the old backup + any possible updates to it... Sorry if I'm missing something very obvious... a. * Alvaro Herrera <[EMAIL PROTECTED]> [081113 13:08]: > I see. > > Since our CRC implementation is a simple byte loop, and since ItemIdData > fits in a uint32, the attached patch should do mostly the same by > copying the line pointer into a uint32, turning off the lp_flags, and > summing the modified copy. > > This patch is also skipping pd_special and the unused area of the page. > > I'm still testing this; please beware that this likely has an even > higher bug density than my regular patches (and some debugging printouts > as well). > > While reading the pg_filedump code I noticed that there's a way to tell > the different index pages apart, so perhaps we can use that to be able > to checksum the special space as well. -- Aidan Van Dyk Create like a god, [EMAIL PROTECTED] command like a king, http://www.highrise.ca/ work like a slave. signature.asc Description: Digital signature
Re: [HACKERS] Simple postgresql.conf wizard
Josh Berkus <[EMAIL PROTECTED]> writes: > Gregory Stark wrote: >> Josh Berkus <[EMAIL PROTECTED]> writes: >>> DW: >>> default_statistics_target = 400 >>> Mixed: >>> default_statistics_target = 100 >> >> You, my friend, are certifiably insane. > Hmmm? Why? I've used those settings in the field, fairly frequently. Even though we all agree default_statistics_target = 10 is too low, proposing a 40X increase in the default value requires more evidence than this. In particular, the prospect of a 1600-fold increase in the typical cost of eqjoinsel() is a mite scary. 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] libpq-events windows gotcha
Tom Lane wrote: And it's not even clear to me that it fixes the problem: wouldn't you get two different handles if you supplied the internal and external addresses of an eventproc? Both #1 and #4 suffer from this issue, internal & external register methods. They also require the same WARNING in the docs. But, #1 solves the instancedata lookup issue. I am not trying to make a case for #1 but it does appear to have a narrower failure window. > libpq has to deal with generating the handles Well lock; if(not_in_map) handle = ++counter; unlock surely won't be to difficult ;-) > (threading issues here) There is no unregister, so the idea won't lock/unlock in high traffic routines. On the whole I vote for #4 out of these. Okay. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] SQL5 budget
Josh, Actually that is a poorly worded page. It really should be something like, "How to submit a patch" or "How to get your patch committed". Yeah, I told Bruce I was going to re-write that page but seem to have been short some Round Tuits. --Josh -- 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] WIP: Automatic view update rules
Decibel! <[EMAIL PROTECTED]> writes: > That seems like a deal-breaker to me... many users could easily be > depending on views not being updateable. Views are generally always > thought of as read-only, so you should need to explicitly mark a view > as being updateable/insertable/deleteable. The SQL standard says differently ... 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] Simple postgresql.conf wizard
Greg, BTW, I think this is still in enough flux that we really ought to make it a pgfoundry project. I don't think we'll have anything ready for 8.4 contrib. --Josh -- 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] WIP: Automatic view update rules
On Nov 11, 2008, at 10:06 PM, Robert Haas wrote: - Should this be an optional behavior? What if I don't WANT my view to be updateable? That seems like a deal-breaker to me... many users could easily be depending on views not being updateable. Views are generally always thought of as read-only, so you should need to explicitly mark a view as being updateable/insertable/deleteable. It's tempting to try and use permissions to try and handle this, but I don't think that's safe either: nothing prevents you from doing GRANT ALL on a view with no rules, and such a view would suddenly become updateable. -- Decibel!, aka Jim C. Nasby, Database Architect [EMAIL PROTECTED] Give your computer some brain candy! www.distributed.net Team #1828 -- 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] Simple postgresql.conf wizard
Gregory Stark wrote: Josh Berkus <[EMAIL PROTECTED]> writes: DW: default_statistics_target = 400 Mixed: default_statistics_target = 100 You, my friend, are certifiably insane. Hmmm? Why? I've used those settings in the field, fairly frequently. I was actually wondering if we should raise the default for web as well, but decided to let it alone. Actually, I think a DW should begin at 400; often it needs to go up to 1000, but I don't think a script should do that. --Josh -- 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] Block-level CRC checks
Alvaro Herrera <[EMAIL PROTECTED]> writes: > I'm still testing this; please beware that this likely has an even > higher bug density than my regular patches (and some debugging printouts > as well). This seems impossibly fragile ... and the non-modular assumptions about what is in a disk page aren't even the worst part :-(. The worst part is the race conditions. In particular, the code added to FlushBuffer effectively assumes that the PD_UNLOGGED_CHANGE bit is set sooner than the actual hint bit change occurs. Even if the tqual.c code did that in the right order, which it doesn't, you can't assume that the updates will become visible to other CPUs in the expected order. This might be fixable with introduction of some memory barrier operations but it's certainly broken as-is. Also, if you do make tqual.c set the bits in that order, it's not clear how you can ever *clear* PD_UNLOGGED_CHANGE without introducing a race condition at that end. (The patch actually neglects to do this anywhere, which means that it won't be long till every page in the DB has got that bit set all the time, which I don't think we want.) I also don't like that you've got different CPUs trying to set or clear the same PD_UNLOGGED_CHANGE bit with no locking. We can tolerate that for ordinary hint bits because it's not critical if an update gets lost. But in this scheme PD_UNLOGGED_CHANGE is not an optional hint bit: you *will* mess up if it fails to get set. Even more insidiously, the scheme will certainly fail if someone ever tries to add another asynchronously-updated hint bit in pd_flags, since an update of one of the bits might overwrite a concurrent update of the other. Also, it's not inconceivable (depending on how wide the processor/memory bus is) that one processor updating PD_UNLOGGED_CHANGE could overwrite some other processor's change to the nearby pd_checksum or pd_lsn or pd_tli fields. Basically, you can't make any critical changes to a shared buffer if you haven't got exclusive lock on it. But that's exactly what this patch is assuming it can do. 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] Simple postgresql.conf wizard
Josh Berkus <[EMAIL PROTECTED]> writes: > DW: > default_statistics_target = 400 > Mixed: > default_statistics_target = 100 You, my friend, are certifiably insane. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres support! -- 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] Sometimes pg_dump generates dump which is not restorable
"Dmitry Koterov" <[EMAIL PROTECTED]> writes: > 3. The function a() calls any OTHER function b() from OTHER namespace (or > uses operators from other namespaces), but does not specify the schema name, > because it is in database search_path: > CREATE FUNCTION a(i integer) RETURNS boolean AS $$ > BEGIN > PERFORM b(); -- b() is is from "nsp" schema > RETURN true; > END;$$ LANGUAGE plpgsql IMMUTABLE; I think your function is broken. You might want to fix it by attaching a local search_path setting to it. 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] Simple postgresql.conf wizard
Greg, Attached version takes all its input via command line switches. If you don't specify an explict number of connections, it also implements setting max_connections via some of the logic from your calcfactors spreadsheet. OK, I'll review. What follows is a review of the *previous* version, because I'm currently on the road and didn't see your message to -hackers. Some of the information in the review will still be relevant; for one thing, I've simplified the "what color is your application" logic to a few calculations. Review of simple_config.py: 1) don't bother with os.sysconf, or make it optional and error-trap it. Instead, solicit the following information from the user: -- Available RAM -- Expected Database Size (to nearest 10x) -- Type of Application -- Web -- Data Warehouse -- Mixed -- Desktop -- Operating System [Linux/Windows/OSX/Solaris/FreeBSD/other] From the above, you can derive all necessary calculations for the basics. In the advanced version, we'll also want to ask: -- Memory used by other applications on the system? -- Analyze queries for performance? -- SSL? -- Production vs. Development status? -- How many connections? -- Logging setup: Syslog Analyze Performance Private log with weekly rotation 2) It's completely unnecessary to account for OS overhead. This can and should be taken into account as part of the calculations for other figures. For example, my 1/4 and 3/4 calculations ignore OS overhead. You only need to reduce Available RAM when the server will be running something else, like a J2EE server or multiple databases. 3) You need to provide a whole bunch more values. shared_buffers and effective_cache_size isn't nearly enough. We should also provide, based on these calculations, and by database type. (I'm happy to argue out the figures below. They are simply based on my direct turning experience with a variety of databases and could probably use more tweaking for the general case.) web / oltp listen_addresses = '*' max_connections = 200 shared_buffers = 1/4 AvRAM effective_cache_size = 3/4 AvRAM work_mem = AvRAM / max_connections, round down maint_work_mem = AvRAM / 16, round up wal_buffers = 8mb autovacuum = on max_fsm_pages = DBsize / PageSize / 8 checkpoint_segments = 8 default_statistics_target = 10 constraint_exclusion = off DW: listen_addresses = '*' max_connections = 20 shared_buffers = 1/4 AvRAM effective_cache_size = 3/4 AvRAM work_mem = AvRAM / max_connections / 2, round down maint_work_mem = AvRAM / 8, round up wal_buffers = 32mb autovacuum = off max_fsm_pages = DBsize / PageSize / 32* (unless LoadSize is known) checkpoint_segments = 64 default_statistics_target = 400 constraint_exclusion = on Mixed: listen_addresses = '*' max_connections = 80 shared_buffers = 1/4 AvRAM effective_cache_size = 3/4 AvRAM work_mem = AvRAM / max_connections / 2, round down maint_work_mem = AvRAM / 16, round up wal_buffers = 8mb autovacuum = on max_fsm_pages = DBsize / PageSize / 8 checkpoint_segments = 16 default_statistics_target = 100 constraint_exclusion = on Desktop: listen_addresses = 'localhost' max_connections = 5 shared_buffers = 1/16 AvRAM effective_cache_size = 1/4 AvRAM work_mem = AvRAM / 32, round down maint_work_mem = AvRAM / 16, round up wal_buffers = 1mb autovacuum = on max_fsm_pages = DBsize / PageSize / 8 checkpoint_segments = 3 default_statistics_target = 10 constraint_exclusion = off 4) Because this comes up so often, we should output to a seperate file a set of sysctl.conf lines to support SysV memory, depending on OS. -- 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] Block-level CRC checks
Gregory Stark wrote:
> I think we're talking past each other. Martin and I are talking about doing
> something like:
>
> for (...)
> ...
> crc(word including hint bits)
> ...
> for (each line pointer)
> crc-negated(word & LP_DEAD<<15)
>
> Because CRC is a cyclic checksum it's possible to add or remove bits
> incrementally.
I see.
Since our CRC implementation is a simple byte loop, and since ItemIdData
fits in a uint32, the attached patch should do mostly the same by
copying the line pointer into a uint32, turning off the lp_flags, and
summing the modified copy.
This patch is also skipping pd_special and the unused area of the page.
I'm still testing this; please beware that this likely has an even
higher bug density than my regular patches (and some debugging printouts
as well).
While reading the pg_filedump code I noticed that there's a way to tell
the different index pages apart, so perhaps we can use that to be able
to checksum the special space as well.
--
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Index: src/backend/access/heap/heapam.c
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/heap/heapam.c,v
retrieving revision 1.269
diff -c -p -r1.269 heapam.c
*** src/backend/access/heap/heapam.c 6 Nov 2008 20:51:14 - 1.269
--- src/backend/access/heap/heapam.c 13 Nov 2008 17:44:23 -
***
*** 4036,4041
--- 4036,4128
}
/*
+ * Perform XLogInsert for hint bits changes in a page. This handles hint
+ * bits set in HeapTupleHeaderData (t_infomask and t_infomask2).
+ *
+ * This is intended to be called right before writing a page from shared
+ * buffers to disk.
+ *
+ * The approach used here, instead of WAL-logging every change, is to produce
+ * a complete record of the current state of hint bits in a page just before
+ * flushing it. There are two downsides to this approach: first, it stores
+ * all hint bits in the page, not only those that changed; and second, that
+ * the flusher of the page needs to flush a lot more of the WAL (namely up
+ * to this new record's LSN) than the original LSN marked on the page.
+ */
+ XLogRecPtr
+ log_hintbits(RelFileNode *rnode, ForkNumber forkNum, BlockNumber blkno,
+ Page page)
+ {
+ xl_heap_hintbits xlrec;
+ OffsetNumber i;
+ XLogRecPtr recptr;
+ XLogRecData rdata[2];
+ char *bits;
+ intpos = 0;
+ StringInfoData buf;
+
+ /*
+ * 1 byte for line pointer bits, 2 bytes for infomask,
+ * 2 bytes for infomask2
+ */
+ bits = palloc(MaxHeapTuplesPerPage * 5);
+
+ initStringInfo(&buf);
+ appendStringInfo(&buf, "page %u: ", blkno);
+
+ for (i = FirstOffsetNumber; i <= PageGetMaxOffsetNumber(page);
+ i = OffsetNumberNext(i))
+ {
+ HeapTupleHeader htup;
+ ItemId lp = PageGetItemId(page, i);
+
+ if (!ItemIdHasStorage(lp))
+ continue;
+
+ appendStringInfo(&buf, "offset %d: ", i);
+
+ htup = (HeapTupleHeader) PageGetItem(page, lp);
+
+ *((uint16 *) (bits + pos)) = htup->t_infomask & HEAP_XACT_MASK;
+ appendStringInfo(&buf, "infomask %04x/%04x ", htup->t_infomask,
+ htup->t_infomask & HEAP_XACT_MASK);
+ pos += 2;
+ *((uint16 *) (bits + pos)) = htup->t_infomask2 & HEAP2_XACT_MASK;
+ appendStringInfo(&buf, "infomask2 %04x/%04x\n", htup->t_infomask2,
+ htup->t_infomask2 & HEAP2_XACT_MASK);
+ pos += 2;
+ }
+
+ elog(LOG, "%s", buf.data);
+ pfree(buf.data);
+
+ /* NO ELOG(ERROR) from here till hint bits are logged */
+ START_CRIT_SECTION();
+
+ xlrec.node = *rnode;
+ xlrec.block = blkno;
+
+ rdata[0].data = (char *) &xlrec;
+ rdata[0].len = SizeOfHeapHintbits;
+ rdata[0].buffer = InvalidBuffer;
+ rdata[0].next = &(rdata[1]);
+
+ rdata[1].data = (char *) bits;
+ rdata[1].len = pos;
+ rdata[1].buffer = InvalidBuffer;
+ rdata[1].next = NULL;
+
+ recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_HINTBITS, rdata);
+
+ PageSetLSN(page, recptr);
+ PageSetTLI(page, ThisTimeLineID);
+
+ END_CRIT_SECTION();
+
+ return recptr;
+ }
+
+ /*
* Handles CLEAN and CLEAN_MOVE record types
*/
static void
***
*** 4153,4158
--- 4240,4324
}
static void
+ heap_xlog_hintbits(XLogRecPtr lsn, XLogRecord *record)
+ {
+ xl_heap_hintbits *xlrec = (xl_heap_hintbits *) XLogRecGetData(record);
+ Buffer buffer;
+ Page page;
+
+ buffer = XLogReadBuffer(xlrec->node, xlrec->block, false);
+ if (!BufferIsValid(buffer))
+ return;
+ page = (Page) BufferGetPage(buffer);
+
+ if (XLByteLE(lsn, PageGetLSN(page)))
+ {
+ UnlockReleaseBuffer(buffer);
+ return;
+ }
+
+ if (record->xl_len > SizeOfHeapHintbits)
+ {
+ char *bits;
+ char *bits_end;
+ OffsetNumber offset = FirstOffsetNumber;
+ StringInfoData buf;
+
+
+ bits = (char *) xlrec + SizeOfHeapHintbits;
+ bits_end = (char *) xlrec + record->xl_len;
+
+ initStringInfo(&bu
Re: [HACKERS] [GENERAL] db_user_namespace, md5 and changing passwords
Magnus Hagander <[EMAIL PROTECTED]> writes: > I am unsure of exactly where this thing hacks into the authentication > stream, but is it really only MD5 that fails? The problem with md5 is that the username is part of the encryption salt for the stored password, so changing it breaks that --- the client will hash the password with what it thinks the username is, but the stored password in pg_authid is hashed with what the server thinks the username is. You might be right that some other auth methods have an issue too, but md5 is the only one anyone's ever reported a problem with. That might or might not just represent lack of testing. 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
[HACKERS] Suppress leap-second timezones in pg_timezone_names view?
Per bug #4528, I'm thinking we should do $SUBJECT. I'm inclined to put a tz_acceptable() check into pg_tzenumerate_next, which is currently only used by that view but might be used for other purposes later. Any objections? 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
[HACKERS] Okay, DLLIMPORT is making me crazy
I did this: http://archives.postgresql.org/pgsql-committers/2008-11/msg00156.php to try to fix this: http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=narwhal&dt=2008-11-12%2021:00:01 only to get this: http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=narwhal&dt=2008-11-13%2015:00:01 Anybody know how to get that stuff to act sanely? Or should we just toss the mingw build overboard? 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] array_agg and array_accum (patch)
Jeff Davis wrote: Here's an updated patch for just array_accum() with some simple docs. I have committed a "best of Robert Haas and Jeff Davis" array_agg() function with standard SQL semantics. I believe this gives the best consistency with other aggregate functions for the no-input-rows case. If some other behavior is wanted, it is a coalesce() away, as the documentation states. -- 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] Re: Updated interval patches - ECPG [was, intervalstyle....]
On Wed, Nov 12, 2008 at 02:28:56PM -0800, Ron Mayer wrote: > Merging of the interval style into ecpg attached. Thanks for caring about the ecpg changes too. > I know little enough about ecpg that I can't really tell if these changes > are for the better or worse. The closer pgtypeslib is to the backend the better. > One thing in the patch that's probably a bug is that the > constants in src/include/utils/dt.h and src/include/utils/datetime.h > under the section "Fields for time decoding" seem not to match, so Assuming you mean src/interfaces/ecpg/pgtypeslib/dt.h. The numbers should match IMO. Also one files seems to be missing, there are no changes to test/expected/pgtypeslib-dt_test.c in the patch, but when changing dt_test.pgc this file should be changed too. Could you add this to your work too? Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: [EMAIL PROTECTED] Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Sometimes pg_dump generates dump which is not restorable
Hello. Why pg_dump dumps CONSTRAINT ... CHECK together with CREATE TABLE queries, but NOT at the end of dump file (as FOREIGN KEY)? Sometimes it causes the generation of invalid dumps which cannot be restored. Details follow. 1. I use database-dedicated search_path: ALTER DATABASE d SET search_path TO nsp, public, pg_catalog; 2. I have a CHECK on table1 which calls a stored function: CREATE TABLE table1 ( i integer, CONSTRAINT table1_chk CHECK ((a(i) = true)) ); 3. The function a() calls any OTHER function b() from OTHER namespace (or uses operators from other namespaces), but does not specify the schema name, because it is in database search_path: CREATE FUNCTION a(i integer) RETURNS boolean AS $$ BEGIN PERFORM b(); -- b() is is from "nsp" schema RETURN true; END;$$ LANGUAGE plpgsql IMMUTABLE; 4. If I dump such schema using pg_dump, later this dump cannot be restored. Look the following piece of generated dump: SET search_path = public, pg_catalog; COPY table1 (i) FROM stdin; 1 \. You see, when COPY is executed, data is inserted, and CHECK is called. So, function a() is called with "public, pg_catalog" search_path. It is errorous! Possible solutions: 1. When generating CREATE TABLE dump query, DO NOT include CONSTRAINT ... CHECK clauses in it. Instead, use ALTER TABLE to add all checks AT THE END of dump, the same as it is done for foreign keys. I have already offered this above. Additionally, seems to me it will speed up the dump restoration. 2. Replace "SET search_path = public, pg_catalog" to "SET search_path = public, pg_catalog, ". It's a worse way, kind a hack.
[HACKERS] pg_filedump for CVS HEAD
Hi,
Who is in charge of pg_filedump now?
I noticed that the latest version (for 8.3) does not play nice with
HEAD, because of changes in ControlFileData. The attached patch fixes
that, allowing it to compile.
I didn't look if there were other changes needed for it to actually
work; any clues?
--
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
*** pg_filedump.c.orig 2008-02-28 11:12:21.0 -0300
--- pg_filedump.c 2008-11-13 13:55:11.0 -0300
***
*** 1170,1178
" Maximum Index Keys: %u\n"
" TOAST Chunk Size: %u\n"
" Date and Time Type Storage: %s\n"
! " Locale Buffer Length: %u\n"
! " lc_collate: %s\n"
! " lc_ctype: %s\n\n",
EQ_CRC32 (crcLocal,
controlData->crc) ? "Correct" : "Not Correct",
controlData->pg_control_version,
--- 1170,1177
" Maximum Index Keys: %u\n"
" TOAST Chunk Size: %u\n"
" Date and Time Type Storage: %s\n"
! " float4 parameter passing: %s\n"
! " float8 parameter passing: %s\n\n",
EQ_CRC32 (crcLocal,
controlData->crc) ? "Correct" : "Not Correct",
controlData->pg_control_version,
***
*** 1204,1212
controlData->toast_max_chunk_size,
(controlData->enableIntTimes ?
"64 bit Integers" : "Floating Point"),
! controlData->localeBuflen,
! controlData->lc_collate,
! controlData->lc_ctype);
}
else
{
--- 1203,1210
controlData->toast_max_chunk_size,
(controlData->enableIntTimes ?
"64 bit Integers" : "Floating Point"),
! controlData->float4ByVal ? "by value" : "by reference",
! controlData->float8ByVal ? "by value" : "by reference");
}
else
{
--
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] WIP: Automatic view update rules
--On Dienstag, November 11, 2008 23:06:08 -0500 Robert Haas <[EMAIL PROTECTED]> wrote: Thanks for your look at this. Unfortunately i was travelling the last 2 days, so i didn't have time to reply earlier, sorry for that. I haven't done a full review of this patch, but here are some thoughts based on a quick read-through: - "make check" fails 16 of 118 tests for me with this patch applied. Most of them are caused by additional NOTICE messages or unexpected additional rules in the rewriter regression tests. I don't see anything critical here. - There are some unnecessary hunks in this diff. For example, some of the gram.y changes appear to move curly braces around, adjust spacing, hmm i didn't see any changes to brackets or adjusting spaces... and replace true and false with TRUE and FALSE (I'm not 100% sure that the last of these isn't substantive... but I hope it's not). The changes to rewriteDefine.c contain some commented-out declarations that need to be cleaned up, and at least one place where a blank line has been added. - The doc changes for ev_kind describe 'e' as meaning "rule was created by user", which confused me because why would you pick "e" for that? Then I realized that "e" was probably intended to mean "explicit"; it would probably be good to work that word into the documentation of that value somehow. okay - Should this be an optional behavior? What if I don't WANT my view to be updateable? I didn't see anything like this in the SQL92 draft...i thought if a view is updatable, it is, without any further adjustments. If you don't want your view updatable you have to REVOKE or GRANT your specific ACLs. - I am wondering if the old_tup_is_implicit variable started off its life as a boolean and morphed into a char. It seems misnamed, now, at any rate. agreed - The capitalization of deleteImplicitRulesOnEvent is inconsistent with the functions that immediately precede it in rewriteRemove.h. I think the "d" should be capitalized. checkTree() also uses this style of capitalization, which I haven't seen elsewhere in the source tree. agreed - This: rhaas=# create table baz (a integer, b integer); CREATE TABLE rhaas=# create or replace view bar as select * from baz; NOTICE: CREATE VIEW will create implicit INSERT/UPDATE/DELETE rules CREATE VIEW Generates this update rule: ON UPDATE TO bar DO INSTEAD UPDATE ONLY foo SET a = new.a, b = new.b WHERE CASE WHEN old.a IS NOT NULL THEN old.a = foo.a ELSE foo.a IS NULL END AND CASE WHEN old.b IS NOT NULL THEN old.b = foo.b ELSE foo.b IS NULL END RETURNING new.a, new.b It seems like this could be simplified using IS NOT DISTINCT FROM. I'll look at this. -- Thanks Bernd -- 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] libpq-events windows gotcha
Andrew Chernow wrote: Tom Lane wrote: Andrew Chernow <[EMAIL PROTECTED]> writes: Just noticed that the last libpqtypes release was broken on windows when dynamically linking. The problem is that windows has two addresses for functions, the import library uses a stub "ordinal" address while the DLL itself is using the real address; yet another m$ annoyance. This breaks the PQEventProc being used as a unique lookup value. This is a big gotcha for any libpq-events implementors. It should probably be documented in some fashion. Hmm. Well, it's not too late to reconsider the use of the function address as a lookup key ... but what else would we use instead? regards, tom lane Here are the options we see: 1. PQregisterEventProc returns a handle, synchronized counter incremented by libpq. A small table could map handle value to proc address, so register always returns the same handle for a provided eventproc. Only register would take an eventproc, instanceData functions would take this magical handle. 2. string key, has collision issues (already been ruled out) 3. have implementors return a static variable address (doesn't seem all that different from returning a static funcaddr) 4. what we do now, but document loudly that PGEventProc must be static. If it can't be referenced outside the DLL directly then the issue can't arise. If you need the function address for calls to PQinstanceData, an implementor can create a public function that returns their private PGEventProc address. Do you have a preference form the list above, or an another idea? If not, we would probably implement #1. Although, the simplest thing is #4 which leaves everything as is and updates the sgml docs with a strong warning to implementors. I am not sure which is best. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] pg_filedump for CVS HEAD
Alvaro Herrera <[EMAIL PROTECTED]> writes: > Who is in charge of pg_filedump now? It's usually me that fixes it for new PG versions. I don't normally try to track CVS HEAD, just update it at release time. > I noticed that the latest version (for 8.3) does not play nice with > HEAD, because of changes in ControlFileData. The attached patch fixes > that, allowing it to compile. Thanks, appreciate the patch. 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] [GENERAL] db_user_namespace, md5 and changing passwords
Tom Lane wrote: > Magnus Hagander <[EMAIL PROTECTED]> writes: >> I am unsure of exactly where this thing hacks into the authentication >> stream, but is it really only MD5 that fails? > > The problem with md5 is that the username is part of the encryption salt > for the stored password, so changing it breaks that --- the client will > hash the password with what it thinks the username is, but the stored > password in pg_authid is hashed with what the server thinks the username > is. > > You might be right that some other auth methods have an issue too, > but md5 is the only one anyone's ever reported a problem with. That > might or might not just represent lack of testing. Right. But say GSSAPI for example. It will get the username from an external source, and compare this to whatever the user specified. If we rewrite what the user specified, we loose. But maybe you can work around that by using pg_ident.conf, so *both* the identities gets rewritten. Not sure I care enough to dive into what it would actually mean. My guess is that it's very uncommon to use db_user_namespace in any of these scenarios (in fact I think it's very uncommon to use it at all, but even more uncommon in these cases) //Magnus -- 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] [GENERAL] db_user_namespace, md5 and changing passwords
Bruce Momjian wrote: > Magnus Hagander wrote: >>> I have developed the attached patch, which documents the inability to >>> use MD5 with db_user_namespace, and throws an error when it is used: >>> >>> psql: FATAL: MD5 authentication is not supported when >>> "db_user_namespace" is enabled >> IMHO it would be much nicer to detect this when we load pg_hba.conf. >> It's easy to do these days :-P >> >> I don't think we need to worry about the "changed postgresql.conf after >> we changed pg_hba.conf" that much, because we'll always reload >> pg_hba.conf after the main config file. >> >> I'd still leave the runtime check in as well to handle the "loaded one >> but not the other" case, but let's try prevent the user from loading the >> broken config file in the first place.. > > [ Thread moved to hackers. ] > > OK, updated patch attached. > > Looks a lot better. I am unsure of exactly where this thing hacks into the authentication stream, but is it really only MD5 that fails? AFAICS, we rewrite what the user puts into the system *before* we do the authentication. Which I think would break all authentication *except* password (without md5) and trust, more or less. //Magnus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] SQL5 budget
Hi, Pgsql-hackers. > many seem (to me) to be overly tied to an "all XML all the time" view. Only for hierarchical result sets. Even in case of http://computer20.euro.ru/site/computer20/en/author/communication_eng.htm Dmitry (SQL50, HTML60) -- 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] WIP: Column-level Privileges
Hello Stephen, Stephen Frost wrote: > Attached patch has this fixed and still passes all regression tests, > etc. Do you have an up-to-date patch laying around? The current one conflicts with some CVS tip changes. I didn't get around writing some docu, yet. Sorry. Regards Markus Wanner -- 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] Re: [BUGS] libpq does not manage SSL callbacks properly when other libraries are involved.
Bruce Momjian wrote:
> Yes, my defines were very messed up; updated version attached.
>
Hi,
I've not done a review of this patch, however I did backport it to 8.3
(as attached in unified diff). The patch wasn't made for PG purposes, so
it's not in context diff. I tested the backported patch and the issues I
was experiencing with the initial bug report have stopped. So the fix
works for the initial reported problem.
Will this be back patched when it's committed?
Regards
Russell
diff -uNr postgresql-8.3.3/src/interfaces/libpq/fe-secure.c postgresql-8.3.3.new/src/interfaces/libpq/fe-secure.c
--- postgresql-8.3.3/src/interfaces/libpq/fe-secure.c 2008-01-29 13:03:39.0 +1100
+++ postgresql-8.3.3.new/src/interfaces/libpq/fe-secure.c 2008-11-13 20:57:40.0 +1100
@@ -142,12 +142,10 @@
#define ERR_pop_to_mark() ((void) 0)
#endif
-#ifdef NOT_USED
-static int verify_peer(PGconn *);
-#endif
static int verify_cb(int ok, X509_STORE_CTX *ctx);
static int client_cert_cb(SSL *, X509 **, EVP_PKEY **);
static int init_ssl_system(PGconn *conn);
+static void destroy_ssl_system(void);
static int initialize_SSL(PGconn *);
static void destroy_SSL(void);
static PostgresPollingStatusType open_client_SSL(PGconn *);
@@ -156,11 +154,19 @@
static void SSLerrfree(char *buf);
#endif
-#ifdef USE_SSL
static bool pq_initssllib = true;
static SSL_CTX *SSL_context = NULL;
+#ifdef ENABLE_THREAD_SAFETY
+static int ssl_open_connections = 0;
+
+#ifndef WIN32
+static pthread_mutex_t ssl_config_mutex = PTHREAD_MUTEX_INITIALIZER;
+#else
+static pthread_mutex_t ssl_config_mutex = NULL;
+static long win32_ssl_create_mutex = 0;
#endif
+#endif/* ENABLE_THREAD_SAFETY */
/*
* Macros to handle disabling and then restoring the state of SIGPIPE handling.
@@ -839,40 +845,53 @@
init_ssl_system(PGconn *conn)
{
#ifdef ENABLE_THREAD_SAFETY
-#ifndef WIN32
- static pthread_mutex_t init_mutex = PTHREAD_MUTEX_INITIALIZER;
-#else
- static pthread_mutex_t init_mutex = NULL;
- static long mutex_initlock = 0;
-
- if (init_mutex == NULL)
- {
- while (InterlockedExchange(&mutex_initlock, 1) == 1)
- /* loop, another thread own the lock */ ;
- if (init_mutex == NULL)
- pthread_mutex_init(&init_mutex, NULL);
- InterlockedExchange(&mutex_initlock, 0);
- }
-#endif
- pthread_mutex_lock(&init_mutex);
-
- if (pq_initssllib && pq_lockarray == NULL)
- {
- int i;
-
- CRYPTO_set_id_callback(pq_threadidcallback);
-
- pq_lockarray = malloc(sizeof(pthread_mutex_t) * CRYPTO_num_locks());
- if (!pq_lockarray)
- {
- pthread_mutex_unlock(&init_mutex);
- return -1;
- }
- for (i = 0; i < CRYPTO_num_locks(); i++)
- pthread_mutex_init(&pq_lockarray[i], NULL);
-
- CRYPTO_set_locking_callback(pq_lockingcallback);
- }
+#ifdef WIN32
+ if (ssl_config_mutex == NULL)
+ {
+ while (InterlockedExchange(&win32_ssl_create_mutex, 1) == 1)
+ /* loop, another thread own the lock */ ;
+ if (ssl_config_mutex == NULL)
+ {
+ if (pthread_mutex_init(&ssl_config_mutex, NULL))
+ return -1;
+ }
+ InterlockedExchange(&win32_ssl_create_mutex, 0);
+ }
+#endif
+ if (pthread_mutex_lock(&ssl_config_mutex))
+ return -1;
+
+ if (pq_initssllib)
+ {
+ if (pq_lockarray == NULL)
+ {
+ int i;
+
+ pq_lockarray = malloc(sizeof(pthread_mutex_t) * CRYPTO_num_locks());
+ if (!pq_lockarray)
+ {
+ pthread_mutex_unlock(&ssl_config_mutex);
+ return -1;
+ }
+ for (i = 0; i < CRYPTO_num_locks(); i++)
+ {
+ if (pthread_mutex_init(&pq_lockarray[i], NULL))
+ {
+ free(pq_lockarray);
+ pq_lockarray = NULL;
+ pthread_mutex_unlock(&ssl_config_mutex);
+ return -1;
+ }
+ }
+ }
+
+ if (ssl_open_connections++ == 0)
+ {
+ /* These are only required for threaded SSL applications */
+ CRYPTO_set_id_callback(pq_threadidcallback);
+ CRYPTO_set_locking_callback(pq_lockingcallback);
+ }
+}
#endif
if (!SSL_context)
{
@@ -894,18 +913,61 @@
err);
SSLerrfree(err);
#ifdef ENABLE_THREAD_SAFETY
- pthread_mutex_unlock(&init_mutex);
+ pthread_mutex_unlock(&ssl_config_mutex);
#endif
return -1;
}
}
#ifdef ENABLE_THREAD_SAFETY
- pthread_mutex_unlock(&init_mutex);
+ pthread_mutex_unlock(&ssl_config_mutex);
#endif
return 0;
}
/*
+ *This function is needed because if the libpq library is unloaded
+ *from the application, the callback functions will no longer exist when
+ *SSL used by other parts of the system. For this reason,
+ *we unregister the SSL callback functions when the last libpq
+ *connection is closed.
+ */
+static void
+destroy_ssl_system(void)
+{
+#ifdef ENABLE_THREAD_SAFETY
+ /* Assume mutex is already created */
+ if (pthread_mutex_lock(&ssl
Re: [HACKERS] 8.3 .4 + Vista + MingW + initdb = ACCESS_DENIED
Charlie Savage wrote: > Just wanted to close off this thread. > > Previously I reported that building 8.3.4 with MingW on Windows resulted > in an initdb executable that couldn't create new databases due to > security restrictions in creating global file mappings in Vista. > > I'm happy to say that the problem seems fixed in 8.3.5, which I have > successfully built, installed, and created new databases with. Great! > And just to note this for anyone else who runs into the issue - if you > build postgresql with MingW then make sure you are using the latest > version of the MingW runtime: > > http://sourceforge.net/project/showfiles.php?group_id=2435&package_id=11598 > > Specifically, use version 3.15.1 released on 2008-10-04 (or hopefully > later). In the previous version, getopt_long is broken, meaning you > cannot use any long style switches to initdb (for example, initdb > --locale=C doesn't work and causes initdb to exit with an error message). There are a lot of earlier versions that work just fine - it's just that there are a number in between that don't :-( I'd recommend anybody who needs to build on mingw (the main recommendations of using binary or msvc still stand, but I realise everybody don't want that) to look at the buildfarm and pick the same versions as are being used there. //Magnus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Client certificate authentication
Attached patch implements client certificate authentication.
I kept this sitting in my tree without sending it in before the
commitfest because it is entirely dependent on the
not-yet-reviewed-and-applied patch for how to configure client
certificate requesting. But now that I learned how to do it right in
git, breaking it out was very easy :-) Good learning experience.
Anyway. Here it is. Builds on top of the "clientcert option for pg_hba"
patch already on the list.
//Magnus
*** a/doc/src/sgml/client-auth.sgml
--- b/doc/src/sgml/client-auth.sgml
***
*** 388,393 hostnossl database user
--- 388,403
+ cert
+
+
+ Authenticate using SSL client certificates. See
+for details.
+
+
+
+
+
pam
***
*** 1114,1119 ldapserver=ldap.example.net prefix="cn=" suffix="dc=example, dc=net"
--- 1124,1148
+
+Certificate authentication
+
+
+ Certificate
+
+
+
+ This authentication method uses SSL client certificates to perform
+ authentication. It is therefore only available for SSL connections.
+ When using this authentication method, the server will require that
+ the client provide a certificate. No password prompt will be sent
+ to the client. The cn attribute of the certificate
+ will be matched with the username the user is trying to log in as,
+ and if they match the login will be allowed. Username mapping can be
+ used if the usernames don't match.
+
+
+
PAM authentication
*** a/doc/src/sgml/runtime.sgml
--- b/doc/src/sgml/runtime.sgml
***
*** 1674,1684 $ kill -INT `head -1 /usr/local/pgsql/data/postmaster.pid`
!PostgreSQL currently does not support authentication
!using client certificates, since it cannot differentiate between
!different users. As long as the user holds any certificate issued
!by a trusted CA it will be accepted, regardless of what account the
!user is trying to connect with.
--- 1674,1682
!You can use the authentication method cert to use the
!client certificate for authenticating users. See
! for details.
*** a/src/backend/libpq/auth.c
--- b/src/backend/libpq/auth.c
***
*** 110,115 ULONG(*__ldap_start_tls_sA) (
--- 110,123
static int CheckLDAPAuth(Port *port);
#endif /* USE_LDAP */
+ /*
+ * Cert authentication
+ *
+ */
+ #ifdef USE_SSL
+ static int CheckCertAuth(Port *port);
+ #endif
+
/*
* Kerberos and GSSAPI GUCs
***
*** 420,425 ClientAuthentication(Port *port)
--- 428,441
#endif
break;
+ case uaCert:
+ #ifdef USE_SSL
+ status = CheckCertAuth(port);
+ #else
+ Assert(false);
+ #endif
+ break;
+
case uaTrust:
status = STATUS_OK;
break;
***
*** 2072,2074 CheckLDAPAuth(Port *port)
--- 2088,2115
}
#endif /* USE_LDAP */
+
+ /*
+ * SSL client certificate authentication
+ *
+ */
+ #ifdef USE_SSL
+ static int
+ CheckCertAuth(Port *port)
+ {
+ Assert(port->ssl);
+
+ /* Make sure we have received a username in the certificate */
+ if (port->peer_cn == NULL ||
+ strlen(port->peer_cn) <= 0)
+ {
+ ereport(LOG,
+ (errmsg("Certificate login failed for user \"%s\": client certificate contains no username",
+ port->user_name)));
+ return STATUS_ERROR;
+ }
+
+ /* Just pass the certificate CN to the usermap check */
+ return check_usermap(port->hba->usermap, port->user_name, port->peer_cn, false);
+ }
+ #endif
*** a/src/backend/libpq/hba.c
--- b/src/backend/libpq/hba.c
***
*** 859,864 parse_hba_line(List *line, int line_num, HbaLine *parsedline)
--- 859,870
#else
unsupauth = "ldap";
#endif
+ else if (strcmp(token, "cert") == 0)
+ #ifdef USE_SSL
+ parsedline->auth_method = uaCert;
+ #else
+ unsupauth = "cert";
+ #endif
else
{
ereport(LOG,
***
*** 893,898 parse_hba_line(List *line, int line_num, HbaLine *parsedline)
--- 899,915
return false;
}
+ if (parsedline->conntype != ctHostSSL &&
+ parsedline->auth_method == uaCert)
+ {
+ ereport(LOG,
+ (errcode(ERRCODE_CONFIG_FILE_ERROR),
+ errmsg("cert authentication is only supported on hostssl connections"),
+ errcontext("line %d of configuration file \"%s\"",
+ line_num, HbaFileName)));
+ return false;
+ }
+
/* Parse remaining arguments */
while ((line_item = lnext(line_item)) != NULL)
{
***
*** 923,930 ***
Re: [HACKERS] Synchronization Primitives
Hi, Hannu Krosing wrote: >> As I know, you can use show mutex status in MySQL to find which mutex >> is hot. But i don't know in PostgreSQL. > > look at pg_locks system view Or read about dtrace to analyze lower level locking contention: http://www.postgresql.org/docs/8.3/static/dynamic-trace.html Regards Markus Wanner -- 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] Synchronization Primitives
On Thu, 2008-11-13 at 18:55 +0800, [EMAIL PROTECTED] wrote: > Hi all: > > I am a fresh men in PostgreSQL. And i work on benchmark study > these days using PostgreSQL. > > Now i have a question: Is there some way to show the lock contention > of PostgreSQL? > > As I know, you can use show mutex status in MySQL to find which mutex > is hot. But i don't know in PostgreSQL. look at pg_locks system view -- -- Hannu Krosing http://www.2ndQuadrant.com PostgreSQL Scalability and Availability Services, Consulting and Training -- 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] [Slony1-general] ERROR: incompatible library
--- On Wed, 12/11/08, Tony Fernandez <[EMAIL PROTECTED]> wrote: > Date: Wednesday, 12 November, 2008, 10:52 PM > Hello lists, > > > > I am trying to run Slony on a Master Postgres 8.1.11 > replicating to a > Slave same version and 2nd Slave Postgres 8.3.4. > > I am getting the following error: > > > > :14: PGRES_FATAL_ERROR load > '$libdir/xxid'; - ERROR: > incompatible library "/usr/lib/pgsql/xxid.so": > missing magic block > > HINT: Extension libraries are required to use the > PG_MODULE_MAGIC > macro. > > :14: Error: the extension for the xxid data > type cannot be loaded > in database 'dbname=hdap host=10.0.100.234 port=6543 > user=myuser > password=mp' I think you've proabably built slony against one version of postgres and then tried to use it with another. You must build against 8.1.11 and then separately for 8.3.4, using the same version of slony ofcourse. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Synchronization Primitives
Hi all: I am a fresh men in PostgreSQL. And i work on benchmark study these days using PostgreSQL. Now i have a question: Is there some way to show the lock contention of PostgreSQL? As I know, you can use *show mutex status* in MySQL to find which mutex is hot. But i don't know in PostgreSQL. Any ideas? Thanks!
Re: [HACKERS] some strange bugs related to upgrade from 8.1 to 8.3
2008/11/5 Tom Lane <[EMAIL PROTECTED]>: > "Pavel Stehule" <[EMAIL PROTECTED]> writes: >> 2008/11/4 Tom Lane <[EMAIL PROTECTED]>: >>> "Pavel Stehule" <[EMAIL PROTECTED]> writes: a) server crash after creating tsearch2 function (I use tsearch2 contrib from 8.3) >>> >>> I couldn't reproduce that with the script you gave. > >> I tested it on fe8, 32 bit without problem, so it's maybe related to 64bit. > > Can't reproduce it on 64-bit either. Looking closer, I don't believe > that you were running this script at all --- the crash backtrace > includes > > #14 0x005e0a0c in exec_simple_query ( >query_string=0x1f69ae8 "CREATE FUNCTION plpgsql_call_handler() > RETURNS language_handler\nAS '/usr/lib64/pgsql/plpgsql', > 'plpgsql_call_handler'\nLANGUAGE c;") at postgres.c:986 > > and there is no such command in this script. I should send complet script on your private address. It has 2MB. > > Something else weird I just noticed: in this script, you've got > > -- > -- Name: position; Type: TABLE; Schema: public; Owner: lmc; Tablespace: > -- > > CREATE TABLE "position" ( >id integer NOT NULL, >title character varying(255) NOT NULL, >description text, >fulltext_index tsvector > ); > > it's strange - you have different error. Script was truncated, but I have error some time before. ALTER FUNCTION SET SET ERROR: type "tsvector" is only a shell LINE 5: fulltext_index tsvector ^ that is wrong. type tsvector is normal type registrated from tsearch2 module. regards Pavel Stehule > ALTER TABLE public."position" OWNER TO lmc; > > -- > -- Name: view_position_uniq; Type: VIEW; Schema: public; Owner: lmc > -- > > CREATE VIEW view_position_uniq AS >SELECT p.uniq_key FROM "position" p; > > > The CREATE VIEW fails because there's no uniq_key column in "position". > Either you edited this script before sending it in, or there's something > a bit broken about pg_dump or the database you dumped from. > >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] [BUGS] BUG #4516: FOUND variable does not work after RETURN QUERY
I am sending patch, that adds FOUND and GET DIAGNOSTICS support for
RETURN QUERY statement
Regards
Pavel Stehule
2008/11/10 Andrew Gierth <[EMAIL PROTECTED]>:
>> "Pavel" == "Pavel Stehule" <[EMAIL PROTECTED]> writes:
>
> >> Well, changing the semantics of an already-released statement
> >> carries a risk of breaking existing apps that aren't expecting it
> >> to change FOUND. So I'd want to see a pretty strong case why this
> >> is important --- not just that it didn't meet someone's
> >> didn't-read-the-manual expectation.
>
> Pavel> It's should do some problems, but I belive much less than
> Pavel> change of casting or tsearch2 integration. And actually it's
> Pavel> not ortogonal. Every not dynamic statement change FOUND
> Pavel> variable.
>
> Regardless of what you think of FOUND, a more serious problem is this:
>
> postgres=# create function test(n integer) returns setof integer language
> plpgsql
> as $f$
>declare
> rc bigint;
>begin
> return query (select i from generate_series(1,n) i);
> get diagnostics rc = row_count;
> raise notice 'rc = %',rc;
>end;
> $f$;
> CREATE FUNCTION
> postgres=# select test(3);
> NOTICE: rc = 0
> test
> --
>1
>2
>3
> (3 rows)
>
> Since GET DIAGNOSTICS is documented as working for every SQL query
> executed in the function, rather than for a specific list of
> constructs, this is clearly a bug.
>
> --
> Andrew (irc:RhodiumToad)
>
> --
> Sent via pgsql-bugs mailing list ([EMAIL PROTECTED])
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-bugs
>
*** ./doc/src/sgml/plpgsql.sgml.orig 2008-11-13 11:44:57.0 +0100
--- ./doc/src/sgml/plpgsql.sgml 2008-11-13 11:48:39.0 +0100
***
*** 1356,1361
--- 1356,1368
execution of other statements within the loop body.
+
+
+ A RETURN QUERY and RETURN QUERY
+ EXECUTE statements sets FOUND
+ true if query returns least one row.
+
+
FOUND is a local variable within each
*** ./src/pl/plpgsql/src/pl_exec.c.orig 2008-11-13 10:53:37.0 +0100
--- ./src/pl/plpgsql/src/pl_exec.c 2008-11-13 11:29:24.0 +0100
***
*** 2285,2290
--- 2285,2291
PLpgSQL_stmt_return_query *stmt)
{
Portal portal;
+ uint32 processed = 0;
if (!estate->retisset)
ereport(ERROR,
***
*** 2326,2331
--- 2327,2333
HeapTuple tuple = SPI_tuptable->vals[i];
tuplestore_puttuple(estate->tuple_store, tuple);
+ processed++;
}
MemoryContextSwitchTo(old_cxt);
***
*** 2335,2340
--- 2337,2345
SPI_freetuptable(SPI_tuptable);
SPI_cursor_close(portal);
+ estate->eval_processed = processed;
+ exec_set_found(estate, processed != 0);
+
return PLPGSQL_RC_OK;
}
*** ./src/test/regress/expected/plpgsql.out.orig 2008-11-13 11:44:34.0 +0100
--- ./src/test/regress/expected/plpgsql.out 2008-11-13 11:42:56.0 +0100
***
*** 3666,3668
--- 3666,3700
(2 rows)
drop function tftest(int);
+ create or replace function rttest()
+ returns setof int as $$
+ declare rc int;
+ begin
+ return query values(10),(20);
+ get diagnostics rc = row_count;
+ raise notice '% %', found, rc;
+ return query select * from (values(10),(20)) f(a) where false;
+ get diagnostics rc = row_count;
+ raise notice '% %', found, rc;
+ return query execute 'values(10),(20)';
+ get diagnostics rc = row_count;
+ raise notice '% %', found, rc;
+ return query execute 'select * from (values(10),(20)) f(a) where false';
+ get diagnostics rc = row_count;
+ raise notice '% %', found, rc;
+ end;
+ $$ language plpgsql;
+ select * from rttest();
+ NOTICE: t 2
+ NOTICE: f 0
+ NOTICE: t 2
+ NOTICE: f 0
+ rttest
+
+ 10
+ 20
+ 10
+ 20
+ (4 rows)
+
+ drop function rttest();
*** ./src/test/regress/sql/plpgsql.sql.orig 2008-11-13 11:32:17.0 +0100
--- ./src/test/regress/sql/plpgsql.sql 2008-11-13 11:41:20.0 +0100
***
*** 2948,2950
--- 2948,2974
select * from tftest(10);
drop function tftest(int);
+
+ create or replace function rttest()
+ returns setof int as $$
+ declare rc int;
+ begin
+ return query values(10),(20);
+ get diagnostics rc = row_count;
+ raise notice '% %', found, rc;
+ return query select * from (values(10),(20)) f(a) where false;
+ get diagnostics rc = row_count;
+ raise notice '% %', found, rc;
+ return query execute 'values(10),(20)';
+ get diagnostics rc = row_count;
+ raise notice '% %', found, rc;
+ return query execute 'select * from (values(10),(20)) f(a) where false';
+ get diagnostics rc = row_count;
+ raise notice '% %', found, rc;
+ end;
+ $$ language plpgsql;
+
+ select * from rttest();
+
+ drop function rttest();
+
--
Sent via pgsql-hackers mailing list (pgsql-h
Re: [HACKERS] SSL cleanups/hostname verification
It means I will go ahead and apply it once I have looked it over once more. Thanks for review+testing! You may now move on to the next ssl patch if you're interested ;) /Magnus On 12 nov 2008, at 17.05, "Alex Hunsaker" <[EMAIL PROTECTED]> wrote: OK now that im using the right env var everything seems to work as described. FYI I also tried to exercise the various new error paths and everything seems good so as far as i'm concerned this looks good to me. Ill go mark it as "ready for commiter" on the wiki. (whatever that means you being a commiter :) ) --- $ PGSSLVERIFY=none ./psql postgres -h 127.0.0.1 psql (8.4devel) SSL connection (cipher: DHE-RSA-AES256-SHA, bits: 256) Type "help" for help. postgres=# \q $ PGSSLVERIFY=cert ./psql postgres -h 127.0.0.1 psql (8.4devel) SSL connection (cipher: DHE-RSA-AES256-SHA, bits: 256) Type "help" for help. postgres=# \q $ ./psql postgres -h 127.0.0.1 psql: server common name 'bahdushka' does not match hostname '127.0.0.1'FATAL: no pg_hba.conf entry for host "127.0.0.1", user "alex", database "postgres", SSL off $ PGHOSTADDR=127.0.0.1 ./psql postgres -h 127.0.0.1 psql: verified SSL connections are only supported when connecting to a hostnameFATAL: no pg_hba.conf entry for host "127.0.0.1", user "alex", database "postgres", SSL off $ rm ~/.postgresql/root.crt $ PGSSLVERIFY=none ./psql postgres -h 127.0.0.1 psql (8.4devel) SSL connection (cipher: DHE-RSA-AES256-SHA, bits: 256) Type "help" for help. postgres=# \q $ PGSSLVERIFY=cert ./psql postgres -h 127.0.0.1 psql: root certificate file (/home/alex/.postgresql/root.crt) not found -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
