[HACKERS] improving \dt++ in psql
Hi I tried to start writing a patch to add "Total Size" column to \dt++ output. in src/bin/psql/describe.c we have this listTables( const char *tabtypes, const char *pattern, bool verbose, bool showSystem) I was (as a long time Pg user) dead sure that psql really sometimes cares about the number of plus signs that you add to meta-command So why this particular function interface has boolean "verbose" parameter? Can't we have higher levels of "verbosiness"? thanks, Filip -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix initdb for path with whitespace and at char
On Wed, Apr 30, 2014 at 4:01 PM, Heikki Linnakangas wrote: > I committed the non-invasive fixes to backbranches (and master too, just to > keep it in sync), but the attached is what I came up with for master. > > There are a couple of places in the code where we have #ifdef WIN32 code > that uses CreateProcess with "CMD /C ..." directly. 1. Do we need similar handling for CreatePipe case where it directly uses executable path such as in function pipe_read_line()? Currently the caller of pipe_read_line() calls canonicalize_path() to change win32 specific path, is that sufficient or do we need SYSTEMQUOTE type of handling for it. 2. system.c #include Do we really need inclusion of assert.h or this is for future use? 3. One more observation is that currently install.bat doesn't work for such paths: install.bat "e:\PostgreSQL\master\install 1\ins@1" 1\ins@1""=="" was unexpected at this time. 4. Similar to Andrew, I also could not reproduce this problem on my Windows system (Windows 7 64 bit) e:\>"e:\PostgreSQL\master\install 1\ins@1\bin\initdb.exe" -D "e: \PostgreSQL\master\Data 1" e:\>"e:\PostgreSQL\master\install 1\ins@1\bin\pg_ctl.exe" start -D "e: \PostgreSQL\master\Data 1" Above commands work fine. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] hooks not working in postgresql 9.3 (building from source)
On Thu, May 1, 2014 at 12:33 AM, Rajmohan C wrote: >I need to use the hook feature of planner in postgresql 9.3 to perform a > task. I am building postgresql from source. This hook is called planner_hook_type and is present in planner.c. > To start with, I wanted to know how to create a hook and attach that shared > libray to postgresql. Hence i tried the examples given in > "wiki.postgresql.org/images/e/e3/Hooks_in_postgresql.pdf" and > "https://github.com/gleu/Hooks-in-PostgreSQL/tree/master/examples/my_client_auth";. > I have copied the "my_client_auth.c" file and Makefile into > contrib/client_auth folder. make and make install is working fine. > This is the output of make install. > > /bin/mkdir -p '/home/rajmohan/projects/lib/postgresql' > /usr/bin/install -c -m 755 my_client_auth.so > '/home/rajmohan/projects/lib/postgresql/' > after that i have added shared_preload_libraries = 'my_client_auth' to > postgresql.conf This is fine. > Then i added the line > ClientAuthentication_hook_type client_auth_hook = NULL; at the top of a file > say planner.c in postgresql code > and inside a method im checking client_auth_hook value. When i rebuild and > run the project, > client_auth_hook value is always zero. Well, you are trying to create a new hook here, and not to use an existing one. You do not need to modify the source code of PostgreSQL when using hooks, that's what make their strength. > It seems my_client_auth.so file is > not linked properly to my postgresql project. > Am i missing any step? how to access methods in my_client_auth.so from > postgresql. Kindly help Are you sure that you set up properly shared_preload_libraries = 'my_client_auth' and then restarted the PostgreSQL server. If the hook library is properly loaded by server, you should see a message similar to that if of course server is pretty verbose when logging things: LOG: loaded library "my_client_auth" You can as well try a hook on an existing session by using LOAD. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Small doc patch for pg_replication_slots
On Tue, Apr 29, 2014 at 5:39 AM, Thomas Reiss wrote: > You can find attached a small patch to fix the pg_replication_slots > documentation. The slot_type and plugin are not in the appropriate > order, slot_name and plugin have a wrong type and xmin appears two times. Without the patch, the description of catalog_xmin is: The xmin, or oldest transaction ID, that this slot forces to be retained in the system catalogs. With the patch it's: The oldest transaction that this slot needs the database to retain. VACUUM cannot remove catalog tuples deleted by any later transaction. That's only one word different from the language for xmin, which doesn't seem quite right. Committed after fixing that. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Display of timestamp in pg_dump custom format
On 01/05/14 12:04, Bruce Momjian wrote: On Thu, May 1, 2014 at 08:27:49AM +1200, Gavin Flower wrote: On 01/05/14 02:51, Bruce Momjian wrote: The table of contents for pg_restore -l shows the time the archive was made as local time (it uses ctime()): ; Archive created at Wed Apr 30 10:03:28 2014 Is this clear enough that it is local time? Should we display this better, perhaps with a time zone designation? I think it would be good to include the time zone, as we are all very international these days - and in Australia, adjacent states have different dates for the summer time transition! Personally, I would like to see the date in the format 2014-04-30, but having the day of the week is good. Milliseconds might be useful, if you want to check logs files. OK, I will work on it for 9.5. Thanks. So the it would then read something like: ; Archive created at Wed 2014-04-30 10:03:28.042 NZST (but with the correct appropriate time zone designation)? Cheers, Gavin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Display of timestamp in pg_dump custom format
On Thu, May 1, 2014 at 08:27:49AM +1200, Gavin Flower wrote: > On 01/05/14 02:51, Bruce Momjian wrote: > >The table of contents for pg_restore -l shows the time the archive was > >made as local time (it uses ctime()): > > > > ; Archive created at Wed Apr 30 10:03:28 2014 > > > >Is this clear enough that it is local time? Should we display this > >better, perhaps with a time zone designation? > > > I think it would be good to include the time zone, as we are all > very international these days - and in Australia, adjacent states > have different dates for the summer time transition! > > Personally, I would like to see the date in the format 2014-04-30, > but having the day of the week is good. > > Milliseconds might be useful, if you want to check logs files. OK, I will work on it for 9.5. Thanks. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] includedir_internal headers are not self-contained
Andres Freund writes: > On 2014-04-28 13:20:47 -0400, Tom Lane wrote: >> Printing anything other than the relation OID here is irrelevant, >> misleading, and inconsistent with our practice everywhere else. > I don't think it's really comparable to the other scenarios. We should > print the oid, just as relation_open() does, but the filenode is also > rather helpful here. How about the attached? Applied along with a bit of other cleanup. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] shm_mq inconsistent behavior of SHM_MQ_DETACHED
On Mon, Apr 28, 2014 at 4:24 PM, Petr Jelinek wrote: > Yes, the patch fixes it for me. OK. I committed it. Thanks for the report. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GSoC on WAL-logging hash indexes
Greg Stark writes: > But imnsho doing nothing is a bad idea. We should have long ago either > added WAL logging or removed the index type. We shouldn't have left a > foot-gun that large lying around for so long. We can't remove the hash index type, nor move it to an extension, because it is the operator classes for the built-in hash index AM that tell the planner and executor how to do hashing for arbitrary datatypes. And we certainly do not want to give up hashing-based query plans, whatever you may think of hash indexes. We really oughta fix the WAL situation, not just band-aid around it. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix initdb for path with whitespace and at char
On 04/30/2014 03:03 PM, Andrew Dunstan wrote: On 04/30/2014 11:58 AM, Andrew Dunstan wrote: On 04/30/2014 06:31 AM, Heikki Linnakangas wrote: Andrew: you have a cygwin installation, don't you? Could you test if "pg_ctl start" works when the binaries are installed to a path that contains both a space and an @ sign, like "C:\white space\at@sign\install". I suspect it doesn't, but the attached patch fixes it. I'll see what I can do. I tried git master like this: foo\ bar/a\@b/bin/initdb.exe data foo\ bar/a\@b/bin/pg_ctl.exe -D data/ -w start and didn't encounter a problem. Platform is Windows 8 Pro 64 bit, with latest 32 bit Cygwin. [ ... ] It looks like possibly the only time this will actually matter on Cygwin is when starting a service. Just running "pg_ctl start" from the command line works fine. But starting the service doesn't. I'll try the patch when I get a chance. No, there is something horribly wrong with the Cygwin service code. I don't have time now to sort it out. I suspect it's been broken for a very long time. I'm not even sure why we have it. Cygwin contains a wrapper (cygrunsrv) for setting up cygwin executables as services, so this is probably worse than redundant. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] includedir_internal headers are not self-contained
I wrote: > How about we change common/relpath.[hc] to export a single version of > relpath() that takes its arguments as separate plain OIDs, and then > make the other versions wrappers that are only declared in some > backend header? The wrappers could possibly be macros, allowing > things like pg_xlogdump to keep using them as long as they didn't > mind importing backend headers. (Though for the RelFileNode case this > would imply multiple evaluation of the macro argument, so maybe it's > not such a great idea.) Since nobody objected, I've committed something along this line. include/common/ is now free of references to backend headers. The patch is certainly too invasive to consider back-patching into 9.3, though. Heikki Linnakangas writes: >> One idea would be to have relpath.h/.c expose a function (not a #define) >> that returns the value of CATALOG_VERSION_NO compiled into it. Then, >> if pg_rewind were to replace all its direct references to >> CATALOG_VERSION_NO (including the value it checks against pg_control) >> with calls of that function, consistency would be ensured. > In pg_rewind, I'd like to compile CATALOG_VERSION_NO into the binary > itself, because pg_rewind is quite version-specific. Even if it happens > to work with libpgport from a different version, I would worry that > there are directory layout changes that would need to be handled in > pg_rewind for it to work safely. So I would like to lock it to a > specific catalog version. > To lock it down, I could call the function and check that it matches the > compiled-in value of CATALOG_VERSION_NO, though. So a function works for > me, even though I don't really need the flexibility. I didn't do anything about this idea, but if you want to follow through with it, feel free to add such a function. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] bgworker crashed or not?
On Mon, Apr 28, 2014 at 4:19 PM, Petr Jelinek wrote: > On 28/04/14 16:27, Robert Haas wrote: >> On Thu, Apr 24, 2014 at 1:39 AM, Craig Ringer >> wrote: >>> On 04/17/2014 08:35 AM, Craig Ringer wrote: >>> I've just noticed that the bgworker control interfaces do not honour >>> bgw.bgw_restart_time = BGW_NEVER_RESTART if you exit with status zero. >>> >>> This means that it's not simply a problem where you can't say "restart >>> me if I crash, but not if I exit normally". >>> >>> You also can't even say "never restart me at all". Because >>> "BGW_NEVER_RESTART" seems to really mean "BGW_NO_RESTART_ON_CRASH". >>> >>> This _needs_fixing before 9.4. >> >> >> It seems we have consensus on what to do about this, but what we >> haven't got is a patch. > > If you mean the consensus that exit status 0 should mean permanent stop then > I think the patch can be as simple as attached. Hmm. Well, at the very least, you need to update the comment. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GSoC on WAL-logging hash indexes
Stephen Frost wrote: > * Greg Stark (st...@mit.edu) wrote: > > I could do the legwork on either the GUC or moving hash indexes to an > > extension if there's a consensus. But I suspect either will be quite > > controversial... > If you'd like to build an extension which provides hash indexes- I say > go for it? Nothing stopping you as far as that's concerned. Extensions can't write/replay WAL, so it's not clear to me how would this work. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [pgsql-advocacy] [HACKERS] GSoC on WAL-logging hash indexes
Is there a good reason for this thread being copied to the advocacy list? It seems to me just on topic for hackers. -- Darren Duncan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Display of timestamp in pg_dump custom format
On 01/05/14 02:51, Bruce Momjian wrote: The table of contents for pg_restore -l shows the time the archive was made as local time (it uses ctime()): ; Archive created at Wed Apr 30 10:03:28 2014 Is this clear enough that it is local time? Should we display this better, perhaps with a time zone designation? I think it would be good to include the time zone, as we are all very international these days - and in Australia, adjacent states have different dates for the summer time transition! Personally, I would like to see the date in the format 2014-04-30, but having the day of the week is good. Milliseconds might be useful, if you want to check logs files. Cheers, Gavin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GSoC on WAL-logging hash indexes
On Wed, Apr 30, 2014 at 12:16 PM, Greg Stark wrote: > I think the key question was if someone wanted to develop a good hash > index would they start from our current hash index or would they be > better off starting from a fresh codebase? If it were me I'd start with the current code. It would be nice if one could just fork the code to have a new type of index (say "hash2") which is initially identical, but I never figured out how to do that. > If the former then we > should add WAL logging and throw GSOC and other people who ask for > projects at it. If the latter then we should throw out the current > codebase and let people develop extensions that add new hash index > code until someone comes up with a good design we want to move to > core. > Extensions have no hooks into the WAL system, and I'm not optimistic that that will ever change. Relegating a fundamentally new index to be an extension virtually *guarantees* that it will never be WAL logged. > > Incidentally something else I've considered would be having a WAL > record type saying "relation corrupted" and emitting one the first > time a hash index is touched after a checkpoint. That could provide a > general mechanism that might be useful for unlogged operations (and > might be combinable with the infrastructure for unlogged tables). But > it seems like a better tool for other objects than hash indexes. > +1. I often lament that unlogged tables cannot be used on a standby or a test server which were derived from a hot backup. In the case of unlogged tables, this does mean we would need a way to checkpoint them with a non-shutdown checkpoint, though. I don't know if that would need a different type of unlogged table, or a different type of checkpoint. Cheers, Jeff
Re: [HACKERS] GSoC on WAL-logging hash indexes
On Wed, Apr 30, 2014 at 11:19 AM, Peter Geoghegan wrote: > On Wed, Apr 30, 2014 at 11:03 AM, Jeff Janes wrote: > > If we don't put in the work to make them useful, then they won't ever > become > > useful. > > > > If we do put in the effort (and it would be considerable) then I think > they > > will be. But you may be correct that the effort required would perhaps > be > > better used in making btree even more better. I don't think we can > conclude > > that definitively without putting in the work to do the experiment. > > My argument doesn't hinge on there being more important work to do. > Rather, I simply don't think that there is never going to be a > compelling reason to use hash indexes in production. I have an indexed text column with an average length of 50, a stddev length of 15, and a pronounced right skew. Currently, the longest value in it is 811. But inevitably someone will need to insert something longer than 2712. When that day comes, I will drop the btree index and add a hash index (unless we remove that limitation from btree indexes in the mean time). It lets me sleep at night knowing that I have that option today, even if it would complicate crash recovery. Apart from the > obvious inflexibility, consider what it takes to make index creation > fast - insertion-style building of indexes is much slower. Consider > multi-key indexes. > I'm pretty sure hash indexes already implement a bulk creation fast path. In any case, I've never noticed them being slow, and I've tested some pretty big ones. > Now, I'm not telling anyone what to work on, and if someone wants to > make hash indexes WAL-logged to plug that hole, don't let me stop you. > It probably makes sense as a project to learn more about Postgres > internals. However, it would be unfair to not speak up given my > misgivings around the practical utility of hash indexes. > Sure, and we all have our own opinions on that. Should we summarize them somewhere easier to follow than a long email thread but more detailed than a TODO entry? Whatever happened with the GSOC people? That should be well under way by now, is anyone working on it? Are the discussions of their efforts on-list, or is it between them and their mentors? Cheers, Jeff
Re: [HACKERS] GSoC on WAL-logging hash indexes
* Greg Stark (st...@mit.edu) wrote: > But imnsho doing nothing is a bad idea. We should have long ago either > added WAL logging or removed the index type. We shouldn't have left a > foot-gun that large lying around for so long. I certainly encourage you to work on it. :) > Incidentally something else I've considered would be having a WAL > record type saying "relation corrupted" and emitting one the first > time a hash index is touched after a checkpoint. That could provide a > general mechanism that might be useful for unlogged operations (and > might be combinable with the infrastructure for unlogged tables). But > it seems like a better tool for other objects than hash indexes. Ugh, please do not make unlogged tables suffer for this. I feel it is *quite* clear when you say "UNLOGGED" or "TEMP" in the CREATE statement that you know what you're gonna get. Perhaps we should require 'UNLOGGED INDEX' to be passed in when someone creates a 'hash' index instead. > Another quick fix would be having a GUC allow_unrecoverable_relations > which defaulted to false. Creating a hash index (and presumably > unlogged tables) would error out with a hint to set that to true so at > least users who create them would have to know what they were in for. -1 > Right now we're seeing a lot of users who create hash indexes and then > complain about corrupted standbys. Uh, we are? If you're saying that $employer is, great, but please clarify that as the rest of us aren't 'in the loop' as far as that goes and we see the various list/IRC traffic instead, and I can't remember ever seeing such a complaint in either place (but I'll admit, my memory isn't the best). > I could do the legwork on either the GUC or moving hash indexes to an > extension if there's a consensus. But I suspect either will be quite > controversial... -1 on the GUC. I'd be alright with finding a way to make it clear, upon creation of the hash index, that it's not WAL'd (maybe even just throw a WARNING, it'd be better than nothing..). Trying to rip out the current hash index wouldn't be great, imv. If you'd like to build an extension which provides hash indexes- I say go for it? Nothing stopping you as far as that's concerned. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] pg_get_viewdefs() indentation considered harmful
On Wed, Apr 30, 2014 at 6:47 PM, Tom Lane wrote: > I pushed a patch that does it that way, and also patches for the other > items discussed in this thread. Great! thanks a lot. This makes a really solid noticeable difference even in relatively simple cases and I know of users for whom this will make a big difference. That indentation code looks like a lot of work to maintain that I hadn't really been aware of before. I wonder if there's some way to combine it with the actual grammar so the input and output can be edited in the same place. -- greg -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GSoC on WAL-logging hash indexes
I think the key question was if someone wanted to develop a good hash index would they start from our current hash index or would they be better off starting from a fresh codebase? If the former then we should add WAL logging and throw GSOC and other people who ask for projects at it. If the latter then we should throw out the current codebase and let people develop extensions that add new hash index code until someone comes up with a good design we want to move to core. But imnsho doing nothing is a bad idea. We should have long ago either added WAL logging or removed the index type. We shouldn't have left a foot-gun that large lying around for so long. Incidentally something else I've considered would be having a WAL record type saying "relation corrupted" and emitting one the first time a hash index is touched after a checkpoint. That could provide a general mechanism that might be useful for unlogged operations (and might be combinable with the infrastructure for unlogged tables). But it seems like a better tool for other objects than hash indexes. Another quick fix would be having a GUC allow_unrecoverable_relations which defaulted to false. Creating a hash index (and presumably unlogged tables) would error out with a hint to set that to true so at least users who create them would have to know what they were in for. Right now we're seeing a lot of users who create hash indexes and then complain about corrupted standbys. I could do the legwork on either the GUC or moving hash indexes to an extension if there's a consensus. But I suspect either will be quite controversial... -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix initdb for path with whitespace and at char
On 04/30/2014 11:58 AM, Andrew Dunstan wrote: On 04/30/2014 06:31 AM, Heikki Linnakangas wrote: Andrew: you have a cygwin installation, don't you? Could you test if "pg_ctl start" works when the binaries are installed to a path that contains both a space and an @ sign, like "C:\white space\at@sign\install". I suspect it doesn't, but the attached patch fixes it. I'll see what I can do. I tried git master like this: foo\ bar/a\@b/bin/initdb.exe data foo\ bar/a\@b/bin/pg_ctl.exe -D data/ -w start and didn't encounter a problem. Platform is Windows 8 Pro 64 bit, with latest 32 bit Cygwin. [ ... ] It looks like possibly the only time this will actually matter on Cygwin is when starting a service. Just running "pg_ctl start" from the command line works fine. But starting the service doesn't. I'll try the patch when I get a chance. cheers andrew cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GSoC on WAL-logging hash indexes
On Wed, Apr 30, 2014 at 11:03 AM, Jeff Janes wrote: > If we don't put in the work to make them useful, then they won't ever become > useful. > > If we do put in the effort (and it would be considerable) then I think they > will be. But you may be correct that the effort required would perhaps be > better used in making btree even more better. I don't think we can conclude > that definitively without putting in the work to do the experiment. My argument doesn't hinge on there being more important work to do. Rather, I simply don't think that there is never going to be a compelling reason to use hash indexes in production. Apart from the obvious inflexibility, consider what it takes to make index creation fast - insertion-style building of indexes is much slower. Consider multi-key indexes. Now, I'm not telling anyone what to work on, and if someone wants to make hash indexes WAL-logged to plug that hole, don't let me stop you. It probably makes sense as a project to learn more about Postgres internals. However, it would be unfair to not speak up given my misgivings around the practical utility of hash indexes. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix initdb for path with whitespace and at char
Heikki Linnakangas writes: > I committed the non-invasive fixes to backbranches (and master too, just > to keep it in sync), but the attached is what I came up with for master. The malloc's in the new system.c file should be pg_malloc, or else have custom defenses against out-of-memory (possibly returning ENOMEM to the caller would be best?). Also, it seems like a good idea to save and restore errno across the ending free() calls. I don't know if Windows' version of free() can change errno, but we've definitely found that to be possible on other platforms. Looks good otherwise. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GSoC on WAL-logging hash indexes
On Wed, Apr 30, 2014 at 2:02 PM, Peter Geoghegan wrote: >> Of course our current hash indexes have *more* not less contention >> than btree but I'm pretty comfortable chalking that up to quality of >> implementation rather than anything intrinsic. > > I am not convinced of that. In commit 76837c1507cb5a5a0048046233568447729e66dd, I remove part (basically half) of the heavyweight locking used by hash indexes, and performance improved rather dramatically: http://www.postgresql.org/message-id/CA+Tgmoaf=nojxlyzgcbrry+pe-0vll0vfhi6tjdm3fftvws...@mail.gmail.com I think it's quite reasonable to suppose that getting rid of the remaining heavyweight locking will help just as much. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GSoC on WAL-logging hash indexes
On 2014-04-30 11:10:22 -0700, Jeff Janes wrote: > I've seen the simple pinning and unpinning of the root page (or the fast root, > whatever the first page we bother to pin on a regular basis is called) be a > point of contention. When one index dominates the entire system workload, > that > one page also drives contention on the spin lock that protects the lwlock that > share-protects whichever buffer mapping partition happens to contain it. To quite some degree that's an implementation deficiency of our lwlocks though. I've seen *massive* improvements with my lwlock patch for that problem. Additionally we need to get rid of the spinlock around pin/unpin. That said, even after those optimizations, there remains a significant amount of cacheline bouncing. That's much easier to avoid for something like hash indexes than btrees. I think another advantage is that hash indexes can be *much* smaller than btree when the individual rows are wide. I wonder though if we couldn't solve that better by introducing "transforms" around the looked up data. E.g. allow to *transparently* use a hash(indexed_column) to be used. If you currently do that a lot of work has to be done in every query... Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GSoC on WAL-logging hash indexes
On Wed, Apr 30, 2014 at 11:02 AM, Peter Geoghegan wrote: > On Wed, Apr 30, 2014 at 10:11 AM, Robert Haas > wrote: > > I thought the theoretical advantage of hash indexes wasn't that they > > were smaller but that you avoided a central contention point (the > > btree root). > > The B-Tree root isn't really a central contention point at all. The > locking/latching protocol that nbtree uses is remarkably > concurrency-friendly. In the real world, there is pretty much no > exclusive locking of the root page's buffer. > I've seen the simple pinning and unpinning of the root page (or the fast root, whatever the first page we bother to pin on a regular basis is called) be a point of contention. When one index dominates the entire system workload, that one page also drives contention on the spin lock that protects the lwlock that share-protects whichever buffer mapping partition happens to contain it. Cheers, Jeff
Re: [HACKERS] GSoC on WAL-logging hash indexes
All, (dropping pgsql-advocacy off the cc list) On 04/30/2014 10:11 AM, Robert Haas wrote: > On Wed, Apr 30, 2014 at 12:54 PM, Peter Geoghegan wrote: >> On Wed, Apr 30, 2014 at 5:55 AM, k...@rice.edu wrote: >>> I do not think that CPU costs matter as much as the O(1) probe to >>> get a result value specifically for very large indexes/tables where >>> even caching the upper levels of a B-tree index would kill your >>> working set in memory. I know, I know, everyone has so much memory >>> and can just buy more... but this does matter. >> >> Have you actually investigated how little memory it takes to store the >> inner pages? It's typically less than 1% of the entire index. AFAIK, >> hash indexes are not used much in any other system. I think MySQL has >> them, and SQL Server 2014 has special in-memory hash table indexes for >> in memory tables, but that's all I can find on Google. Hash indexes are more important for MySQL because they have index-organized tables. > I thought the theoretical advantage of hash indexes wasn't that they > were smaller but that you avoided a central contention point (the > btree root). Yes. And being smaller isn't insignificant; think of billion-row tables with fairly random access over the whole table. Also, *theoretically*, a hash index could avoid the rebalancing issues which cause our btree indexes to become bloated and need a REINDEX with certain update patterns. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GSoC on WAL-logging hash indexes
On Wed, Apr 30, 2014 at 12:26 AM, Peter Geoghegan wrote: > On Mon, Mar 3, 2014 at 8:12 AM, Robert Haas wrote: > >> As a GSoC student, I will implement WAL recovery of hash indexes using > the > >> other index types' WAL code as a guide. > > Frankly, I'm skeptical of the idea that hash indexes will ever really > be useful. I realize that that's a counter-intuitive conclusion, but > there are many things we could do to improve B-Tree CPU costs to make > them closer to those of hash indexes, without making them any less > flexible. I myself would much rather work on that, and intend to. > If we don't put in the work to make them useful, then they won't ever become useful. If we do put in the effort (and it would be considerable) then I think they will be. But you may be correct that the effort required would perhaps be better used in making btree even more better. I don't think we can conclude that definitively without putting in the work to do the experiment. One advantage of the hash indexes is that the code is simple enough for someone to actually understand it in a summer. Whether it would still be like that after WAL logging was implemented, I don't know. The O(1) cost seems attractive when you consider that that only > requires that we read one index page from disk to service any given > index scan, but in fact B-Trees almost always only require the same. > They are of course also much more flexible. The concurrency > characteristics B-Trees are a lot better understood. Not sure what you mean there. The concurrency issues of the hash index has a lot less that needs to be understand. I think I understand it pretty well (unlike B-tree), I just don't know what to with that knowledge. > I sincerely > suggest that we forget about conventional hash table type indexes. I > fear they're a lost cause. > I understand that those are the only ones worth fighting for. :) Cheers, Jeff
Re: [HACKERS] GSoC on WAL-logging hash indexes
On Wed, Apr 30, 2014 at 11:01 AM, Tom Lane wrote: > It might lead to good things later; and even if > it doesn't, the lack of WAL support is an embarrassment. I don't think it will, but I do agree that the current state of affairs is an embarrassment. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GSoC on WAL-logging hash indexes
On Wed, Apr 30, 2014 at 10:11 AM, Robert Haas wrote: > I thought the theoretical advantage of hash indexes wasn't that they > were smaller but that you avoided a central contention point (the > btree root). The B-Tree root isn't really a central contention point at all. The locking/latching protocol that nbtree uses is remarkably concurrency-friendly. In the real world, there is pretty much no exclusive locking of the root page's buffer. > Of course our current hash indexes have *more* not less contention > than btree but I'm pretty comfortable chalking that up to quality of > implementation rather than anything intrinsic. I am not convinced of that. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GSoC on WAL-logging hash indexes
Robert Haas writes: > I thought the theoretical advantage of hash indexes wasn't that they > were smaller but that you avoided a central contention point (the > btree root). > Of course our current hash indexes have *more* not less contention > than btree but I'm pretty comfortable chalking that up to quality of > implementation rather than anything intrinsic. The long and the short of it is that there are *lots* of implementation deficiences in our hash indexes. There's no real way to know whether they'd be competitive if all those things were rectified, except by doing the work to fix 'em. And it's hard to justify putting much effort into hash indexes so long as there's an elephant in the room of the size of "no WAL support". So I'm in favor of getting that fixed, if we have somebody who's willing to do it. It might lead to good things later; and even if it doesn't, the lack of WAL support is an embarrassment. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_get_viewdefs() indentation considered harmful
I wrote: > I'm still dubious about halving the step distance, because there are > assorted places that adjust the indentation of specific keywords by > distances that aren't a multiple of 2 (look for odd last arguments to > appendContextKeyword). I'm not sure how that will look after we make such > a change. Possibly it would work to only apply the scale factor to > context->indentLevel, and not the indentPlus number? I pushed a patch that does it that way, and also patches for the other items discussed in this thread. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GSoC on WAL-logging hash indexes
On Wed, Apr 30, 2014 at 12:54 PM, Peter Geoghegan wrote: > On Wed, Apr 30, 2014 at 5:55 AM, k...@rice.edu wrote: >> I do not think that CPU costs matter as much as the O(1) probe to >> get a result value specifically for very large indexes/tables where >> even caching the upper levels of a B-tree index would kill your >> working set in memory. I know, I know, everyone has so much memory >> and can just buy more... but this does matter. > > Have you actually investigated how little memory it takes to store the > inner pages? It's typically less than 1% of the entire index. AFAIK, > hash indexes are not used much in any other system. I think MySQL has > them, and SQL Server 2014 has special in-memory hash table indexes for > in memory tables, but that's all I can find on Google. I thought the theoretical advantage of hash indexes wasn't that they were smaller but that you avoided a central contention point (the btree root). Of course our current hash indexes have *more* not less contention than btree but I'm pretty comfortable chalking that up to quality of implementation rather than anything intrinsic. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GSoC on WAL-logging hash indexes
On Wed, Apr 30, 2014 at 5:55 AM, k...@rice.edu wrote: > I do not think that CPU costs matter as much as the O(1) probe to > get a result value specifically for very large indexes/tables where > even caching the upper levels of a B-tree index would kill your > working set in memory. I know, I know, everyone has so much memory > and can just buy more... but this does matter. Have you actually investigated how little memory it takes to store the inner pages? It's typically less than 1% of the entire index. AFAIK, hash indexes are not used much in any other system. I think MySQL has them, and SQL Server 2014 has special in-memory hash table indexes for in memory tables, but that's all I can find on Google. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Sequential disk access during VACUUM for GiST/GIN
Hello.There is a task "Sequential disk access during VACUUM for GiST/GIN " in list GSOC14.Nobody is working on this task?Do I understand this task correctly?I must recode gistbulkdelete.GistBDItem *stack is must have items with sequential blkno as possible.I have a question:where are access to disk in this function? ReadBufferExtended?Thanks
Re: [HACKERS] tests for client programs
On 2014-04-30 18:09:15 +0200, Andres Freund wrote: > The issues here don't seem to have been addressed in the commit. At > least the latin1 thing should be fixed. As an additional issue it currently doesn't seem to work in VPATH builds. That's imo a must fix. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tests for client programs
On 2014-04-04 16:44:46 +0200, Andres Freund wrote:
> On 2014-02-27 21:44:48 -0500, Peter Eisentraut wrote:
> > +open HBA, ">>$tempdir/pgdata/pg_hba.conf";
> > +print HBA "local replication all trust\n";
> > +print HBA "host replication all 127.0.0.1/32 trust\n";
> > +print HBA "host replication all ::1/128 trust\n";
> > +close HBA;
>
> Given the recent make check security discussions, this doesn't seem like
> a good idea...
>
> > +issues_sql_like(['createdb', 'foobar1'], qr/statement: CREATE DATABASE
> > foobar1/, 'SQL CREATE DATABASE run');
> > +issues_sql_like(['createdb', 'foobar2', '-l', 'C', '-E', 'LATIN1', '-T',
> > 'template0'], qr/statement: CREATE DATABASE foobar2 ENCODING 'LATIN1'/,
> > 'create database with encoding');
>
> Hm. Are all platforms guaranteed to provide latin1?
>
> > diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
> > +if (!$ENV{PGPORT}) {
> > + $ENV{PGPORT} = 65432;
> > +}
> > +
> > +$ENV{PGPORT} = int($ENV{PGPORT}) % 65536;
>
> Hm. I think this should use logical similar to what pg_regress is using,
> namely test a few ports.
>
> > +sub start_test_server {
> > + my ($tempdir) = @_;
> > + my $ret;
> > +
> > + system "initdb -D $tempdir/pgdata -A trust -N >/dev/null";
> > + $ret = system 'pg_ctl', '-D', "$tempdir/pgdata", '-s', '-w', '-l',
> > "$tempdir/logfile", '-o', "--fsync=off -k $tempdir --listen-addresses=''
> > --log-statement=all", 'start';
> > +
> > + if ($ret != 0) {
> > + system('cat', "$tempdir/logfile");
> > + BAIL_OUT("pg_ctl failed");
> > + }
> > +
> > + $ENV{PGHOST} = $tempdir;
> > + $test_server_datadir = "$tempdir/pgdata";
> > + $test_server_logfile = "$tempdir/logfile";
> > +}
>
> Should stuff like --fsync-off, -k, really be on by default?
>
> I think the code to massage pg_hba.conf should also be here, there'll be
> a fair number of tests that need it.
The issues here don't seem to have been addressed in the commit. At
least the latin1 thing should be fixed.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix initdb for path with whitespace and at char
On 04/30/2014 06:31 AM, Heikki Linnakangas wrote: Andrew: you have a cygwin installation, don't you? Could you test if "pg_ctl start" works when the binaries are installed to a path that contains both a space and an @ sign, like "C:\white space\at@sign\install". I suspect it doesn't, but the attached patch fixes it. I'll see what I can do. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] folder:lk/lk date:yesterday..
On 2014-04-30 17:39:27 +0200, Andres Freund wrote:
> Hi,
>
> Coverity flagged a couple of issues that seem to worth addressing by
> changing the code instead of ignoring them:
>
> 01) heap_xlog_update() looks to coverity as if it could trigger a NULL
> pointer dereference. That's because it thinks that oldtup.t_data is
> NULL if XLR_BKP_BLOCK(0) while reconstructing incremental
> tuples. That fortunately can't happen as incremental updates are
> only performed if the tuples are on the same page.
> Add an Assert().
> 02) Be a bit more consistent in what type to use for a size
> variable. Inconsequential, but more consistent.
> 03) Don't leak memory after connection aborts in pg_recvlogical.
> 04) Use a sensible parameter for memset() when randomizing memory in
> reorderbuffer. Inconsequential.
>
> Could somebody please pick these up?
That might be easier with actual patches...
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
>From 67a566a57b66e1e47430f9b74fc82228e1c9130f Mon Sep 17 00:00:00 2001
From: Andres Freund
Date: Wed, 30 Apr 2014 17:18:55 +0200
Subject: [PATCH 1/4] Assert that pre/post-fix updated tuples are on the same
page during replay.
If they were not 'oldtup.t_data' would be dereferenced while set to
NULL in case of a full page image for block 0.
Do so primarily to silenence coverity; but also to make sure this
prerequisite isn't changed without adapting the replay routine as that
would appear to work in many cases.
---
src/backend/access/heap/heapam.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index a047632..336fbb0 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8115,11 +8115,13 @@ newsame:;
if (xlrec->flags & XLOG_HEAP_PREFIX_FROM_OLD)
{
+ Assert(samepage);
memcpy(&prefixlen, recdata, sizeof(uint16));
recdata += sizeof(uint16);
}
if (xlrec->flags & XLOG_HEAP_SUFFIX_FROM_OLD)
{
+ Assert(samepage);
memcpy(&suffixlen, recdata, sizeof(uint16));
recdata += sizeof(uint16);
}
--
1.8.3.251.g1462b67
>From 43c3c46cf5b35296df2f1121940997cdd419eb4e Mon Sep 17 00:00:00 2001
From: Andres Freund
Date: Wed, 30 Apr 2014 17:27:06 +0200
Subject: [PATCH 2/4] Use Size instead of uint32 to measure ... size in
snapbuild.c.
Silences coverity and is more consistent with other functions in the
same file.
---
src/backend/replication/logical/snapbuild.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index c462e90..36034db 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -1431,7 +1431,7 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
char path[MAXPGPATH];
int ret;
struct stat stat_buf;
- uint32 sz;
+ Size sz;
Assert(lsn != InvalidXLogRecPtr);
Assert(builder->last_serialized_snapshot == InvalidXLogRecPtr ||
--
1.8.3.251.g1462b67
>From c41faf9047e0d133b0cc233e6d7ca0be4e54e589 Mon Sep 17 00:00:00 2001
From: Andres Freund
Date: Wed, 30 Apr 2014 17:29:04 +0200
Subject: [PATCH 3/4] Don't leak memory after connection aborts in
pg_recvlogical.
Noticed by coverity.
---
src/bin/pg_basebackup/pg_recvlogical.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 8141ba3..fe902cf 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -547,9 +547,6 @@ StreamLog(void)
}
PQclear(res);
- if (copybuf != NULL)
- PQfreemem(copybuf);
-
if (outfd != -1 && strcmp(outfile, "-") != 0)
{
int64 t = feGetCurrentTimestamp();
@@ -563,6 +560,11 @@ StreamLog(void)
}
outfd = -1;
error:
+ if (copybuf != NULL)
+ {
+ PQfreemem(copybuf);
+ copybuf = NULL;
+ }
destroyPQExpBuffer(query);
PQfinish(conn);
conn = NULL;
--
1.8.3.251.g1462b67
>From 84c8333f02abd3f973b3009d02dcbecc880ce909 Mon Sep 17 00:00:00 2001
From: Andres Freund
Date: Wed, 30 Apr 2014 17:35:15 +0200
Subject: [PATCH 4/4] Pass sensible value to memset() when randomizing
reorderbuffer's tuple slab.
This is entirely harmless, but still wrong. Noticed by coverity.
---
src/backend/replication/logical/reorderbuffer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 4493930..0b21be9 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -463,7 +463,7 @@ ReorderBufferGetTupleBuf(ReorderBuffer *rb)
tuple = slist_container(ReorderBufferTupleBuf, node,
slist_pop_head_node(&rb->cached_tuplebufs));
#ifdef USE_ASSERT_CHECKING
- memset(
[HACKERS] folder:lk/lk date:yesterday..
Hi, Coverity flagged a couple of issues that seem to worth addressing by changing the code instead of ignoring them: 01) heap_xlog_update() looks to coverity as if it could trigger a NULL pointer dereference. That's because it thinks that oldtup.t_data is NULL if XLR_BKP_BLOCK(0) while reconstructing incremental tuples. That fortunately can't happen as incremental updates are only performed if the tuples are on the same page. Add an Assert(). 02) Be a bit more consistent in what type to use for a size variable. Inconsequential, but more consistent. 03) Don't leak memory after connection aborts in pg_recvlogical. 04) Use a sensible parameter for memset() when randomizing memory in reorderbuffer. Inconsequential. Could somebody please pick these up? I have to say, I am not particularly happy with the complexity of the control flow in heap_xlog_update() :(. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] hooks not working in postgresql 9.3 (building from source)
hi, I need to use the hook feature of planner in postgresql 9.3 to perform a task. I am building postgresql from source. To start with, I wanted to know how to create a hook and attach that shared libray to postgresql. Hence i tried the examples given in " wiki.postgresql.org/images/e/e3/Hooks_in_postgresql.pdf" and " https://github.com/gleu/Hooks-in-PostgreSQL/tree/master/examples/my_client_auth ". I have copied the "my_client_auth.c" file and Makefile into contrib/client_auth folder. make and make install is working fine. This is the output of make install. /bin/mkdir -p '/home/rajmohan/projects/lib/postgresql' /usr/bin/install -c -m 755 my_client_auth.so '/home/rajmohan/projects/lib/postgresql/' after that i have added shared_preload_libraries = 'my_client_auth' to postgresql.conf Then i added the line ClientAuthentication_hook_type client_auth_hook = NULL; at the top of a file say planner.c in postgresql code and inside a method im checking client_auth_hook value. When i rebuild and run the project, client_auth_hook value is always zero. It seems my_client_auth.so file is not linked properly to my postgresql project. Am i missing any step? how to access methods in my_client_auth.so from postgresql. Kindly help
[HACKERS] Display of timestamp in pg_dump custom format
The table of contents for pg_restore -l shows the time the archive was made as local time (it uses ctime()): ; Archive created at Wed Apr 30 10:03:28 2014 Is this clear enough that it is local time? Should we display this better, perhaps with a time zone designation? -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] "Considerer Harmful Considered Harmful" categorized as Mostly Harmless
On 04/30/2014 02:53 PM, Andrew Dunstan wrote: > > On 04/30/2014 02:35 AM, Heikki Linnakangas wrote: >> On 04/30/2014 01:27 AM, Stephen Frost wrote: >>> * Tom Lane (t...@sss.pgh.pa.us) wrote: Josh Berkus writes: > ... so let's stop using that phrase, OK? > http://meyerweb.com/eric/comment/chech.html Shrug ... what I see there is a rant from a guy with no sense of humor. >>> >>> +1 >>> >>> 'pt', I say. >> >> I wasn't sure if the whole article was a parody. >> >> > > > The rebuttal would be: "'Considered Harmful' Considered Harmful" > Considered Harmful. > > Don't you just love recursion? Nah, I'd categorize it as "Mostly Harmless" :) Cheers -- Hannu Krosing PostgreSQL Consultant Performance, Scalability and High Availability 2ndQuadrant Nordic OÜ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GSoC on WAL-logging hash indexes
On Wed, Apr 30, 2014 at 12:26:20AM -0700, Peter Geoghegan wrote: > On Mon, Mar 3, 2014 at 8:12 AM, Robert Haas wrote: > >> As a GSoC student, I will implement WAL recovery of hash indexes using the > >> other index types' WAL code as a guide. > > Frankly, I'm skeptical of the idea that hash indexes will ever really > be useful. I realize that that's a counter-intuitive conclusion, but > there are many things we could do to improve B-Tree CPU costs to make > them closer to those of hash indexes, without making them any less > flexible. I myself would much rather work on that, and intend to. > > The O(1) cost seems attractive when you consider that that only > requires that we read one index page from disk to service any given > index scan, but in fact B-Trees almost always only require the same. > They are of course also much more flexible. The concurrency > characteristics B-Trees are a lot better understood. I sincerely > suggest that we forget about conventional hash table type indexes. I > fear they're a lost cause. > > -- > Peter Geoghegan > Hi Peter, I do not think that CPU costs matter as much as the O(1) probe to get a result value specifically for very large indexes/tables where even caching the upper levels of a B-tree index would kill your working set in memory. I know, I know, everyone has so much memory and can just buy more... but this does matter. I also think that development of hash indexes has been stalled waiting for WAL logging. For example, hash indexes can almost trivially become more space efficient as they grow in size by utilizing the page number to represent the prefix bits of the hash value for a bucket. My 2 cents. Ken -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Considerer Harmful Considered Harmful
On 04/30/2014 02:35 AM, Heikki Linnakangas wrote: On 04/30/2014 01:27 AM, Stephen Frost wrote: * Tom Lane (t...@sss.pgh.pa.us) wrote: Josh Berkus writes: ... so let's stop using that phrase, OK? http://meyerweb.com/eric/comment/chech.html Shrug ... what I see there is a rant from a guy with no sense of humor. +1 'pt', I say. I wasn't sure if the whole article was a parody. The rebuttal would be: "'Considered Harmful' Considered Harmful" Considered Harmful. Don't you just love recursion? cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] includedir_internal headers are not self-contained
On 04/29/2014 06:00 PM, Tom Lane wrote: Andrew Dunstan writes: On 04/29/2014 02:56 AM, Heikki Linnakangas wrote: On 04/28/2014 10:32 PM, Tom Lane wrote: Meh. I still think it's a bad idea to have CATALOG_VERSION_NO getting compiled into libpgcommon.a, where there will be no way to cross-check that it matches anything. But I guess I'm losing this argument. FWIW, I agree it's a bad idea. I just have no better ideas (and haven't given it much thought anyway). Sure sounds like a bad idea. One idea would be to have relpath.h/.c expose a function (not a #define) that returns the value of CATALOG_VERSION_NO compiled into it. Then, if pg_rewind were to replace all its direct references to CATALOG_VERSION_NO (including the value it checks against pg_control) with calls of that function, consistency would be ensured. In pg_rewind, I'd like to compile CATALOG_VERSION_NO into the binary itself, because pg_rewind is quite version-specific. Even if it happens to work with libpgport from a different version, I would worry that there are directory layout changes that would need to be handled in pg_rewind for it to work safely. So I would like to lock it to a specific catalog version. To lock it down, I could call the function and check that it matches the compiled-in value of CATALOG_VERSION_NO, though. So a function works for me, even though I don't really need the flexibility. A notational problem is that if pg_rewind or similar program is directly using the TABLESPACE_VERSION_DIRECTORY macro anywhere, this wouldn't work. But we could perhaps expose a function to return that string too. pg_rewind doesn't use TABLESPACE_VERSION_DIRECTORY directly. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix initdb for path with whitespace and at char
I committed the non-invasive fixes to backbranches (and master too, just
to keep it in sync), but the attached is what I came up with for master.
There are a couple of places in the code where we have #ifdef WIN32 code
that uses CreateProcess with "CMD /C ..." directly. I believe those are
currently (ie. before this patch) wrong for cygwin builds. SYSTEMQUOTE
is defined as:
#if defined(WIN32) && !defined(__CYGWIN__)
#define SYSTEMQUOTE "\""
#else
#define SYSTEMQUOTE ""
#endif
I presume the !CYGWIN part is because cygwin version of system() and
popen() don't require the extra quoting, because cygwin does that for
us. But when we use CreateProcess directly, e.g like this in pg_ctl.c:
snprintf(cmd, MAXPGPATH, "CMD /C " SYSTEMQUOTE "\"\"%s\" %s%s <
\"%s\" 2>&1\"" SYSTEMQUOTE,
exec_path, pgdata_opt, post_opts, DEVNULL);
if (!CreateRestrictedProcess(cmd, &pi, false))
return GetLastError();
we would need the extra quotes, but SYSTEMQUOTE doesn't provide them
with cygwin.
Andrew: you have a cygwin installation, don't you? Could you test if
"pg_ctl start" works when the binaries are installed to a path that
contains both a space and an @ sign, like "C:\white
space\at@sign\install". I suspect it doesn't, but the attached patch
fixes it.
- Heikki
>From b00134385de28361194e7ba0a050343bb581e058 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas
Date: Wed, 30 Apr 2014 10:23:14 +0300
Subject: [PATCH] Replace SYSTEMQUOTEs with Windows-specific wrapper functions.
It's easy to forget using SYSTEMQUOTEs whe constructing command strings
for system() or popen(). We are currently missing it e.g. in COPY TO/FROM
PROGRAM calls. Even if we fix all the places missing it now, it is bound
to be forgotten again in the future. To eliminate the need for that, add
wrapper functions that do the the extra quoting for you, and get rid of
SYSTEMQUOTEs in all the callers.
diff --git a/configure.in b/configure.in
index fc9c52f..52357a6 100644
--- a/configure.in
+++ b/configure.in
@@ -1353,6 +1353,7 @@ if test "$PORTNAME" = "win32"; then
AC_REPLACE_FUNCS(gettimeofday)
AC_LIBOBJ(kill)
AC_LIBOBJ(open)
+ AC_LIBOBJ(system)
AC_LIBOBJ(win32env)
AC_LIBOBJ(win32error)
AC_LIBOBJ(win32setlocale)
diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c
index 56e912d..d22b6d3 100644
--- a/contrib/pg_upgrade/check.c
+++ b/contrib/pg_upgrade/check.c
@@ -970,7 +970,7 @@ get_bin_version(ClusterInfo *cluster)
int pre_dot,
post_dot;
- snprintf(cmd, sizeof(cmd), SYSTEMQUOTE "\"%s/pg_ctl\" --version" SYSTEMQUOTE, cluster->bindir);
+ snprintf(cmd, sizeof(cmd), "\"%s/pg_ctl\" --version", cluster->bindir);
if ((output = popen(cmd, "r")) == NULL ||
fgets(cmd_output, sizeof(cmd_output), output) == NULL)
diff --git a/contrib/pg_upgrade/controldata.c b/contrib/pg_upgrade/controldata.c
index fa0a005..476c6be 100644
--- a/contrib/pg_upgrade/controldata.c
+++ b/contrib/pg_upgrade/controldata.c
@@ -110,7 +110,7 @@ get_control_data(ClusterInfo *cluster, bool live_check)
pg_putenv("LC_ALL", NULL);
pg_putenv("LC_MESSAGES", "C");
- snprintf(cmd, sizeof(cmd), SYSTEMQUOTE "\"%s/%s \"%s\"" SYSTEMQUOTE,
+ snprintf(cmd, sizeof(cmd), "\"%s/%s \"%s\"",
cluster->bindir,
live_check ? "pg_controldata\"" : "pg_resetxlog\" -n",
cluster->pgdata);
diff --git a/contrib/pg_upgrade/exec.c b/contrib/pg_upgrade/exec.c
index 7f01301..91e66e6 100644
--- a/contrib/pg_upgrade/exec.c
+++ b/contrib/pg_upgrade/exec.c
@@ -59,14 +59,14 @@ static DWORD mainThreadId = 0;
mainThreadId = GetCurrentThreadId();
#endif
- written = strlcpy(cmd, SYSTEMQUOTE, sizeof(cmd));
+ written = 0;
va_start(ap, fmt);
written += vsnprintf(cmd + written, MAXCMDLEN - written, fmt, ap);
va_end(ap);
if (written >= MAXCMDLEN)
pg_fatal("command too long\n");
written += snprintf(cmd + written, MAXCMDLEN - written,
- " >> \"%s\" 2>&1" SYSTEMQUOTE, log_file);
+ " >> \"%s\" 2>&1", log_file);
if (written >= MAXCMDLEN)
pg_fatal("command too long\n");
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index b53fa8b..83b7f6e 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -1130,11 +1130,11 @@ test_config_settings(void)
test_buffs = MIN_BUFS_FOR_CONNS(test_conns);
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "\"%s\" --boot -x0 %s "
+ "\"%s\" --boot -x0 %s "
"-c max_connections=%d "
"-c shared_buffers=%d "
"-c dynamic_shared_memory_type=none "
- "< \"%s\" > \"%s\" 2>&1" SYSTEMQUOTE,
+ "< \"%s\" > \"%s\" 2>&1",
backend_exec, boot_options,
test_conns, test_buffs,
DEVNULL, DEVNULL);
@@ -1165,11 +1165,11 @@ test_config_settings(void)
}
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "\"%s\" --boot -x0 %s "
+ "\"%s\" --boot -x0 %s "
"-c max_connections=%d "
"-c shared_buffers=%d "
"-c dynamic_shared_memory_type=none "
- "< \"%s\" > \"%s\" 2>&1" SYSTEMQUOTE,
+ "< \"%s\"
Re: [HACKERS] Minor improvement to fdwhandler.sgml
(2014/04/28 23:31), Robert Haas wrote: On Thu, Apr 24, 2014 at 7:59 AM, Etsuro Fujita wrote: The patch attached improves docs in fdwhandler.sgml a little bit. When you submit a patch, it's helpful to describe what the patch actually does, rather than just saying it makes things better. For example, I think that this patch could be described as "in fdwhandler.sgml, mark references to scan_clauses with tags". I thought so. Sorry, my explanation wasn't enough. A problem with that idea is that scan_clauses is not a field in any struct. I was mistaken. I think those should be marked with tags. Patch attached. Thanks, Best regards, Etsuro Fujita diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml index 9c818cd..6b5c8b7 100644 --- a/doc/src/sgml/fdwhandler.sgml +++ b/doc/src/sgml/fdwhandler.sgml @@ -873,11 +873,11 @@ GetForeignServerByName(const char *name, bool missing_ok); In GetForeignPlan, generally the passed-in target list can - be copied into the plan node as-is. The passed scan_clauses list + be copied into the plan node as-is. The passed scan_clauses list contains the same clauses as baserel->baserestrictinfo, but may be re-ordered for better execution efficiency. In simple cases the FDW can just strip RestrictInfo nodes from the - scan_clauses list (using extract_actual_clauses) and put + scan_clauses list (using extract_actual_clauses) and put all the clauses into the plan node's qual list, which means that all the clauses will be checked by the executor at run time. More complex FDWs may be able to check some of the clauses internally, in which case those @@ -895,7 +895,7 @@ GetForeignServerByName(const char *name, bool missing_ok); affect the cost estimate for the path. The path's fdw_private field would probably include a pointer to the identified clause's RestrictInfo node. Then - GetForeignPlan would remove that clause from scan_clauses, + GetForeignPlan would remove that clause from scan_clauses, but add the sub_expression to fdw_exprs to ensure that it gets massaged into executable form. It would probably also put control information into the plan node's -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix initdb for path with whitespace and at char
On 04/30/2014 07:39 AM, Amit Kapila wrote: On Wed, Apr 30, 2014 at 3:57 AM, Tom Lane wrote: Heikki Linnakangas writes: This looks correct to me. popen() requires SYSTEMQUOTEs on Windows, like system() does. It seems right now SYSTEMQUOTE is used before popen both for Windows and non-Windows, ex. adjust_data_dir() in pg_ctl.c Yeah, but it's defined to an empty string on non-Windows platforms. We might forget to use the wrapper function too, if it has a nonstandard name, no? A better idea would be to redefine popen() and system() on Windows. It looks like we're already using a #define to redefine popen(). Right, that's exactly what I meant by a wrapper function. Won't defining variant of popen just for Windows to add SYSTEMQUOTE effect such (where currently it is used for both win and non-winows) usage? Also, I think we might want to remove use of SYSTEMQUOTE before popen calls where ever it is currently used to avoid usage of the same two times. Yep. I'll write up a patch to do that for git master. For back-branches, I just added the missing SYSTEMQUOTEs. There are a couple of places where changing the behavior might break existing installations. In particular, in the stable branches it's probably best to not add the SYSTEMQUOTEs to things like archive_command, restore_command, and COPY TO/FROM PROGRAM, where the command is specified by the user. Because people might already have added the extra quotes to the command to make it work, and if we suddenly start adding yet another pair of quotes, it will stop working. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GSoC on WAL-logging hash indexes
On Mon, Mar 3, 2014 at 8:12 AM, Robert Haas wrote: >> As a GSoC student, I will implement WAL recovery of hash indexes using the >> other index types' WAL code as a guide. Frankly, I'm skeptical of the idea that hash indexes will ever really be useful. I realize that that's a counter-intuitive conclusion, but there are many things we could do to improve B-Tree CPU costs to make them closer to those of hash indexes, without making them any less flexible. I myself would much rather work on that, and intend to. The O(1) cost seems attractive when you consider that that only requires that we read one index page from disk to service any given index scan, but in fact B-Trees almost always only require the same. They are of course also much more flexible. The concurrency characteristics B-Trees are a lot better understood. I sincerely suggest that we forget about conventional hash table type indexes. I fear they're a lost cause. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
