Re: [HACKERS] proposal - assign result of query to psql variable
Hello fast reply 2012/10/14 Tom Lane : > Pavel Stehule writes: >> [ gset_08.diff ] > > In the course of adding a new backslash command, this patch manages to > touch: > > * the main loop's concept of what types of backslash commands exist > (PSQL_CMD_NOSEND ... what's the point of that, rather than making > this work the same as \g?) > * SendQuery's concept of how to process command results (again, why > isn't this more like \g?) If I remember, I had to do because I had a problem with shell, but I have to diagnose it again > * ExecQueryUsingCursor's concept of how to process command results > (why? surely we don't need \gset to use a cursor) There was two possibilities, but hardly non using cursor is better way > * the psql lexer (adding a whole bunch of stuff that probably doesn't > belong there) ?? > * the core psql settings construct (to store something that is in > no way a persistent setting) > ?? > Surely there is a less ugly and invasive way to do this. The fact > that the reviewer keeps finding bizarre bugs like "another backslash > command on the same line doesn't work" seems to me to be a good > indication that this is touching things it shouldn't. - all these bugs are based on lexer construct. A little modification of lexer is possible Regards Pavel > > regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] string escaping in tutorial/syscat.source
Hi all, It seems the queries in ./src/tutorial/syscat.source use string escaping with the assumption that standard_conforming_strings is off, and thus give wrong results with modern versions. A simple fix is attached. Josh syscat.source_escaping.diff Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Making the planner more tolerant of implicit/explicit casts
Robert Haas writes: > On Thu, Oct 11, 2012 at 5:59 PM, Tom Lane wrote: >> I'm reasonably convinced that this is a good fix for HEAD, but am of two >> minds whether to back-patch it or not. The problem complained of in >> bug #7598 may seem a bit narrow, but the real point is that whether you >> write a cast explicitly or not shouldn't affect planning if the >> semantics are the same. This might well be a significant though >> previously unrecognized performance issue, particularly for people who >> use varchar columns heavily. > I have had a few bad experiences with people getting *really* upset > about plan changes in minor releases, so I would be disinclined to > back-patch this, even if we're fairly sure that it will be an > improvement in most/all cases. It's just not worth the risk of > discovering otherwise. I stuck it into 9.2, but not further back. 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] [RFC][PATCH] wal decoding, attempt #2 - Design Documents (really attached)
On Thu, Oct 11, 2012 at 3:15 AM, Heikki Linnakangas wrote: > IMHO that's a good thing, and I'd hope this new logical replication to live > outside core as well, as much as possible. But whether or not something is > in core is just a political decision, not a reason to implement something > new. > > If the only meaningful advantage is reducing the amount of WAL written, I > can't help thinking that we should just try to address that in the existing > solutions, even if it seems "easy to solve at a first glance, but a solution > not using a normal transactional table for its log/queue has to solve a lot > of problems", as the document says. Sorry to be a naysayer, but I'm pretty > scared of all the new code and complexity these patches bring into core. I think what we're really missing at the moment is a decent way of decoding WAL. There are a decent number of customers who, when presented with replication system, start by asking whether it's trigger-based or WAL-based. When you answer that it's trigger-based, their interest goes... way down. If you tell them the triggers are written in anything but C, you lose a bunch more points. Sure, some people's concerns are overblown, but it's hard to escape the conclusion that a WAL-based solution can be a lot more efficient than a trigger-based solution, and EnterpriseDB has gotten comments from a number of people who upgraded to 9.0 or 9.1 to the effect that SR was way faster than Slony. I do not personally believe that a WAL decoding solution adequate to drive logical replication can live outside of core, at least not unless core exposes a whole lot more interface than we do now, and probably not even then. Even if it could, I don't see the case for making every replication solution reinvent that wheel. It's a big wheel to be reinventing, and everyone needs pretty much the same thing. That having been said, I have to agree that the people working on this project seem to be wearing rose-colored glasses when it comes to the difficulty of implementing a full-fledged solution in core. I'm right on board with everything up to the point where we start kicking out a stream of decoded changes to the user... and that's about it. To pick on Slony for the moment, as the project that has been around for the longest and has probably the largest user base (outside of built-in SR, perhaps), they've got a project that they have been developing for years and years and years. What have they been doing all that time? Maybe they are just stupid, but Chris and Jan and Steve don't strike me that way, so I think the real answer is that they are solving problems that we haven't even started to think about yet, especially around control logic: how do you turn it on? how do you turn it off? how do you handle node failures? how do you handle it when a node gets behind? We are not going to invent good solutions to all of those problems between now and January, or even between now and next January. > PS. I'd love to see a basic Slony plugin for this, for example, to see how > much extra code on top of the posted patches you need to write in a plugin > like that to make it functional. I'm worried that it's a lot.. I agree. I would go so far as to say that if Slony can't integrate with this work and use it in place of their existing change-capture facility, that's sufficient grounds for unconditional rejection. -- 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] Making the planner more tolerant of implicit/explicit casts
On Thu, Oct 11, 2012 at 5:59 PM, Tom Lane wrote: > I'm reasonably convinced that this is a good fix for HEAD, but am of two > minds whether to back-patch it or not. The problem complained of in > bug #7598 may seem a bit narrow, but the real point is that whether you > write a cast explicitly or not shouldn't affect planning if the > semantics are the same. This might well be a significant though > previously unrecognized performance issue, particularly for people who > use varchar columns heavily. I have had a few bad experiences with people getting *really* upset about plan changes in minor releases, so I would be disinclined to back-patch this, even if we're fairly sure that it will be an improvement in most/all cases. It's just not worth the risk of discovering otherwise. -- 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
[HACKERS] smgrsettransient mechanism is full of bugs
I got a bit suspicious of the transient-file mechanism introduced in commit fba105b1099f4f5fa7283bb17cba6fed2baa8d0c after noticing that CleanupTempFiles seemed to take an unreasonable amount of time in a test case that didn't involve any temp files, cf http://archives.postgresql.org/message-id/7110.1349392...@sss.pgh.pa.us After further review, I have become convinced that in fact it's completely broken and needs to be redone from scratch. The temp-file marking at the fd.c level can easily get out of sync with the marking at the smgr level, and that marking isn't too consistent with reality either, which means we have all of the following problems: (1) It can leak kernel descriptors, as reported in http://archives.postgresql.org/message-id/b9bea448-978f-4a14-a088-3fd82214f...@pvv.ntnu.no The triggering sequence for that appears to be: * Transaction 1 does a blind write, sets FD_XACT_TRANSIENT. * At transaction close, we close kernel FD and clear FD_XACT_TRANSIENT in the VFD, but the VFD, the smgr relation, and the md.c data structure are all still there. * Transaction 2 does another blind write on same file. This does not cause FD_XACT_TRANSIENT to get set because md.c data structure already exists. * Now we are carrying a "leaked" kernel FD that will never get closed short of a CacheInvalidateSmgr message. Which doesn't happen in a dropdb scenario. (That might be a bug in itself.) (2) FlushBuffer will set the smgr-level transient flag even if we have a relcache entry for the relation. (The fact that we're doing a blind write to flush a dirty buffer does not prove that the rel is one we're not interested in.) This can result in unnecessary forced closures of kernel FDs, and it also results in too many scans of the VFD array, because have_pending_fd_cleanup can get set unnecessarily. (3) If the smgr-level flag gets cleared intra-transaction (ie, we did a blind write and later started doing normal accesses to the same relation), this fails to propagate to the VFD level, again resulting in undesirable FD closures. (4) After a blind write, we will close the kernel FD at transaction end, but we don't flush the VFD array entry. This results in VFD array bloat over time. The combination of this and (2) seems to explain the performance problem I complained of above: there are too many VFD searches done by CleanupTempFiles, and they have to pass over too many useless entries. I believe that we probably ought to revert this mechanism entirely, and build a new implementation based on these concepts: * An SMgrRelation is transient if and only if it doesn't have an "owning" relcache entry. Keep a list of all such SmgrRelations, and close them all at transaction end. (Obviously, an SMgrRelation gets removed from the list if it acquires an owner mid-transaction.) * There's no such concept as FD_XACT_TRANSIENT at the fd.c level. Rather, we close and delete the VFD entry when told to by SmgrRelation closure. Comments? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecating RULES
Simon, * Simon Riggs (si...@2ndquadrant.com) wrote: > You also mention that 3 years wasn't long enough for some people, but > I am unsure as to your point there. It might be that we should take > longer than 3 years to deprecate things, or that the same pain will be > felt however long we leave it. I think the latter, since the standard > conforming strings change didn't go much better even though we took > ages to do it. RULEs, being what they are, deserve at least 3 years, imo. > In many people's opinion, RULEs are a strangeness that are seldom used > in production and long since should have been removed. Peter shows a > paper that details things wrong with them from 15 years ago. Indeed. Unfortunately, our documentation doesn't reflect that (yet). > Deprecating rules is a much, much smaller change than any of the > recent deprecations. Everything else we say needs to have that > context. It's smaller. I don't agree that it's "much, much smaller". > My answer to that > is that rules are pretty useless and any reasonable developer will > discover that before putting anything live. To be honest, I don't believe we would be having this discussion were your statement above accurate. RULEs are used quite a bit 'in the wild', as it were, particularly to address our lack of proper partitioning. > If they do put it live, > they might not have noticed rules are actually broken, so deprecating > rules in this way will actually avoid bugs and data corruption for > those people. Your proposal was to explicitly break RULEs for them on an upgrade, wasn't it..? Or did you propose something else? Regardless of *how* that breakage happens, I do not believe our users would appreciate RULEs breaking without sufficient notice for them to do something about it. I understand your suggestion that they could simply remove the breakage, but I do not agree that it is sufficient for them. Either they will do it as a matter of course during the upgrade and promptly forget about it, or they'll decide that they need to fix it in a very tight timeframe leading up to their upgrade (after they discover it in testing)- neither is good. > For me, most developers either 1) use ORMs, none of > which use RULEs, 2) speak to people in PostgreSQL community/consulting > companies - almost all of whom will advise to avoid RULEs or 3) have > read books that advise against their use or 4) read that rules are not > SQL standard and so wisely avoid unnecessarily non-standard coding. As > a result, I think the number of people likely to adopt rules in the > near future is approximately zero and the number affected by this > change will be very low and unlikely to cause embarrassment for us. I completely disagree with this while our documentation talks about it, describes it as a user feature, and even encourages use of RULEs for partitions. Indeed, changing the documentation would be the correct first step to deprecating RULEs. > IMHO the risk of losing people to other databases seems higher from > providing broken features than it does from removing broken features, > which seems like useful and proactive management. Since I believe > there is something to be lost from maintaining the status quo, and > little to be gained from delay, I proposed a way of speeding up the > process that allowed a back out plan. Personally, I don't believe your plan is sufficient with regard to giving users time to move off of RULEs. I don't disagree that we need to get rid of them as a user-visible/encouraged feature. > Daniel has made the point that we must enforce deprecation without any > option of reversion. Given neither of us likes to be hostile to users, > I presume we both disagree with him on that? One thing I would like to > avoid is providing another GUC for compatibility, since the > combinatorial explosion of GUC settings introduces considerable bug > risks. I agree that we can't simply disable them in the next release. My suggestion would be along the lines of: updating our documentation, issuing a warning when they're used in our next major release, make them only something a superuser can create, eventually make them unable for anyone to create. > Having said that, I've got no particular reason to hurry other than my > own personal embarrassment at explaining that, yes, some of our > features are broken. But I would like to see actions begin, however > long the timescale. Let's, please, start with a communication plan that is initiatied by updating our documentation and making an announcement to -announce regarding the planned deprecation of RULEs. > If we wish to make some noise, I think docs changes are not enough. > Darren's suggestion that doc additions that explain that advice has > been backpatched is useful, but we should also make Announcements of > impending deprecations as well if we truly wish to make noise. And if > we do that, as Daniel points out, sorting out hash indexes at the same > time also makes sense.
Re: [HACKERS] Deprecating RULES
> -Original Message- > From: pgsql-hackers-ow...@postgresql.org [mailto:pgsql-hackers- > ow...@postgresql.org] On Behalf Of Simon Riggs > Sent: Sunday, October 14, 2012 5:30 PM > To: Tom Lane > Cc: Greg Stark; Peter Geoghegan; PostgreSQL-development > Subject: Re: [HACKERS] Deprecating RULES > > On 14 October 2012 17:35, Tom Lane wrote: > > Simon Riggs writes: > >> On 12 October 2012 19:48, Greg Stark wrote: > >>> On Fri, Oct 12, 2012 at 7:55 AM, Simon Riggs > wrote: > AFAICS all RULEs can be re-expressed as Triggers or Views. > > > >>> This is a bizarre discussion. Firstly this isn't even close to true. > >>> The whole source of people's discontentment is that triggers are > >>> *not* equivalent to rules. If they were then they wouldn't be so upset. > > > >> I'm not aware of any rule that can't be rewritten as a trigger or a > >> view. Please can anyone show me some examples of those? > > > > Sorry, you're thinking you can put the burden of proof on other > > people, but this doesn't work like that. If you want to deprecate > > rules on the grounds that triggers are an adequate substitute, it's up > > to you to prove that claim, not for other people to disprove it. > > It's fair comment that if one thing can replace another, the burden of proof is > on the claimant. > > But I'm not just saying rules should be deprecated because they are > superfluous. Rules are broken in various ways that can cause data corruption, > security issues and user unhappiness. Those downsides outweigh the few > possible advantages. If we rely on people that dislike RULEs to provide examples of where RULEs necessary then through simple ignorance we are likely to deduce that RULEs are of no practical use. People who use rules because there are no viable alternatives need to elaborate on the how and why RULEs are used so that the ignorant can become educated and maybe even provide current or future alternatives to using RULEs. It does seem that considerable documentation work needs to be done regardless of whether a final decision to deprecate is made. Furthermore, the results of that documentation process will provide valuable insight toward that decision. This thread should probably end and a new one discussing the design and content of said documentation should probably be started. As I do not have a concrete idea of how that process would look with respect to this community I'll leave the initial posting to someone with more intimate knowledge of the community dynamics. David J. -- 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] Deprecating RULES
On Sun, Oct 14, 2012 at 9:30 AM, Simon Riggs wrote: > On 12 October 2012 19:48, Greg Stark wrote: >> On Fri, Oct 12, 2012 at 7:55 AM, Simon Riggs wrote: >>> AFAICS all RULEs can be re-expressed as Triggers or Views. >> >> This is a bizarre discussion. Firstly this isn't even close to true. >> The whole source of people's discontentment is that triggers are *not* >> equivalent to rules. If they were then they wouldn't be so upset. > > This may be a confusion on the point of equivalence; clearly the > features work differently. > > I'm not aware of any rule that can't be rewritten as a trigger or a > view. Please can anyone show me some examples of those? Huh? The one thing we currently use rules for, implementing views, couldn't be done in triggers. In general if your source table is empty then there's *nothing* you could cause to happen with triggers because no triggers will fire. The analogy to this discussion would be something like "users get confused by macros in C and usually what they're trying to do can be better done with functions. now that we have functions we should deprecate macros" All of the preconditions in that sentence are true but it doesn't follow because macros exist for a reason. In fact it's not a very good analogy because the situation is *precisely* the same -- rules *are* macros and manipulate the raw sql before it's run and the reason they can't be replaced by triggers is because, like functions, triggers happen after the code is compiled and run. -- 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] Potential autovacuum optimization: new tables
On Sat, Oct 13, 2012 at 3:13 AM, Stephen Frost wrote: > Josh's concern is about autovacuum causing lots of stats churn, which is > understandable, we don't want it constantly rescanning a table I don't think rescanning the table is a big concern. autovacuum will only scan as often as it feels like in the first place and these are by definition small tables anyways. Josh's stated concern was about the churn in the stats table. That could cause extra vacuums on the stats table which could be a fairly substantial table. Hopefully HOT updates and the visibility bitmap would protect against that being too bad though. -- 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] pg_dump restore error
Le 14/10/2012 18:50, Tom Lane a écrit : > Gilles Darold writes: >> Restoring a backup generated with pg_dump/pg_dumpall in plain text >> format and the --clean option will report errors if the backup is loaded >> in an other or empty database. > This is intentional. What you propose amounts to a fundamental change > in the semantics of --clean, and I don't think that it's really much > of an improvement. I'm agree that this is not an improvement, I used to work with it because I know it's harmless error messages. The request comes from a client that claims that replaying his backup on a new server was always producing those errors. I was just thinking that cleaning if exists was also harmless, but it is probably better to have error reported in most of the cases. Regards, -- Gilles Darold Administrateur de bases de données http://dalibo.com - http://dalibo.org -- 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] Visual Studio 2012 RC
Noah Misch wrote: I decided to try a 32-bit build, but Solution::DeterminePlatform detected it as x64. Its shibboleth is no longer valid; the cl.exe shipping with VS 2012 Express for Desktop has a /favor option for both architectures: 32clhelp:/favor: select processor to optimize for, one of: 64clhelp:/favor: select processor to optimize for, one of: Overlaying the first attached change fixed detection for this particular compiler, but I have not checked compatibility with older versions. Do you have VS 2008 and/or VS 2010 handy? Older compilers work fine but localized ones will probably cause trouble (for -> für in german). I've decided to change the regex to "/^\/favor:<.+AMD64/" in my current version of the patch as this is not very likely to appear in a 32-bit environment and will not be subject ot localization problems. Having worked around that, the build eventually failed like this: Creating library Debug\postgres\postgres.lib and object Debug\postgres\postgres.exp postgres.exp : error LNK2001: unresolved external symbol _xmm@41f0 [c:\cygwin\home\nm\src\pg\postgresql\postgres.vcxproj] postgres.exp : error LNK2001: unresolved external symbol _xmm@80008000 [c:\cygwin\home\nm\src\pg\postgresql\postgres.vcxproj] postgres.exp : error LNK2001: unresolved external symbol _xmm@8000800080008000 [c:\cygwin\home\nm\src\pg\postgresql\postgres.vcxproj] .\Debug\postgres\postgres.exe : fatal error LNK1120: 3 unresolved externals [c:\cygwin\home\nm\src\pg\postgresql\postgres.vcxproj] The command exited with code 1120. Done executing task "Link" -- FAILED. This compiler emits _xmm symbols automatically, where needed. The second attached change lets the build complete and pass tests, but I can't readily explain why it's necessary. In the 64-bit build, the _xmm symbols export normally (albeit, I presume, needlessly). I hoped to find some rationale for the preexisting gendef.pl exclusion of _real, which seems to resemble _xmm in origin and use. Magnus/anyone, can you shed light on our exclusion of "_real" symbols from .def files? I kind of feel like excluding the _xmm symbols is the right thing to do but - like you - I can't explain why they cause problems in a x86 build. Regards, Brar -- 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] Deprecating RULES
On 14 October 2012 17:35, Tom Lane wrote: > Simon Riggs writes: >> On 12 October 2012 19:48, Greg Stark wrote: >>> On Fri, Oct 12, 2012 at 7:55 AM, Simon Riggs wrote: AFAICS all RULEs can be re-expressed as Triggers or Views. > >>> This is a bizarre discussion. Firstly this isn't even close to true. >>> The whole source of people's discontentment is that triggers are *not* >>> equivalent to rules. If they were then they wouldn't be so upset. > >> I'm not aware of any rule that can't be rewritten as a trigger or a >> view. Please can anyone show me some examples of those? > > Sorry, you're thinking you can put the burden of proof on other people, > but this doesn't work like that. If you want to deprecate rules on the > grounds that triggers are an adequate substitute, it's up to you to > prove that claim, not for other people to disprove it. It's fair comment that if one thing can replace another, the burden of proof is on the claimant. But I'm not just saying rules should be deprecated because they are superfluous. Rules are broken in various ways that can cause data corruption, security issues and user unhappiness. Those downsides outweigh the few possible advantages. We have no theoretical model that describes how they should behave and it would seem nobody cares. The reality is that rules aren't going to be fixed and I think we should admit that and put rules to rest. Stonebraker has moved on, and so should we, from his bad ideas. > Personally, I don't think I believe it, on both practical and > theoretical grounds: > > * Triggers have been available for a very long time, and have been > documented as the better choice for quite a while, but people still try > to use rules. Whether it's for (admittedly mostly illusory) ease of use > or some other reason, there's some attraction there that we need to > match, not just decree that people can't have it anymore. I agree there is a huge initial attraction, just an equally huge disappointment later. I used to quite like them myself, once. > * Triggers necessarily operate on a row-at-a-time basis. In theory, > for at least some bulk operations, a rule could greatly outperform > a trigger. It's difficult to walk away from that - unless somebody > can prove that the advantage doesn't ever accrue in practice. But > the fact that a rule won't work in some cases doesn't mean it won't > work for any practical use-case. The absence of COPY support makes any use case of rules moot for bulk data cases, IMHO. It's possible rules are good for something and almost impossible for me to prove that's not true. > * AFTER triggers fall over on sufficiently large updates, since we still > lack any method for spilling the trigger event queue to disk when it > gets too big. It's fairly difficult to tell people they have to use > triggers as long as that limitation exists and rules don't have it. > (I wonder if this could be alleviated with some sort of "immediate > after" trigger mode that fires after the event is frozen, but without > queueing...) I think we need that mode for RI anyway, to allow us to optimize cases that repeatedly perform the same action, such as fk locks against a small reference table. Not sure if that is instead of or as well as scroll to disk. I don't think rules solve those problems well enough to be a genuine substitute. I think people just turn RI off and run checks offline. Why say this all now? No reason, apart from never seems to be the wrong answer. Apart from docs, I'll desist. -- Simon Riggs 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] Successor of MD5 authentication, let's use SCRAM
On Sun, Oct 14, 2012 at 2:04 AM, Magnus Hagander wrote: > There's a lot of shades of gray to that one. Way too many to say > they're right *or* wrong, IMHO. We can agree it is 'sub-ideal', but there is not one doubt in my mind that it is not 'right' given the scope of Debian's task, which does *not* include pushing applied cryptography beyond its current pitiful state. Debian not making self-signed certs available by default will just result in a huge amount of plaintext database authentication and traffic available over the internet, especially when you consider the sslmode=prefer default, and as a result eliminate protection from the most common class of attack for users with low-value (or just low-vigilance) use cases. In aggregate, that is important, because there are a lot of them. It would be a net disaster for security. > It *does* make people think they have "full ssl security by default", > which they *don't*.They do have partial protection, which helps in > some (fairly common) scenarios. But if you compare it to the > requirements that people *do* have when they use SSL, it usually > *doesn't* protect them the whole way - but they get the illusion that > it does. Sure, they'd have to read up on the details in order to get > secure whether it's on by default or not - that's why I think it's > hard to call it either right or wrong, but it's rather somewhere in > between. If there is such blame to go around, I place such blame squarely on clients. More secure is the JDBC library, which makes you opt into logging into a server that has no verified identity via configuration. The problem there is that it's a pain to get signed certs in, say, a test environment, so "don't check certs" will make its way into the default configuration, and now you have all pain and no gain. -- fdr -- 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] Visual Studio 2012 RC
Noah Misch wrote: My build log filled 8.8 MiB, a large increase from the 432 KiB of the "chough" build log. This isn't strictly a problem, but do you happen to have ideas for curbing the noise? Not yet. I find no functional problems with the patch, but some comment updates and other trivia need attention. The patch itself was reversed; it applied cleanly with "patch -R". I regenerated it in the usual direction for the portions I quote below. src/tools/msvc/MSBuildProject.pm:4:# Package that encapsulates a MSBuild (Visual C++ 2010) project file Say something like "Visual C++ 2010 or greater". src/tools/msvc/VSObjectFactory.pm:93:# we use nmake as it has existed for a long time and still exists in visual studio 2010 Likewise, modify this comnent to be open-ended. If nmake ever disappears, we'll be updating this code and can see to change the comment. fixed *** a/doc/src/sgml/install-windows.sgml --- b/doc/src/sgml/install-windows.sgml *** *** 22,28 Microsoft tools is to install a supported version of the Microsoft Windows SDK and use the included compiler. It is also possible to build with the full ! Microsoft Visual C++ 2005, 2008 or 2010. In some cases that requires the installation of the Windows SDK in addition to the compiler. --- 22,28 Microsoft tools is to install a supported version of the Microsoft Windows SDK and use the included compiler. It is also possible to build with the full ! Microsoft Visual C++ 2005, 2008, 2010 or 2012. In some cases that requires the installation of the Windows SDK in addition to the compiler. I think this paragraph should be more like the one in the next patch hunk: call out Visual Studio 2012 Express for Windows Desktop and Windows SDK 7.1 as the main recommendations. Perhaps even demote the SDK entirely and just recommend VS 2012. It'd odd to recommend only a past-version tool if a current-version tool works just as well. fixed. I would write "Windows SDK 7.1" here and remove the parenthesized bit. There's a later mention of support for older versions. ! (<= 7.1) or those from Visual Studio Express 2012 for Windows ! Desktop, which are both free downloads from Microsoft. fixed. The part ending here looks like this: Microsoft Windows SDK It is recommended that you upgrade to the latest supported version of the Microsoft Windows SDK (currently version 7.1), available for download from http://www.microsoft.com/downloads/";>. You must always include the Windows Headers and Libraries part of the SDK. If you install the Windows SDK including the Visual C++ Compilers, you don't need Visual Studio to build. Note that as of Version 8.0a the Windows SDK no longer ships with a complete command-line build environment. Since SDK version 7.1 is named as the "latest supported version", I understand from that text that installing SDK version 8.0a along with compilers from another source (VS 2012 full, VS 2012 Express for Desktop) is considered "unsupported" as a PostgreSQL build environment. Is that your intent? No, not really. What I want to say is that you'll need the SDK to build postgres. Using a Visual Studio version that ships with a supported SDK version (full versions of VS 2005 to 2010 as well as any version of VS 2012) will work. On the other hand standalone SDK versions that ship with compilers will also work. The major gotcha here is the fact that old sdk versions ship without compilers and old VS Express versions ship without SDK and you'll need both to build. I've tried to change the wording to make this more clear but perhaps someone else (native speaker) finds a better aproach to make this clear. *** a/src/tools/msvc/MSBuildProject.pm --- b/src/tools/msvc/MSBuildProject.pm *** *** 397,400 sub new --- 397,440 return $self; } + package VC2012Project; + + # + # Package that encapsulates a Visual C++ 2012 project file + # + + use strict; + use warnings; + use base qw(MSBuildProject); + + sub new + { + my $classname = shift; + my $self = $classname->SUPER::_new(@_); + bless($self, $classname); + + $self->{vcver} = '11.00'; + + return $self; + } + + sub WriteConfigurationPropertyGroup Please add a comment explaining what this override does differently. (I think it just adds the "" element.) done. Regards, Brar diff -Napcdr -x .git postgresql/doc/src/sgml/install-windows.sgml postgresql_dev/doc/src/sgml/install-windows.sgml *** postgresql/doc/src/sgml/install-windows.sgmlSun Oct 14 09:47:50 2012 --- postgresql_dev/doc/src/sgml/install-windows.sgmlSun Oct 14 17:52:14 2012 *** *** 19,26 There are several different ways of building PostgreSQL on Windows. The simplest way to build with ! Microsoft tool
Re: [HACKERS] proposal - assign result of query to psql variable
Pavel Stehule writes: > [ gset_08.diff ] In the course of adding a new backslash command, this patch manages to touch: * the main loop's concept of what types of backslash commands exist (PSQL_CMD_NOSEND ... what's the point of that, rather than making this work the same as \g?) * SendQuery's concept of how to process command results (again, why isn't this more like \g?) * ExecQueryUsingCursor's concept of how to process command results (why? surely we don't need \gset to use a cursor) * the psql lexer (adding a whole bunch of stuff that probably doesn't belong there) * the core psql settings construct (to store something that is in no way a persistent setting) Surely there is a less ugly and invasive way to do this. The fact that the reviewer keeps finding bizarre bugs like "another backslash command on the same line doesn't work" seems to me to be a good indication that this is touching things it shouldn't. 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] [PATCH] Make pg_basebackup configure and start standby [Review]
2012-10-14 22:26 keltezéssel, Boszormenyi Zoltan írta: 2012-10-14 22:23 keltezéssel, Boszormenyi Zoltan írta: Hi, 2012-10-14 18:41 keltezéssel, Boszormenyi Zoltan írta: 2012-10-14 18:02 keltezéssel, Fujii Masao írta: Thanks for updating the patch! On Sun, Oct 14, 2012 at 8:41 PM, Boszormenyi Zoltan wrote: Backing up a standby server without -R preserves the original recovery.conf of the standby, it points to the standby's source server. Backing up a standby server with -R overwrites the original recovery.conf with the new one pointing to the standby instead of the standby's source server. Without -Ft, it is obvious. With -Ft, there are two recovery.conf files in the tar file and upon extracting it, the last written one (the one generated via -R) overwrites the original. The tar file is always extracted such way in all platform which PostgreSQL supports? I'm just concerned about that some tool in some platform might prefer the original recovery.conf when extracting tar file. If the spec of tar format specifies such behavior (i.e., the last written file of the same name is always preferred), it's OK. Since tar is a sequential archive format, I think this is the behaviour of every tar extractor. But I will look at adding code to skip the original recovery.conf if it exists in the tar file. I found the bug that recovery.conf is included in the tar file of the tablespace instead of base.tar, when there are tablespaces in the server. You are right, I am looking into this. But I don't know how it got there, I check for (rownum == 0 && writerecoveryconf) and rownum == 0 supposedly means that it's the base.tar. Looking again. I made a mistake in the previous check, rownum is not reliable. The tablespaces are sent first and base backup as the last. Now recovery.conf is written into base.tar. Maybe this is nitpicky problem but... If port number is not explicitly specified in pg_basebackup, the port number is not included to primary_conninfo in recovery.conf which is created during the backup. That is, the standby server using such recovery.conf tries to connect to the default port number because the port number is not supplied in primary_conninfo. This assumes that the default port number is the same between the master and standby. But this is not true. The default port number can be changed in --with-pgport configure option, so the default port number might be different between the master and standby. To avoid this uncertainty, pg_basebackup -R should always include the port number in primary_conninfo? I think you are right. But, I wouldn't restrict it only to the port setting. Any of the values that are set and equal to the compiled-in default, it should be written into recovery.conf. Now all values that are set (even those being equal to the compiled-in default) are put into recovery.conf. When the password is required to connect to the server, pg_basebackup -R always writes the password setting into primary_conninfo in recovery.conf. But if the password is supplied from .pgpass, ISTM that the password setting doesn't need to be written into primary_conninfo. Right? How can you deduce it from the PQconninfoOption structure? Also, if the machine you take the base backup on is different from the one where you actually use the backup on, it can be different not only in the --with-pgport compilation option but in the presence of .pgpass or the PGPASSWORD envvar, too. The administrator is there for a reason or there is no .pgpass or PGPASSWORD at all. +The password written into recovery.conf is not escaped even if special +characters appear in it. The administrator must review recovery.conf +to ensure proper escaping. Is it difficult to make pg_basebackup escape the special characters in the password? It's better if we can remove this restriction. It's not difficult. What other characters need to be escaped besides single quotes? All written values are escaped. Other changes: the recovery.conf in base.tar is correctly skipped if it exists and -R is given. The new recovery.conf is written with padding to round up to 512, the TAR chunk size. Also, the check for conflict between -R and -x/-X is now removed. Really the last one, for today at least. The buffer for recovery.conf is freed in both the -Fp and -Ft cases. The PQconninfo patch is also attached but didn't change since the last mail. I've not reviewed PQconninfo patch yet. Will review. Thanks in advance. Best regards, Zoltán Böszörményi -- -- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt, Austria Web: http://www.postgresql-support.de http://www.postgresql.at/ diff -durpN postgresql/doc/src/sgml/libpq.sgml postgresql.1/doc/src/sgml/libpq.sgml --- postgresql/doc/src/sgml/libpq.sgml 2012-08-03 09:39:30.114266570 +0200 +++ postgresql.1/doc/src/sgml/libpq.sgml 2012-10-14 11:12:07.049196259 +0200 @@ -4
Re: [HACKERS] [PATCH] Make pg_basebackup configure and start standby [Review]
2012-10-14 22:23 keltezéssel, Boszormenyi Zoltan írta: Hi, 2012-10-14 18:41 keltezéssel, Boszormenyi Zoltan írta: 2012-10-14 18:02 keltezéssel, Fujii Masao írta: Thanks for updating the patch! On Sun, Oct 14, 2012 at 8:41 PM, Boszormenyi Zoltan wrote: Backing up a standby server without -R preserves the original recovery.conf of the standby, it points to the standby's source server. Backing up a standby server with -R overwrites the original recovery.conf with the new one pointing to the standby instead of the standby's source server. Without -Ft, it is obvious. With -Ft, there are two recovery.conf files in the tar file and upon extracting it, the last written one (the one generated via -R) overwrites the original. The tar file is always extracted such way in all platform which PostgreSQL supports? I'm just concerned about that some tool in some platform might prefer the original recovery.conf when extracting tar file. If the spec of tar format specifies such behavior (i.e., the last written file of the same name is always preferred), it's OK. Since tar is a sequential archive format, I think this is the behaviour of every tar extractor. But I will look at adding code to skip the original recovery.conf if it exists in the tar file. I found the bug that recovery.conf is included in the tar file of the tablespace instead of base.tar, when there are tablespaces in the server. You are right, I am looking into this. But I don't know how it got there, I check for (rownum == 0 && writerecoveryconf) and rownum == 0 supposedly means that it's the base.tar. Looking again. I made a mistake in the previous check, rownum is not reliable. The tablespaces are sent first and base backup as the last. Now recovery.conf is written into base.tar. Maybe this is nitpicky problem but... If port number is not explicitly specified in pg_basebackup, the port number is not included to primary_conninfo in recovery.conf which is created during the backup. That is, the standby server using such recovery.conf tries to connect to the default port number because the port number is not supplied in primary_conninfo. This assumes that the default port number is the same between the master and standby. But this is not true. The default port number can be changed in --with-pgport configure option, so the default port number might be different between the master and standby. To avoid this uncertainty, pg_basebackup -R should always include the port number in primary_conninfo? I think you are right. But, I wouldn't restrict it only to the port setting. Any of the values that are set and equal to the compiled-in default, it should be written into recovery.conf. Now all values that are set (even those being equal to the compiled-in default) are put into recovery.conf. When the password is required to connect to the server, pg_basebackup -R always writes the password setting into primary_conninfo in recovery.conf. But if the password is supplied from .pgpass, ISTM that the password setting doesn't need to be written into primary_conninfo. Right? How can you deduce it from the PQconninfoOption structure? Also, if the machine you take the base backup on is different from the one where you actually use the backup on, it can be different not only in the --with-pgport compilation option but in the presence of .pgpass or the PGPASSWORD envvar, too. The administrator is there for a reason or there is no .pgpass or PGPASSWORD at all. +The password written into recovery.conf is not escaped even if special +characters appear in it. The administrator must review recovery.conf +to ensure proper escaping. Is it difficult to make pg_basebackup escape the special characters in the password? It's better if we can remove this restriction. It's not difficult. What other characters need to be escaped besides single quotes? All written values are escaped. Other changes: the recovery.conf in base.tar is correctly skipped if it exists and -R is given. The new recovery.conf is written with padding to round up to 512, the TAR chunk size. Also, the check for conflict between -R and -x/-X is now removed. The PQconninfo patch is also attached but didn't change since the last mail. I've not reviewed PQconninfo patch yet. Will review. Thanks in advance. Best regards, Zoltán Böszörményi -- -- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt, Austria Web: http://www.postgresql-support.de http://www.postgresql.at/ -- 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] [PATCH] Make pg_basebackup configure and start standby [Review]
Hi, 2012-10-14 18:41 keltezéssel, Boszormenyi Zoltan írta: 2012-10-14 18:02 keltezéssel, Fujii Masao írta: Thanks for updating the patch! On Sun, Oct 14, 2012 at 8:41 PM, Boszormenyi Zoltan wrote: Backing up a standby server without -R preserves the original recovery.conf of the standby, it points to the standby's source server. Backing up a standby server with -R overwrites the original recovery.conf with the new one pointing to the standby instead of the standby's source server. Without -Ft, it is obvious. With -Ft, there are two recovery.conf files in the tar file and upon extracting it, the last written one (the one generated via -R) overwrites the original. The tar file is always extracted such way in all platform which PostgreSQL supports? I'm just concerned about that some tool in some platform might prefer the original recovery.conf when extracting tar file. If the spec of tar format specifies such behavior (i.e., the last written file of the same name is always preferred), it's OK. Since tar is a sequential archive format, I think this is the behaviour of every tar extractor. But I will look at adding code to skip the original recovery.conf if it exists in the tar file. I found the bug that recovery.conf is included in the tar file of the tablespace instead of base.tar, when there are tablespaces in the server. You are right, I am looking into this. But I don't know how it got there, I check for (rownum == 0 && writerecoveryconf) and rownum == 0 supposedly means that it's the base.tar. Looking again. I made a mistake in the previous check, rownum is not reliable. The tablespaces are sent first and base backup as the last. Now recovery.conf is written into base.tar. Maybe this is nitpicky problem but... If port number is not explicitly specified in pg_basebackup, the port number is not included to primary_conninfo in recovery.conf which is created during the backup. That is, the standby server using such recovery.conf tries to connect to the default port number because the port number is not supplied in primary_conninfo. This assumes that the default port number is the same between the master and standby. But this is not true. The default port number can be changed in --with-pgport configure option, so the default port number might be different between the master and standby. To avoid this uncertainty, pg_basebackup -R should always include the port number in primary_conninfo? I think you are right. But, I wouldn't restrict it only to the port setting. Any of the values that are set and equal to the compiled-in default, it should be written into recovery.conf. Now all values that are set (even those being equal to the compiled-in default) are put into recovery.conf. When the password is required to connect to the server, pg_basebackup -R always writes the password setting into primary_conninfo in recovery.conf. But if the password is supplied from .pgpass, ISTM that the password setting doesn't need to be written into primary_conninfo. Right? How can you deduce it from the PQconninfoOption structure? Also, if the machine you take the base backup on is different from the one where you actually use the backup on, it can be different not only in the --with-pgport compilation option but in the presence of .pgpass or the PGPASSWORD envvar, too. The administrator is there for a reason or there is no .pgpass or PGPASSWORD at all. +The password written into recovery.conf is not escaped even if special +characters appear in it. The administrator must review recovery.conf +to ensure proper escaping. Is it difficult to make pg_basebackup escape the special characters in the password? It's better if we can remove this restriction. It's not difficult. What other characters need to be escaped besides single quotes? All written values are escaped. Other changes: the recovery.conf in base.tar is correctly skipped if it exists and -R is given. The new recovery.conf is written with padding to round up to 512, the TAR chunk size. The PQconninfo patch is also attached but didn't change since the last mail. I've not reviewed PQconninfo patch yet. Will review. Thanks in advance. Best regards, Zoltán Böszörményi -- -- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt, Austria Web: http://www.postgresql-support.de http://www.postgresql.at/ diff -durpN postgresql/doc/src/sgml/libpq.sgml postgresql.1/doc/src/sgml/libpq.sgml --- postgresql/doc/src/sgml/libpq.sgml 2012-08-03 09:39:30.114266570 +0200 +++ postgresql.1/doc/src/sgml/libpq.sgml 2012-10-14 11:12:07.049196259 +0200 @@ -496,6 +496,37 @@ typedef struct + + PQconninfoPQconninfo + + + Returns the connection options used by a live connection. + +PQconninfoOption *PQconninfo(PGconn *conn, bool for_replication); + + + + + Returns a connection options array. This
Re: [HACKERS] proposal - assign result of query to psql variable
Hi Pavel, On Sat, Oct 13, 2012 at 12:58 AM, Pavel Stehule wrote: * merge Shigeru's doc patch * rename psql regression test from "psql" to "psql_cmd" Those changes seem good. Besides, I found an error message which doesn't end with '¥n' in common.c. In general, messages passed to psql_error end with '¥n', unless additional information follows. Please see attached patch for additional change. After you determine whether it's ok or unnecessary, I'll mark this patch as "Ready for committer". Regards, -- Shigeru HANADA *** a/doc/src/sgml/ref/psql-ref.sgml --- b/doc/src/sgml/ref/psql-ref.sgml *** *** 1483,1490 testdb=> way. Use \i for that.) This means that if the query ends with (or contains) a semicolon, it is immediately executed. Otherwise it will merely wait in the ! query buffer; type semicolon or \g to send it, or ! \r to cancel. --- 1483,1490 way. Use \i for that.) This means that if the query ends with (or contains) a semicolon, it is immediately executed. Otherwise it will merely wait in the ! query buffer; type semicolon, \g or ! \gset to send it, or \r to cancel. *** *** 1617,1622 Tue Oct 26 21:40:57 CEST 1999 --- 1617,1644 + \gset variable [ ,variable ... ] + + + + Sends the current query input buffer to the server and stores the + query's output into corresponding variable. The preceding query must + return only one row, and the number of variables must be same as the + number of elements in SELECT list. If you don't + need any of items in SELECT list, you can omit + corresponding variable. + Example: + + foo=> SELECT 'hello', 'wonderful', 'world!' \gset var1,,var3 + foo=> \echo :var1 :var3 + hello world! + + + + + + \h or \help [ command ] *** a/src/bin/psql/command.c --- b/src/bin/psql/command.c *** *** 748,753 exec_command(const char *cmd, --- 748,776 status = PSQL_CMD_SEND; } + /* \gset send query and store result */ + else if (strcmp(cmd, "gset") == 0) + { + boolerror; + + pset.gvars = psql_scan_slash_vars(scan_state, &error); + + if (!pset.gvars && !error) + { + psql_error("\\%s: missing required argument\n", cmd); + status = PSQL_CMD_NOSEND; + } + else if (error) + { + psql_error("\\%s: syntax error\n", cmd); + status = PSQL_CMD_NOSEND; + tglist_free(pset.gvars); + pset.gvars = NULL; + } + else + status = PSQL_CMD_SEND; + } + /* help */ else if (strcmp(cmd, "h") == 0 || strcmp(cmd, "help") == 0) { *** a/src/bin/psql/command.h --- b/src/bin/psql/command.h *** *** 16,21 typedef enum _backslashResult --- 16,22 { PSQL_CMD_UNKNOWN = 0, /* not done parsing yet (internal only) */ PSQL_CMD_SEND, /* query complete; send off */ + PSQL_CMD_NOSEND,/* query complete, don't send */ PSQL_CMD_SKIP_LINE, /* keep building query */ PSQL_CMD_TERMINATE, /* quit program */ PSQL_CMD_NEWEDIT, /* query buffer was changed (e.g., via \e) */ *** a/src/bin/psql/common.c --- b/src/bin/psql/common.c *** *** 816,821 PrintQueryResults(PGresult *results) --- 816,919 return success; } + /* + * StoreQueryResult: store first row of result to selected variables + * + * Note: Utility function for use by SendQuery() only. + * + * Returns true if the query executed successfully, false otherwise. + */ + static bool + StoreQueryResult(PGresult *result) + { + bool success; + + switch (PQresultStatus(result)) + { + case PGRES_TUPLES_OK: + { + int i; + + if (PQntuples(result) < 1) + { + psql_error("no data found\n"); + success = false; + } + else if (PQntuples(result) > 1) + { + psql_error("too many rows\n"); + success = false; + } + else + { +
Re: [HACKERS] proposal - assign result of query to psql variable
Hello 2012/10/14 Erik Rijkers : > On Sat, October 13, 2012 19:26, Pavel Stehule wrote: >> 2012/10/13 Shigeru HANADA : >>> After you determine whether it's ok or unnecessary, I'll mark this patch as >>> "Ready for committer". >>> >> > > I found this behaviour which I think must count as a bug. > \gset doesn't allow more \\-separated lines behind it: > > Only the last of these commands is problematic, and giving the syntax error > > $ psql > psql > (9.3devel-psql_var-20121012_2345-8b728e5c6e0ce6b6d6f54b92b390f14aa1aca6db) > Type "help" for help. > > testdb=# select 1,2 \gset x,y > testdb=# \echo :x > 1 > testdb=# \echo :y > 2 > testdb=# \echo :x \\ \echo :y > 1 > 2 > testdb=# select 1,2 \gset x,y \\ \echo :x > \gset: syntax error > testdb=# > > It'd be nice if it could be made to work > done Regards Pavel > Thanks > > Erik Rijkers > gset_08.diff Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecating RULES
On Oct 14, 2012, at 12:35 PM, Tom Lane wrote: > Simon Riggs writes: >> On 12 October 2012 19:48, Greg Stark wrote: >>> On Fri, Oct 12, 2012 at 7:55 AM, Simon Riggs wrote: AFAICS all RULEs can be re-expressed as Triggers or Views. > >>> This is a bizarre discussion. Firstly this isn't even close to true. >>> The whole source of people's discontentment is that triggers are *not* >>> equivalent to rules. If they were then they wouldn't be so upset. > >> I'm not aware of any rule that can't be rewritten as a trigger or a >> view. Please can anyone show me some examples of those? > > Sorry, you're thinking you can put the burden of proof on other people, > but this doesn't work like that. If you want to deprecate rules on the > grounds that triggers are an adequate substitute, it's up to you to > prove that claim, not for other people to disprove it. It seems there are two somewhat separate issues for discussion, one is the question of what to do about rules, the second is deprecation policy in general. Having worked for major software vendors, this are is always a headache. Consider the Microsoft, one of the more powerful software vendors of the PC era, is still trying to get people to upgrade to IE6, but is facing the obstacle of businesses refraining because internally written applications. The discussions around rules and hash indexes going on concurrently on this list share features which would benefit from having a general policy discussion abstracted from the attachments or dislikes of particular features. I would suggest that a thread be spawned off to consider deprecation policy, including substantive reasons for deprecation, the burden of proof on those proposing deprecation, means of communicating to users. This will cause some thrash up front, but will go a long way to triaging deprecation discussions, and having a work flow in place for when such decisions are made. -- 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] Rethinking placement of latch self-pipe initialization
On 14 October 2012 19:17, Tom Lane wrote: > Anyway, the simplest working solution proved to be to put the > InitializeLatchSupport calls in InitProcess and InitAuxiliaryProcess, > plus add them in a few background process types that use InitLatch but > don't call either of those functions. Patch attached. Any objections? Looks good to me. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and 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] Rethinking placement of latch self-pipe initialization
I wrote: > Sean Chittenden recently reported that 9.2 can crash after logging > "FATAL: pipe() failed" if the kernel is short of file descriptors: > http://archives.postgresql.org/pgsql-general/2012-10/msg00202.php > ... > What I think would be a better idea is to fix things so that OwnLatch > cannot fail except as a result of internal logic errors, by splitting > out the acquisition of the self-pipe into a separate function named say > InitializeLatchSupport. The question then becomes where to put the > InitializeLatchSupport calls. My first thought is to put them near the > signal-setup stanzas for the various processes (ie, the pqsignal calls) > similarly to what we did recently for initialization of timeout support. I experimented with that approach, and found out it didn't work at all, because there are assorted code paths in which InitProcess and InitAuxiliaryProcess are called well before we enable signals. I find that a bit disturbing; it means there is a significant amount of process-startup code that has a latch available but can't actually wait on the latch. (Well, it could, but it would never awaken because it would never react to the signal.) At some point in the future we may have to do some refactoring in this area. On the other hand, moving signal-enabling earlier carries its own hazards: the signal handlers might expect yet other infrastructure to be alive already, and any bug of that ilk would probably be a lot harder to reproduce and diagnose. Anyway, the simplest working solution proved to be to put the InitializeLatchSupport calls in InitProcess and InitAuxiliaryProcess, plus add them in a few background process types that use InitLatch but don't call either of those functions. Patch attached. Any objections? regards, tom lane diff --git a/src/backend/port/unix_latch.c b/src/backend/port/unix_latch.c index 335e9f66afb52163470ee9b69119b7fa16d2bc1c..bbd1810ea530547c75255f02c5be44c559cc03de 100644 *** a/src/backend/port/unix_latch.c --- b/src/backend/port/unix_latch.c *** static volatile sig_atomic_t waiting = f *** 60,70 static int selfpipe_readfd = -1; static int selfpipe_writefd = -1; ! /* private function prototypes */ ! static void initSelfPipe(void); ! static void drainSelfPipe(void); static void sendSelfPipeByte(void); /* * Initialize a backend-local latch. --- 60,100 static int selfpipe_readfd = -1; static int selfpipe_writefd = -1; ! /* Private function prototypes */ static void sendSelfPipeByte(void); + static void drainSelfPipe(void); + + + /* + * Initialize the process-local latch infrastructure. + * + * This must be called once during startup of any process that can wait on + * latches, before it issues any InitLatch() or OwnLatch() calls. + */ + void + InitializeLatchSupport(void) + { + int pipefd[2]; + + Assert(selfpipe_readfd == -1); + + /* + * Set up the self-pipe that allows a signal handler to wake up the + * select() in WaitLatch. Make the write-end non-blocking, so that + * SetLatch won't block if the event has already been set many times + * filling the kernel buffer. Make the read-end non-blocking too, so that + * we can easily clear the pipe by reading until EAGAIN or EWOULDBLOCK. + */ + if (pipe(pipefd) < 0) + elog(FATAL, "pipe() failed: %m"); + if (fcntl(pipefd[0], F_SETFL, O_NONBLOCK) < 0) + elog(FATAL, "fcntl() failed on read-end of self-pipe: %m"); + if (fcntl(pipefd[1], F_SETFL, O_NONBLOCK) < 0) + elog(FATAL, "fcntl() failed on write-end of self-pipe: %m"); + selfpipe_readfd = pipefd[0]; + selfpipe_writefd = pipefd[1]; + } /* * Initialize a backend-local latch. *** static void sendSelfPipeByte(void); *** 72,80 void InitLatch(volatile Latch *latch) { ! /* Initialize the self-pipe if this is our first latch in the process */ ! if (selfpipe_readfd == -1) ! initSelfPipe(); latch->is_set = false; latch->owner_pid = MyProcPid; --- 102,109 void InitLatch(volatile Latch *latch) { ! /* Assert InitializeLatchSupport has been called in this process */ ! Assert(selfpipe_readfd >= 0); latch->is_set = false; latch->owner_pid = MyProcPid; *** InitSharedLatch(volatile Latch *latch) *** 116,126 void OwnLatch(volatile Latch *latch) { ! Assert(latch->is_shared); ! /* Initialize the self-pipe if this is our first latch in this process */ ! if (selfpipe_readfd == -1) ! initSelfPipe(); /* sanity check */ if (latch->owner_pid != 0) --- 145,154 void OwnLatch(volatile Latch *latch) { ! /* Assert InitializeLatchSupport has been called in this process */ ! Assert(selfpipe_readfd >= 0); ! Assert(latch->is_shared); /* sanity check */ if (latch->owner_pid != 0) *** latch_sigusr1_handler(void) *** 514,543 sendSelfPipeByte(); } - /* initialize the self-pipe */ - static void - initSelfPipe(void) - { - int pipe
Re: [HACKERS] Deprecating Hash Indexes
On 14 October 2012 17:47, Tom Lane wrote: > Simon Riggs writes: >> As discussed on other threads, Hash Indexes are currently a broken >> feature and bring us into disrepute. > > Yeah ... > >> Suggested actions are > > I find it curious that you don't even bother to list "add WAL support to > hash indexes" as a possible solution. It's certainly doable, and > probably not even very much more work than something like moving hash > support to a contrib module would be. (Assuming that's even possible, > which I doubt, because the core code relies on hash index opclasses even > if not on hash indexes.) It is doable, yes. I think that's at least 2-3 months total work, much of it low-level bug fixing and eventual production bug hunting. Its gone for 9.3 already, so the earliest this would see production code is Sept 2014. It's a task beyond most people's ability and beyond the range of professionals without funding. I don't see anyone funding a medium-low importance feature ahead of the higher ones out there, so I don't expect the situation to change for (more) years. If you personally have time to spare on performance features, there are many optimizer tasks more worthy of your attention and more popular also, so I don't ask for a code sprint on this. -- Simon Riggs 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] pg_dump restore error
Gilles Darold writes: > Restoring a backup generated with pg_dump/pg_dumpall in plain text > format and the --clean option will report errors if the backup is loaded > in an other or empty database. This is intentional. What you propose amounts to a fundamental change in the semantics of --clean, and I don't think that it's really much of an improvement. 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] Deprecating Hash Indexes
Simon Riggs writes: > As discussed on other threads, Hash Indexes are currently a broken > feature and bring us into disrepute. Yeah ... > Suggested actions are I find it curious that you don't even bother to list "add WAL support to hash indexes" as a possible solution. It's certainly doable, and probably not even very much more work than something like moving hash support to a contrib module would be. (Assuming that's even possible, which I doubt, because the core code relies on hash index opclasses even if not on hash indexes.) 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_stat_lwlocks view - lwlocks statistics, round 2
Satoshi Nagayasu writes: > (2012/10/14 13:26), Fujii Masao wrote: >> The tracing lwlock usage seems to still cause a small performance >> overhead even if reporting is disabled. I believe some users would >> prefer to avoid such overhead even if pg_stat_lwlocks is not available. >> It should be up to a user to decide whether to trace lwlock usage, e.g., >> by using trace_lwlock parameter, I think. > Frankly speaking, I do not agree with disabling performance > instrument to improve performance. DBA must *always* monitor > the performance metrix when having such heavy workload. This brings up a question that I don't think has been honestly considered, which is exactly whom a feature like this is targeted at. TBH I think it's of about zero use to DBAs (making the above argument bogus). It is potentially of use to developers, but a DBA is unlikely to be able to do anything about lwlock-level contention even if he has the knowledge to interpret the data. So I feel it isn't something that should be turned on in production builds. I'd vote for enabling it by a non-default configure option, and making sure that it doesn't introduce any overhead when the option is off. 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] [PATCH] Make pg_basebackup configure and start standby [Review]
2012-10-14 18:02 keltezéssel, Fujii Masao írta: Thanks for updating the patch! On Sun, Oct 14, 2012 at 8:41 PM, Boszormenyi Zoltan wrote: Backing up a standby server without -R preserves the original recovery.conf of the standby, it points to the standby's source server. Backing up a standby server with -R overwrites the original recovery.conf with the new one pointing to the standby instead of the standby's source server. Without -Ft, it is obvious. With -Ft, there are two recovery.conf files in the tar file and upon extracting it, the last written one (the one generated via -R) overwrites the original. The tar file is always extracted such way in all platform which PostgreSQL supports? I'm just concerned about that some tool in some platform might prefer the original recovery.conf when extracting tar file. If the spec of tar format specifies such behavior (i.e., the last written file of the same name is always preferred), it's OK. Since tar is a sequential archive format, I think this is the behaviour of every tar extractor. But I will look at adding code to skip the original recovery.conf if it exists in the tar file. I found the bug that recovery.conf is included in the tar file of the tablespace instead of base.tar, when there are tablespaces in the server. You are right, I am looking into this. But I don't know how it got there, I check for (rownum == 0 && writerecoveryconf) and rownum == 0 supposedly means that it's the base.tar. Looking again. Maybe this is nitpicky problem but... If port number is not explicitly specified in pg_basebackup, the port number is not included to primary_conninfo in recovery.conf which is created during the backup. That is, the standby server using such recovery.conf tries to connect to the default port number because the port number is not supplied in primary_conninfo. This assumes that the default port number is the same between the master and standby. But this is not true. The default port number can be changed in --with-pgport configure option, so the default port number might be different between the master and standby. To avoid this uncertainty, pg_basebackup -R should always include the port number in primary_conninfo? I think you are right. But, I wouldn't restrict it only to the port setting. Any of the values that are set and equal to the compiled-in default, it should be written into recovery.conf. When the password is required to connect to the server, pg_basebackup -R always writes the password setting into primary_conninfo in recovery.conf. But if the password is supplied from .pgpass, ISTM that the password setting doesn't need to be written into primary_conninfo. Right? How can you deduce it from the PQconninfoOption structure? Also, if the machine you take the base backup on is different from the one where you actually use the backup on, it can be different not only in the --with-pgport compilation option but in the presence of .pgpass or the PGPASSWORD envvar, too. The administrator is there for a reason or there is no .pgpass or PGPASSWORD at all. +The password written into recovery.conf is not escaped even if special +characters appear in it. The administrator must review recovery.conf +to ensure proper escaping. Is it difficult to make pg_basebackup escape the special characters in the password? It's better if we can remove this restriction. It's not difficult. What other characters need to be escaped besides single quotes? I've not reviewed PQconninfo patch yet. Will review. Thanks in advance. Best regards, Zoltán Böszörményi -- -- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt, Austria Web: http://www.postgresql-support.de http://www.postgresql.at/ -- 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] Deprecating RULES
Simon Riggs writes: > On 12 October 2012 19:48, Greg Stark wrote: >> On Fri, Oct 12, 2012 at 7:55 AM, Simon Riggs wrote: >>> AFAICS all RULEs can be re-expressed as Triggers or Views. >> This is a bizarre discussion. Firstly this isn't even close to true. >> The whole source of people's discontentment is that triggers are *not* >> equivalent to rules. If they were then they wouldn't be so upset. > I'm not aware of any rule that can't be rewritten as a trigger or a > view. Please can anyone show me some examples of those? Sorry, you're thinking you can put the burden of proof on other people, but this doesn't work like that. If you want to deprecate rules on the grounds that triggers are an adequate substitute, it's up to you to prove that claim, not for other people to disprove it. Personally, I don't think I believe it, on both practical and theoretical grounds: * Triggers have been available for a very long time, and have been documented as the better choice for quite a while, but people still try to use rules. Whether it's for (admittedly mostly illusory) ease of use or some other reason, there's some attraction there that we need to match, not just decree that people can't have it anymore. * Triggers necessarily operate on a row-at-a-time basis. In theory, for at least some bulk operations, a rule could greatly outperform a trigger. It's difficult to walk away from that - unless somebody can prove that the advantage doesn't ever accrue in practice. But the fact that a rule won't work in some cases doesn't mean it won't work for any practical use-case. * AFTER triggers fall over on sufficiently large updates, since we still lack any method for spilling the trigger event queue to disk when it gets too big. It's fairly difficult to tell people they have to use triggers as long as that limitation exists and rules don't have it. (I wonder if this could be alleviated with some sort of "immediate after" trigger mode that fires after the event is frozen, but without queueing...) 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] [PATCH] Make pg_basebackup configure and start standby [Review]
On Mon, Oct 15, 2012 at 12:57 AM, Boszormenyi Zoltan wrote: > 2012-10-11 02:02 keltezéssel, Fujii Masao írta: > >> On Thu, Oct 11, 2012 at 3:36 AM, Boszormenyi Zoltan >> wrote: >>> >>> 2012-10-10 18:23 keltezéssel, Fujii Masao írta: When tar output format is specified together with -R option, recovery.conf is not included in base.tar. I think it should. >>> >>> >>> Why? This patch only promises to write the recovery.conf into the >>> directory specified with -D. >> >> Because it's more user-friendly. If recovery.conf is not included in >> base.tar, >> when base.tar is extracted to disk to use the backup, a user always needs >> to copy recovery.conf to the extracted directory. OTOH if it's included in >> base.tar, such copy operation is not required and we can simplify the >> procedures to use the backup a bit. > > > It's implemented now. Thanks a lot! +setting up the standby. Since creating a backup for a standalone +server with -x or -X and adding +a recovery.conf to it wouldn't make a working standby, these options +naturally conflict. Is this right? ISTM that basically we can use pg_basebackup -x to take a base backup for starting the standby for now. No? >>> >>> >>> I don't know. Pointers? >> >> There is no restriction that the backup which was taken by using >> pg_basebackup -x cannot be used to start the standby. So I wonder >> why -R option cannot work together with -x. It's useful if recovery.conf >> is automatically written even with pg_basebackup -x. > > > There was a problem with 9.0.x (maybe even with 9.1) that the backup > failed to come up as a standby if -x or -X was specified. I don't know > if it was a bug, limitation or intended behaviour. It sounds a bug to me. It's helpful if you provide the self-contained test case. > Before removing the check for conflicting options, I would like to ask: > is there such a known conflict with --xlog-method=stream? AFAIK, No, we can use the backup which pg_basebackup --xlog-method=stream took, to start the standby. BTW, --xlog-method=stream cannot be specified together with -F tar. >> And I found another problem: when -(stdout) is specified in -D option, >> recovery.conf fails to be written. >> >> $ pg_basebackup -D- -F t -c fast -R > hoge.tar >> NOTICE: WAL archiving is not enabled; you must ensure that all >> required WAL segments are copied through other means to complete the >> backup >> pg_basebackup: cannot create -/recovery.conf: No such file or directory > > > Now it works with recovery.conf written into the tar itself. I also tried > > $ pg_basebackup -D- -Ft -R > > and the directory named "-" was created and of course the recovery.conf > inside it. Is this the intended behaviour regarding "stdout is to be treated > as a directory"? Yes. Thanks. Regards, -- Fujii Masao -- 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] [PATCH] Make pg_basebackup configure and start standby [Review]
Thanks for updating the patch! On Sun, Oct 14, 2012 at 8:41 PM, Boszormenyi Zoltan wrote: > Backing up a standby server without -R preserves the original recovery.conf > of the > standby, it points to the standby's source server. > > Backing up a standby server with -R overwrites the original recovery.conf > with the new > one pointing to the standby instead of the standby's source server. Without > -Ft, it is > obvious. With -Ft, there are two recovery.conf files in the tar file and > upon extracting it, > the last written one (the one generated via -R) overwrites the original. The tar file is always extracted such way in all platform which PostgreSQL supports? I'm just concerned about that some tool in some platform might prefer the original recovery.conf when extracting tar file. If the spec of tar format specifies such behavior (i.e., the last written file of the same name is always preferred), it's OK. I found the bug that recovery.conf is included in the tar file of the tablespace instead of base.tar, when there are tablespaces in the server. Maybe this is nitpicky problem but... If port number is not explicitly specified in pg_basebackup, the port number is not included to primary_conninfo in recovery.conf which is created during the backup. That is, the standby server using such recovery.conf tries to connect to the default port number because the port number is not supplied in primary_conninfo. This assumes that the default port number is the same between the master and standby. But this is not true. The default port number can be changed in --with-pgport configure option, so the default port number might be different between the master and standby. To avoid this uncertainty, pg_basebackup -R should always include the port number in primary_conninfo? When the password is required to connect to the server, pg_basebackup -R always writes the password setting into primary_conninfo in recovery.conf. But if the password is supplied from .pgpass, ISTM that the password setting doesn't need to be written into primary_conninfo. Right? +The password written into recovery.conf is not escaped even if special +characters appear in it. The administrator must review recovery.conf +to ensure proper escaping. Is it difficult to make pg_basebackup escape the special characters in the password? It's better if we can remove this restriction. I've not reviewed PQconninfo patch yet. Will review. Regards, -- Fujii Masao -- 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] [PATCH] Make pg_basebackup configure and start standby [Review]
2012-10-11 02:02 keltezéssel, Fujii Masao írta: On Thu, Oct 11, 2012 at 3:36 AM, Boszormenyi Zoltan wrote: 2012-10-10 18:23 keltezéssel, Fujii Masao írta: When tar output format is specified together with -R option, recovery.conf is not included in base.tar. I think it should. Why? This patch only promises to write the recovery.conf into the directory specified with -D. Because it's more user-friendly. If recovery.conf is not included in base.tar, when base.tar is extracted to disk to use the backup, a user always needs to copy recovery.conf to the extracted directory. OTOH if it's included in base.tar, such copy operation is not required and we can simplify the procedures to use the backup a bit. It's implemented now. +setting up the standby. Since creating a backup for a standalone +server with -x or -X and adding +a recovery.conf to it wouldn't make a working standby, these options +naturally conflict. Is this right? ISTM that basically we can use pg_basebackup -x to take a base backup for starting the standby for now. No? I don't know. Pointers? There is no restriction that the backup which was taken by using pg_basebackup -x cannot be used to start the standby. So I wonder why -R option cannot work together with -x. It's useful if recovery.conf is automatically written even with pg_basebackup -x. There was a problem with 9.0.x (maybe even with 9.1) that the backup failed to come up as a standby if -x or -X was specified. I don't know if it was a bug, limitation or intended behaviour. Before removing the check for conflicting options, I would like to ask: is there such a known conflict with --xlog-method=stream? And I found another problem: when -(stdout) is specified in -D option, recovery.conf fails to be written. $ pg_basebackup -D- -F t -c fast -R > hoge.tar NOTICE: WAL archiving is not enabled; you must ensure that all required WAL segments are copied through other means to complete the backup pg_basebackup: cannot create -/recovery.conf: No such file or directory Now it works with recovery.conf written into the tar itself. I also tried $ pg_basebackup -D- -Ft -R and the directory named "-" was created and of course the recovery.conf inside it. Is this the intended behaviour regarding "stdout is to be treated as a directory"? Best regards, Zoltán Böszörményi -- -- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt, Austria Web: http://www.postgresql-support.de http://www.postgresql.at/ -- 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] tuplesort memory usage: grow_memtuples
On 14 October 2012 09:19, Simon Riggs wrote: > This is a very useful optimisation, for both the low and the high end. I suppose it's possible to not think of it as an optimisation at all. Rather, it could be considered a way of making tuplesort really use the work_mem allotted to it, or at least use it more effectively, so that DBAs don't have to oversize work_mem. You're getting the same behaviour as when you oversized work_mem, except that you don't run the risk of thrashing due to excessive paging if ever there is a sort big enough to have that effect. > The current coding allows full use of memory iff the memory usage is > an exact power of 2, so this patch will allow an average of 50% and as > much as 100% gain in memory for sorts. We need to be careful to note > this as a change on the release notes, since users may well have set > work_mem to x2 what was actually needed to get it to work - that means > most users will see a sudden leap in actual memory usage, which could > lead to swapping in edge cases. Yes, that's probably true. > I notice that cost_sort doesn't take into account the space used for > sorting other than tuple space so apparently requires no changes with > this patch. ISTM that cost_sort should be less optimistic about memory > efficiency than it is. Perhaps. I don't have an intuitive sense of what is and is not worth modelling in the optimiser, so I can't really comment here. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and 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] pg_dump restore error
Hello, Restoring a backup generated with pg_dump/pg_dumpall in plain text format and the --clean option will report errors if the backup is loaded in an other or empty database. I mean that the backup file contains all SQL order to drop the database's objects before recreating them, so if you load this backup into a new database it will throw errors on each DROP call complaining that the objects doesn't exists. This is not very important because everything goes fine but these error reports can be easily prevented with the addition of IF EXISTS clauses and this will probably be less confusing. I've attached a patch adding those IF EXISTS on each DROP and ALTER statements. Best regards, -- Gilles Darold http://dalibo.com - http://dalibo.org diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 9920d96..e61cdd6 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -1965,7 +1965,7 @@ dumpDatabase(Archive *fout) } - appendPQExpBuffer(delQry, "DROP DATABASE %s;\n", + appendPQExpBuffer(delQry, "DROP DATABASE IF EXISTS %s;\n", fmtId(datname)); dbDumpId = createDumpId(); @@ -7345,7 +7345,7 @@ dumpNamespace(Archive *fout, NamespaceInfo *nspinfo) qnspname = pg_strdup(fmtId(nspinfo->dobj.name)); - appendPQExpBuffer(delq, "DROP SCHEMA %s;\n", qnspname); + appendPQExpBuffer(delq, "DROP SCHEMA IF EXISTS %s;\n", qnspname); appendPQExpBuffer(q, "CREATE SCHEMA %s;\n", qnspname); @@ -7404,7 +7404,7 @@ dumpExtension(Archive *fout, ExtensionInfo *extinfo) qextname = pg_strdup(fmtId(extinfo->dobj.name)); - appendPQExpBuffer(delq, "DROP EXTENSION %s;\n", qextname); + appendPQExpBuffer(delq, "DROP EXTENSION IF EXISTS %s;\n", qextname); if (!binary_upgrade) { @@ -7575,7 +7575,7 @@ dumpEnumType(Archive *fout, TypeInfo *tyinfo) * CASCADE shouldn't be required here as for normal types since the I/O * functions are generic and do not get dropped. */ - appendPQExpBuffer(delq, "DROP TYPE %s.", + appendPQExpBuffer(delq, "DROP TYPE IF EXISTS %s.", fmtId(tyinfo->dobj.namespace->dobj.name)); appendPQExpBuffer(delq, "%s;\n", fmtId(tyinfo->dobj.name)); @@ -7697,7 +7697,7 @@ dumpRangeType(Archive *fout, TypeInfo *tyinfo) * CASCADE shouldn't be required here as for normal types since the I/O * functions are generic and do not get dropped. */ - appendPQExpBuffer(delq, "DROP TYPE %s.", + appendPQExpBuffer(delq, "DROP TYPE IF EXISTS %s.", fmtId(tyinfo->dobj.namespace->dobj.name)); appendPQExpBuffer(delq, "%s;\n", fmtId(tyinfo->dobj.name)); @@ -8029,7 +8029,7 @@ dumpBaseType(Archive *fout, TypeInfo *tyinfo) * the type and its I/O functions makes it impossible to drop the type any * other way. */ - appendPQExpBuffer(delq, "DROP TYPE %s.", + appendPQExpBuffer(delq, "DROP TYPE IF EXISTS %s.", fmtId(tyinfo->dobj.namespace->dobj.name)); appendPQExpBuffer(delq, "%s CASCADE;\n", fmtId(tyinfo->dobj.name)); @@ -8281,7 +8281,7 @@ dumpDomain(Archive *fout, TypeInfo *tyinfo) /* * DROP must be fully qualified in case same name appears in pg_catalog */ - appendPQExpBuffer(delq, "DROP DOMAIN %s.", + appendPQExpBuffer(delq, "DROP DOMAIN IF EXISTS %s.", fmtId(tyinfo->dobj.namespace->dobj.name)); appendPQExpBuffer(delq, "%s;\n", fmtId(tyinfo->dobj.name)); @@ -8794,7 +8794,7 @@ dumpProcLang(Archive *fout, ProcLangInfo *plang) else lanschema = NULL; - appendPQExpBuffer(delqry, "DROP PROCEDURAL LANGUAGE %s;\n", + appendPQExpBuffer(delqry, "DROP PROCEDURAL LANGUAGE IF EXISTS %s;\n", qlanname); if (useParams) @@ -9335,7 +9335,7 @@ dumpFunc(Archive *fout, FuncInfo *finfo) /* * DROP must be fully qualified in case same name appears in pg_catalog */ - appendPQExpBuffer(delqry, "DROP FUNCTION %s.%s;\n", + appendPQExpBuffer(delqry, "DROP FUNCTION IF EXISTS %s.%s;\n", fmtId(finfo->dobj.namespace->dobj.name), funcsig); @@ -9553,7 +9553,7 @@ dumpCast(Archive *fout, CastInfo *cast) delqry = createPQExpBuffer(); labelq = createPQExpBuffer(); - appendPQExpBuffer(delqry, "DROP CAST (%s AS %s);\n", + appendPQExpBuffer(delqry, "DROP CAST IF EXISTS (%s AS %s);\n", getFormattedTypeName(fout, cast->castsource, zeroAsNone), getFormattedTypeName(fout, cast->casttarget, zeroAsNone)); @@ -9823,7 +9823,7 @@ dumpOpr(Archive *fout, OprInfo *oprinfo) /* * DROP must be fully qualified in case same name appears in pg_catalog */ - appendPQExpBuffer(delq, "DROP OPERATOR %s.%s;\n", + appendPQExpBuffer(delq, "DROP OPERATOR IF EXISTS %s.%s;\n", fmtId(oprinfo->dobj.namespace->dobj.name), oprid->data); @@ -10109,7 +10109,7 @@ dumpOpclass(Archive *fout, OpclassInfo *opcinfo) /* * DROP must be fully qualified in case same name appears in pg_catalog */ - appendPQExpBuffer(delq, "DROP OPERATOR CLASS %s", + appendPQExpBuffer(delq, "DROP OPERATOR CLASS IF EXISTS %s", fmtId(opcinfo->dobj.namespace->dobj.name));
Re: [HACKERS] Deprecating Hash Indexes
On 10/14/2012 09:45 AM, Simon Riggs wrote: As discussed on other threads, Hash Indexes are currently a broken feature and bring us into disrepute. I describe them as broken because they are neither crash safe, nor could they properly be called unlogged indexes (or if so, why just hash?). And also because if somebody suggested implementing them the way they are now, they would be told they were insane because silent data corruption is not something we tolerate anymore. We know why they are like that, but its time to remove the rest of the historical research legacy. It's hard to say "We respect your data [except if you press here]" and be taken seriously. Suggested actions are * Put WARNINGs in the docs against the use of hash indexes, backpatch to 8.3. CREATE INDEX gives no warning currently, though Index Types does mention a caution. * Mention in the current docs that hash indexes are likely to be deprecated completely in future releases. Should anybody ever make them work, we can change that advice quickly but I don't think we're going to. Personally, I would like to see them removed into a contrib module to allow people to re-add them if they understand the risks. ISTM better to confiscate all foot-guns before they go off and then re-issue them to marksmen, at the risk of annoying the people that use them with full knowledge but that's likely a contentious issue. Alternate views? I think we'd be better off to start by implementing unlogged indexes generally. I have a client who is using hash indexes for the performance gain on a very heavy insert load precisely because they are unlogged, and who can get away with it because all their index lookup requirements are equality. They are aware of the consequences of a crash, and can manage the risk accordingly. But there is a lot of attraction in the idea of unlogged btree indexes for example. And the danger of an unlogged index is substantially less than that of an unlogged table. After all, your data is still there, quite crash-safe, and you can thus just rebuild the index after a crash. 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] pg_stat_lwlocks view - lwlocks statistics, round 2
On Sun, Oct 14, 2012 at 7:52 PM, Satoshi Nagayasu wrote: > (2012/10/14 13:26), Fujii Masao wrote: >> >> On Sun, Oct 14, 2012 at 10:46 AM, Satoshi Nagayasu >> wrote: >>> >>> HEAD >>> >>> number of transactions actually processed: 3439971 >>> tps = 57331.891602 (including connections establishing) >>> tps = 57340.932324 (excluding connections establishing) >> >> >>> >>> pg_stat_lwlocks patch (reporting disabled) >>> == >>> number of transactions actually processed: 3429370 >>> tps = 57155.286475 (including connections establishing) >>> tps = 57163.996943 (excluding connections establishing) >>> >>> So, I think some additional hack to reduce reporting is needed. >>> Would it be acceptable in terms of the performance? >> >> >> The tracing lwlock usage seems to still cause a small performance >> overhead even if reporting is disabled. I believe some users would >> prefer to avoid such overhead even if pg_stat_lwlocks is not available. >> It should be up to a user to decide whether to trace lwlock usage, e.g., >> by using trace_lwlock parameter, I think. > > > Frankly speaking, I do not agree with disabling performance > instrument to improve performance. DBA must *always* monitor > the performance metrix when having such heavy workload. > > But it's ok to add a parameter to switch enable/disable it. > Any other comments? I thought that pg_stat_lwlocks would be mostly used for the debugging purpose. IOW most users would disable that in production system. But, if you think it should be enabled even in production system and most users would usually improve performance by using that view, I think that you need to reconsider the lwlockid in pg_stat_lwlocks view. It's hard to understand what kind of lwlock each lwlockid indicates. Which means that it's hard for most user to investigate something from current pg_stat_lwlocks. You might need to expose the lwlock name and its short description Another comment is; local_calls/waits/time_ms are really required? I'm not sure how those info would help the performance debugging. >>> >>> >>> >>> I think there are some needs to observe/determine how your test >>> query is affected by the other workload from the other sessions. >>> So, splitting local and shared statistics would be nice to have. >>> Just my thought though. >> >> >> What I don't like is that a session can see only local stats of its own >> session. It's hard to monitor local stats. Imagine the case where you'd >> like to monitor local stats of each pgbench session. To monitor such >> stats, you need to modify pgbench so that its each session monitors >> its own local stats. Even if you run a monitoring software, it cannot >> collect those stats because they don't belong to the session that it uses. > > > Ok. I'm waiting more comments from others. > Dropping it is easy for me, but any other comments? Josh? What I was thinking useful is to make the stats collector collect also local stats and report them for each backend ID or distinct query (e.g., adding such stats into pg_stat_statements, though this would not be good idea). Which enables every session to see any local stats. Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Deprecating Hash Indexes
As discussed on other threads, Hash Indexes are currently a broken feature and bring us into disrepute. I describe them as broken because they are neither crash safe, nor could they properly be called unlogged indexes (or if so, why just hash?). And also because if somebody suggested implementing them the way they are now, they would be told they were insane because silent data corruption is not something we tolerate anymore. We know why they are like that, but its time to remove the rest of the historical research legacy. It's hard to say "We respect your data [except if you press here]" and be taken seriously. Suggested actions are * Put WARNINGs in the docs against the use of hash indexes, backpatch to 8.3. CREATE INDEX gives no warning currently, though Index Types does mention a caution. * Mention in the current docs that hash indexes are likely to be deprecated completely in future releases. Should anybody ever make them work, we can change that advice quickly but I don't think we're going to. Personally, I would like to see them removed into a contrib module to allow people to re-add them if they understand the risks. ISTM better to confiscate all foot-guns before they go off and then re-issue them to marksmen, at the risk of annoying the people that use them with full knowledge but that's likely a contentious issue. Alternate views? -- Simon Riggs 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] [PATCH] Make pg_basebackup configure and start standby [Review]
2012-10-12 22:21 keltezéssel, Robert Haas írta: On Wed, Oct 10, 2012 at 8:02 PM, Fujii Masao wrote: On Thu, Oct 11, 2012 at 3:36 AM, Boszormenyi Zoltan wrote: 2012-10-10 18:23 keltezéssel, Fujii Masao írta: When tar output format is specified together with -R option, recovery.conf is not included in base.tar. I think it should. Why? This patch only promises to write the recovery.conf into the directory specified with -D. Because it's more user-friendly. If recovery.conf is not included in base.tar, when base.tar is extracted to disk to use the backup, a user always needs to copy recovery.conf to the extracted directory. OTOH if it's included in base.tar, such copy operation is not required and we can simplify the procedures to use the backup a bit. +1. OK, out of popular demand, I implemented writing into the base.tar if both -R and -Ft was specified. The code to create a tar header and the checksum inside the header was copied from bin/pg_dump/pg_backup_tar.c. I tested these scenarios ("server" can be either a master and standby): - backup a server with -Fp (no recovery.conf in the target directory was written) - backup a server with -Ftar (no recovery.conf was written into the target directory or base.tar) - backup a server with -Ftar -z (no recovery.conf was written into the target directory or base.tar.gz) - backup a server with -R -Fp (the new recovery.conf was written into the target directory) - backup a server with -R -Ftar (the new recovery.conf was written into the base.tar) - backup a server with -R -Ftar -z (the new recovery.conf was written into the base.tar.gz) Backing up a standby server without -R preserves the original recovery.conf of the standby, it points to the standby's source server. Backing up a standby server with -R overwrites the original recovery.conf with the new one pointing to the standby instead of the standby's source server. Without -Ft, it is obvious. With -Ft, there are two recovery.conf files in the tar file and upon extracting it, the last written one (the one generated via -R) overwrites the original. Best regards, Zoltán Böszörményi -- -- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt, Austria Web: http://www.postgresql-support.de http://www.postgresql.at/ diff -durpN postgresql/doc/src/sgml/libpq.sgml postgresql.1/doc/src/sgml/libpq.sgml --- postgresql/doc/src/sgml/libpq.sgml 2012-08-03 09:39:30.114266570 +0200 +++ postgresql.1/doc/src/sgml/libpq.sgml 2012-10-14 11:12:07.049196259 +0200 @@ -496,6 +496,37 @@ typedef struct + + PQconninfoPQconninfo + + + Returns the connection options used by a live connection. + +PQconninfoOption *PQconninfo(PGconn *conn, bool for_replication); + + + + + Returns a connection options array. This can be used to determine + all possible PQconnectdb options and their + current values that were used to connect to the server. The return + value points to an array of PQconninfoOption + structures, which ends with an entry having a null keyword + pointer. Every notes above for PQconndefaults also apply. + + + + The for_replication argument can be used to exclude some + options from the list which are used by the walreceiver module. + pg_basebackup uses this pre-filtered list + to construct primary_conninfo in the automatically generated + recovery.conf file. + + + + + + PQconninfoParsePQconninfoParse diff -durpN postgresql/src/interfaces/libpq/exports.txt postgresql.1/src/interfaces/libpq/exports.txt --- postgresql/src/interfaces/libpq/exports.txt 2012-10-09 09:58:14.342782974 +0200 +++ postgresql.1/src/interfaces/libpq/exports.txt 2012-10-14 11:12:07.049196259 +0200 @@ -164,3 +164,4 @@ PQsetSingleRowMode161 lo_lseek64162 lo_tell64 163 lo_truncate64 164 +PQconninfo165 diff -durpN postgresql/src/interfaces/libpq/fe-connect.c postgresql.1/src/interfaces/libpq/fe-connect.c --- postgresql/src/interfaces/libpq/fe-connect.c 2012-09-09 08:11:09.470401480 +0200 +++ postgresql.1/src/interfaces/libpq/fe-connect.c 2012-10-14 11:12:07.053196284 +0200 @@ -137,6 +137,9 @@ static int ldapServiceLookup(const char * PQconninfoOptions[] *must* be NULL. In a working copy, non-null "val" * fields point to malloc'd strings that should be freed when the working * array is freed (see PQconninfoFree). + * + * If you add a new connection option to this list, remember to add it to + * PQconninfoMappings[] below. * -- */ static const PQconninfoOption PQconninfoOptions[] = { @@ -264,6 +267,66 @@ static const PQconninfoOption PQconninfo NULL, NULL, 0} }; +/* + * We need a mapping between the PQconninfoOptions[] array + * and PGconn members. We have to keep it separate from + * PQconninfoOptio
Re: [HACKERS] pg_stat_lwlocks view - lwlocks statistics, round 2
(2012/10/14 13:26), Fujii Masao wrote: On Sun, Oct 14, 2012 at 10:46 AM, Satoshi Nagayasu wrote: HEAD number of transactions actually processed: 3439971 tps = 57331.891602 (including connections establishing) tps = 57340.932324 (excluding connections establishing) pg_stat_lwlocks patch (reporting disabled) == number of transactions actually processed: 3429370 tps = 57155.286475 (including connections establishing) tps = 57163.996943 (excluding connections establishing) So, I think some additional hack to reduce reporting is needed. Would it be acceptable in terms of the performance? The tracing lwlock usage seems to still cause a small performance overhead even if reporting is disabled. I believe some users would prefer to avoid such overhead even if pg_stat_lwlocks is not available. It should be up to a user to decide whether to trace lwlock usage, e.g., by using trace_lwlock parameter, I think. Frankly speaking, I do not agree with disabling performance instrument to improve performance. DBA must *always* monitor the performance metrix when having such heavy workload. But it's ok to add a parameter to switch enable/disable it. Any other comments? Another comment is; local_calls/waits/time_ms are really required? I'm not sure how those info would help the performance debugging. I think there are some needs to observe/determine how your test query is affected by the other workload from the other sessions. So, splitting local and shared statistics would be nice to have. Just my thought though. What I don't like is that a session can see only local stats of its own session. It's hard to monitor local stats. Imagine the case where you'd like to monitor local stats of each pgbench session. To monitor such stats, you need to modify pgbench so that its each session monitors its own local stats. Even if you run a monitoring software, it cannot collect those stats because they don't belong to the session that it uses. Ok. I'm waiting more comments from others. Dropping it is easy for me, but any other comments? Josh? Regards, Regards, -- Satoshi Nagayasu Uptime Technologies, LLC. http://www.uptime.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] September 2012 commitfest
On 12 October 2012 20:07, Robert Haas wrote: > On Thu, Oct 11, 2012 at 3:37 PM, Simon Riggs wrote: >> On 11 October 2012 20:30, Robert Haas wrote: >>> On Thu, Oct 11, 2012 at 2:42 PM, Andrew Dunstan wrote: I have a quietish few days starting on Saturday, will be looking at this then. Is it only the Windows aspect that needs reviewing? Are we more or less happy with the rest? >>> >>> I think the Windows issues were the biggest thing, but I suspect there >>> may be a few other warts as well. It's a lot of code, and it's >>> modifying pg_dump, which is an absolute guarantee that it's built on a >>> foundation made out of pure horse manure. >> >> That may be so, but enough people dependent upon it that now I'm >> wondering whether we should be looking to create a new utility >> altogether, or at least have pg_dump_parallel and pg_dump to avoid any >> screw ups with people's backups/restores. > > Well, I think pg_dump may well need a full rewrite to be anything like > sane. But I'm not too keen about forking it as part of adding > parallel dump. I think we can sanely hack this patch into what's > there now. It's liable to be a bit hard to verify, but in the long > run having two copies of the code is going to be a huge maintenance > headache, so we should avoid that. I agree that maintaining 2 copies of the code would be a huge maintenance headache in the long run. I wasn't suggesting we do it for more than 1 release, after which we just have 2 names for same program. I think the phrase "a bit hard to verify" probably isn't good in conjunction with backup utilities, where stability is preferred. I'm not wedded to my suggestion of how we handle the risk of it going wrong, but if we see a risk then we should do something about it. -- Simon Riggs 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] Deprecating RULES
On 13 October 2012 21:15, Joshua Berkus wrote: > Simon, > >> I think its sad we can't even attempt a technical conversation >> without >> you making snide ad hominem attacks that aren't even close to being >> true on a personal level, nor accurate in a technical sense. > > I would prefer it if you actually addressed my substantive arguments, which, > so far, you haven't. I would prefer it if you stuck to your substantive arguments, after reading and understanding others points, which so far, you haven't. So lets now try and stick to technical points. Your substantive argument, as I understand it, is that deprecating something can cause user annoyance at upgrade. I agree with that point, as I would expect everybody to do so. You also mention that 3 years wasn't long enough for some people, but I am unsure as to your point there. It might be that we should take longer than 3 years to deprecate things, or that the same pain will be felt however long we leave it. I think the latter, since the standard conforming strings change didn't go much better even though we took ages to do it. In many people's opinion, RULEs are a strangeness that are seldom used in production and long since should have been removed. Peter shows a paper that details things wrong with them from 15 years ago. Deprecating rules is a much, much smaller change than any of the recent deprecations. Everything else we say needs to have that context. You also mention that we must make noise for at least 18 months before making any change, to avoid race conditions where new users adopt RULEs and are then surprised when we deprecate them. My answer to that is that rules are pretty useless and any reasonable developer will discover that before putting anything live. If they do put it live, they might not have noticed rules are actually broken, so deprecating rules in this way will actually avoid bugs and data corruption for those people. For me, most developers either 1) use ORMs, none of which use RULEs, 2) speak to people in PostgreSQL community/consulting companies - almost all of whom will advise to avoid RULEs or 3) have read books that advise against their use or 4) read that rules are not SQL standard and so wisely avoid unnecessarily non-standard coding. As a result, I think the number of people likely to adopt rules in the near future is approximately zero and the number affected by this change will be very low and unlikely to cause embarrassment for us. IMHO the risk of losing people to other databases seems higher from providing broken features than it does from removing broken features, which seems like useful and proactive management. Since I believe there is something to be lost from maintaining the status quo, and little to be gained from delay, I proposed a way of speeding up the process that allowed a back out plan. Daniel has made the point that we must enforce deprecation without any option of reversion. Given neither of us likes to be hostile to users, I presume we both disagree with him on that? One thing I would like to avoid is providing another GUC for compatibility, since the combinatorial explosion of GUC settings introduces considerable bug risks. Having said that, I've got no particular reason to hurry other than my own personal embarrassment at explaining that, yes, some of our features are broken. But I would like to see actions begin, however long the timescale. If we wish to make some noise, I think docs changes are not enough. Darren's suggestion that doc additions that explain that advice has been backpatched is useful, but we should also make Announcements of impending deprecations as well if we truly wish to make noise. And if we do that, as Daniel points out, sorting out hash indexes at the same time also makes sense. Where rules do exist it seems possible to write simple code to transform them into triggers or views. I'll write some docs to explain. -- Simon Riggs 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] Successor of MD5 authentication, let's use SCRAM
On Sun, Oct 14, 2012 at 5:59 AM, Daniel Farina wrote: > On Sat, Oct 13, 2012 at 7:00 AM, Andrew Dunstan wrote: >> Does Debian they create a self-signed certificate? If so, count me as >> unimpressed. I'd argue that's worse than doing nothing. Here's what the docs >> say (rightly) about such certificates: > > Debian will give you a self signed certificate by default. Protecting > against passive eavesdroppers is not an inconsiderable benefit to get > for "free", and definitely not a marginal attack technique: it's > probably the most common. > > For what they can possibly know about the end user, Debian has it right here. There's a lot of shades of gray to that one. Way too many to say they're right *or* wrong, IMHO. It *does* make people think they have "full ssl security by default", which they *don't*.They do have partial protection, which helps in some (fairly common) scenarios. But if you compare it to the requirements that people *do* have when they use SSL, it usually *doesn't* protect them the whole way - but they get the illusion that it does. Sure, they'd have to read up on the details in order to get secure whether it's on by default or not - that's why I think it's hard to call it either right or wrong, but it's rather somewhere in between. They also enable things like encryption on all localhost connections. I consider that plain wrong, regardless. Though it provides for some easy "performance tuning" for consultants... -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecating RULES
On 12 October 2012 19:48, Greg Stark wrote: > On Fri, Oct 12, 2012 at 7:55 AM, Simon Riggs wrote: >> AFAICS all RULEs can be re-expressed as Triggers or Views. > > This is a bizarre discussion. Firstly this isn't even close to true. > The whole source of people's discontentment is that triggers are *not* > equivalent to rules. If they were then they wouldn't be so upset. This may be a confusion on the point of equivalence; clearly the features work differently. I'm not aware of any rule that can't be rewritten as a trigger or a view. Please can anyone show me some examples of those? Assuming examples exist, do we think that is wide enough to be considered a useful feature, given the other downsides of rules such as not abiding by COPY - which causes data corruption for those who thought rules would always be obeyed. -- Simon Riggs 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] tuplesort memory usage: grow_memtuples
On 17 August 2012 00:26, Peter Geoghegan wrote: > This patch works by not doubling the size of state->memtupsize when > available memory is less than half of allowed memory (i.e. we are one > call to grow_memtuples() away from reporting ). Rather, in that > situation, it is set to a size that extrapolates the likely size of > the total amount of memory needed in a way that is quite flawed, but > likely to work well for the pass-by-value Datum case. Now, on the face > of it, this may appear to be a re-hashing of the question of "how > paranoid do we need to be about wasting memory in memory-constrained > situations - should we consider anything other than a geometric growth > rate, to be parsimonious with memory at the risk of thrashing?". > However, this is not really the case, because this is only a single, > last-ditch effort to avoid a particularly undesirable eventuality. We > have little to lose and quite a bit to gain. A cheap guestimation > seems reasonable. This is a very useful optimisation, for both the low and the high end. The current coding allows full use of memory iff the memory usage is an exact power of 2, so this patch will allow an average of 50% and as much as 100% gain in memory for sorts. We need to be careful to note this as a change on the release notes, since users may well have set work_mem to x2 what was actually needed to get it to work - that means most users will see a sudden leap in actual memory usage, which could lead to swapping in edge cases. I notice that cost_sort doesn't take into account the space used for sorting other than tuple space so apparently requires no changes with this patch. ISTM that cost_sort should be less optimistic about memory efficiency than it is. -- Simon Riggs 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