Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters
On 2011-12-06 17:58, Kevin Grittner wrote: Kevin Grittnerkgri...@wicourts.gov wrote: If there are no objections, I suggest that Yeb implement the mixed notation for cursor parameters. Hearing no objections -- Yeb, are you OK with doing this, and do you feel this is doable for this CF? It is not a large change, I'll be able to do it tomorrow if nothing unexpected happens, or monday otherwise. regards, Yeb -- 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] [DOCS] Moving tablespaces
On Tue, Dec 6, 2011 at 17:07, Tom Lane t...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: There is some nice precedent in the CREATE TABLESPACE command (though dependent on HAVE_SYMLINK and not HAVE_READLINK), so I'm just going to copy the error message from there. Fair enough. Looking at the existing readlink use in port/exec.c, it strikes me that another thing you'd better do is include a check for buffer overrun, ie the test needs to be more like rllen = readlink(fname, link_buf, sizeof(link_buf)); if (rllen 0 || rllen = sizeof(link_buf)) ... fail ... Seems reasonable, yeah. I'll go put a similar check in the basebackup.c file as well when I'm done here. Also, you're assuming that the result is already null-terminated, which is incorrect. No, I'm not - I'm MemSet()ing the whole buffer to 0 before I start. But I'll change that to work the same way as the on in port/exec.c, for consistency. -- 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] [DOCS] Moving tablespaces
On Wed, Dec 7, 2011 at 10:05, Magnus Hagander mag...@hagander.net wrote: On Tue, Dec 6, 2011 at 17:07, Tom Lane t...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: There is some nice precedent in the CREATE TABLESPACE command (though dependent on HAVE_SYMLINK and not HAVE_READLINK), so I'm just going to copy the error message from there. Fair enough. Looking at the existing readlink use in port/exec.c, it strikes me that another thing you'd better do is include a check for buffer overrun, ie the test needs to be more like rllen = readlink(fname, link_buf, sizeof(link_buf)); if (rllen 0 || rllen = sizeof(link_buf)) ... fail ... Seems reasonable, yeah. I'll go put a similar check in the basebackup.c file as well when I'm done here. To close this thread (hopefully): Fixed and applied. -- 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] [COMMITTERS] pgsql: plpython: Add SPI cursor support
On 06/12/11 19:33, Jan Urbański wrote: On 06/12/11 19:23, Tom Lane wrote: Peter Eisentraut pete...@gmx.net writes: plpython: Add SPI cursor support Buildfarm member narwhal does not like this patch. It looks like PyObject_SelfIter is not a compile-time constant on its version of python (2.5, apparently). I'll try to dig around to see what can be causing that... Seems that PyObject_GenericGetIter has been renamed to PyObject_SelfIter at some point: http://hg.python.org/cpython/rev/fd5ef7003469 I'm trying to muster whatever mercurial-foo I have to check if there's been a 2.5 version tagged without that patch... Jan -- 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] [COMMITTERS] pgsql: plpython: Add SPI cursor support
On 07/12/11 11:16, Jan Urbański wrote: On 06/12/11 19:33, Jan Urbański wrote: On 06/12/11 19:23, Tom Lane wrote: Peter Eisentraut pete...@gmx.net writes: plpython: Add SPI cursor support Buildfarm member narwhal does not like this patch. It looks like PyObject_SelfIter is not a compile-time constant on its version of python (2.5, apparently). I'll try to dig around to see what can be causing that... Seems that PyObject_GenericGetIter has been renamed to PyObject_SelfIter at some point: http://hg.python.org/cpython/rev/fd5ef7003469 I'm trying to muster whatever mercurial-foo I have to check if there's been a 2.5 version tagged without that patch... So no, the renaming happened on Mar 17 2003 and 2.5 was tagged on Sep 18 2006. The source tarball for 2.5 contains PyObject_SelfIter. Both fennec and mastodon build OK with Python 2.5. Is it possible that narwhal's Python installation is in some way broken? -- 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] Configuration include directory
On 11/17/2011 11:03 AM, Tom Lane wrote: So as long as the include-directory code path doesn't interfere with tracking that nesting depth, I don't think it needs any extra protection against include-the-same-directory. That was the theory in Magnus's original patch, and I don't believe anything has broken that part; I did glance at it. Since I have a pile of good feedback from Noah now, I'll specifically test this as part of submitting the next patch update. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us -- 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] Inlining comparators as a performance optimisation
On 7 December 2011 03:45, Robert Haas robertmh...@gmail.com wrote: In this regard, I think Heikki's remarks upthread are worth some thought. If inlining is a win just because it avoids saving and restoring registers or allows better instruction scheduling, then inlining is the (probably?) the only way to get the benefit. But if most of the benefit is in having a separate path for the single-sort-key case, we can do that without duplicating the qsort() code and get the benefit for every data type without much code bloat. I'd like to see us dig into that a little, so that we get the broadest possible benefit out of this work. It doesn't bother me that not every optimization will apply to every case, and I don't object to optimizations that are intrinsically narrow (within some reasonable limits). But I'd rather not take what could be a fairly broad-based optimization and apply it only narrowly, all things being equal. I think we're in agreement then. It's important to realise, though I'm sure you do, that once you've done what I did with sorting single scanKey integers, that's it - there isn't really any way that I can think of to speed up quick sorting further, other than parallelism which has major challenges, and isn't particularly appropriate at this granularity. The real significance of inlining is, well, that's it, that's all there is - after inlining, I predict that the well will run dry, or practically dry. Inlining actually is a pretty great win when you look at sorting in isolation, but it's just that there's been a number of other significant wins in my patch, and of course there is overhead from a number of other areas, so perhaps it's easy to lose sight of that. -- 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] documentation issue - extensions
Hello I am trying to create simple extension according to doc http://www.postgresql.org/docs/9.1/interactive/extend-extensions.html I have a problem with line that contains backslash statement -- complain if script is sourced in psql, rather than via CREATE EXTENSION \echo Use CREATE EXTENSION pair to load this file. \quit this content causes a error postgres=# create extension gdlib; ERROR: syntax error at or near \ without this line extension is created fine. Is documentation correct? Regards Pavel -- 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] Configuration include directory
On Wed, Nov 16, 2011 at 06:53, Greg Smith g...@2ndquadrant.com wrote: -Considers all names in that directory that end with *.conf [Discussion concluded more flexibility here would be of limited value relative to how it complicates the implementation] I'd suggest also excluding hidden files -- files that start with a dot. That's how glob/fnmatch functions work and most include all files implementations are based on that. Regards, Marti -- 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] RangeVarGetRelid()
On Tue, Dec 06, 2011 at 04:02:31PM -0500, Robert Haas wrote: On Mon, Dec 5, 2011 at 2:09 AM, Noah Misch n...@leadboat.com wrote: ? ? ? ? ? ? ? AcceptInvalidationMessages(); The above call can go away, now. Doesn't that still protect us against namespace-shadowing issues? RangeVarGetRelid doesn't actually AcceptInvalidationMessages() at the top. It narrows the window for race conditions of that genesis, but isn't doing so an anti-feature? Even if not, doing that _only_ in RemoveRelations() is odd. FWIW, this is fairly independent of your latest patches -- I just happened to notice it due to the incidental quotation. Attached please find a patch with some more fixes on this same general theme. This one tackles renaming of relations, columns, and triggers; and changing the schema of relations. In these cases, the current code does a permissions check before locking the table (which is good) and uses RangeVarGetRelid() to guard against cache lookup failure errors caused by concurrent DDL (also good). However, if the referent of the name changes during the lock wait, we don't recheck permissions; we just allow the rename or schema change on the basis that the user had permission to do it to the relation that formerly had that name. While this is pretty minor as security concerns go, it seems best to clean it up, so this patch does that. I'd suggest limiting callback functions to checks that benefit greatly from preceding the lock acquisition. In these cases, that's only the target object ownership check; all other checks can wait for the lock. Writing correct callback code is relatively tricky; we have no lock, so the available catalog entries can change arbitrarily often as the function operates. It's worth the trouble of navigating that hazard to keep the mob from freely locking all objects. However, if the owner of some_table writes ALTER VIEW some_table RENAME TO foo, I see no commensurate benefit from reporting the relkind mismatch before locking the relation. This would also permit sharing a callback among almost all the call sites you've improved so far. Offhand, I think only ReindexIndex() would retain a bespoke callback. This patch removes two of the three callers of CheckRelationOwnership(), copying some of its logic to two other sites. I'd suggest fixing the third caller (standard_ProcessUtility() for CREATE INDEX) in this same patch. That way, we can either delete the function now or adjust it to permit continued sharing of that code under the new regime. I like how this patch reduces the arbitrary division of authorization checks between alter.c and tablecmds.c. Thanks, nm -- 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_restore --no-post-data and --post-data-only
Note that this feature has the odd effect that some constraints are loaded at the same time as the tables and some are loaded with the post-data. This is consistent with how text-mode pg_dump has always worked, but will seem odd to the user. This also raises the possibility of a future pg_dump/pg_restore optimization. That does seem odd. Why do we do it that way? Beats me. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_stat_statements with query tree based normalization
On Wed, Dec 7, 2011 at 03:19, Peter Geoghegan pe...@2ndquadrant.com wrote: The results are...taking the median value of each set of runs as representative, my patch appears to run marginally faster than head. Of course, there is no reason to believe that it should, and I'm certain that the difference can be explained by noise, even though I've naturally strived to minimise noise. You should use the t-test to distinguish whether two data sets show a consistent difference or whether it's just noise. Excel/OpenOffice have the TTEST() macro for this purpose. For statistics doofuses like me, just pick mode=2 and type=3 as that's the most conservative. If the TTEST result is less than 0.05 then you have 95% certainty that the two dataset are consistently different. If not, you need more consistent data. More information here: http://www.gifted.uconn.edu/siegle/research/t-test/t-test.html Regards, Marti -- 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] Inlining comparators as a performance optimisation
On Tue, Dec 6, 2011 at 8:46 PM, Tom Lane t...@sss.pgh.pa.us wrote: 1. Adding sortsupport infrastructure for more datatypes. 2. Revising nbtree and related code to use this infrastructure. 3. Integrating Peter's work into this framework. I'll try to take care of #1 for at least a few key datatypes before I commit, but I think #2 is best done as a separate patch, so I'll postpone that till later. I see you've committed a chunk of this now. Does it make sense to do #1 for every data type we support, or should we be more selective than that? My gut feeling would be to add it across the board and introduce an opr_sanity check for it. The general utility of adding it to deprecated types like abstime is perhaps questionable, but it strikes me that the value of making everything consistent probably outweighs the cost of a few extra lines of code. Are you planning to do anything about #2 or #3? -- 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] review: CHECK FUNCTION statement
Pavel Stehule wrote: there is a updated patch. it support multi check, options and custom check functions are not supported yet. I don't plan to implement custom check functions in this round - I has not any example of usage - but we have agreement on syntax and behave, so this should not be problem. I changed reporting - from exception to warnings. The patch applies and builds cleanly. The syntax error messages are still inadequate; all I can get is 'syntax error at or near %s'. They should be more detailed. Many other messages and code comments are in bad English. It might be a good idea to add some regression tests for the CHECK FUNCTION ALL variants. Functionality: -- I noticed an oddity: postgres=# CHECK FUNCTION ALL; ERROR: syntax error at or near ; LINE 1: CHECK FUNCTION ALL; ^ postgres=# CHECK FUNCTION ALL IN LANGUAGE plpgsql; NOTICE: nothing to check postgres=# CHECK FUNCTION ALL IN SCHEMA pg_catalog; [prints lots of NOTICEs] According to the syntax diagram and my intuition CHECK FUNCTION ALL without additional clauses should work. Regarding the syntax: I know I suggested it myself, but after several times of typing IN LANGUAGE plpgsql I think that omitting the IN would be better and more like other commands (e.g. CREATE FUNCTION). It is a pity that the CHECK FUNCTION ALL variants will not check trigger functions, but I understand the difficulty -- it would require checking all trigger functions on all tables where they occur in a trigger. I think that the checker function should be shown in psql's \dL+ output. Barring these little gripes, the functionality seems ready for committer from my point of view. Code review: I do not feel competent for a thorough code review. Documentation: -- This is where I see the greatest shortcomings. - The documentation for the system catalog pg_pltemplate should be extended to include tmplchecker. - The documentation patch for CREATE LANGUAGE is plain wrong and contains a syntax error. - CHECK FUNCTION and CHECK TRIGGER should be treated as different SQL statements. It is misleading to have CHECK TRIGGER listed under CHECK FUNCTION. If they have to be together, the statement should be called CHECK and not CHECK TRIGGER, but I think they should be separate. - There is still no documentation patch for plhandler.sgml. I think that at least the documentation should be improved before I am ready to set this as ready for committer. Yours, Laurenz Albe -- 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] Inlining comparators as a performance optimisation
Robert Haas robertmh...@gmail.com writes: On Tue, Dec 6, 2011 at 8:46 PM, Tom Lane t...@sss.pgh.pa.us wrote: 1. Adding sortsupport infrastructure for more datatypes. 2. Revising nbtree and related code to use this infrastructure. 3. Integrating Peter's work into this framework. I'll try to take care of #1 for at least a few key datatypes before I commit, but I think #2 is best done as a separate patch, so I'll postpone that till later. I see you've committed a chunk of this now. Does it make sense to do #1 for every data type we support, or should we be more selective than that? Basically, I tried to do #1 for every datatype for which the comparator was cheap enough that reducing the call overhead seemed likely to make a useful difference. I'm not in favor of adding sortsupport functions where this is not true, as I think it'll be useless code and catalog bloat. I don't want to add 'em for cruft like abstime either. There's some stuff that's debatable according to this criterion --- in particular, I wondered whether it'd be worth having a fast path for bttextcmp, especially if we pre-tested the collate_is_c condition and had a separate version that just hardwired the memcmp code path. (The idea of doing that was one reason I insisted on collation being known at the setup step.) But it would still have to be prepared for detoasting, so in the end I was unenthused. Anyone who feels like testing could try to prove me wrong about it though. Are you planning to do anything about #2 or #3? I am willing to do #2, but not right now; I feel what I need to do next is go review SPGist. I don't believe that #2 blocks progress on #3 anyway. I think #3 is in Peter's court, or yours if you want to do it. (BTW, I agree with your comments yesterday about trying to break down the different aspects of what Peter did, and put as many of them as we can into the non-inlined code paths.) 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] Inlining comparators as a performance optimisation
On Wed, Dec 7, 2011 at 10:09 AM, Tom Lane t...@sss.pgh.pa.us wrote: There's some stuff that's debatable according to this criterion --- in particular, I wondered whether it'd be worth having a fast path for bttextcmp, especially if we pre-tested the collate_is_c condition and had a separate version that just hardwired the memcmp code path. (The idea of doing that was one reason I insisted on collation being known at the setup step.) But it would still have to be prepared for detoasting, so in the end I was unenthused. Anyone who feels like testing could try to prove me wrong about it though. I think that'd definitely be worth investigating (although I'm not sure I have the time to do it myself any time real soon). Are you planning to do anything about #2 or #3? I am willing to do #2, but not right now; I feel what I need to do next is go review SPGist. Yeah, makes sense. That one seems likely to be a challenge to absorb. I don't believe that #2 blocks progress on #3 anyway. I think #3 is in Peter's court, or yours if you want to do it. (BTW, I agree with your comments yesterday about trying to break down the different aspects of what Peter did, and put as many of them as we can into the non-inlined code paths.) Cool. Peter, can you rebase your patch and integrate it into the sortsupport framework that's now committed? -- 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] review: CHECK FUNCTION statement
2011/12/7 Albe Laurenz laurenz.a...@wien.gv.at: Pavel Stehule wrote: there is a updated patch. it support multi check, options and custom check functions are not supported yet. I don't plan to implement custom check functions in this round - I has not any example of usage - but we have agreement on syntax and behave, so this should not be problem. I changed reporting - from exception to warnings. The patch applies and builds cleanly. The syntax error messages are still inadequate; all I can get is 'syntax error at or near %s'. They should be more detailed. this system is based on error messages that generates a plpgsql engine or bison engine. I can correct only a few percent from these messages :( internally I didn't wrote a compiler or plpgsql checker - this is just tool that can emit some plpgsql interpret subprocess - and when these subprocesses raises exceptions, then takes their messages. Many other messages and code comments are in bad English. It might be a good idea to add some regression tests for the CHECK FUNCTION ALL variants. Functionality: -- I noticed an oddity: postgres=# CHECK FUNCTION ALL; ERROR: syntax error at or near ; LINE 1: CHECK FUNCTION ALL; ^ postgres=# CHECK FUNCTION ALL IN LANGUAGE plpgsql; NOTICE: nothing to check postgres=# CHECK FUNCTION ALL IN SCHEMA pg_catalog; [prints lots of NOTICEs] According to the syntax diagram and my intuition CHECK FUNCTION ALL without additional clauses should work. this is question - this will check all functions in postgres.It's 2421 function, so one criterium as minimum should be good idea. We can remove buildin functions from list - so it will check all function in database. Regarding the syntax: I know I suggested it myself, but after several times of typing IN LANGUAGE plpgsql I think that omitting the IN would be better and more like other commands (e.g. CREATE FUNCTION). IN should be syntactic sugar It is a pity that the CHECK FUNCTION ALL variants will not check trigger functions, but I understand the difficulty -- it would require checking all trigger functions on all tables where they occur in a trigger. I think that the checker function should be shown in psql's \dL+ output. Barring these little gripes, the functionality seems ready for committer from my point of view. Code review: I do not feel competent for a thorough code review. Documentation: -- This is where I see the greatest shortcomings. - The documentation for the system catalog pg_pltemplate should be extended to include tmplchecker. - The documentation patch for CREATE LANGUAGE is plain wrong and contains a syntax error. - CHECK FUNCTION and CHECK TRIGGER should be treated as different SQL statements. It is misleading to have CHECK TRIGGER listed under CHECK FUNCTION. If they have to be together, the statement should be called CHECK and not CHECK TRIGGER, but I think they should be separate. - There is still no documentation patch for plhandler.sgml. I think that at least the documentation should be improved before I am ready to set this as ready for committer. please, can you send a correction to documentation or error messages? I am not able to write documentation Regards Pavel Yours, Laurenz Albe -- 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] review: CHECK FUNCTION statement
Pavel Stehule wrote: The syntax error messages are still inadequate; all I can get is 'syntax error at or near %s'. They should be more detailed. this system is based on error messages that generates a plpgsql engine or bison engine. I can correct only a few percent from these messages :( internally I didn't wrote a compiler or plpgsql checker - this is just tool that can emit some plpgsql interpret subprocess - and when these subprocesses raises exceptions, then takes their messages. I see. I think that at least the documentation should be improved before I am ready to set this as ready for committer. please, can you send a correction to documentation or error messages? I am not able to write documentation I'll give it a try. Yours, Laurenz Albe -- 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] Inlining comparators as a performance optimisation
On 7 December 2011 15:15, Robert Haas robertmh...@gmail.com wrote: On Wed, Dec 7, 2011 at 10:09 AM, Tom Lane t...@sss.pgh.pa.us wrote: But it would still have to be prepared for detoasting, so in the end I was unenthused. Anyone who feels like testing could try to prove me wrong about it though. I think that'd definitely be worth investigating (although I'm not sure I have the time to do it myself any time real soon). I'll at least take a look at it. Sorting text is a fairly common case. I'm not hugely enthused about spending too much time on work that will only be useful if collate_is_c. I don't believe that #2 blocks progress on #3 anyway. I think #3 is in Peter's court, or yours if you want to do it. (BTW, I agree with your comments yesterday about trying to break down the different aspects of what Peter did, and put as many of them as we can into the non-inlined code paths.) I'm confident that we should have everything for the simple case of ordering by a single int4 and int8 column, and I think you'd probably agree with that - they're extremely common cases. Anything beyond that will need to be justified, probably in part by running additional benchmarks. Cool. Peter, can you rebase your patch and integrate it into the sortsupport framework that's now committed? Yes, I'd be happy to, though I don't think I'm going to be getting around to it this side of Friday. Since it isn't a blocker, I assume that's okay. The rebased revision will come complete with a well thought-out rationale for my use of inlining specialisations, that takes account of the trade-off against binary bloat that Tom highlighted. I wasn't ignoring that issue, but I did fail to articulate my thoughts there, mostly because I felt the need to do some additional research to justify my position. -- 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] Inlining comparators as a performance optimisation
On Wed, Dec 7, 2011 at 10:58 AM, Peter Geoghegan pe...@2ndquadrant.com wrote: On 7 December 2011 15:15, Robert Haas robertmh...@gmail.com wrote: On Wed, Dec 7, 2011 at 10:09 AM, Tom Lane t...@sss.pgh.pa.us wrote: But it would still have to be prepared for detoasting, so in the end I was unenthused. Anyone who feels like testing could try to prove me wrong about it though. I think that'd definitely be worth investigating (although I'm not sure I have the time to do it myself any time real soon). I'll at least take a look at it. Sorting text is a fairly common case. I'm not hugely enthused about spending too much time on work that will only be useful if collate_is_c. Well, text_pattern_ops is not exactly an uncommon case, but sure. And there might be some benefit for other collations, too. Cool. Peter, can you rebase your patch and integrate it into the sortsupport framework that's now committed? Yes, I'd be happy to, though I don't think I'm going to be getting around to it this side of Friday. Since it isn't a blocker, I assume that's okay. Sounds fine to me. -- 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] Measuring relation free space
On Mon, Nov 28, 2011 at 5:40 AM, Greg Smith g...@2ndquadrant.com wrote: Unless I am missing something, all indexes are handled via a procedure designed for BTree indices, GetBTRelationFreeSpace. I don't know that the ultimate behavior of this is wrong, but it seems unusual. If I get some more time, I will try to explore what is actually going on when called on other types of indexes. This I think I'll punt back toward Jaime, as well as asking did you have a plan for TOAST here? for indexes. it seems pageinspect only deals with btree indexes and i neglected to put a similar limitation on this function... now, because the free space is calculated using PageGetFreeSpace() for indexes it should be doing the right thing for all kind of indexes, i only put the function there because i was trying to avoid to create a new file. But if the function is right for all kind of indexes that's maybe enough to create a new file and rename the helper function so is obvious that it can manage all kind of indexes for toast tables. a simple test here seems to show that is as easy as to add toast tables in the supported objects and treat them as normal pages... or there is something i'm missing? -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación -- 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] documentation issue - extensions
Pavel Stehule pavel.steh...@gmail.com writes: I am trying to create simple extension according to doc http://www.postgresql.org/docs/9.1/interactive/extend-extensions.html I have a problem with line that contains backslash statement -- complain if script is sourced in psql, rather than via CREATE EXTENSION \echo Use CREATE EXTENSION pair to load this file. \quit this content causes a error postgres=# create extension gdlib; ERROR: syntax error at or near \ Are you testing in an up-to-date server? We added the ability to handle that post-9.1.0. 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_restore --no-post-data and --post-data-only
Josh Berkus j...@agliodbs.com writes: Note that this feature has the odd effect that some constraints are loaded at the same time as the tables and some are loaded with the post-data. This is consistent with how text-mode pg_dump has always worked, but will seem odd to the user. This also raises the possibility of a future pg_dump/pg_restore optimization. That does seem odd. Why do we do it that way? Beats me. Performance, mostly --- we prefer to apply checks during the original data load if possible, but for indexes and FK constraints it's faster to apply them later. Also, we can separate constraints from the original table declaration if it's necessary to break a reference circularity. This isn't something that would be wise to whack around. 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] IDLE in transaction introspection
On Tue, Dec 6, 2011 at 6:38 AM, Magnus Hagander mag...@hagander.net wrote: On Sat, Nov 19, 2011 at 02:55, Scott Mead sco...@openscg.com wrote: On Thu, Nov 17, 2011 at 11:58 AM, Scott Mead sco...@openscg.com wrote: On Wed, Nov 16, 2011 at 4:09 PM, Scott Mead sco...@openscg.com wrote: On Tue, Nov 15, 2011 at 1:18 PM, Robert Treat r...@xzilla.net wrote: On Tue, Nov 15, 2011 at 12:00 PM, Greg Smith g...@2ndquadrant.com wrote: On 11/15/2011 09:44 AM, Scott Mead wrote: Fell off the map last week, here's v2 that: * RUNNING = active * all states from CAPS to lower case This looks like what I was hoping someone would add here now. Patch looks good, only issue I noticed was a spaces instead of a tab goof where you set beentry_st_state at line 2419 in src/backend/postmaster/pgstat.c Missing pieces: -There is one regression test that uses pg_stat_activity that is broken now. -The documentation should list the new field and all of the states it might include. That's a serious doc update from the minimal info available right now. V3 Attached: * Updates Documentation -- Adds a separate table to cover each column of pg_stat_activity I like that a lot - we should've done taht years ago :-) For consistency, either both datname and usename should refer to their respective oid, or none of them. application_name should probably have a link to the libpq documentation for said parameter. field is not set should probably be field is NULL Thanks for pointing these out. Boolean, if the query is waiting because of a block / lock reads really strange to me. Boolean indicating if would be a good start, but I'm not sure what you're trying to say with block / lock at all? Yeah, me neither. What about: Boolean indicating if a query is in a wait state due to a conflict with another backend. The way the list of states is built up looks really strange. I think it should be built out of variablelist like other such places instead - but I admit to not having checked what that actually looks like in the output. Agreed. I'll look at variablelist The use of single quotes in the descriptions, things like This is the 'state' of seems wrong. If we should use anything, it should be double quotes, but why do we need double quotes around that? And the reference to think time is just strange, IMO. Yeah, that's a 'Scottism' (see, I used it again :-). I'll clean that up In some other cases, the single quotes should probably be replaced with literal. NB: should probably be note. I am actually looking for some helping in describing the FASTPATH function call state. Any takers? * Adds 'state_change' (timestamp) column -- Tracks when the 'state' column is updated independently of when the 'query' column is updated Very useful. * renames procpid = pid Shouldn't we do this consistently if we do it? It's change in pg_stat_get_activity() but not pg_stat_get_wal_senders()? I think we either change in both functions, or none of them Agreed * Bug Fixes -- If a backend had never before issued a query, another session looking at pg_stat_activity would print command string not enabled in the query column. -- query_start was being updated on a 'state' change, now only updated on query change * Fixed 'rules' regression failure -- Added new columns for pg_stat_activity Also, I think state=-1 needs a symbolic value. STATE_UNDEFINED or something like that. Agreed. There's also a lot of random trailing whitespace in the patch - git diff (which you seem to be using already, that's why I mention it) will happily alert you about that - they should be removed. Or the committer can clean that up of course, but it makes for less work ;) Thanks for pointing that out. The code is starting to look really good, but I think the docs definitely need another round of work. Yeah, I figured they would. Thanks for going through them! -- Scott Mead OpenSCG, http://www.openscg.com -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] pg_restore --no-post-data and --post-data-only
On 12/07/2011 11:31 AM, Tom Lane wrote: Josh Berkusj...@agliodbs.com writes: Note that this feature has the odd effect that some constraints are loaded at the same time as the tables and some are loaded with the post-data. This is consistent with how text-mode pg_dump has always worked, but will seem odd to the user. This also raises the possibility of a future pg_dump/pg_restore optimization. That does seem odd. Why do we do it that way? Beats me. Performance, mostly --- we prefer to apply checks during the original data load if possible, but for indexes and FK constraints it's faster to apply them later. Also, we can separate constraints from the original table declaration if it's necessary to break a reference circularity. This isn't something that would be wise to whack around. Yeah, and if we did want to change it that should be a TODO and not hold up this feature. 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] [v9.2] Fix Leaky View Problem
On Mon, Sep 26, 2011 at 3:59 PM, Tom Lane t...@sss.pgh.pa.us wrote: Kohei KaiGai kai...@kaigai.gr.jp writes: Sorry, are you saying the current (in other words, rte-security_barrier stores the state of reloption) approach is not a good idea? Yes. I think the same as Robert: the way to handle this is to store it in RelOptInfo for the duration of planning, and pull it from the catalogs during planner startup (cf plancat.c). Having looked at this more, I'm starting to believe KaiGai has this part right after all. The trouble is that the rewriter does this: /* * Now, plug the view query in as a subselect, replacing the relation's * original RTE. */ rte = rt_fetch(rt_index, parsetree-rtable); rte-rtekind = RTE_SUBQUERY; rte-relid = InvalidOid; rte-subquery = rule_action; rte-inh = false; /* must not be set for a subquery */ In other words, by the time the planner comes along and tries to decide whether or not it should flatten this subquery, the view has already been rewritten into a subquery - and that subquery is in most respects indistinguishable from a subquery that the user wrote directly. There is one difference: the permission check that would have been done against the view gets attached to the OLD entry in the subquery's range table. It would probably be possible to make this work by having the code paths that need to know whether or not a given subquery originated from a security-barrier-enabled view do that same trick: peek down into the OLD entry in the subquery rangetable, extract the view OID from there, and go check its reloptions. But that seems awfully complicated and error-prone, hence my feeling that just flagging the subquery explicitly is probably a better approach. One other possibility that comes to mind is that, instead of adding bool security_view to the RTE, we could instead add a new RTEKind, something like RTE_SECURITY_VIEW. That would mean going through and finding all the places that refer to RTE_SUBQUERY and adjusting them to handle RTE_SECURITY_VIEW in either the same way or differently as may be appropriate. The possible advantage of this approach is that it doesn't bloat the RTE structure (and stored rules that use it) with an additional attribute that (I think) will always be false - because security_barrier can only be set on a subquery RTE after rewriting has happened, and stored rules are haven't been rewritten yet. It might also force people to think a bit more carefully about how security views should be handled during future code changes, which could also be viewed as a plus. I'm attaching my current version of KaiGai's patch (with substantial cleanup of the comments and documentation, and some other changes) for reference. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company leaky-views-20111207.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [PATCH] pg_test_fsync: Delete temporary file when aborted by a signal
Hi list,
I found a 'pg_test_fsync.out' file in my $PGDATA, which was probably
left around because I aborted pg_test_fsync with ^C back when setting
up the server.
Here's a patch to delete that file via a signal handler for
SIGINT/SIGTERM/SIGHUP.
Not tested on Windows, but should work according to MSDN documentation.
Regards,
Marti
From 5531c177bf108b840563e5aecbf3321fe7efbf87 Mon Sep 17 00:00:00 2001
From: Marti Raudsepp ma...@juffo.org
Date: Wed, 7 Dec 2011 18:40:58 +0200
Subject: [PATCH] pg_test_fsync: Delete temporary file when aborted by a
signal
---
contrib/pg_test_fsync/pg_test_fsync.c | 20
1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/contrib/pg_test_fsync/pg_test_fsync.c b/contrib/pg_test_fsync/pg_test_fsync.c
index 3791f5a..5fe6228 100644
--- a/contrib/pg_test_fsync/pg_test_fsync.c
+++ b/contrib/pg_test_fsync/pg_test_fsync.c
@@ -9,6 +9,7 @@
#include sys/time.h
#include time.h
#include unistd.h
+#include signal.h
#include getopt_long.h
#include access/xlogdefs.h
@@ -44,6 +45,7 @@ static void test_sync(int writes_per_op);
static void test_open_syncs(void);
static void test_open_sync(const char *msg, int writes_size);
static void test_file_descriptor_sync(void);
+static void signal_cleanup(int sig);
#ifdef HAVE_FSYNC_WRITETHROUGH
static int pg_fsync_writethrough(int fd);
@@ -59,6 +61,14 @@ main(int argc, char *argv[])
handle_args(argc, argv);
+ /* Prevent leaving behind the test file */
+ signal(SIGINT, signal_cleanup);
+ signal(SIGTERM, signal_cleanup);
+#ifdef SIGHUP
+ /* Not defined on win32 */
+ signal(SIGHUP, signal_cleanup);
+#endif
+
prepare_buf();
test_open();
@@ -490,6 +500,16 @@ test_non_sync(void)
print_elapse(start_t, stop_t);
}
+static void
+signal_cleanup(int signum)
+{
+ /* Delete the file if it exists. Ignore errors */
+ unlink(filename);
+ /* Finish incomplete line on stdout */
+ puts();
+ exit(signum);
+}
+
#ifdef HAVE_FSYNC_WRITETHROUGH
static int
--
1.7.8
--
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] [v9.2] Fix Leaky View Problem
Robert Haas robertmh...@gmail.com writes: Having looked at this more, I'm starting to believe KaiGai has this part right after all. Yeah, you have a point. The rewriter is intentionally trying to make an expanded view look just the same as an in-line SELECT-in-FROM, and we need it to be easier to distinguish them. One other possibility that comes to mind is that, instead of adding bool security_view to the RTE, we could instead add a new RTEKind, something like RTE_SECURITY_VIEW. That would mean going through and finding all the places that refer to RTE_SUBQUERY and adjusting them to handle RTE_SECURITY_VIEW in either the same way or differently as may be appropriate. The possible advantage of this approach is that it doesn't bloat the RTE structure (and stored rules that use it) with an additional attribute that (I think) will always be false - because security_barrier can only be set on a subquery RTE after rewriting has happened, and stored rules are haven't been rewritten yet. It might also force people to think a bit more carefully about how security views should be handled during future code changes, which could also be viewed as a plus. Hmm. The question is whether the places where we need to care about this would naturally be looking at RTEKind anyway. If they are, or many are, then I think this might be a good idea. However if a lot of the action is elsewhere then I don't know if we get much leverage from the new RTEKind. I haven't read the patch lately so can't opine on that. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [v9.2] Fix Leaky View Problem
On Wed, Dec 7, 2011 at 1:45 PM, Tom Lane t...@sss.pgh.pa.us wrote: One other possibility that comes to mind is that, instead of adding bool security_view to the RTE, we could instead add a new RTEKind, something like RTE_SECURITY_VIEW. That would mean going through and finding all the places that refer to RTE_SUBQUERY and adjusting them to handle RTE_SECURITY_VIEW in either the same way or differently as may be appropriate. The possible advantage of this approach is that it doesn't bloat the RTE structure (and stored rules that use it) with an additional attribute that (I think) will always be false - because security_barrier can only be set on a subquery RTE after rewriting has happened, and stored rules are haven't been rewritten yet. It might also force people to think a bit more carefully about how security views should be handled during future code changes, which could also be viewed as a plus. Hmm. The question is whether the places where we need to care about this would naturally be looking at RTEKind anyway. If they are, or many are, then I think this might be a good idea. However if a lot of the action is elsewhere then I don't know if we get much leverage from the new RTEKind. I haven't read the patch lately so can't opine on that. *reads through the code* It looks to me like most places that look at RTE_SUBQUERY really have no reason to care about this. So probably it's just as well to have a separate flag for it. -- 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] const correctness
On ons, 2011-11-09 at 21:15 +, Thomas Munro wrote: I've attached a new patch, which simply adds the keyword 'const' in lots of places, no new functions etc. This version generates no warnings under -Wcast-qual (now that I've read Peter E's thread and been inspired to fix up some places that previously cast away const) for all code under backend/nodes. To achieve that I had to stray outside backend/nodes and change get_leftop and get_rightop (from clauses.h). Patch committed. -- 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] documentation issue - extensions
2011/12/7 Tom Lane t...@sss.pgh.pa.us: Pavel Stehule pavel.steh...@gmail.com writes: I am trying to create simple extension according to doc http://www.postgresql.org/docs/9.1/interactive/extend-extensions.html I have a problem with line that contains backslash statement -- complain if script is sourced in psql, rather than via CREATE EXTENSION \echo Use CREATE EXTENSION pair to load this file. \quit this content causes a error postgres=# create extension gdlib; ERROR: syntax error at or near \ Are you testing in an up-to-date server? We added the ability to handle that post-9.1.0. it was tested on 9.1.1 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] documentation issue - extensions
Pavel Stehule pavel.steh...@gmail.com writes: 2011/12/7 Tom Lane t...@sss.pgh.pa.us: Pavel Stehule pavel.steh...@gmail.com writes: I have a problem with line that contains backslash statement Are you testing in an up-to-date server? We added the ability to handle that post-9.1.0. it was tested on 9.1.1 [ checks release notes... ] Well, we added it in 9.1.2, so there you are. 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] documentation issue - extensions
2011/12/7 Tom Lane t...@sss.pgh.pa.us: Pavel Stehule pavel.steh...@gmail.com writes: 2011/12/7 Tom Lane t...@sss.pgh.pa.us: Pavel Stehule pavel.steh...@gmail.com writes: I have a problem with line that contains backslash statement Are you testing in an up-to-date server? We added the ability to handle that post-9.1.0. it was tested on 9.1.1 [ checks release notes... ] Well, we added it in 9.1.2, so there you are. I can upgrade - it's not problem - just it is surprise for me - because documentation is not related to mayor version. Maybe this issue can be documented somewhere 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
Re: [HACKERS] documentation issue - extensions
Pavel Stehule pavel.steh...@gmail.com wrote: I can upgrade - it's not problem - just it is surprise for me - because documentation is not related to mayor version. Maybe this issue can be documented somewhere Don't the release notes, mentioning that the bug is fixed in 9.1.2, cover that? -Kevin -- 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] ecmascript 5 DATESTYLE
On tis, 2011-12-06 at 15:15 -0500, Tom Lane wrote: TBH, I think that inventing a new datestyle setting ECMA would be a more appropriate investment of effort. So we'd have a setting called ECMA that's really ISO, and a setting called ISO that's really SQL, and a setting called SQL that's really Postgres, and a setting called Postgres that's also Postgres but different. -- 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] ecmascript 5 DATESTYLE
On Dec 7, 2011, at 3:56 PM, Peter Eisentraut wrote: On tis, 2011-12-06 at 15:15 -0500, Tom Lane wrote: TBH, I think that inventing a new datestyle setting ECMA would be a more appropriate investment of effort. So we'd have a setting called ECMA that's really ISO, and a setting called ISO that's really SQL, and a setting called SQL that's really Postgres, and a setting called Postgres that's also Postgres but different. ...and a setting called XSD that's also ISO. for now i'm backing away from the ECMA option - what i was thinking of would be exactly the same as XSD except rather than a timezone of '+00:00' it would be a 'Z'. from some quick searching, it seems that XSD should be capable of understanding 'Z' rather than '+00:00' so if i was going to do anything i'd work towards making that change to 'XSD'. however, as it turns out, the constraint i have that is requiring me to use 'Z' is not actually from ECMAScript 5 but from json-schema (http://tools.ietf.org/html/draft-zyp-json-schema-03#section-5.23 ). XSD is fully compatible with ECMAScript 5 date time string format (http://es5.github.com/#x15.9.1.15 ) so i'm going to sit on this again for a little while and think some more. maybe try to convince json-schema to relax their definition of date-time format. i'll be back when i have a clear picture of what i think makes the most sense. thanks, ben...
Re: [HACKERS] documentation issue - extensions
2011/12/7 Kevin Grittner kevin.gritt...@wicourts.gov: Pavel Stehule pavel.steh...@gmail.com wrote: I can upgrade - it's not problem - just it is surprise for me - because documentation is not related to mayor version. Maybe this issue can be documented somewhere Don't the release notes, mentioning that the bug is fixed in 9.1.2, cover that? It is little bit difficult detect this issue as bug Regards Pavel -Kevin -- 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] ecmascript 5 DATESTYLE
2011/12/7 ben hockey neonstalw...@gmail.com: On Dec 7, 2011, at 3:56 PM, Peter Eisentraut wrote: On tis, 2011-12-06 at 15:15 -0500, Tom Lane wrote: TBH, I think that inventing a new datestyle setting ECMA would be a more appropriate investment of effort. So we'd have a setting called ECMA that's really ISO, and a setting called ISO that's really SQL, and a setting called SQL that's really Postgres, and a setting called Postgres that's also Postgres but different. ...and a setting called XSD that's also ISO. for now i'm backing away from the ECMA option - what i was thinking of would be exactly the same as XSD except rather than a timezone of '+00:00' it would be a 'Z'. from some quick searching, it seems that XSD should be capable of understanding 'Z' rather than '+00:00' so if i was going to do anything i'd work towards making that change to 'XSD'. however, as it turns out, the constraint i have that is requiring me to use 'Z' is not actually from ECMAScript 5 but from json-schema (http://tools.ietf.org/html/draft-zyp-json-schema-03#section-5.23). XSD is fully compatible with ECMAScript 5 date time string format (http://es5.github.com/#x15.9.1.15) so i'm going to sit on this again for a little while and think some more. maybe try to convince json-schema to relax their definition of date-time format. i'll be back when i have a clear picture of what i think makes the most sense. please do it - we still would to have JSON support, so date style can be processed together Regards Pavel thanks, ben... -- 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] Caching for stable expressions with constant arguments v3
On Wed, Dec 7, 2011 at 00:29, Marti Raudsepp ma...@juffo.org wrote: ExecInitExpr enables the cache when its 'PlanState *parent' attribute isn't NULL [...] On the other hand, a few places lose caching support this way since they don't go through the planner: * Column defaults in a COPY FROM operation. Common use case is 'timestamp default now()' This might be a significant loss in some data-loading scenarios. * ALTER TABLE t ALTER col TYPE x USING some_expr(); No big loss here. Let me rephrase that as a question: Does it seem worthwhile to add a new argument to ExecInitExpr to handle those two cases? Does relying on the PlanState argument being NULL seem like a bad idea for any reason? PS: I forgot to mention that 2 test cases covering the two above query types are deliberately left failing in the v4-wip patch. Regards, Marti -- 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] Caching for stable expressions with constant arguments v3
Marti Raudsepp ma...@juffo.org writes: Let me rephrase that as a question: Does it seem worthwhile to add a new argument to ExecInitExpr to handle those two cases? Possibly. Another way would be to keep its API as-is and introduce a different function name for the other behavior. I would think that we'd always know for any given caller which behavior we need, so a flag as such isn't notationally helpful. Does relying on the PlanState argument being NULL seem like a bad idea for any reason? Yes, that seemed like a pretty horrid idea when I read your description, but I hadn't got round to looking at just how awful it might be. 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] documentation issue - extensions
On Wed, Dec 7, 2011 at 4:37 PM, Pavel Stehule pavel.steh...@gmail.com wrote: 2011/12/7 Kevin Grittner kevin.gritt...@wicourts.gov: Pavel Stehule pavel.steh...@gmail.com wrote: I can upgrade - it's not problem - just it is surprise for me - because documentation is not related to mayor version. Maybe this issue can be documented somewhere Don't the release notes, mentioning that the bug is fixed in 9.1.2, cover that? It is little bit difficult detect this issue as bug It's not a bug. It's an not-forward-compatible behavior change in a minor release. -- 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] [v9.2] Fix Leaky View Problem
On Sat, Dec 3, 2011 at 3:19 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote: I rebased my patch set. New functions in pg_proc.h prevented to apply previous revision cleanly. Here is no functional changes. I was thinking that my version of this (attached to an email from earlier today) might be about ready to commit. But while I was trolling through the archives on this problem trying to figure out who to credit, I found an old complaint of Tom's that we never fixed, and which represents a security exposure for this patch: rhaas=# create table foo (a integer); CREATE TABLE rhaas=# insert into foo select generate_series(1,10); INSERT 0 10 rhaas=# insert into foo values (1); INSERT 0 1 rhaas=# analyze foo; ANALYZE rhaas=# create view safe_foo with (security_barrier) as select * from foo where a 5; CREATE VIEW rhaas=# grant select on safe_foo to bob; GRANT Secure in the knowledge that Bob will only be able to see rows where a is 6 or higher, we go to bed. But Bob finds a way to outsmart us: rhaas= create or replace function leak(integer,integer) returns boolean as $$begin raise notice 'leak % %', $1, $2; return false; end$$ language plpgsql; CREATE FUNCTION rhaas= create operator !! (procedure = leak, leftarg = integer, rightarg = integer, restrict = eqsel); CREATE OPERATOR rhaas= explain select * from safe_foo where a !! 0; NOTICE: leak 1 0 QUERY PLAN - Subquery Scan on safe_foo (cost=0.00..2.70 rows=1 width=4) Filter: (safe_foo.a !! 0) - Seq Scan on foo (cost=0.00..1.14 rows=6 width=4) Filter: (a 5) (4 rows) OOPS. The *executor* has been persuaded not to apply the possibly-nefarious operator !! to the data until after applying the security-critical qual a 5. But the *planner* has no such compunctions, and has cheerfully leaked the most common value in the table, which the user wasn't supposed to see. I guess it's hopeless to suppose that we're going to completely conceal the list of MCVs from the user, since it might change the plan - and even if ProcessUtility_hook or somesuch is used to disable EXPLAIN, the user can still try to ferret out the MCVs via a timing attack. That having been said, the above behavior doesn't sit well with me: letting the user probe for MCVs via a timing attack or a plan change is one thing; printing them out on request is a little bit too convenient for my taste. :-( -- 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] pg_dump --exclude-table-data
On 11/04/2011 10:21 AM, Robert Haas wrote: A slightly updated patch is attached, the main change being that I removed use of a short option and only support the long name option. -D didn't seem sufficiently mnemonic to me. I'll add this to the November commitfest, but I'd like to get it committed ASAP as it will simplify setting up the -pre and -post data patches. Instead of: do NOT dump data for the named table(s) How about: dump only schema for the named table(s) I have no great objection to the wording change. I'm also a bit concerned about the relationship between this and the existing -s option. It seems odd that you use --schema-only to get the behavior database-wide, and --exclude-table-data to get it for just one table. Is there some way we can make that a bit more consistent? It's consistent, and was designed to be, with the --exclude-table option. I'm not sure what you want it to look like instead. But TBH I'm more interested in getting the functionality than in how it's spelled. 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] documentation issue - extensions
2011/12/8 Robert Haas robertmh...@gmail.com: On Wed, Dec 7, 2011 at 4:37 PM, Pavel Stehule pavel.steh...@gmail.com wrote: 2011/12/7 Kevin Grittner kevin.gritt...@wicourts.gov: Pavel Stehule pavel.steh...@gmail.com wrote: I can upgrade - it's not problem - just it is surprise for me - because documentation is not related to mayor version. Maybe this issue can be documented somewhere Don't the release notes, mentioning that the bug is fixed in 9.1.2, cover that? It is little bit difficult detect this issue as bug It's not a bug. It's an not-forward-compatible behavior change in a minor release. so this can be mentioned in documentation elsewhere than release notes - the best is near related examples - it is different behave than postgresql's user expect. I understand to reason now - and it has sense - but it is surprising - not all has newer postgres - this version is mostly in development environments than production in this moment. Regards Pavel -- 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] Lots of FSM-related fragility in transaction commit
Joseph Shraibman reported some rather unpleasant behavior that seems to be due to trying to drop a table whose FSM file has got no permissions: http://archives.postgresql.org/pgsql-general/2011-12/msg00246.php One question is how the file got that way, and whether Postgres did anything wrong to cause it to not have permissions. But what I'm on the warpath about now is the abysmally bad error response. What ought to happen if we fail to delete a file during a DROP TABLE is a WARNING, no more. Not a PANIC that recurs during crash recovery. I tried to reproduce this, and found a different but equally bad behavior: regression=# create table foo as select * from tenk1; SELECT 1 regression=# delete from foo where unique1%10 = 0; DELETE 1000 regression=# vacuum foo; VACUUM regression=# select 'foo'::regclass::oid; oid --- 52860 (1 row) [ go and chmod 000 52860_fsm ] regression=# drop table foo; WARNING: AbortTransaction while in COMMIT state ERROR: could not open file base/27666/52860_fsm: Permission denied PANIC: cannot abort transaction 7413, it was already committed ERROR: could not open file base/27666/52860_fsm: Permission denied PANIC: cannot abort transaction 7413, it was already committed The connection to the server was lost. Attempting reset: Failed. I thought that removing the unreadable file would let me restart, but after rm 52860_fsm and trying again to start the server, there's a different problem: LOG: database system was interrupted while in recovery at 2011-12-08 00:56:11 EST HINT: This probably means that some data is corrupted and you will have to use the last backup for recovery. LOG: database system was not properly shut down; automatic recovery in progress LOG: consistent recovery state reached at 0/5D71BA8 LOG: redo starts at 0/5100050 WARNING: page 18 of relation base/27666/52860 is uninitialized CONTEXT: xlog redo visible: rel 1663/27666/52860; blk 18 PANIC: WAL contains references to invalid pages CONTEXT: xlog redo visible: rel 1663/27666/52860; blk 18 LOG: startup process (PID 14507) was terminated by signal 6 LOG: aborting startup due to startup process failure Note that this isn't even the same symptom Shraibman hit, since apparently he was failing on replaying the commit record. How is it that the main table file managed to have uninitialized pages? I suspect that redo visible WAL processing is breaking one or more of the fundamental WAL rules, perhaps by not generating a full-page image when it needs to. So this is really a whole lot worse than our behavior was in pre-FSM days, and it needs to get fixed. 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] Tracking latest timeline in standby mode
We are using 9.1., We have a set up like a master and 2 standby servers. M -- S1,S2 . Both standby S1 and S2 share the same archive. Master will have an Virtual IP. Both stand by servers will be replicated using this virtual ip. Assume the master fails,using our heart beat mechanism Virtual IP bound to S1(if S1 is ahead or equal to S2 XLOG)., Is it required to copy the time line history file that is generated at time of S1 promotion as master to the archive directory of S2 for replication to work (i.e S1(new master) to S2.) Without doing this history file copy from S1 to S2, S2 keeps throwing the following error message., 2011-12-07 17:29:46 IST::@:[18879]:FATAL: could not receive data from WAL stream: server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. cp: cannot stat `../archive/00010005': No such file or directory 2011-12-07 17:29:49 IST::@:[18875]:LOG: record with zero length at 0/5D8FFC0 cp: cannot stat `../archive/00010005': No such file or directory cp: cannot stat `../archive/0002.history': No such file or directory 2011-12-07 17:29:49 IST::@:[20362]:FATAL: timeline 2 of the primary does not match recovery target timeline 1 cp: cannot stat `../archive/00010005': No such file or directory cp: cannot stat `../archive/00010005': No such file or directory cp: cannot stat `../archive/0002.history': No such file or directory 2011-12-07 17:29:54 IST::@:[20367]:FATAL: timeline 2 of the primary does not match recovery target timeline 1 cp: cannot stat `../archive/00010005': No such file or directory cp: cannot stat `../archive/00010005': No such file or directory cp: cannot stat `../archive/0002.history': No such file or directory -- View this message in context: http://postgresql.1045698.n5.nabble.com/Tracking-latest-timeline-in-standby-mode-tp3238829p5057733.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
