Re: [HACKERS] [PATCHES] WAL logging freezing
Simon Riggs [EMAIL PROTECTED] writes: I don't agree: If the truncation points are at 1 million, 2 million etc, then if we advance the relvacuumxid from 1.2 million to 1.5 million, then crash, the hints bits for that last vacuum are lost. Sounds bad, but we have not truncated clog, so there is no danger. You're still wrong though. Suppose that VACUUM moves a particular rel's relvacuumxid from 1.9 to 2.1 million, but because this rel is not currently the oldest vacuumxid, it doesn't truncate clog. Then we crash and lose hint bits, but not the relvacuumxid change. Then VACUUM vacuums some other rel and advances its relvacuumxid from 1.9 to 2.1 million --- but this time that *was* the globally oldest value, and now we think we can truncate clog at 2 million. But the first rel might still have some unhinted xids around 1.9 million. If you look at this another way, maybe you'll see what I'm saying: Only update relvacuumxid iff the update would allow us to truncate the clog. Then you'll never update it at all, because there will always be some other rel constraining the global min. regards, tom lane ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] [PATCHES] WAL logging freezing
On Mon, 2006-10-30 at 19:18 -0500, Tom Lane wrote: Simon Riggs [EMAIL PROTECTED] writes: I don't agree: If the truncation points are at 1 million, 2 million etc, then if we advance the relvacuumxid from 1.2 million to 1.5 million, then crash, the hints bits for that last vacuum are lost. Sounds bad, but we have not truncated clog, so there is no danger. You're still wrong though. Frequently, I'd say :-) Suppose that VACUUM moves a particular rel's relvacuumxid from 1.9 to 2.1 million, but because this rel is not currently the oldest vacuumxid, it doesn't truncate clog. Then we crash and lose hint bits, but not the relvacuumxid change. Then VACUUM vacuums some other rel and advances its relvacuumxid from 1.9 to 2.1 million --- but this time that *was* the globally oldest value, and now we think we can truncate clog at 2 million. But the first rel might still have some unhinted xids around 1.9 million. That was understood; in the above example I agree you need to flush. If you don't pass a truncation point, you don't need to flush whether or not you actually truncate. So we don't need to flush *every* time, so IMHO we don't need to play safe and keep clog the size of an iceberg. Anyway, if PITR is safe again, I'd like to sleepzz -- Simon Riggs EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] WAL logging freezing
Simon Riggs [EMAIL PROTECTED] writes: That was understood; in the above example I agree you need to flush. If you don't pass a truncation point, you don't need to flush whether or not you actually truncate. So we don't need to flush *every* time, OK, but does that actually do much of anything for your performance complaint? Just after GlobalXmin has passed a truncation point, *every* vacuum the system does will start performing a flush-n-fsync, which seems like exactly what you didn't like. If the syncs were spread out in time for different rels then maybe this idea would help, but AFAICS they won't be. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] WAL logging freezing
Simon Riggs [EMAIL PROTECTED] writes: [I've just coded the relcache invalidation WAL logging patch also.] What? That doesn't make any sense to me. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] WAL logging freezing
On Fri, 2006-10-27 at 12:01 -0400, Tom Lane wrote: Heikki Linnakangas [EMAIL PROTECTED] writes: Tom Lane wrote: I think it's premature to start writing patches until we've decided how this really needs to work. Not logging hint-bit updates seems safe to me. As long as we have the clog, the hint-bit is just a hint. The problem with freezing is that after freezing tuples, the corresponding clog page can go away. Actually clog can go away much sooner than that, at least in normal operation --- that's what datvacuumxid is for, to track where we can truncate clog. So we definitely have a nasty problem here. VACUUM FREEZE is just a loaded gun right now. Maybe it's OK to say that during WAL replay we keep it all the way back to the freeze horizon, but I'm not sure how we keep the system from wiping clog it still needs right after switching to normal operation. Maybe we should somehow not xlog updates of datvacuumxid? Thinking... Also, we should probably be setting all the hint bits for pages during recovery then, so we don't need to re-write them again later. Another thing I'm concerned about is the scenario where a PITR hot-standby machine tracks a master over a period of more than 4 billion transactions. I'm not sure what will happen in the slave's pg_clog directory, but I'm afraid it won't be good :-( I think we'll need to error-out at that point, plus produce messages when we pass 2 billion transactions recovered. It makes sense to produce a new base backup regularly anyway. We'll also need to produce an error message on the primary server so that we take a new base backup every 2 billion transactions. There are better solutions, but I'm not sure it makes sense to try and fix them right now, since that could well delay the release. If we think it is a necessary fix for the 8.2 line then we could get a better fix into 8.2.1 [I've just coded the relcache invalidation WAL logging patch also.] -- Simon Riggs EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] [PATCHES] WAL logging freezing
On Fri, 2006-10-27 at 22:19 +0100, Simon Riggs wrote: So we definitely have a nasty problem here. VACUUM FREEZE is just a loaded gun right now. Maybe it's OK to say that during WAL replay we keep it all the way back to the freeze horizon, but I'm not sure how we keep the system from wiping clog it still needs right after switching to normal operation. Maybe we should somehow not xlog updates of datvacuumxid? Thinking... Suggestions: 1. Create a new Utility rmgr that can issue XLOG_UTIL_FREEZE messages for each block that has had any tuples frozen on it during normal VACUUMs. We need log only the relid, blockid and vacuum's xid to redo the freeze operation. 2. VACUUM FREEZE need not generate any additional WAL records, but will do an immediate sync following execution and before clog truncation. That way the large number of changed blocks will all reach disk before we do the updates to the catalog. 3. We don't truncate the clog during WAL replay, so the clog will grow during recovery. Nothing to do there to make things safe. 4. When InArchiveRecovery we should set all of the datminxid and datvacuumxid fields to be the Xid from where recovery started, so that clog is not truncated soon after recovery. Performing a VACUUM FREEZE after a recovery would be mentioned as an optional task at the end of a PITR recovery on a failover/second server. 5. At 3.5 billion records during recovery we should halt the replay, do a full database scan to set hint bits, truncate clog, then restart replay. (Automatically within the recovery process). 6. During WAL replay, put out a warning message every 1 billion rows saying that a hint bit scan will eventually be required if recovery continues. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] GUC description cleanup
Neil, Sure, I'll wait for 8.3 to branch. I have some cleanup I want to do for 8.3 too. Josh Berkus PostgreSQL @ Sun San Francisco 415-752-2500 ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] Eliminating phase 3 requirement for varlen increases via ALTER COLUMN
Jonah H. Harris [EMAIL PROTECTED] writes: On 10/26/06, Tom Lane [EMAIL PROTECTED] wrote: This makes some really quite unacceptable assumptions about the meaning and encoding of typmod ... True, so VARCHAR seems like the only one? That's the only one I've really encountered in the field on a fairly regular basis. I think what you want is to add a new method entry in pg_type to allow a type to declare a method to tell you whether a change is work-free or not. Then any type, even user-defined types, can allow some changes to be work-free and some not without exposing any implementation details outside the type. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] [PATCHES] Eliminating phase 3 requirement for varlen increases via ALTER COLUMN
On 10/26/06, Gregory Stark [EMAIL PROTECTED] wrote: I think what you want is to add a new method entry in pg_type to allow a type to declare a method to tell you whether a change is work-free or not. Then any type, even user-defined types, can allow some changes to be work-free and some not without exposing any implementation details outside the type. Seems like too much work for a fairly simple use-case. -- Jonah H. Harris, Software Architect | phone: 732.331.1300 EnterpriseDB Corporation| fax: 732.331.1301 33 Wood Ave S, 2nd Floor| [EMAIL PROTECTED] Iselin, New Jersey 08830| http://www.enterprisedb.com/ ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] [PATCHES] zic with msvc
+ #ifdef WIN32 + #define _WIN32_WINNT 0x0400 + #endif Hmm ... in pg_ctl.c I see #define _WIN32_WINNT 0x0500 Is there a reason for these to be different? Are there other places that will need this (ie, maybe it should be in c.h instead?) Not really. The default appears to be 0x0400 for MingW (or it wouldn't have worked before), but 0x0350 or so for Visual C++. If we define it to 0x0500 we pull in headers that will only work on 2000 or newer. Hm. Actually, if the rest of the backend compiles without this, then I guess the real question is what's zic.c doing that needs it? pg_ctl.c has an excuse because it's doing weird MS-specific junk, but zic.c is supposed to be bog-standard portable code. It really shouldn't have anything that's further out there than you could find in ten other places in the backend. Only almost. There is a small function to emulate link(), which uses CopyFileEx(). That's the one. //Magnus ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] zic with msvc
Magnus Hagander [EMAIL PROTECTED] writes: + #ifdef WIN32 + #define _WIN32_WINNT 0x0400 + #endif Hmm ... in pg_ctl.c I see #define _WIN32_WINNT 0x0500 Is there a reason for these to be different? Are there other places that will need this (ie, maybe it should be in c.h instead?) Not really. The default appears to be 0x0400 for MingW (or it wouldn't have worked before), but 0x0350 or so for Visual C++. If we define it to 0x0500 we pull in headers that will only work on 2000 or newer. Hm. Actually, if the rest of the backend compiles without this, then I guess the real question is what's zic.c doing that needs it? pg_ctl.c has an excuse because it's doing weird MS-specific junk, but zic.c is supposed to be bog-standard portable code. It really shouldn't have anything that's further out there than you could find in ten other places in the backend. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] array_accum aggregate
On Thu, Oct 12, 2006 at 06:58:52PM -0400, Tom Lane wrote: I wrote: aggregate_state would have no other uses in the system, and its input and output functions would raise an error, so type safety is assured --- there would be no way to call either the sfunc or ffunc manually, except by passing a NULL value, which should be safe because that's what they'd expect as the aggregate initial condition. Um, no, I take that back, unless you want to invent a separate pseudotype for each such aggregate. Otherwise you can crash it with snip What this really calls for is a type that users are forbidden to interact with directly. Basically, the type may only be used by C functions and such C functions may not appear in an SQL query. Seems the only way to safely deal with datums you don't know the representation of. The name internal would be nice, but it's taken :( Have a nice day, -- Martijn van Oosterhout kleptog@svana.org http://svana.org/kleptog/ From each according to his ability. To each according to his ability to litigate. signature.asc Description: Digital signature
Re: [HACKERS] [PATCHES] array_accum aggregate
Martijn van Oosterhout kleptog@svana.org writes: What this really calls for is a type that users are forbidden to interact with directly. Basically, the type may only be used by C functions and such C functions may not appear in an SQL query. That's not really the flavor of solution I'd like to have. Ideally, it'd actually *work* to write my_ffunc(my_sfunc(my_sfunc(null, 1), 2)) and get the same result as aggregating over the values 1 and 2. The trick is to make sure that my_sfunc and my_ffunc can only be used together. Maybe we do need a type for each such aggregate ... In any case, internal isn't quite the right model, because with that, the type system is basically disclaiming all knowledge of the data's properties. With an aggregate_state datatype, the type system would be asserting that it's OK to use this type (or more accurately, these functions) in an aggregate. regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] [PATCHES] array_accum aggregate
* Tom Lane ([EMAIL PROTECTED]) wrote: That's not really the flavor of solution I'd like to have. Ideally, it'd actually *work* to write my_ffunc(my_sfunc(my_sfunc(null, 1), 2)) and get the same result as aggregating over the values 1 and 2. The trick is to make sure that my_sfunc and my_ffunc can only be used together. Maybe we do need a type for each such aggregate ... In general I like this idea but there are some complications, the main one being where would the memory be allocated? I guess we could just always use the QueryContext. The other issue is, in the above scenario is it acceptable to modify the result of my_sfunc(null, 1) in the ,2 call? Normally it's unacceptable to modify your input, aggregates get a special exception when called as an aggregate, but in this case you'd have to copy the entire custom structure underneath, I'd think. As for a type for each such aggregate, that seems reasonable to me, honestly. I'd like for the type creation to be reasonably simple though, if possible. ie: could we just provide NULLs for the input/output functions instead of having to implement functions that just return an error? Or perhaps have a polymorphic function which can be reused that just returns an error. Additionally, we'd have to be able to mark the types as being polymorhpic along the same lines as anyelement/anyarray. Hopefully that's already easy, but if not it'd be nice if it could be made easy... Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] [PATCHES] array_accum aggregate
Stephen Frost [EMAIL PROTECTED] writes: That's not really the flavor of solution I'd like to have. Ideally, it'd actually *work* to write my_ffunc(my_sfunc(my_sfunc(null, 1), 2)) In general I like this idea but there are some complications, the main one being where would the memory be allocated? In the agg context if called with that context, else CurrentMemoryContext will do fine. The other issue is, in the above scenario is it acceptable to modify the result of my_sfunc(null, 1) in the ,2 call? Yes, because the only place a nonnull value of the type could have come from is a my_sfunc call; since it's a pseudotype, we don't allow it on disk. (We might also need a hack to prevent the type from being used as a record-type component ... not sure if that comes for free with being a pseudotype currently.) As for a type for each such aggregate, that seems reasonable to me, honestly. The ugly part is that we'd still need a way for the planner to recognize this class of types. Additionally, we'd have to be able to mark the types as being polymorhpic along the same lines as anyelement/anyarray. What for? regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] [PATCHES] array_accum aggregate
* Tom Lane ([EMAIL PROTECTED]) wrote: Stephen Frost [EMAIL PROTECTED] writes: The other issue is, in the above scenario is it acceptable to modify the result of my_sfunc(null, 1) in the ,2 call? Yes, because the only place a nonnull value of the type could have come from is a my_sfunc call; since it's a pseudotype, we don't allow it on disk. (We might also need a hack to prevent the type from being used as a record-type component ... not sure if that comes for free with being a pseudotype currently.) Ah, ok. As for a type for each such aggregate, that seems reasonable to me, honestly. The ugly part is that we'd still need a way for the planner to recognize this class of types. Hrm, true. I would agree that they would need more memory than most aggregates. It seems likely to me that worst offenders are going to be of the array_accum type- store all tuples coming in. Therefore, my suggestion would be that the memory usage be estimated along those lines and allow for hashagg when there's enough memory to keep all the tuples (or rather the portion of the tuple going into the aggregate) in memory (plus some amount of overhead, maybe 10%). That doesn't help with how to get the planner to recognize this class of types though. I don't suppose we already have a class framework which the planner uses to group types which have similar characteristics? Additionally, we'd have to be able to mark the types as being polymorhpic along the same lines as anyelement/anyarray. What for? So that the finalfunc can be polymorphic along the lines of my suggested aaccum_sfunc(anyarray,anyelement) returns anyarray and aaccum_ffunc(anyarray) returns anyarray. If the state type isn't polymorphic then PG won't let me create a function from it which returns a polymorphic, aiui. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] [PATCHES] array_accum aggregate
Stephen Frost [EMAIL PROTECTED] writes: Additionally, we'd have to be able to mark the types as being polymorhpic along the same lines as anyelement/anyarray. What for? So that the finalfunc can be polymorphic along the lines of my suggested aaccum_sfunc(anyarray,anyelement) returns anyarray and aaccum_ffunc(anyarray) returns anyarray. Hmm ... there's not any real need for this at the level of the aggregate, because we resolve a polymorphic aggregate's output type directly from its input type(s), and we've already established that the general-purpose agg code doesn't need to be able to infer anything about the transition data type. However the function code is going to complain if you try to declare my_sfunc(aggregate_state) returns anyarray because that looks ill-formed ... and indeed that kinda kills the idea of being able to call my_sfunc standalone anyway. Maybe we need a more radical solution in which the sfunc/ffunc don't exist as separately callable entities at all. That would sidestep the whole need to have a type-system representation for the state data, as well as eliminate worries about whether we've sufficiently plugged the security risks of being able to call one of them in an unexpected context. Not sure what this should look like in the catalogs though. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] test: please ignore
Magnus Hagander [EMAIL PROTECTED] writes: I've posted a 6.5kB patch (as an attachment) three times over the past few days but haven't seen it hit the lists. Checking to see if this goes through. Did you by any chance gzip it? IIRC, mails with gzipped attachments are silently dropped on- patches for some reason. Hm? They've always worked fine for me, and for a lot of other people. You should ask Marc to look into this. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] test: please ignore
Tom Lane wrote: Magnus Hagander [EMAIL PROTECTED] writes: I've posted a 6.5kB patch (as an attachment) three times over the past few days but haven't seen it hit the lists. Checking to see if this goes through. Did you by any chance gzip it? IIRC, mails with gzipped attachments are silently dropped on- patches for some reason. Hm? They've always worked fine for me, and for a lot of other people. You should ask Marc to look into this. It depends on the MIME type IIRC. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] [PATCHES] DOC: catalog.sgml
Zdenek Kotala wrote: Alvaro Herrera wrote: What's global? A maybe-useful flag would be telling that a table is shared. Is that it? Mind you, it's not useful to me because I know which tables are shared, but I guess for someone not so familiar with the catalogs it could have some use. Global means share table stored in global directory :-). Ok I change it. There is new version of catalogs overview patch. This version add only one column into overview table which contains Oid/Filename for each catalog table. Oid information is important if someone need make relation with filename on disk and related catalog table. In this column shared table are marked as well. Other information like bootstrap and with-oids are really useless and they is not mentioned. Zdenek Index: doc/src/sgml/catalogs.sgml === RCS file: /projects/cvsroot/pgsql/doc/src/sgml/catalogs.sgml,v retrieving revision 2.134 diff -c -r2.134 catalogs.sgml *** doc/src/sgml/catalogs.sgml 22 Sep 2006 23:20:13 - 2.134 --- doc/src/sgml/catalogs.sgml 4 Oct 2006 12:17:49 - *** *** 33,47 Most system catalogs are copied from the template database during database creation and are thereafter database-specific. A few catalogs are physically shared across all databases in a cluster; !these are noted in the descriptions of the individual catalogs. /para table id=catalog-table titleSystem Catalogs/title !tgroup cols=2 thead row entryCatalog Name/entry entryPurpose/entry /row --- 33,49 Most system catalogs are copied from the template database during database creation and are thereafter database-specific. A few catalogs are physically shared across all databases in a cluster; !these are noted in the Oid/Filename column and in the descriptions !of the individual catalogs. /para table id=catalog-table titleSystem Catalogs/title !tgroup cols=3 thead row + entryOid/Filename/entry entryCatalog Name/entry entryPurpose/entry /row *** *** 49,214 --- 51,249 tbody row + entry2600/entry entrylink linkend=catalog-pg-aggregatestructnamepg_aggregate/structname/link/entry entryaggregate functions/entry /row row + entry2601/entry entrylink linkend=catalog-pg-amstructnamepg_am/structname/link/entry entryindex access methods/entry /row row + entry2602/entry entrylink linkend=catalog-pg-amopstructnamepg_amop/structname/link/entry entryaccess method operators/entry /row row + entry2603/entry entrylink linkend=catalog-pg-amprocstructnamepg_amproc/structname/link/entry entryaccess method support procedures/entry /row row + entry2604/entry entrylink linkend=catalog-pg-attrdefstructnamepg_attrdef/structname/link/entry entrycolumn default values/entry /row row + entry1249/entry entrylink linkend=catalog-pg-attributestructnamepg_attribute/structname/link/entry entrytable columns (quoteattributes/quote)/entry /row row + entry1260 (shared)/entry entrylink linkend=catalog-pg-authidstructnamepg_authid/structname/link/entry entryauthorization identifiers (roles)/entry /row row + entry1261 (shared)/entry entrylink linkend=catalog-pg-auth-membersstructnamepg_auth_members/structname/link/entry entryauthorization identifier membership relationships/entry /row row + entry1248/entry entrylink linkend=catalog-pg-autovacuumstructnamepg_autovacuum/structname/link/entry entryper-relation autovacuum configuration parameters/entry /row row + entry2605/entry entrylink linkend=catalog-pg-caststructnamepg_cast/structname/link/entry entrycasts (data type conversions)/entry /row row + entry1259/entry entrylink linkend=catalog-pg-classstructnamepg_class/structname/link/entry entrytables, indexes, sequences, views (quoterelations/quote)/entry /row row + entry2606/entry entrylink linkend=catalog-pg-constraintstructnamepg_constraint/structname/link/entry entrycheck constraints, unique constraints, primary key constraints, foreign key constraints/entry /row row + entry2607/entry entrylink linkend=catalog-pg-conversionstructnamepg_conversion/structname/link/entry entryencoding conversion information/entry /row row + entry1262 (shared)/entry entrylink linkend=catalog-pg-databasestructnamepg_database/structname/link/entry entrydatabases within this database cluster/entry
Re: [HACKERS] [PATCHES] DOC: catalog.sgml
Zdenek Kotala [EMAIL PROTECTED] writes: There is new version of catalogs overview patch. This version add only one column into overview table which contains Oid/Filename for each catalog table. Oid information is important if someone need make relation with filename on disk and related catalog table. I still say this is just confusing clutter. The proposed patch even goes so far as to give the OID pride of place as the most important item you could possibly want to know about a catalog, which is surely silly. People who actually want to know this information can look into the pg_class catalog, which has the advantages of being complete (eg, it covers indexes too), guaranteed up-to-date, and easily program-readable. I really do not see the value of putting it in the sgml docs. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] DOC: catalog.sgml
Tom Lane wrote: Zdenek Kotala [EMAIL PROTECTED] writes: There is new version of catalogs overview patch. This version add only one column into overview table which contains Oid/Filename for each catalog table. Oid information is important if someone need make relation with filename on disk and related catalog table. I still say this is just confusing clutter. The proposed patch even goes so far as to give the OID pride of place as the most important item you could possibly want to know about a catalog, which is surely silly. You have right that OID is not important information in many cases, but how I said It is useful when you want know relation between filename and catalog table. People who actually want to know this information can look into the pg_class catalog, which has the advantages of being complete (eg, it covers indexes too), guaranteed up-to-date, and easily program-readable. I really do not see the value of putting it in the sgml docs. You can look into pg_class catalog only if database is running. If you have some data corruption problem, OID should help during recovery. But you have right, that pg_class have complex information and who want to play with datafiles, he must know more than OID. OK, thanks for response, forget to this patch Zdenek ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Bad bug in fopen() wrapper code
Magnus, is this the right fix? Well, actually msdn states: Return Value If successful, _setmode returns the previous translation mode. A return value of -1 indicates an error So, shouldn't we be testing for -1 instead of 0 ? The thing is probably academic, since _setmode is only supposed to fail on invalid file handle or invalid mode. So basically, given our code, it should only fail if filemode is (O_BINARY | O_TEXT) both flags set. Andreas ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] Bad bug in fopen() wrapper code
Zeugswetter Andreas DCP SD [EMAIL PROTECTED] writes: If successful, _setmode returns the previous translation mode. A return value of -1 indicates an error So, shouldn't we be testing for -1 instead of 0 ? I think the usual convention is to test for 0, unless there are other negative return values that are legal. This is doubtless a silly cycle-shaving habit (on nearly all machines, test against 0 is a bit more compact than test against other constants), but it is a widespread habit anyway, and if you sometimes do it one way and sometimes another you just create a distraction for readers. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] Bad bug in fopen() wrapper code
I agree that this code is both wrong and unreadable (although in practice the _setmode will probably never fail, which is why our attention hasn't been drawn to it). Is someone going to submit a patch? I'm hesitant to change the code myself since I'm not in a position to test it. I can look at fixing that. Is it something we want to do for 8.2, or wait until 8.3? If there's a hidden bug, I guess 8.2? Magnus, is this the right fix? Claudio sent me some small updates to the patch; updated version attached. Tested, passes all regression tests on MingW. //Magnus ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] [PATCHES] Bad bug in fopen() wrapper code
Applied.
---
Bruce Momjian wrote:
Bruce Momjian wrote:
Magnus Hagander wrote:
Now, I still twist my head around the lines:
if ((fd = _open_osfhandle((long) h, fileFlags O_APPEND)) 0
||
(fileFlags (O_TEXT | O_BINARY) (_setmode(fd, fileFlags
(O_TEXT
| O_BINARY)) 0)))
Without having studied it closely, it might also highlight a bug
on
failure of the second clause -- if the _setmode fails, shouldn't
_close be called instead of CloseHandle, and -1 returned?
(CloseHandle would still be called on failure of the
_open_osfhandle,
obviously)
I agree that this code is both wrong and unreadable (although in
practice the _setmode will probably never fail, which is why our
attention hasn't been drawn to it). Is someone going to submit a
patch? I'm hesitant to change the code myself since I'm not in a
position to test it.
I can look at fixing that. Is it something we want to do for 8.2, or
wait until 8.3? If there's a hidden bug, I guess 8.2?
Magnus, is this the right fix?
Claudio sent me some small updates to the patch; updated version
attached.
--
Bruce Momjian [EMAIL PROTECTED]
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
[ text/x-diff is unsupported, treating like TEXT/PLAIN ]
Index: src/port/open.c
===
RCS file: /cvsroot/pgsql/src/port/open.c,v
retrieving revision 1.15
diff -c -c -r1.15 open.c
*** src/port/open.c 24 Sep 2006 17:19:53 - 1.15
--- src/port/open.c 3 Oct 2006 01:16:43 -
***
*** 105,113
}
/* _open_osfhandle will, on error, set errno accordingly */
! if ((fd = _open_osfhandle((long) h, fileFlags O_APPEND)) 0 ||
! (fileFlags (O_TEXT | O_BINARY) (_setmode(fd, fileFlags
(O_TEXT | O_BINARY)) 0)))
CloseHandle(h); /* will not affect errno */
return fd;
}
--- 105,119
}
/* _open_osfhandle will, on error, set errno accordingly */
! if ((fd = _open_osfhandle((long) h, fileFlags O_APPEND)) 0)
CloseHandle(h); /* will not affect errno */
+ else if (fileFlags (O_TEXT | O_BINARY)
+ _setmode(fd, fileFlags (O_TEXT | O_BINARY)) 0)
+ {
+ _close(fd);
+ return -1;
+ }
+
return fd;
}
--
Bruce Momjian [EMAIL PROTECTED]
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Bad bug in fopen() wrapper code
Now, I still twist my head around the lines: if ((fd = _open_osfhandle((long) h, fileFlags O_APPEND)) 0 || (fileFlags (O_TEXT | O_BINARY) (_setmode(fd, fileFlags (O_TEXT | O_BINARY)) 0))) Without having studied it closely, it might also highlight a bug on failure of the second clause -- if the _setmode fails, shouldn't _close be called instead of CloseHandle, and -1 returned? (CloseHandle would still be called on failure of the _open_osfhandle, obviously) I agree that this code is both wrong and unreadable (although in practice the _setmode will probably never fail, which is why our attention hasn't been drawn to it). Is someone going to submit a patch? I'm hesitant to change the code myself since I'm not in a position to test it. I can look at fixing that. Is it something we want to do for 8.2, or wait until 8.3? If there's a hidden bug, I guess 8.2? //Magnus ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] Bad bug in fopen() wrapper code
Magnus Hagander wrote:
Now, I still twist my head around the lines:
if ((fd = _open_osfhandle((long) h, fileFlags O_APPEND)) 0
||
(fileFlags (O_TEXT | O_BINARY) (_setmode(fd, fileFlags
(O_TEXT
| O_BINARY)) 0)))
Without having studied it closely, it might also highlight a bug
on
failure of the second clause -- if the _setmode fails, shouldn't
_close be called instead of CloseHandle, and -1 returned?
(CloseHandle would still be called on failure of the
_open_osfhandle,
obviously)
I agree that this code is both wrong and unreadable (although in
practice the _setmode will probably never fail, which is why our
attention hasn't been drawn to it). Is someone going to submit a
patch? I'm hesitant to change the code myself since I'm not in a
position to test it.
I can look at fixing that. Is it something we want to do for 8.2, or
wait until 8.3? If there's a hidden bug, I guess 8.2?
Magnus, is this the right fix?
--
Bruce Momjian [EMAIL PROTECTED]
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: src/port/open.c
===
RCS file: /cvsroot/pgsql/src/port/open.c,v
retrieving revision 1.15
diff -c -c -r1.15 open.c
*** src/port/open.c 24 Sep 2006 17:19:53 - 1.15
--- src/port/open.c 3 Oct 2006 01:16:43 -
***
*** 105,113
}
/* _open_osfhandle will, on error, set errno accordingly */
! if ((fd = _open_osfhandle((long) h, fileFlags O_APPEND)) 0 ||
! (fileFlags (O_TEXT | O_BINARY) (_setmode(fd, fileFlags (O_TEXT | O_BINARY)) 0)))
CloseHandle(h); /* will not affect errno */
return fd;
}
--- 105,119
}
/* _open_osfhandle will, on error, set errno accordingly */
! if ((fd = _open_osfhandle((long) h, fileFlags O_APPEND)) 0)
CloseHandle(h); /* will not affect errno */
+ else if (fileFlags (O_TEXT | O_BINARY)
+ _setmode(fd, fileFlags (O_TEXT | O_BINARY)) 0)
+ {
+ _close(fd)/* will not affect errno */
+ return -1;
+ }
+
return fd;
}
---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match
Re: [HACKERS] [PATCHES] Bad bug in fopen() wrapper code
Bruce Momjian wrote:
Magnus Hagander wrote:
Now, I still twist my head around the lines:
if ((fd = _open_osfhandle((long) h, fileFlags O_APPEND)) 0
||
(fileFlags (O_TEXT | O_BINARY) (_setmode(fd, fileFlags
(O_TEXT
| O_BINARY)) 0)))
Without having studied it closely, it might also highlight a bug
on
failure of the second clause -- if the _setmode fails, shouldn't
_close be called instead of CloseHandle, and -1 returned?
(CloseHandle would still be called on failure of the
_open_osfhandle,
obviously)
I agree that this code is both wrong and unreadable (although in
practice the _setmode will probably never fail, which is why our
attention hasn't been drawn to it). Is someone going to submit a
patch? I'm hesitant to change the code myself since I'm not in a
position to test it.
I can look at fixing that. Is it something we want to do for 8.2, or
wait until 8.3? If there's a hidden bug, I guess 8.2?
Magnus, is this the right fix?
Claudio sent me some small updates to the patch; updated version
attached.
--
Bruce Momjian [EMAIL PROTECTED]
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: src/port/open.c
===
RCS file: /cvsroot/pgsql/src/port/open.c,v
retrieving revision 1.15
diff -c -c -r1.15 open.c
*** src/port/open.c 24 Sep 2006 17:19:53 - 1.15
--- src/port/open.c 3 Oct 2006 01:16:43 -
***
*** 105,113
}
/* _open_osfhandle will, on error, set errno accordingly */
! if ((fd = _open_osfhandle((long) h, fileFlags O_APPEND)) 0 ||
! (fileFlags (O_TEXT | O_BINARY) (_setmode(fd, fileFlags (O_TEXT | O_BINARY)) 0)))
CloseHandle(h); /* will not affect errno */
return fd;
}
--- 105,119
}
/* _open_osfhandle will, on error, set errno accordingly */
! if ((fd = _open_osfhandle((long) h, fileFlags O_APPEND)) 0)
CloseHandle(h); /* will not affect errno */
+ else if (fileFlags (O_TEXT | O_BINARY)
+ _setmode(fd, fileFlags (O_TEXT | O_BINARY)) 0)
+ {
+ _close(fd);
+ return -1;
+ }
+
return fd;
}
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] Bad bug in fopen() wrapper code
Without having studied it closely, it might also
highlight a bug
on
failure of the second clause -- if the _setmode
fails, shouldn't
_close be called instead of CloseHandle, and -1 returned?
(CloseHandle would still be called on failure of the
_open_osfhandle,
obviously)
I agree that this code is both wrong and unreadable
(although in
practice the _setmode will probably never fail, which
is why our
attention hasn't been drawn to it). Is someone going
to submit a
patch? I'm hesitant to change the code myself since
I'm not in a
position to test it.
I can look at fixing that. Is it something we want to do
for 8.2, or
wait until 8.3? If there's a hidden bug, I guess 8.2?
Magnus, is this the right fix?
Claudio sent me some small updates to the patch; updated
version attached.
If we're going for maximum readability, I'd actually split
+ else if (fileFlags (O_TEXT | O_BINARY)
+ _setmode(fd, fileFlags (O_TEXT | O_BINARY)) 0)
into two different if statements as wlel. Ee.g.
else if (fileFlags (O_TEXT | O_BINARY)) {
if (_setmode() 0) {
failure();
}
}
Haven't tested the patch yet, but it looks ok.
//Magnus
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] Patch for UUID datatype (beta)
Mark,A model that intended to try and guarantee uniqueness would provide aUUID generation service for the entire host, that was not specific to any application, or database, possibly accessible via the loopbackaddress. It would ensure that at any given time, either the time isnew, or the sequence is new for the time. If computer time ever wentbackwards, it could keep the last time issued persistent, and increment from this point forward through the clock sequence valuesuntil real time catches up. An alternative would be along the lines ofa /dev/uuid device, that like /dev/random, would be responsible foroutputting unique uuid values for the system. Who does this? Probably nobody. I'm tempted to implement it, though, for my uses. :-)That is an excellent summary. There is just one wrong assumption in it:Probably nobody. Within win32 there is an API call, which provides you with an GUID / UUID with to my knowledge exactly the features you are describing. win32 is installed on some computers. So for PostgreSQL on win32 the new_guid() you describe in detail would be quite simple to implement: a call to CoCreateGuid.The challenging part is: I use PostgreSQL in a mixed environment. And Linux i.e. does not provide CoCreateGuid. That's why I am voting to have it in PostgreSQL :) Harald-- GHUM Harald Massapersuadere et programmareHarald Armin MassaReinsburgstraße 202b70197 Stuttgart0173/9409607-Let's set so double the killer delete select all.
Re: [HACKERS] [PATCHES] setseed() doc
Neil Conway [EMAIL PROTECTED] writes: On Mon, 2006-09-04 at 15:19 -0400, Tom Lane wrote: AFAICT it's just junk. It happens to be the input times MAX_RANDOM_VALUE, but what use is that? I wonder if we shouldn't change the function to return VOID I agree. Given how soon we want to get an 8.2 beta out the door, perhaps this change would be best postponed to 8.3 (unless there's another outstanding 8.2 patch that requires initdb?). Nothing outstanding at the moment. Although this is surely a small change, it's also pretty low-priority, so I'd counsel leaving it for 8.3 rather than trying to cram it in now. We have more important things to be worrying about ... regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Patch for UUID datatype (beta)
On Mon, Sep 18, 2006 at 07:45:07PM -0400, [EMAIL PROTECTED] wrote: I would not use a 100% random number generator for a UUID value as was suggested. I prefer inserting the MAC address and the time, to at least allow me to control if a collision is possible. This is not easy to do using a few lines of C code. I'd rather have a UUID type in core with no generation routine, than no UUID type in core because the code is too complicated to maintain, or not portable enough. As others have mentioned, using MAC address doesn't remove the possibility of a collision. Maybe a good compromise that would allow a generator function to go into the backend would be to combine the current time with a random number. That will ensure that you won't get a dupe, so long as your clock never runs backwards. -- Jim Nasby[EMAIL PROTECTED] EnterpriseDB http://enterprisedb.com 512.569.9461 (cell) ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] Patch for UUID datatype (beta)
On Tue, Sep 19, 2006 at 08:20:13AM -0500, Jim C. Nasby wrote: On Mon, Sep 18, 2006 at 07:45:07PM -0400, [EMAIL PROTECTED] wrote: I would not use a 100% random number generator for a UUID value as was suggested. I prefer inserting the MAC address and the time, to at least allow me to control if a collision is possible. This is not easy to do using a few lines of C code. I'd rather have a UUID type in core with no generation routine, than no UUID type in core because the code is too complicated to maintain, or not portable enough. As others have mentioned, using MAC address doesn't remove the possibility of a collision. It does, as I control the MAC address. I can choose not to overwrite it. I can choose to ensure that any cases where it is overwritten, it is overwritten with a unique value. Random number does not provide this level of control. Maybe a good compromise that would allow a generator function to go into the backend would be to combine the current time with a random number. That will ensure that you won't get a dupe, so long as your clock never runs backwards. Which standard UUID generation function would you be thinking of? Inventing a new one doesn't seem sensible. I'll have to read over the versions again... Cheers, mark -- [EMAIL PROTECTED] / [EMAIL PROTECTED] / [EMAIL PROTECTED] __ . . _ ._ . . .__. . ._. .__ . . . .__ | Neighbourhood Coder |\/| |_| |_| |/|_ |\/| | |_ | |/ |_ | | | | | | \ | \ |__ . | | .|. |__ |__ | \ |__ | Ottawa, Ontario, Canada One ring to rule them all, one ring to find them, one ring to bring them all and in the darkness bind them... http://mark.mielke.cc/ ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] Patch for UUID datatype (beta)
On Tue, Sep 19, 2006 at 09:51:23AM -0400, [EMAIL PROTECTED] wrote: On Tue, Sep 19, 2006 at 08:20:13AM -0500, Jim C. Nasby wrote: On Mon, Sep 18, 2006 at 07:45:07PM -0400, [EMAIL PROTECTED] wrote: I would not use a 100% random number generator for a UUID value as was suggested. I prefer inserting the MAC address and the time, to at least allow me to control if a collision is possible. This is not easy to do using a few lines of C code. I'd rather have a UUID type in core with no generation routine, than no UUID type in core because the code is too complicated to maintain, or not portable enough. As others have mentioned, using MAC address doesn't remove the possibility of a collision. It does, as I control the MAC address. I can choose not to overwrite it. I can choose to ensure that any cases where it is overwritten, it is overwritten with a unique value. Random number does not provide this level of control. Maybe a good compromise that would allow a generator function to go into the backend would be to combine the current time with a random number. That will ensure that you won't get a dupe, so long as your clock never runs backwards. Which standard UUID generation function would you be thinking of? Inventing a new one doesn't seem sensible. I'll have to read over the versions again... I don't think it exists, but I don't see how that's an issue. Let's look at an extreme case: take the amount of random entropy used for the random-only generation method. Append that to the current time in UTC, and hash it. Thanks to the time component, you've now greatly reduced the odds of a duplicate, probably by many orders of magnitude. Ultimately, I'm OK with a generator that's only in contrib, provided that there's at least one that will work on all OSes. -- Jim Nasby[EMAIL PROTECTED] EnterpriseDB http://enterprisedb.com 512.569.9461 (cell) ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] DOC: catalog.sgml
Tom Lane napsal(a): Zdenek Kotala [EMAIL PROTECTED] writes: I little bit enhanced overview catalog tables. I added two new columns. First one is OID of catalog table and second one contains attributes which determine if the table is bootstrap, with oid and global. Why is this a good idea? It seems like mere clutter. I'm working on pg_upgrade and these information are important for me and I think that They should be interest some else. You can easy determine the file related to if you know the OID. Specially when database is shutdown is good to have some information source. If catalog table is global/share or local is very important and it is not mentioned anywhere. If it is created with oid or bootstrap it is not important for standard purpose, it is only for fullness. I know that people who hacking postgres ten years know this, however it is internals chapter and for newbies it should be useful. And by the way it is documentation and this is completion of information. You can say why we have page layout there because it is described in the source code and so on... Zdenek ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] DOC: catalog.sgml
Alvaro Herrera wrote: Tom Lane wrote: Zdenek Kotala [EMAIL PROTECTED] writes: I little bit enhanced overview catalog tables. I added two new columns. First one is OID of catalog table and second one contains attributes which determine if the table is bootstrap, with oid and global. Why is this a good idea? It seems like mere clutter. What's global? A maybe-useful flag would be telling that a table is shared. Is that it? Mind you, it's not useful to me because I know which tables are shared, but I guess for someone not so familiar with the catalogs it could have some use. Global means share table stored in global directory :-). Ok I change it. Thanks for comment Zdenek ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] Patch for UUID datatype (beta)
[EMAIL PROTECTED] wrote: On Tue, Sep 19, 2006 at 08:20:13AM -0500, Jim C. Nasby wrote: On Mon, Sep 18, 2006 at 07:45:07PM -0400, [EMAIL PROTECTED] wrote: I would not use a 100% random number generator for a UUID value as was suggested. I prefer inserting the MAC address and the time, to at least allow me to control if a collision is possible. This is not easy to do using a few lines of C code. I'd rather have a UUID type in core with no generation routine, than no UUID type in core because the code is too complicated to maintain, or not portable enough. As others have mentioned, using MAC address doesn't remove the possibility of a collision. It does, as I control the MAC address. What happens if you have two postmaster running on the same machine? -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] Patch for UUID datatype (beta)
On Tue, Sep 19, 2006 at 11:21:51PM -0400, Alvaro Herrera wrote: [EMAIL PROTECTED] wrote: On Tue, Sep 19, 2006 at 08:20:13AM -0500, Jim C. Nasby wrote: On Mon, Sep 18, 2006 at 07:45:07PM -0400, [EMAIL PROTECTED] wrote: I would not use a 100% random number generator for a UUID value as was suggested. I prefer inserting the MAC address and the time, to at least allow me to control if a collision is possible. This is not easy to do using a few lines of C code. I'd rather have a UUID type in core with no generation routine, than no UUID type in core because the code is too complicated to maintain, or not portable enough. As others have mentioned, using MAC address doesn't remove the possibility of a collision. It does, as I control the MAC address. What happens if you have two postmaster running on the same machine? Could be bad things. :-) For the case of two postmaster processes, I assume you mean two different databases? If you never intend to merge the data between the two databases, the problem is irrelevant. There is a much greater chance that any UUID form is more unique, or can be guaranteed to be unique, within a single application instance, than across all application instances in existence. If you do intend to merge the data, you may have a problem. If I have two connections to PostgreSQL - would the plpgsql procedures be executed from two different processes? With an in-core generation routine, I think it is possible for it to collide unless inter-process synchronization is used (unlikely) to ensure generation of unique time/sequence combinations each time. I use this right now (mostly), but as I've mentioned, it isn't my favourite. It's convenient. I don't believe it provides the sort of guarantees that a SERIAL provides. A model that intended to try and guarantee uniqueness would provide a UUID generation service for the entire host, that was not specific to any application, or database, possibly accessible via the loopback address. It would ensure that at any given time, either the time is new, or the sequence is new for the time. If computer time ever went backwards, it could keep the last time issued persistent, and increment from this point forward through the clock sequence values until real time catches up. An alternative would be along the lines of a /dev/uuid device, that like /dev/random, would be responsible for outputting unique uuid values for the system. Who does this? Probably nobody. I'm tempted to implement it, though, for my uses. :-) Cheers, mark -- [EMAIL PROTECTED] / [EMAIL PROTECTED] / [EMAIL PROTECTED] __ . . _ ._ . . .__. . ._. .__ . . . .__ | Neighbourhood Coder |\/| |_| |_| |/|_ |\/| | |_ | |/ |_ | | | | | | \ | \ |__ . | | .|. |__ |__ | \ |__ | Ottawa, Ontario, Canada One ring to rule them all, one ring to find them, one ring to bring them all and in the darkness bind them... http://mark.mielke.cc/ ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] Patch for UUID datatype (beta)
On Mon, Sep 18, 2006 at 10:33:22AM -0400, Tom Lane wrote: Andreas Pflug [EMAIL PROTECTED] writes: Isn't guaranteed uniqueness the very attribute that's expected? AFAIK there's a commonly accepted algorithm providing this. Anyone who thinks UUIDs are guaranteed unique has been drinking too much of the kool-aid. They're at best probably unique. Some generator algorithms might make it more probable than others, but you simply cannot guarantee it for UUIDs generated on noncommunicating machines. The versions that include a MAC address, time, and serial number for the machine come pretty close, presuming that the user has not overwritten the MAC address with something else. It's unique at manufacturing time. If the generation is performed from a library with the same state, on the same machine, on the off chance that you do request multiple generations at the same exact time (from my experience, this is already unlikely) the serial number should be bumped for that time. So yeah - if you set your MAC address, or if your machine time is ever set back, or if you assume a serial number of 0 each time (generation routine isn't shared among processes on the system), you can get overlap. All of these can be controlled, making it possible to eliminate overlap. One of the big reasons that I'm hesitant to put a UUID generation function into core is the knowledge that none of them are or can be perfect ... so people might need different ones depending on local conditions. I'm inclined to think that a reasonable setup would put the datatype (with input, output, comparison and indexing support) into core, but provide a generation function as a contrib module, making it easily replaceable. I have UUID generation in core in my current implementation. In the last year that I've been using it, I have already chosen twice to generate UUIDs from my calling program. I find it faster, as it avoids have to call out to PostgreSQL twice. Once to generate the UUID, and once to insert the row using it. I have no strong need for UUID generation to be in core, and believe there does exist strong reasons not to. Performance is better when not in core. Portability of PostgreSQL is better when not in core. Ability to control how UUID is defined is better when not in control. The only thing an in-core version provides is convenience for those that do not have easy access to a UUID generation library. I don't care for that convenience. Cheers, mark -- [EMAIL PROTECTED] / [EMAIL PROTECTED] / [EMAIL PROTECTED] __ . . _ ._ . . .__. . ._. .__ . . . .__ | Neighbourhood Coder |\/| |_| |_| |/|_ |\/| | |_ | |/ |_ | | | | | | \ | \ |__ . | | .|. |__ |__ | \ |__ | Ottawa, Ontario, Canada One ring to rule them all, one ring to find them, one ring to bring them all and in the darkness bind them... http://mark.mielke.cc/ ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] [PATCHES] Patch for UUID datatype (beta)
If you're going to yank it, please at least include a generator in contrib. Personally, I'd like to see at least some kind of generator in core, with appropriate info/disclaimers in the docs. A simple random-number generator is probably the best way to go in that regard. I think that most people know that UUID generation isn't 100.0% perfect. BTW, at a former company we used SHA1s to identify files that had been uploaded. We were wondering on the odds of 2 different files hashing to the same value and found some statistical comparisons of probabilities. I don't recall the details, but the odds of duplicating a SHA1 (1 in 2^160) are so insanely small that it's hard to find anything in the physical world that compares. To duplicate random 256^256 numbers you'd probably have to search until the heat-death of the universe. On Mon, Sep 18, 2006 at 05:14:22PM +0200, Gevik Babakhani wrote: Completely agreed. I can remove the function from the patch. The temptation was just too high not to include the new_guid() in the patch :) On Mon, 2006-09-18 at 10:33 -0400, Tom Lane wrote: Andreas Pflug [EMAIL PROTECTED] writes: Isn't guaranteed uniqueness the very attribute that's expected? AFAIK there's a commonly accepted algorithm providing this. Anyone who thinks UUIDs are guaranteed unique has been drinking too much of the kool-aid. They're at best probably unique. Some generator algorithms might make it more probable than others, but you simply cannot guarantee it for UUIDs generated on noncommunicating machines. One of the big reasons that I'm hesitant to put a UUID generation function into core is the knowledge that none of them are or can be perfect ... so people might need different ones depending on local conditions. I'm inclined to think that a reasonable setup would put the datatype (with input, output, comparison and indexing support) into core, but provide a generation function as a contrib module, making it easily replaceable. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings -- Jim Nasby[EMAIL PROTECTED] EnterpriseDB http://enterprisedb.com 512.569.9461 (cell) ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Patch for UUID datatype (beta)
On Mon, Sep 18, 2006 at 04:00:22PM -0500, Jim C. Nasby wrote: BTW, at a former company we used SHA1s to identify files that had been uploaded. We were wondering on the odds of 2 different files hashing to the same value and found some statistical comparisons of probabilities. I don't recall the details, but the odds of duplicating a SHA1 (1 in 2^160) are so insanely small that it's hard to find anything in the physical world that compares. To duplicate random 256^256 numbers you'd probably have to search until the heat-death of the universe. The birthday paradox gives you about 2^80 (about 10^24) files before a SHA1 match, which is huge enough as it is. AIUI a UUID is only 2^128 bits so that would make 2^64 (about 10^19) random strings before you get a duplicate. Embed the time in there and the chance becomes *really* small, because then you have to get it in the same second. Have a nice day, -- Martijn van Oosterhout kleptog@svana.org http://svana.org/kleptog/ From each according to his ability. To each according to his ability to litigate. signature.asc Description: Digital signature
Re: [HACKERS] [PATCHES] Patch for UUID datatype (beta)
On Mon, Sep 18, 2006 at 12:23:16PM -0400, [EMAIL PROTECTED] wrote: I have UUID generation in core in my current implementation. In the last year that I've been using it, I have already chosen twice to generate UUIDs from my calling program. I find it faster, as it avoids have to call out to PostgreSQL twice. Once to generate the UUID, and once to insert the row using it. I have no strong need for UUID generation to be in core, and believe there does exist strong reasons not to. Performance is better when not in core. Portability of PostgreSQL is better when not in core. Ability to control how UUID is defined is better when not in control. That's kinda short-sighted. You're assuming that the only place you'll want to generate UUIDs is outside the database. What about a stored procedure that's adding data to the database? How about populating a table via a SELECT INTO? There's any number of cases where you'd want to generate a UUID inside the database. The only thing an in-core version provides is convenience for those that do not have easy access to a UUID generation library. I don't care for that convenience. It's not about access to a library, it's about how do you get to that library from inside the database, which may not be very easy. You may not care for that convenience, but I certainly would. -- Jim Nasby[EMAIL PROTECTED] EnterpriseDB http://enterprisedb.com 512.569.9461 (cell) ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] Patch for UUID datatype (beta)
On Mon, 2006-09-18 at 16:00 -0500, Jim C. Nasby wrote: BTW, at a former company we used SHA1s to identify files that had been uploaded. We were wondering on the odds of 2 different files hashing to the same value and found some statistical comparisons of probabilities. I don't recall the details, but the odds of duplicating a SHA1 (1 in 2^160) are so insanely small that it's hard to find anything in the physical world that compares. To duplicate random 256^256 numbers you'd probably have to search until the heat-death of the universe. That assumes you have good random data. Usually there is some kind of tradeoff between the randomness and the performance. If you read /dev/random each time, that eliminates some applications that need to generate UUIDs very quickly. If you use pseudorandom data, you are vulnerable in the case a clock is set back or the data repeats. Regards, Jeff Davis ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] Patch for UUID datatype (beta)
On Mon, Sep 18, 2006 at 04:17:50PM -0500, Jim C. Nasby wrote: On Mon, Sep 18, 2006 at 12:23:16PM -0400, [EMAIL PROTECTED] wrote: I have UUID generation in core in my current implementation. In the last year that I've been using it, I have already chosen twice to generate UUIDs from my calling program. I find it faster, as it avoids have to call out to PostgreSQL twice. Once to generate the UUID, and once to insert the row using it. I have no strong need for UUID generation to be in core, and believe there does exist strong reasons not to. Performance is better when not in core. Portability of PostgreSQL is better when not in core. Ability to control how UUID is defined is better when not in control. That's kinda short-sighted. You're assuming that the only place you'll want to generate UUIDs is outside the database. What about a stored procedure that's adding data to the database? How about populating a table via a SELECT INTO? There's any number of cases where you'd want to generate a UUID inside the database. contrib module. The only thing an in-core version provides is convenience for those that do not have easy access to a UUID generation library. I don't care for that convenience. It's not about access to a library, it's about how do you get to that library from inside the database, which may not be very easy. You may not care for that convenience, but I certainly would. Then load the contrib module. I do both. I'd happily reduce my contrib module to be based upon a built-in UUID type within PostgreSQL, providing the necessary UUID generation routines. I would not use a 100% random number generator for a UUID value as was suggested. I prefer inserting the MAC address and the time, to at least allow me to control if a collision is possible. This is not easy to do using a few lines of C code. I'd rather have a UUID type in core with no generation routine, than no UUID type in core because the code is too complicated to maintain, or not portable enough. Cheers, mark -- [EMAIL PROTECTED] / [EMAIL PROTECTED] / [EMAIL PROTECTED] __ . . _ ._ . . .__. . ._. .__ . . . .__ | Neighbourhood Coder |\/| |_| |_| |/|_ |\/| | |_ | |/ |_ | | | | | | \ | \ |__ . | | .|. |__ |__ | \ |__ | Ottawa, Ontario, Canada One ring to rule them all, one ring to find them, one ring to bring them all and in the darkness bind them... http://mark.mielke.cc/ ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] [PATCHES] plpgsql, return can contains any
This has been saved for the 8.3 release: http://momjian.postgresql.org/cgi-bin/pgpatches_hold --- Pavel Stehule wrote: Pavel Stehule [EMAIL PROTECTED] writes: This patch doesn't seem to cope with cases where the supplied tuple has the wrong number of columns, and it doesn't look like it's being careful about dropped columns either. Also, that's a mighty bizarre-looking choice of cache memory context in coerce_to_tuple ... but then again, why are you bothering with a cache at all for temporary arrays? I am sorry, Tom. But I don't understand. I can check number of columns, ofcourse and I'll do it. What cache for temporary arrays do you mean? I thought that making coerce_to_tuple depend on estate-err_func was pretty bizarre, and that there was no need for any cache at all for arrays that need only live as long as the function runs. All you are saving here is a palloc/pfree cycle, which is not worth the obscurantism and risk of bugs (are you sure natts can never change?). No, cache there is ugly. But I don't have idea about more efective implementation of it :-(. First version of this patch was more clean. and little bit slow. This cache save 10%. BTW, if you want this patch to make it into 8.2, it needs to be fixed and resubmitted *very* soon. I understand, but I am not able work on it in next four days. And I need help with it from Neil. It will be for 8.3. Thank you Pavel _ Emotikony a pozadi programu MSN Messenger ozivi vasi konverzaci. http://messenger.msn.cz/ ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] Resurrecting per-page cleaner for
OK, removed from open item list. --- Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Tom Lane wrote: I've applied this but I'm now having some second thoughts about it, because I'm seeing an actual *decrease* in pgbench numbers from the immediately prior CVS HEAD code. The attached patch requires the new row to fit, and 10% to be free on the page. Would someone test that? At the moment, I cannot replicate any consistent difference between CVS head with the patch, without the patch, with the patch plus your BLCKSZ/10 limit addition, or with a variant BLCKSZ/32 limit addition. That's whether I use HEAD's broken version of pgbench or one from late July. So I'm feeling a tad frustrated ... but I have no evidence in favor of changing what is in CVS, and accordingly recommend that we leave well enough alone for 8.2. regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] [PATCHES] Resurrecting per-page cleaner for btree
Bruce Momjian [EMAIL PROTECTED] writes: Tom Lane wrote: I've applied this but I'm now having some second thoughts about it, because I'm seeing an actual *decrease* in pgbench numbers from the immediately prior CVS HEAD code. The attached patch requires the new row to fit, and 10% to be free on the page. Would someone test that? At the moment, I cannot replicate any consistent difference between CVS head with the patch, without the patch, with the patch plus your BLCKSZ/10 limit addition, or with a variant BLCKSZ/32 limit addition. That's whether I use HEAD's broken version of pgbench or one from late July. So I'm feeling a tad frustrated ... but I have no evidence in favor of changing what is in CVS, and accordingly recommend that we leave well enough alone for 8.2. regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] Backend SSL configuration enhancement
On Tue, Sep 05, 2006 at 10:17:15AM +0400, Victor Wagner wrote: It's a pity that it's to late for patch to get into 8.2. It means that during all 8.2 lifecycle we'll have to maintain this patch separately. Hmm? After 8.2 releases, if it's ready, it will go straight into CVS at which point it'll be in the next release. Have a nice day, -- Martijn van Oosterhout kleptog@svana.org http://svana.org/kleptog/ From each according to his ability. To each according to his ability to litigate. signature.asc Description: Digital signature
Re: [HACKERS] [PATCHES] extension for sql update
Bruce Momjian wrote: Susanne Ebrecht wrote: Is it too hard to rip it back out once the full row support arrives? That seems speculation at best anyway. That's what I was thinking. Glad someone else replied. ;-) If you're looking for votes, +1. I'll gladly take a subset of the SQL standard UPDATE table SET (...) = (...) over having nothing. +1 here, too. :) +1 I am working now to get this into 8.2. I am glad to read this. But what does it mean to me? Shall I change the patch someway? I have merged your patch into current CVS and applied it; attached. There was quite a bit of code drift. One drift area was the new RETURNING clause; that was easy to fix. A more complex case is the code no longer has values as ResTargets --- it is a simple a_expr list, so I changed the critical assignment in gram.y from: res_col-val = (Node *)copyObject(res_val-val); to: res_col-val = (Node *)copyObject(res_val); Hope that is OK. Without that fix, it crashed. I also merged your SGML syntax and grammer addition into the exiting UPDATE main entry. Of course it is ok. Many thanks. Susanne ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] [PATCHES] Gen_fmgrtab.sh fails with LANG=et_EE
Peter Eisentraut [EMAIL PROTECTED] writes: Well, the line of code is cpp_define=`echo $OIDSFILE | tr abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ | sed -e 's/[^A-Z]/_/g'` so it ought to be pretty obvious what the correct solution for the problem character ranges are locale-dependent is. Doh. Patched that way. Curiously, I couldn't replicate the failure on Fedora 5 --- Marko's platform must have different locale behavior for et_EE. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Gen_fmgrtab.sh fails with LANG=et_EE
On 9/5/06, Tom Lane [EMAIL PROTECTED] wrote: Peter Eisentraut [EMAIL PROTECTED] writes: Well, the line of code is cpp_define=`echo $OIDSFILE | tr abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ | sed -e 's/[^A-Z]/_/g'` so it ought to be pretty obvious what the correct solution for the problem character ranges are locale-dependent is. Doh. Patched that way. Curiously, I couldn't replicate the failure on Fedora 5 --- Marko's platform must have different locale behavior for et_EE. Did you add it to locale-gen config and ran it? Btw, I removed all the pipeline in my patch, because I felt such messy pipeline for such a tiny thing is ugly. Especially as the filename wont change that much. Thus I though it would be cleaner to just put the symbol together with filename definition. -- marko ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Interval month, week - day
On Sun, Sep 03, 2006 at 10:21:11PM -0400, Bruce Momjian wrote:
When I tried the ecpg regression tests it complained there was no
results/ directory. I created one and it worked.
Hmm, anyone else experiencing this? The pg_regress.sh has this code that
should create it:
outputdir=results/
if [ ! -d $outputdir ]; then
mkdir -p $outputdir || { (exit 2); exit; }
fi
Michael
--
Michael Meskes
Email: Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: [EMAIL PROTECTED]
Go SF 49ers! Go Rhein Fire! Use Debian GNU/Linux! Use PostgreSQL!
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Interval month, week - day
Michael Meskes [EMAIL PROTECTED] writes:
On Sun, Sep 03, 2006 at 10:21:11PM -0400, Bruce Momjian wrote:
When I tried the ecpg regression tests it complained there was no
results/ directory. I created one and it worked.
Hmm, anyone else experiencing this? The pg_regress.sh has this code that
should create it:
outputdir=results/
if [ ! -d $outputdir ]; then
mkdir -p $outputdir || { (exit 2); exit; }
fi
I'll bet you should lose the slash in $outputdir. test(1) might or
might not be friendly about stripping that off.
regards, tom lane
---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
Re: [HACKERS] [PATCHES] Interval month, week - day
Tom Lane wrote:
Michael Meskes [EMAIL PROTECTED] writes:
On Sun, Sep 03, 2006 at 10:21:11PM -0400, Bruce Momjian wrote:
When I tried the ecpg regression tests it complained there was no
results/ directory. I created one and it worked.
Hmm, anyone else experiencing this? The pg_regress.sh has this code that
should create it:
outputdir=results/
if [ ! -d $outputdir ]; then
mkdir -p $outputdir || { (exit 2); exit; }
fi
I'll bet you should lose the slash in $outputdir. test(1) might or
might not be friendly about stripping that off.
Yep, I saw this error:
mkdir: results/: No such file or directory
gmake: *** [installcheck] Error 2
I have removed the trailing slash from CVS; tests run fine now.
Thanks.
--
Bruce Momjian [EMAIL PROTECTED]
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
---(end of broadcast)---
TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] Trivial patch to double vacuum speed
Patch applied. Thanks.
---
Gregory Stark wrote:
Tom Lane [EMAIL PROTECTED] writes:
The reason the patch is so short is that it's a kluge. If we really
cared about supporting this case, more wide-ranging changes would be
needed (eg, there's no need to eat maintenance_work_mem worth of RAM
for the dead-TIDs array); and a decent respect to the opinions of
mankind would require some attention to updating the header comments
and function descriptions, too.
The only part that seems klugy to me is how it releases the lock and
reacquires it rather than wait in the first place until it can acquire the
lock. Fixed that and changed lazy_space_alloc to allocate only as much space
as is really necessary.
Gosh, I've never been accused of offending all mankind before.
--- vacuumlazy.c 31 Jul 2006 21:09:00 +0100 1.76
+++ vacuumlazy.c 28 Aug 2006 09:58:41 +0100
@@ -16,6 +16,10 @@
* perform a pass of index cleanup and page compaction, then resume the heap
* scan with an empty TID array.
*
+ * As a special exception if we're processing a table with no indexes we can
+ * vacuum each page as we go so we don't need to allocate more space than
+ * enough to hold as many heap tuples fit on one page.
+ *
* We can limit the storage for page free space to MaxFSMPages entries,
* since that's the most the free space map will be willing to remember
* anyway. If the relation has fewer than that many pages with free space,
@@ -106,7 +110,7 @@
TransactionId
OldestXmin);
static BlockNumber count_nondeletable_pages(Relation onerel,
LVRelStats *vacrelstats,
TransactionId OldestXmin);
-static void lazy_space_alloc(LVRelStats *vacrelstats, BlockNumber relblocks);
+static void lazy_space_alloc(LVRelStats *vacrelstats, BlockNumber relblocks,
unsigned nindexes);
static void lazy_record_dead_tuple(LVRelStats *vacrelstats,
ItemPointer itemptr);
static void lazy_record_free_space(LVRelStats *vacrelstats,
@@ -206,7 +210,8 @@
* This routine sets commit status bits, builds lists of dead
tuples
* and pages with free space, and calculates statistics on the
number
* of live tuples in the heap. When done, or when we run low on
space
- * for dead-tuple TIDs, invoke vacuuming of indexes and heap.
+ * for dead-tuple TIDs, or after every page if the table has no
indexes
+ * invoke vacuuming of indexes and heap.
*
* It also updates the minimum Xid found anywhere on the table in
* vacrelstats-minxid, for later storing it in pg_class.relminxid.
@@ -247,7 +252,7 @@
vacrelstats-rel_pages = nblocks;
vacrelstats-nonempty_pages = 0;
- lazy_space_alloc(vacrelstats, nblocks);
+ lazy_space_alloc(vacrelstats, nblocks, nindexes);
for (blkno = 0; blkno nblocks; blkno++)
{
@@ -282,8 +287,14 @@
buf = ReadBuffer(onerel, blkno);
- /* In this phase we only need shared access to the buffer */
- LockBuffer(buf, BUFFER_LOCK_SHARE);
+ /* In this phase we only need shared access to the buffer
unless we're
+ * going to do the vacuuming now which we do if there are no
indexes
+ */
+
+ if (nindexes)
+ LockBuffer(buf, BUFFER_LOCK_SHARE);
+ else
+ LockBufferForCleanup(buf);
page = BufferGetPage(buf);
@@ -450,6 +461,12 @@
{
lazy_record_free_space(vacrelstats, blkno,
PageGetFreeSpace(page));
+ } else if (!nindexes) {
+ /* If there are no indexes we can vacuum the page right
now instead
+ * of doing a second scan */
+ lazy_vacuum_page(onerel, blkno, buf, 0, vacrelstats);
+ lazy_record_free_space(vacrelstats, blkno,
PageGetFreeSpace(BufferGetPage(buf)));
+ vacrelstats-num_dead_tuples = 0;
}
/* Remember the location of the last page with nonremovable
tuples */
@@ -891,16 +908,20 @@
* See the comments at the head of this file for rationale.
*/
static void
-lazy_space_alloc(LVRelStats *vacrelstats, BlockNumber relblocks)
+lazy_space_alloc(LVRelStats *vacrelstats, BlockNumber relblocks, unsigned
nindexes)
{
longmaxtuples;
int maxpages;
- maxtuples = (maintenance_work_mem * 1024L) / sizeof(ItemPointerData);
- maxtuples = Min(maxtuples, INT_MAX);
- maxtuples = Min(maxtuples, MaxAllocSize /
Re: [HACKERS] [PATCHES] Trivial patch to double vacuum speed on tables with no indexes
Bruce Momjian [EMAIL PROTECTED] writes: Patch applied. Thanks. Wait a minute. This patch changes the behavior so that LockBufferForCleanup is applied to *every* heap page, not only the ones where there are removable tuples. It's not hard to imagine scenarios where that results in severe system-wide performance degradation. Has there been any real-world testing of this idea? regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] [PATCHES] Trivial patch to double vacuum speed
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Patch applied. Thanks. Wait a minute. This patch changes the behavior so that LockBufferForCleanup is applied to *every* heap page, not only the ones where there are removable tuples. It's not hard to imagine scenarios where that results in severe system-wide performance degradation. Has there been any real-world testing of this idea? I see the no-index case now: + if (nindexes) + LockBuffer(buf, BUFFER_LOCK_SHARE); + else + LockBufferForCleanup(buf); Let's see what Greg says, or revert. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Trivial patch to double vacuum speed on tables with no indexes
Bruce Momjian [EMAIL PROTECTED] writes: Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Patch applied. Thanks. Wait a minute. This patch changes the behavior so that LockBufferForCleanup is applied to *every* heap page, not only the ones where there are removable tuples. It's not hard to imagine scenarios where that results in severe system-wide performance degradation. Has there been any real-world testing of this idea? I see the no-index case now: + if (nindexes) + LockBuffer(buf, BUFFER_LOCK_SHARE); + else + LockBufferForCleanup(buf); Let's see what Greg says, or revert. Hm, that's a good point. I could return it to the original method where it released the share lock and did he LockBufferForCleanup only if necessary. I thought it was awkward to acquire a lock then release it to acquire a different lock on the same buffer but it's true that it doesn't always have to acquire the second lock. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] [PATCHES] DOC: catalog.sgml
On Sun, Sep 03, 2006 at 12:01:06AM -0400, Tom Lane wrote: But ever since 7.3 the convention for identifying system objects has been pretty well-defined: anything that lives in one of the predefined schemas. What problem were you having using that approach in newsysviews? It was just an issue of trawling through pg_dump to confirm that. -- Jim C. Nasby, Database Architect [EMAIL PROTECTED] 512.569.9461 (cell) http://jim.nasby.net ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] [PATCHES] [COMMITTERS] pgsql: Change FETCH/MOVE
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Tom Lane wrote: There is *no* credible use case for this (hint: MOVE FORWARD/BACKWARD ALL does not need this to work for 2G tables). Already done because of bad coding. You want the TODO item removed too? As I said, I see no use case for it. Maybe if Moore's Law holds up for another five or ten years, it'll look like a useful feature then ... Removed. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] Interval month, week - day
Michael Glaesemann [EMAIL PROTECTED] writes: On Sep 4, 2006, at 9:41 , Tom Lane wrote: This patch fails to apply --- looks like whitespace got mangled in transit. Please resend as an attachment. Please let me know if you have any problems with this one. Ah, that one works --- applied. A few comments: * You worried about the tmask coding in your original message, but I think that's OK as-is. The point of that code, IIUC, is to reject multiple specifications of the same field type, eg '1 day 2 days'. If we changed it then we'd reject '1.5 month 2 days', whereas I think least surprise would dictate adding the components to give 1 month 17 days. * AFAICT the ecpg regression tests are not affected by this change. * You mentioned being unable to get the ecpg tests to run on your machine. I'm sure Michael and Joachim would like the details. The ecpg regression tests are pretty new and some portability problems are to be expected, but they seem to be passing on all the machines Michael and Joachim and I have access to. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] Interval month, week - day
Tom Lane wrote: You mentioned being unable to get the ecpg tests to run on your machine. I'm sure Michael and Joachim would like the details. The ecpg regression tests are pretty new and some portability problems are to be expected, but they seem to be passing on all the machines Michael and Joachim and I have access to. I have just today released a new version of the buildfarm client that includes ECPG regression testing for HEAD (until now that was in our CVS tip but not in a released version). cheers andrew ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Resurrecting per-page cleaner for
Tom, should I apply this patch now? Are you still considering other options for this? --- Bruce Momjian wrote: Tom, I ran your tests with fsync off (as you did), and saw numbers bouncing between 400-700 tps without my patch, and sticking at 700 tps with my patch. --- Bruce Momjian wrote: The attached patch requires the new row to fit, and 10% to be free on the page. Would someone test that? --- Tom Lane wrote: ITAGAKI Takahiro [EMAIL PROTECTED] writes: This is a revised patch originated by Junji TERAMOTO for HEAD. [BTree vacuum before page splitting] http://archives.postgresql.org/pgsql-patches/2006-01/msg00301.php I think we can resurrect his idea because we will scan btree pages at-atime now; the missing-restarting-point problem went away. I've applied this but I'm now having some second thoughts about it, because I'm seeing an actual *decrease* in pgbench numbers from the immediately prior CVS HEAD code. Using pgbench -i -s 10 bench pgbench -c 10 -t 1000 bench (repeat this half a dozen times) with fsync off but all other settings factory-stock, what I'm seeing is that the first run looks really good but subsequent runs tail off in spectacular fashion :-( Pre-patch there was only minor degradation in successive runs. What I think is happening is that because pgbench depends so heavily on updating existing records, we get into a state where an index page is about full and there's one dead tuple on it, and then for each insertion we have * check for uniqueness marks one more tuple dead (the next-to-last version of the tuple) * newly added code removes one tuple and does a write * now there's enough room to insert one tuple * lather, rinse, repeat, never splitting the page. The problem is that we've traded splitting a page every few hundred inserts for doing a PageIndexMultiDelete, and emitting an extra WAL record, on *every* insert. This is not good. Had you done any performance testing on this patch, and if so what tests did you use? I'm a bit hesitant to try to fix it on the basis of pgbench results alone. One possible fix that comes to mind is to only perform the cleanup if we are able to remove more than one dead tuple (perhaps about 10 would be good). Or do the deletion anyway, but then go ahead and split the page unless X amount of space has been freed (where X is more than just barely enough for the incoming tuple). After all the thought we've put into this, it seems a shame to just abandon it :-(. But it definitely needs more tweaking. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] [PATCHES] Resurrecting per-page cleaner for
Bruce Momjian [EMAIL PROTECTED] writes: Tom, should I apply this patch now? Are you still considering other options for this? Please wait. This issue is very far down the to-list in terms of size or significance ... but I'll get to it. regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] [PATCHES] extension for sql update
Susanne Ebrecht wrote:
Is it too hard to rip it back out once the full row support
arrives? That seems speculation at best anyway.
That's what I was thinking. Glad someone else replied. ;-)
If you're looking for votes, +1. I'll gladly take a subset of the
SQL standard UPDATE table SET (...) = (...) over having nothing.
+1 here, too. :)
+1
I am working now to get this into 8.2.
I am glad to read this. But what does it mean to me? Shall I change the
patch someway?
I have merged your patch into current CVS and applied it; attached.
There was quite a bit of code drift. One drift area was the new
RETURNING clause; that was easy to fix. A more complex case is the
code no longer has values as ResTargets --- it is a simple a_expr list,
so I changed the critical assignment in gram.y from:
res_col-val = (Node *)copyObject(res_val-val);
to:
res_col-val = (Node *)copyObject(res_val);
Hope that is OK. Without that fix, it crashed. I also merged your SGML
syntax and grammer addition into the exiting UPDATE main entry.
--
Bruce Momjian [EMAIL PROTECTED]
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: doc/src/sgml/ref/update.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/ref/update.sgml,v
retrieving revision 1.38
retrieving revision 1.39
diff -c -r1.38 -r1.39
*** doc/src/sgml/ref/update.sgml 12 Aug 2006 02:52:03 - 1.38
--- doc/src/sgml/ref/update.sgml 2 Sep 2006 20:34:47 - 1.39
***
*** 21,27
refsynopsisdiv
synopsis
UPDATE [ ONLY ] replaceable class=PARAMETERtable/replaceable [ [ AS ] replaceable class=parameteralias/replaceable ]
! SET replaceable class=PARAMETERcolumn/replaceable = { replaceable class=PARAMETERexpression/replaceable | DEFAULT } [, ...]
[ FROM replaceable class=PARAMETERfromlist/replaceable ]
[ WHERE replaceable class=PARAMETERcondition/replaceable ]
[ RETURNING * | replaceable class=parameteroutput_expression/replaceable [ AS replaceable class=parameteroutput_name/replaceable ] [, ...] ]
--- 21,28
refsynopsisdiv
synopsis
UPDATE [ ONLY ] replaceable class=PARAMETERtable/replaceable [ [ AS ] replaceable class=parameteralias/replaceable ]
! [ SET replaceable class=PARAMETERcolumn/replaceable = { replaceable class=PARAMETERexpression/replaceable | DEFAULT } [, ...] |
! SET ( replaceable class=PARAMETERcolumn/replaceable [, ...] ) = ( { replaceable class=PARAMETERexpression/replaceable | DEFAULT } [, ...] ) [, ...] ]
[ FROM replaceable class=PARAMETERfromlist/replaceable ]
[ WHERE replaceable class=PARAMETERcondition/replaceable ]
[ RETURNING * | replaceable class=parameteroutput_expression/replaceable [ AS replaceable class=parameteroutput_name/replaceable ] [, ...] ]
***
*** 251,256
--- 252,261
UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
WHERE city = 'San Francisco' AND date = '2003-07-03';
/programlisting
+ programlisting
+ UPDATE weather SET (temp_lo, temp_hi, prcp) = (temp_lo+1, temp_lo+15, DEFAULT)
+ WHERE city = 'San Francisco' AND date = '2003-07-03';
+ /programlisting
/para
para
Index: src/backend/parser/gram.y
===
RCS file: /cvsroot/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.560
retrieving revision 2.562
diff -c -r2.560 -r2.562
*** src/backend/parser/gram.y 2 Sep 2006 18:17:17 - 2.560
--- src/backend/parser/gram.y 2 Sep 2006 20:52:01 - 2.562
***
*** 237,243
name_list from_clause from_list opt_array_bounds
qualified_name_list any_name any_name_list
any_operator expr_list attrs
! target_list update_target_list insert_column_list
values_list def_list indirection opt_indirection
group_clause TriggerFuncArgs select_limit
opt_select_limit opclass_item_list
--- 237,244
name_list from_clause from_list opt_array_bounds
qualified_name_list any_name any_name_list
any_operator expr_list attrs
! target_list update_col_list update_target_list
! update_value_list set_opt insert_column_list
values_list def_list indirection opt_indirection
group_clause TriggerFuncArgs select_limit
opt_select_limit opclass_item_list
***
*** 308,314
%type jexpr joined_table
%type range relation_expr
%type range relation_expr_opt_alias
! %type target target_el update_target_el insert_column_item
%type typnam Typename SimpleTypename ConstTypename
GenericType Numeric opt_float
--- 309,316
%type jexpr joined_table
%type range relation_expr
%type range relation_expr_opt_alias
! %type target target_el update_target_el update_col_list_el insert_column_item
! %type list update_target_lists_list
Re: [HACKERS] [PATCHES] Use of backslash in tsearch2
Thanks. Yes, it is need for two reasons. In 8.2 you can set standard_conforming_strings to on, meaning \' is really treated as \ and ', and because some encodings now can't support \' for security reasons, though I don't think tsearch2 supports those multibyte encodings. Anyway, applied to 8.2 only, not backpatched. Thanks. --- Teodor Sigaev wrote: Patch isn't full, simple test (values are took from regression.diffs): and try dump table and restore: ERROR: syntax error CONTEXT: COPY tt, line 5, column tq: '1 ''2' Attached cumulative patch fixes problem, but I have some doubts, is it really needed? -- Teodor Sigaev E-mail: [EMAIL PROTECTED] WWW: http://www.sigaev.ru/ [ application/x-tar is not supported, skipping... ] ---(end of broadcast)--- TIP 6: explain analyze is your friend -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] [PATCHES] DOC: catalog.sgml
On Fri, Sep 01, 2006 at 12:36:11PM -0400, Alvaro Herrera wrote: Tom Lane wrote: Zdenek Kotala [EMAIL PROTECTED] writes: I little bit enhanced overview catalog tables. I added two new columns. First one is OID of catalog table and second one contains attributes which determine if the table is bootstrap, with oid and global. Why is this a good idea? It seems like mere clutter. What's global? A maybe-useful flag would be telling that a table is shared. Is that it? Mind you, it's not useful to me because I know which tables are shared, but I guess for someone not so familiar with the catalogs it could have some use. The OIDs may be useful to people inspecting pg_depend, for example; but then, it's foolish not to be using regclass in that case. Whether a table is bootstrap or not doesn't seem useful to me. Something that might be handy would be a method to determine if an object is a system object or not (perhaps what the OP means by bootstrap). We spent quite some time figuring out how to handle that when we were working on newsysviews. In that case, we wanted the info because it's handy to be able to query a view that's not cluttered up with a bunch of system-defined stuff. Having a way to get a list of only user-defined functions, for example. -- Jim C. Nasby, Database Architect [EMAIL PROTECTED] 512.569.9461 (cell) http://jim.nasby.net ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] extension for sql update
bruce wrote: I have merged your patch into current CVS and applied it; attached. There was quite a bit of code drift. One drift area was the new RETURNING clause; that was easy to fix. A more complex case is the code no longer has values as ResTargets --- it is a simple a_expr list, so I changed the critical assignment in gram.y from: res_col-val = (Node *)copyObject(res_val-val); to: res_col-val = (Node *)copyObject(res_val); Hope that is OK. Without that fix, it crashed. I also merged your SGML syntax and grammer addition into the exiting UPDATE main entry. The copyObject() is not required. Removed. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] [COMMITTERS] pgsql: Change FETCH/MOVE
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: This patch has broken half the buildfarm, and I've still not seen a rationale why we need to make such a change at all. Fixed with attached patch. The use case for this was not FETCH, but MOVE for 2gig tables. There is *no* credible use case for this (hint: MOVE FORWARD/BACKWARD ALL does not need this to work for 2G tables). It is not worth the extra computational cycles that it imposes on every machine whether they use the feature or not, and it is certainly not worth the developer time we're expending to fix this poorly written patch. Please revert it. Already done because of bad coding. You want the TODO item removed too? -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] [PATCHES] DOC: catalog.sgml
Jim C. Nasby [EMAIL PROTECTED] writes: On Fri, Sep 01, 2006 at 12:36:11PM -0400, Alvaro Herrera wrote: Whether a table is bootstrap or not doesn't seem useful to me. Something that might be handy would be a method to determine if an object is a system object or not (perhaps what the OP means by bootstrap). No, what the OP meant by bootstrap is one of the four core BKI_BOOTSTRAP catalogs, and the reason they're so core is that bootstrap mode itself doesn't really work till they exist. (I agree with Alvaro that by the time you get interested in what BKI_BOOTSTRAP does, you probably don't need to have any hand-holding from catalogs.sgml; you're already at least an apprentice wizard.) But ever since 7.3 the convention for identifying system objects has been pretty well-defined: anything that lives in one of the predefined schemas. What problem were you having using that approach in newsysviews? regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] [COMMITTERS] pgsql: Change FETCH/MOVE to use int8.
Bruce Momjian [EMAIL PROTECTED] writes: Tom Lane wrote: There is *no* credible use case for this (hint: MOVE FORWARD/BACKWARD ALL does not need this to work for 2G tables). Already done because of bad coding. You want the TODO item removed too? As I said, I see no use case for it. Maybe if Moore's Law holds up for another five or ten years, it'll look like a useful feature then ... regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] DOC: catalog.sgml
Tom Lane wrote: Zdenek Kotala [EMAIL PROTECTED] writes: I little bit enhanced overview catalog tables. I added two new columns. First one is OID of catalog table and second one contains attributes which determine if the table is bootstrap, with oid and global. Why is this a good idea? It seems like mere clutter. What's global? A maybe-useful flag would be telling that a table is shared. Is that it? Mind you, it's not useful to me because I know which tables are shared, but I guess for someone not so familiar with the catalogs it could have some use. The OIDs may be useful to people inspecting pg_depend, for example; but then, it's foolish not to be using regclass in that case. Whether a table is bootstrap or not doesn't seem useful to me. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] Contrib module to examine client
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: I assume this is something we want in /contrib, right? Peter posted an updated version, I believe. Ah, it was lower in my mailbox. Thanks. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] Updatable views
Am Mittwoch, 30. August 2006 18:01 schrieb Tom Lane: This is the first time I've actually looked at this patch, and I am dismayed. viewUpdate.c looks like nothing so much as a large program with a small program struggling to get out. What is all the stuff about handling multiple base rels? SQL92, at least, does not say that a join is updatable, and AFAICT this patch is rejecting that too ... But later SQL versions allow some of that, so at least it shouldn't hurt to have some parts of the code to be more general in preparation of that. I'm unclear as to why you've got DO INSTEAD NOTHING rules in there --- You need to have one unconditional rule if you have a bunch of conditional ones. The system does not see through the fact that the conditional ones cover all cases. The pg_dump changes seem pretty odd too. Why wouldn't you just ignore implicit rules during a dump, expecting the system to regenerate them when the view is reloaded? Right. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] [PATCHES] log_statement output for protocol
On 8/30/06, Bruce Momjian [EMAIL PROTECTED] wrote: I thought about this, and because we are placing two pieces of information on the same line, it seems | is the best choice. Good idea. It's far more readable with a pipe. Oh. You want to pull the parameters out of that. I am thinking you need something that will go over the line character by character with some type of state machine, rather than just regex. Yes, that's what I did but I usually prefer a regex. Additional comments? I confirm it now works with NULL. I'm just wondering if the notation is really consistent: $result = pg_execute($dbconn, insert_query, array(null)); gives: DETAIL: prepare: INSERT INTO shop (name) VALUES($1) | bind: $1 = NULL However: $result = pg_execute($dbconn, insert_query, array(4)); gives: DETAIL: prepare: INSERT INTO shop (name) VALUES($1) | bind: $1 = '4' But I don't think it's possible to have 4 in this case. Can you confirm? I have all the different cases parsed correctly by my parser and I can build the query from the logs so it's OK for me. In the above case, with an int, I remove the quotes if the content is numeric. It's not perfect but I suppose it will be OK most of the time. -- Guillaume ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] [PATCHES] Updatable views
Peter Eisentraut [EMAIL PROTECTED] writes: Am Mittwoch, 30. August 2006 18:01 schrieb Tom Lane: This is the first time I've actually looked at this patch, and I am dismayed. viewUpdate.c looks like nothing so much as a large program with a small program struggling to get out. But later SQL versions allow some of that, so at least it shouldn't hurt to have some parts of the code to be more general in preparation of that. If it bloats the code to unreadability, it's bad. I'm unclear as to why you've got DO INSTEAD NOTHING rules in there --- You need to have one unconditional rule if you have a bunch of conditional ones. The system does not see through the fact that the conditional ones cover all cases. AFAICS, for the cases we are able to implement within the existing rule mechanism, there should be exactly one unconditional rule. If you propose more, then you are going to have insurmountable problems with the usual sorts of multiple-evaluation risks. The proposed WITH CHECK OPTION implementation is unworkable for exactly this reason --- it will give the wrong answers in the presence of volatile functions such as nextval(). I believe that we cannot implement WITH CHECK OPTION as a rule. It's a constraint, instead, and will have to be checked the way the executor presently checks constraints, ie after forming the finished new tuple(s). (Someday we're going to have to look into redesigning the rule system so that it can cope better with the kinds of situations that give rise to multiple-evaluation problems. But today is not that day.) It's possible that if we strip the patch down to SQL92-equivalent functionality (no multiple base rels) without WITH CHECK OPTION, we would have something that would work reliably atop the existing rule mechanism. It's getting mighty late in the 8.2 cycle to be doing major rework though. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] Updatable views
Am Donnerstag, 31. August 2006 15:55 schrieb Tom Lane: I'm unclear as to why you've got DO INSTEAD NOTHING rules in there --- You need to have one unconditional rule if you have a bunch of conditional ones. The system does not see through the fact that the conditional ones cover all cases. AFAICS, for the cases we are able to implement within the existing rule mechanism, there should be exactly one unconditional rule. If you propose more, then you are going to have insurmountable problems with the usual sorts of multiple-evaluation risks. I'm not sure what you are saying here ... The implementation creates, for each of the three actions INSERT, UPDATE, DELETE, one conditional rule that redirects the action from the view into the unterlying table, conditional on the view condition being fulfilled. The unconditional DO INSTEAD NOTHING rule then catches the cases where the view condition is not fulfilled. So there is, for each action, exactly one conditional and one unconditional rule. Which is consistent with what you said above, so I don't see the problem. The proposed WITH CHECK OPTION implementation is unworkable for exactly this reason --- it will give the wrong answers in the presence of volatile functions such as nextval(). I'm not sure why anyone would want to define a view condition containing a volatile function. At least it wouldn't put a major dent into this feature if such views were decreed not updatable. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Updatable views
Peter Eisentraut [EMAIL PROTECTED] writes:
Am Donnerstag, 31. August 2006 15:55 schrieb Tom Lane:
The proposed WITH CHECK OPTION implementation is unworkable for exactly
this reason --- it will give the wrong answers in the presence of
volatile functions such as nextval().
I'm not sure why anyone would want to define a view condition containing a
volatile function. At least it wouldn't put a major dent into this feature
if such views were decreed not updatable.
The problem is not with the view condition. Consider
CREATE TABLE data (id serial primary key, ...);
CREATE VIEW only_new_data AS SELECT * FROM data WHERE id 12345
WITH CHECK OPTION;
INSERT INTO only_new_data VALUES(nextval('data_id_seq'), ...);
The proposed implementation will execute nextval twice (bad), and will
apply the WITH CHECK OPTION test to the value that isn't the one stored
(much worse). It doesn't help if the id is defaulted.
regards, tom lane
---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?
http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] log_statement output for protocol
On 8/29/06, Bruce Momjian [EMAIL PROTECTED] wrote:
Good point. I thought it was clear enough, but obviously not. I had a
similar case with bind, and used a comma to separate them:
LOG: statement: prepare sel1, SELECT $1;
LOG: statement: bind sel1, $1 = 'a''b'
For this one, I must admit I prefer the colon we used before.
Something like:
LOG: statement: prepare sel1: SELECT $1;
LOG: statement: bind sel1: $1 = 'a''b'
seems better to me as after the colon, we have the details of the
command which is the common sense of a colon.
I am concerned a dash isn't clear enough, and a semicolon is confusing.
Using a comma the new output is:
LOG: duration: 0.023 ms execute sel1
DETAIL: prepare: SELECT $1;, bind: $1 = 'a''b'
A dash is clearer in this case IMHO. ;, is not very readable. But I
can parse easily both forms so it's not a problem for me if you prefer
a comma.
Is that OK? Patch attached and committed. I also fixed the null bind
parameter bug. It now displays $1 = NULL (no quotes used).
Cool. I'll test that.
Other suggestions?
I'll compile this new version and make tests in the next few days to
check everything is consistent and let you know.
I'm still struggling to find a regexp good enough to parse statement:
execute y ('a', 4, 'text, with a comma'::text); :).
Thanks a lot for your work on this subject. It will help us a lot when
we use the JDBC driver.
--
Guillaume
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] updated patch for selecting large results
On Tue, 2006-08-29 at 18:31 -0400, Tom Lane wrote: [EMAIL PROTECTED] writes: here comes the latest version (version 7) of the patch to handle large result sets with psql. As previously discussed, a cursor is used for SELECT queries when \set FETCH_COUNT some_value 0 Applied with revisions ... I didn't like the fact that the code was restricted to handle only unaligned output format, so I fixed print.c to be able to deal with emitting output in sections. This is not ideal for aligned output mode, because we compute column widths separately for each FETCH group, but all the other output modes work nicely. I also did a little hacking to make \timing and pager output work as expected. regards, tom lane Cool! I specially like that as a side effect of your work for applying this, psql is faster now. Thanks to all people that helped with this (lots...:) Bye, Chris. -- Chris Mair http://www.1006.org ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] Updatable views
On Wed, Aug 30, 2006 at 12:01:25PM -0400, Tom Lane wrote: Bernd Helmle [EMAIL PROTECTED] writes: [ latest views patch ] This is the first time I've actually looked at this patch, and I am dismayed. viewUpdate.c looks like nothing so much as a large program with a small program struggling to get out. What is all the stuff about handling multiple base rels? SQL92, at least, does not say that a join is updatable, and AFAICT this patch is rejecting that too ... though it's hard to tell with the conditions for allowing the join to be updatable scattered through a lot of different functions. And some of the code seems to be expecting multiple implicit rules and other parts not. I get the impression that a lot of this code is left over from a more ambitious first draft and ought to be removed in the name of readability/maintainability. If that code is on the right path to allowing things like updates to the many side of a join then it would be worth adding comments to that effect. Or maybe a comment referencing whatever version of the file the code was yanked out of. -- Jim C. Nasby, Sr. Engineering Consultant [EMAIL PROTECTED] Pervasive Software http://pervasive.comwork: 512-231-6117 vcard: http://jim.nasby.net/pervasive.vcf cell: 512-569-9461 ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] log_statement output for protocol
Bruce, I made a few tests here and the backend terminates with a SIG11 when a parameter has the NULL value (it was logged as (null) before). I suspect the new code broke something (perhaps it's due to the escaping). -- Guillaume ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] log_statement output for protocol
On 8/29/06, Bruce Momjian [EMAIL PROTECTED] wrote: DETAIL: prepare: SELECT $1; bind: $1 = 'a''b' I attached a trivial patch to add a dash between the prepare part and the bind part. People usually don't finish their queries with a semi colon so it's more readable with a separator. DETAIL: prepare: SELECT $1 bind: $1 = 'a''b' becomes DETAIL: prepare: SELECT $1 - bind: $1 = 'a''b' -- Guillaume Index: src/backend/tcop/postgres.c === RCS file: /projects/cvsroot/pgsql/src/backend/tcop/postgres.c,v retrieving revision 1.501 diff -c -r1.501 postgres.c *** src/backend/tcop/postgres.c 29 Aug 2006 02:32:41 - 1.501 --- src/backend/tcop/postgres.c 29 Aug 2006 11:46:15 - *** *** 1782,1788 *portal_name ? portal_name : ), errdetail(prepare: %s%s%s, sourceText, /* optionally print bind parameters */ ! bindText ? bind: : , bindText ? bindText : ))); BeginCommand(portal-commandTag, dest); --- 1782,1788 *portal_name ? portal_name : ), errdetail(prepare: %s%s%s, sourceText, /* optionally print bind parameters */ ! bindText ? - bind: : , bindText ? bindText : ))); BeginCommand(portal-commandTag, dest); *** *** 1896,1902 *portal_name ? portal_name : ), errdetail(prepare: %s%s%s, sourceText, /* optionally print bind parameters */ ! bindText ? bind: : , bindText ? bindText : ))); } } --- 1896,1902 *portal_name ? portal_name : ), errdetail(prepare: %s%s%s, sourceText, /* optionally print bind parameters */ ! bindText ? - bind: : , bindText ? bindText : ))); } } ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] log_statement output for protocol
Guillaume Smet wrote:
On 8/29/06, Bruce Momjian [EMAIL PROTECTED] wrote:
DETAIL: prepare: SELECT $1; bind: $1 = 'a''b'
I attached a trivial patch to add a dash between the prepare part and
the bind part. People usually don't finish their queries with a semi
colon so it's more readable with a separator.
DETAIL: prepare: SELECT $1 bind: $1 = 'a''b'
becomes
DETAIL: prepare: SELECT $1 - bind: $1 = 'a''b'
Good point. I thought it was clear enough, but obviously not. I had a
similar case with bind, and used a comma to separate them:
LOG: statement: prepare sel1, SELECT $1;
LOG: statement: bind sel1, $1 = 'a''b'
I am concerned a dash isn't clear enough, and a semicolon is confusing.
Using a comma the new output is:
LOG: duration: 0.023 ms execute sel1
DETAIL: prepare: SELECT $1;, bind: $1 = 'a''b'
or with no semicolon:
LOG: duration: 0.023 ms execute sel1
DETAIL: prepare: SELECT $1, bind: $1 = 'a''b'
Is that OK? Patch attached and committed. I also fixed the null bind
parameter bug. It now displays $1 = NULL (no quotes used). Other
suggestions?
--
Bruce Momjian [EMAIL PROTECTED]
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: src/backend/tcop/postgres.c
===
RCS file: /cvsroot/pgsql/src/backend/tcop/postgres.c,v
retrieving revision 1.501
diff -c -c -r1.501 postgres.c
*** src/backend/tcop/postgres.c 29 Aug 2006 02:32:41 - 1.501
--- src/backend/tcop/postgres.c 29 Aug 2006 19:54:08 -
***
*** 1539,1555
-1);
/* Save the parameter values */
! appendStringInfo(bind_values_str, %s$%d = ',
bind_values_str.len ? , : ,
paramno + 1);
! for (p = pstring; *p; p++)
{
! if (*p == '\'') /* double single quotes */
appendStringInfoChar(bind_values_str, *p);
! appendStringInfoChar(bind_values_str, *p);
}
! appendStringInfoChar(bind_values_str, '\'');
!
/* Free result of encoding conversion, if any */
if (pstring pstring != pbuf.data)
pfree(pstring);
--- 1539,1561
-1);
/* Save the parameter values */
! appendStringInfo(bind_values_str, %s$%d = ,
bind_values_str.len ? , : ,
paramno + 1);
! if (pstring)
{
! appendStringInfoChar(bind_values_str, '\'');
! for (p = pstring; *p; p++)
! {
! if (*p == '\'') /* double single quotes */
! appendStringInfoChar(bind_values_str, *p);
appendStringInfoChar(bind_values_str, *p);
! }
! appendStringInfoChar(bind_values_str, '\'');
}
! else
! appendStringInfo(bind_values_str, NULL);
!
/* Free result of encoding conversion, if any */
if (pstring pstring != pbuf.data)
pfree(pstring);
***
*** 1782,1788
*portal_name ? portal_name : ),
errdetail(prepare: %s%s%s, sourceText,
/* optionally print bind parameters */
! bindText ? bind: : ,
bindText ? bindText : )));
BeginCommand(portal-commandTag, dest);
--- 1788,1794
*portal_name ? portal_name : ),
errdetail(prepare: %s%s%s, sourceText,
/* optionally print bind parameters */
! bindText ? , bind: : ,
bindText ? bindText : )));
BeginCommand(portal-commandTag, dest);
***
*** 1896,1902
*portal_name ? portal_name : ),
errdetail(prepare: %s%s%s, sourceText,
/* optionally print bind parameters */
! bindText ? bind: : ,
bindText ? bindText : )));
}
}
--- 1902,1908
*portal_name ? portal_name : ),
errdetail(prepare: %s%s%s, sourceText,
/* optionally print bind parameters */
! bindText ? , bind: : ,
bindText ? bindText : )));
}
}
---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
Re: [HACKERS] [PATCHES] Trivial patch to double vacuum speed on tables with no indexes
Tom Lane [EMAIL PROTECTED] writes:
The reason the patch is so short is that it's a kluge. If we really
cared about supporting this case, more wide-ranging changes would be
needed (eg, there's no need to eat maintenance_work_mem worth of RAM
for the dead-TIDs array); and a decent respect to the opinions of
mankind would require some attention to updating the header comments
and function descriptions, too.
The only part that seems klugy to me is how it releases the lock and
reacquires it rather than wait in the first place until it can acquire the
lock. Fixed that and changed lazy_space_alloc to allocate only as much space
as is really necessary.
Gosh, I've never been accused of offending all mankind before.
--- vacuumlazy.c31 Jul 2006 21:09:00 +0100 1.76
+++ vacuumlazy.c28 Aug 2006 09:58:41 +0100
@@ -16,6 +16,10 @@
* perform a pass of index cleanup and page compaction, then resume the heap
* scan with an empty TID array.
*
+ * As a special exception if we're processing a table with no indexes we can
+ * vacuum each page as we go so we don't need to allocate more space than
+ * enough to hold as many heap tuples fit on one page.
+ *
* We can limit the storage for page free space to MaxFSMPages entries,
* since that's the most the free space map will be willing to remember
* anyway. If the relation has fewer than that many pages with free space,
@@ -106,7 +110,7 @@
TransactionId
OldestXmin);
static BlockNumber count_nondeletable_pages(Relation onerel,
LVRelStats *vacrelstats,
TransactionId OldestXmin);
-static void lazy_space_alloc(LVRelStats *vacrelstats, BlockNumber relblocks);
+static void lazy_space_alloc(LVRelStats *vacrelstats, BlockNumber relblocks,
unsigned nindexes);
static void lazy_record_dead_tuple(LVRelStats *vacrelstats,
ItemPointer itemptr);
static void lazy_record_free_space(LVRelStats *vacrelstats,
@@ -206,7 +210,8 @@
* This routine sets commit status bits, builds lists of dead
tuples
* and pages with free space, and calculates statistics on the
number
* of live tuples in the heap. When done, or when we run low on
space
- * for dead-tuple TIDs, invoke vacuuming of indexes and heap.
+ * for dead-tuple TIDs, or after every page if the table has no
indexes
+ * invoke vacuuming of indexes and heap.
*
* It also updates the minimum Xid found anywhere on the table in
* vacrelstats-minxid, for later storing it in pg_class.relminxid.
@@ -247,7 +252,7 @@
vacrelstats-rel_pages = nblocks;
vacrelstats-nonempty_pages = 0;
- lazy_space_alloc(vacrelstats, nblocks);
+ lazy_space_alloc(vacrelstats, nblocks, nindexes);
for (blkno = 0; blkno nblocks; blkno++)
{
@@ -282,8 +287,14 @@
buf = ReadBuffer(onerel, blkno);
- /* In this phase we only need shared access to the buffer */
- LockBuffer(buf, BUFFER_LOCK_SHARE);
+ /* In this phase we only need shared access to the buffer
unless we're
+* going to do the vacuuming now which we do if there are no
indexes
+*/
+
+ if (nindexes)
+ LockBuffer(buf, BUFFER_LOCK_SHARE);
+ else
+ LockBufferForCleanup(buf);
page = BufferGetPage(buf);
@@ -450,6 +461,12 @@
{
lazy_record_free_space(vacrelstats, blkno,
PageGetFreeSpace(page));
+ } else if (!nindexes) {
+ /* If there are no indexes we can vacuum the page right
now instead
+* of doing a second scan */
+ lazy_vacuum_page(onerel, blkno, buf, 0, vacrelstats);
+ lazy_record_free_space(vacrelstats, blkno,
PageGetFreeSpace(BufferGetPage(buf)));
+ vacrelstats-num_dead_tuples = 0;
}
/* Remember the location of the last page with nonremovable
tuples */
@@ -891,16 +908,20 @@
* See the comments at the head of this file for rationale.
*/
static void
-lazy_space_alloc(LVRelStats *vacrelstats, BlockNumber relblocks)
+lazy_space_alloc(LVRelStats *vacrelstats, BlockNumber relblocks, unsigned
nindexes)
{
longmaxtuples;
int maxpages;
- maxtuples = (maintenance_work_mem * 1024L) / sizeof(ItemPointerData);
- maxtuples = Min(maxtuples, INT_MAX);
- maxtuples = Min(maxtuples, MaxAllocSize / sizeof(ItemPointerData));
- /* stay sane if small maintenance_work_mem */
- maxtuples = Max(maxtuples, MaxHeapTuplesPerPage);
+ if (nindexes) {
+
Re: [HACKERS] [PATCHES] Trivial patch to double vacuum speed on tables with no indexes
Gregory Stark wrote: Tom Lane [EMAIL PROTECTED] writes: The reason the patch is so short is that it's a kluge. If we really cared about supporting this case, more wide-ranging changes would be needed (eg, there's no need to eat maintenance_work_mem worth of RAM for the dead-TIDs array); and a decent respect to the opinions of mankind would require some attention to updating the header comments and function descriptions, too. The only part that seems klugy to me is how it releases the lock and reacquires it rather than wait in the first place until it can acquire the lock. Fixed that and changed lazy_space_alloc to allocate only as much space as is really necessary. Gosh, I've never been accused of offending all mankind before. Does that feel good or bad? I won't comment on the spirit of the patch but I'll observe that you should respect mankind a little more by observing brace position in if/else ;-) -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] log_statement output for protocol
Guillaume Smet wrote:
On 8/7/06, Bruce Momjian [EMAIL PROTECTED] wrote:
Updated patch attached. It prints the text bind parameters on a single
detail line. I still have not seen portal names generated by libpq.
I'm currently testing CVS tip to generate sample log files. I noticed
that Bruce only patched log_statement and not
log_min_duration_statement which still has the old behaviour ie:
[1-1] LOG: duration: 0.097 ms execute my_query: SELECT * FROM shop
WHERE name = $1
The problem of not having the bind parameters still remains.
A lot of people use log_min_duration_statement and it's usually
recommended to use it instead of log_statement because log_statement
generates far too much output.
I tried to find a way to fix it but it's not so simple as when we bind
the statement, we don't know if the query will be slower than
log_min_duration_statement.
My first idea is that we should add a DETAIL line with the parameter
values to the execute log line when we are in the
log_min_duration_statement case. AFAICS the values are in the portal
but I don't know the overhead introduced by generating the detail line
from the portal.
Does anyone have a better idea on how we could fix it?
Yes, I do. I have applied the attached patch to fix this issue and
several others. The fix was to save the bind parameters in the portal,
and display those in the executor output, if available.
The other changes were to use single quotes for bind values, instead of
double quotes, and double literal single quotes in bind values (and
document that). I also made use of the DETAIL line to have much cleaner
output.
With the new output, bind displays prepare as detail, and execute
displays prepare and optionally bind. I re-added the statement: label
so people will understand why the line is being printed (it is
log_*statement behavior). I am now happy that the display is clear and
not cluttered.
-- SQL using log_statement
LOG: set log_statement = 'all';
LOG: statement: begin;
LOG: statement: prepare x as select $1::text;
LOG: statement: execute x ('a');
DETAIL: prepare: prepare x as select $1::text;
LOG: statement: commit;
-- SQL using log_min_duration_statement
LOG: statement: set log_statement = 'none';
LOG: duration: 0.242 ms statement: set log_min_duration_statement = 0;
LOG: duration: 0.155 ms statement: begin;
LOG: duration: 0.174 ms statement: prepare y as select $1::text;
LOG: duration: 0.252 ms statement: execute y ('a');
DETAIL: prepare: prepare y as select $1::text;
LOG: duration: 0.093 ms statement: commit;
-- protocol using log_statement
LOG: statement: prepare sel1, SELECT $1;
LOG: statement: bind sel1, $1 = 'a''b'
DETAIL: prepare: SELECT $1;
LOG: statement: execute sel1
DETAIL: prepare: SELECT $1; bind: $1 = 'a''b'
-- protocol using log_min_duration_statement
LOG: duration: 0.497 ms statement: SET log_min_duration_statement = 0;
LOG: duration: 0.027 ms execute sel1
DETAIL: prepare: SELECT $1; bind: $1 = 'a''b'
The last output doesn't have bind or prepare because we don't print
those because we don't do any duration timing for them. Should we print
the statement: lines of log_min_duration_statement == 0?
Also, this code assumes that a protocol bind and execute always has
prepared statement text, which I believe is true.
The use of brackets is gone. :-)
--
Bruce Momjian [EMAIL PROTECTED]
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: doc/src/sgml/config.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/config.sgml,v
retrieving revision 1.75
diff -c -c -r1.75 config.sgml
*** doc/src/sgml/config.sgml 17 Aug 2006 23:04:03 - 1.75
--- doc/src/sgml/config.sgml 28 Aug 2006 23:59:41 -
***
*** 2839,2845
prepare, bind, and execute commands are logged only if
varnamelog_statement/ is literalall/. Bind parameter
values are also logged if they are supplied in literaltext/
! format.
/para
para
The default is literalnone/. Only superusers can change this
--- 2839,2845
prepare, bind, and execute commands are logged only if
varnamelog_statement/ is literalall/. Bind parameter
values are also logged if they are supplied in literaltext/
! format (literal single quotes are doubled).
/para
para
The default is literalnone/. Only superusers can change this
Index: src/backend/commands/portalcmds.c
===
RCS file: /cvsroot/pgsql/src/backend/commands/portalcmds.c,v
retrieving revision 1.50
diff
Re: [HACKERS] [PATCHES] log_statement output for protocol
Bruce Momjian [EMAIL PROTECTED] writes: Yes, I do. I have applied the attached patch to fix this issue and several others. The fix was to save the bind parameters in the portal, and display those in the executor output, if available. I have a feeling you just blew away the 4% savings in psql I've spent the evening on. What's the overhead of this patch? regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] log_statement output for protocol
BTom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Yes, I do. I have applied the attached patch to fix this issue and several others. The fix was to save the bind parameters in the portal, and display those in the executor output, if available. I have a feeling you just blew away the 4% savings in psql I've spent the evening on. What's the overhead of this patch? The only overhead I see is calling log_after_parse() all the time, rather than only when log_statement is all. I could fix it by checking log_statement and log_min_duration_statement = 0. Does log_after_parse() look heavy to you? -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] [PATCHES] log_statement output for protocol
bruce wrote: BTom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Yes, I do. I have applied the attached patch to fix this issue and several others. The fix was to save the bind parameters in the portal, and display those in the executor output, if available. I have a feeling you just blew away the 4% savings in psql I've spent the evening on. What's the overhead of this patch? The only overhead I see is calling log_after_parse() all the time, rather than only when log_statement is all. I could fix it by checking log_statement and log_min_duration_statement = 0. Does log_after_parse() look heavy to you? OK, I applied this patch to call log_after_parse() only if necessary. The 'if' statement looked pretty ugly, so the optimization seemed overkill, but maybe it will be useful. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/backend/tcop/postgres.c === RCS file: /cvsroot/pgsql/src/backend/tcop/postgres.c,v retrieving revision 1.500 diff -c -c -r1.500 postgres.c *** src/backend/tcop/postgres.c 29 Aug 2006 02:11:29 - 1.500 --- src/backend/tcop/postgres.c 29 Aug 2006 02:29:48 - *** *** 871,877 parsetree_list = pg_parse_query(query_string); /* Log immediately if dictated by log_statement */ ! was_logged = log_after_parse(parsetree_list, query_string, prepare_string); /* * Switch back to transaction context to enter the loop. --- 871,879 parsetree_list = pg_parse_query(query_string); /* Log immediately if dictated by log_statement */ ! if (log_statement != LOGSTMT_NONE || log_duration || ! log_min_duration_statement = 0) ! was_logged = log_after_parse(parsetree_list, query_string, prepare_string); /* * Switch back to transaction context to enter the loop. ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Trivial patch to double vacuum speed on tables with no indexes
Gregory Stark [EMAIL PROTECTED] writes: Tom Lane [EMAIL PROTECTED] writes: How often does that case come up in the real world, for tables that are large enough that you'd care about vacuum performance? I would have had the same objection if it resulted in substantially more complex code but it was so simple that it doesn't seem like a concern. The reason the patch is so short is that it's a kluge. If we really cared about supporting this case, more wide-ranging changes would be needed (eg, there's no need to eat maintenance_work_mem worth of RAM for the dead-TIDs array); and a decent respect to the opinions of mankind would require some attention to updating the header comments and function descriptions, too. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] log_statement output for protocol
On 8/7/06, Bruce Momjian [EMAIL PROTECTED] wrote: Updated patch attached. It prints the text bind parameters on a single detail line. I still have not seen portal names generated by libpq. I'm currently testing CVS tip to generate sample log files. I noticed that Bruce only patched log_statement and not log_min_duration_statement which still has the old behaviour ie: [1-1] LOG: duration: 0.097 ms execute my_query: SELECT * FROM shop WHERE name = $1 The problem of not having the bind parameters still remains. A lot of people use log_min_duration_statement and it's usually recommended to use it instead of log_statement because log_statement generates far too much output. I tried to find a way to fix it but it's not so simple as when we bind the statement, we don't know if the query will be slower than log_min_duration_statement. My first idea is that we should add a DETAIL line with the parameter values to the execute log line when we are in the log_min_duration_statement case. AFAICS the values are in the portal but I don't know the overhead introduced by generating the detail line from the portal. Does anyone have a better idea on how we could fix it? Regards, -- Guillaume ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] psql 'none' as a HISTFILE special case
Martin Atukunda [EMAIL PROTECTED] writes: On 8/25/06, Tom Lane [EMAIL PROTECTED] wrote: There's probably no way to get Apple's libedit to not try the fchmod, so what do we want to do here? Maybe special-case the string /dev/null? If this is OK, I can up with a patch that special cases /dev/null as a HISTFILE if libedit is found. I was thinking of basically a one-liner addition to write_history to skip the whole thing if strcmp(fname, DEVNULL) == 0. Should be reasonably inoffensive on anyone's machine. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] psql 'none' as a HISTFILE special case
On 8/25/06, Tom Lane [EMAIL PROTECTED] wrote: Martin Atukunda [EMAIL PROTECTED] writes: On 8/25/06, Tom Lane [EMAIL PROTECTED] wrote: There's probably no way to get Apple's libedit to not try the fchmod, so what do we want to do here? Maybe special-case the string /dev/null? If this is OK, I can up with a patch that special cases /dev/null as a HISTFILE if libedit is found. I was thinking of basically a one-liner addition to write_history to skip the whole thing if strcmp(fname, DEVNULL) == 0. Should be reasonably inoffensive on anyone's machine. I guess you meant saveHistory instead of write_history here. :) something like the attached diff - Martin - special_case_DEVNULL.diff Description: Binary data ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] psql 'none' as a HISTFILE special case
Am Freitag, 25. August 2006 17:03 schrieb Martin Atukunda: hmm, setting HISTFILE to /dev/null doesn't work on my MacOSX here. Please elaborate on doesn't work. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] [PATCHES] psql 'none' as a HISTFILE special case
On 8/25/06, Peter Eisentraut [EMAIL PROTECTED] wrote: Am Freitag, 25. August 2006 17:03 schrieb Martin Atukunda: hmm, setting HISTFILE to /dev/null doesn't work on my MacOSX here. Please elaborate on doesn't work. without any .psqlrc file I get the following error when quitting a psql session: could not save history to file /Users/matlads/.psql_history: Invalid argument When I set HISTFILE to /dev/null I get the following: could not save history to file /dev/null: Operation not permitted - Martin - ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] psql 'none' as a HISTFILE special case
Martin Atukunda [EMAIL PROTECTED] writes: On 8/25/06, Peter Eisentraut [EMAIL PROTECTED] wrote: Please elaborate on doesn't work. without any .psqlrc file I get the following error when quitting a psql session: could not save history to file /Users/matlads/.psql_history: Invalid argument That is fixed in CVS HEAD. The current coding looks like: /* * return value of write_history is not standardized across GNU * readline and libedit. Therefore, check for errno becoming set * to see if the write failed. */ errno = 0; (void) write_history(fname); if (errno == 0) return true; psql_error(could not save history to file \%s\: %s\n, fname, strerror(errno)); When I set HISTFILE to /dev/null I get the following: could not save history to file /dev/null: Operation not permitted Hm. ktrace shows this happening: 23279 psql CALL open(0x302d70,0x601,0x1b6) 23279 psql NAMI /dev/null 23279 psql RET open 3 23279 psql CALL fchmod(0x3,0x180) 23279 psql RET fchmod -1 errno 1 Operation not permitted 23279 psql CALL close(0x3) 23279 psql RET close 0 23279 psql CALL write(0x2,0xb180,0x44) 23279 psql GIO fd 2 wrote 68 bytes could not save history to file /dev/null: Operation not permitted 23279 psql RET write 68/0x44 23279 psql CALL exit(0) There's probably no way to get Apple's libedit to not try the fchmod, so what do we want to do here? Maybe special-case the string /dev/null? regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] psql 'none' as a HISTFILE special case
On 8/25/06, Tom Lane [EMAIL PROTECTED] wrote: When I set HISTFILE to /dev/null I get the following: could not save history to file /dev/null: Operation not permitted Hm. ktrace shows this happening: 23279 psql CALL open(0x302d70,0x601,0x1b6) 23279 psql NAMI /dev/null 23279 psql RET open 3 23279 psql CALL fchmod(0x3,0x180) 23279 psql RET fchmod -1 errno 1 Operation not permitted 23279 psql CALL close(0x3) 23279 psql RET close 0 23279 psql CALL write(0x2,0xb180,0x44) 23279 psql GIO fd 2 wrote 68 bytes could not save history to file /dev/null: Operation not permitted 23279 psql RET write 68/0x44 23279 psql CALL exit(0) There's probably no way to get Apple's libedit to not try the fchmod, so what do we want to do here? Maybe special-case the string /dev/null? If this is OK, I can up with a patch that special cases /dev/null as a HISTFILE if libedit is found. - Martin - ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] [PATCHES] COPY view
Tom Lane írta: Zoltan Boszormenyi [EMAIL PROTECTED] writes: How about the callback solution for the SELECT case that was copied from the original? Should I consider open-coding in copy.c what ExecutorRun() does to avoid the callback? Adding a DestReceiver type is a good solution ... although that static variable is not. Instead define a DestReceiver extension struct that can carry the CopyState pointer for you. Done. You could also consider putting the copy-from-view-specific state fields into DestReceiver instead of CopyState, though this is a bit asymmetric with the relation case so maybe it's not really cleaner. Left it alone for now. BTW, lose the tuple_to_values function --- it's an extremely bad reimplementation of heap_deform_tuple. Done. copy_dest_printtup also seems coded without regard for the TupleTableSlot access API (read printtup() to see what to do instead). I am still interpreting it. Can you give me some hints besides using slot_getallattrs(slot)? And what's the point of factoring out the heap_getnext loop as CopyRelationTo? It's not like that lets you share any more code. The inside of the loop, ie what you've called CopyValuesTo, is the sharable part. Done. The option parsing and error checking is now common. I also changed it to use transformStmt() in analyze.c. However, both the UNION and sunselect cases give me something like this: ERROR: could not open relation 1663/16384/16723: No such file or directory What else can I do with it? Best regards, Zoltán Böszörményi pgsql-copyselect-4.patch.gz Description: Unix tar archive ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
