Re: [COMMITTERS] pgsql: Tag 9.1rc1.
On 19 August 2011 15:15, Magnus Hagander wrote: > May I humbly suggest that we actually start calling it "stamp" > instead, to make it very clear that this is a different operation from > the "git tag" operation that's done on the tree a bit later? > Reasonable? +1 I agree that the ambiguity is pretty confusing, and unnecessarily so. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Properly call strerror() in thread test; add comments.
Why have you removed the GetLastError() call? MSDN says that "The return value is the calling thread's last-error code". -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: plpython: Add SPI cursor support
On 9 January 2012 17:06, Dave Page wrote: > Is there a way I can get gcc to spit out the expanded definition in a > readable format that you know of? Yes. Figure out what flags gcc is given when building the TU. Then, add the -E flag and see what is generated: http://www.network-theory.co.uk/docs/gccintro/gccintro_36.html -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Make bgwriter sleep longer when it has no work to do, to save el
On 26 January 2012 16:43, Heikki Linnakangas wrote: > Make bgwriter sleep longer when it has no work to do, to save electricity. Perhaps this is pedantic, but shouldn't we be initialising bgwriterLatch to NULL within InitProcGlobal(), much as we already do for the other pointers in that structure? -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Don't install hstore--1.0.sql any more.
On 23 February 2012 16:25, Tom Lane wrote: > Robert Haas writes: >> Don't install hstore--1.0.sql any more. >> Since the current version is 1.1, the 1.0 file isn't really needed. We do >> need the 1.0--1.1 upgrade file, so people on 1.0 can upgrade. > > Shouldn't this commit have removed the 1.0 file from git altogether? > It's quite useless if it's not going to get installed. It's worth noting that the recent commit "Make EXPLAIN (BUFFERS) track blocks dirtied, as well as those written" did not remove pg_stat_statements--1.0.sql either. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: More duplicate word removal.
On 2 May 2012 14:43, Thom Brown wrote: > "in" looks like an unnecessary duplicate, but maybe someone who speaks > Italian can confirm. Gabriele Bartolini, a prominent contributor to the .it translation and a colleague of mine, confirms privately that it's an error. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Further corrections from the department of redundancy department
I think I figured out why these went unnoticed for so long: http://i.imgur.com/TnwtZ.png -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Improve tests for postmaster death in auxiliary processes.
On 10 May 2012 05:55, Tom Lane wrote: > In checkpointer and walwriter, avoid calling PostmasterIsAlive unless > WaitLatch has reported WL_POSTMASTER_DEATH. This saves a kernel call per > iteration of the process's outer loop, which is not all that much, but a > cycle shaved is a cycle earned. I had already removed the unconditional > PostmasterIsAlive calls in bgwriter and pgstat in previous patches, but > forgot that WL_POSTMASTER_DEATH is supposed to be treated as untrustworthy > (per comment in unix_latch.c); so adjust those two cases to match. I'm not sure why we're pushing the responsibility to call PostmasterIsAlive() onto latch clients. Why not just do it within WaitLatchOrSocket just as the WL_POSTMASTER_DEATH bit is set? It's not as if someone could conceivably care that the Postmaster might have died, but not enough to want to be sure. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Make WaitLatch's WL_POSTMASTER_DEATH result trustworthy; simplif
On 10 May 2012 19:35, Tom Lane wrote: > Remove weasel wording about falsely-set result bits from > WaitLatch's API contract. Aren't those weasel words still applicable to the case where sock != PGINVALID_SOCKET ? -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Remove reviewers from 9.2 release notes; improve attributions.
On 22 May 2012 22:17, Bruce Momjian wrote: > Remove reviewers from 9.2 release notes; improve attributions. I am unhappy with now being listed as a secondary author of pg_stat_statements normalisation. I am credited as a co-author within the docs (alongside the original 8.4 author, Takahiro Itagaki), and am solely credited within the original commit message. Tom did of course clean up the patch, but most of my earlier design decisions, many of which went against the grain until very late in the last commitfest, remain. These include: * The whole decision as to what type of tree to hash. I was strongly urged to first consider hashing plan trees, and then later the raw parse tree. I more-or-less stood by my earliest decision to hash a serialisation of the Query struct (though switched to pre rather than post rewrite stage hashing). * The jumble concept (central to how the feature works), macro infrastructure, etc. * The decision to not use the query_tree_walker infrastructure when walking trees. * The whole idea of using the low-level scanner API to calculate the lengths of constants, which were not directly available from the core system. * The approach to managing sticky-entry decay, and indeed the very idea of sticky entries. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Mention Peter Geoghegan as primary author of pg_stat_statements
On 23 May 2012 15:12, Bruce Momjian wrote: > Mention Peter Geoghegan as primary author of pg_stat_statements changes. Thank you. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Introduce timeout handling framework
On 17 July 2012 14:43, Andrew Dunstan wrote: > This seems to have broken Windows builds. (And if people need reminding, > cross-compiling is pretty easy: > <http://people.planetpostgresql.org/andrew/index.php?/archives/264-Cross-compiling-PostgreSQL-for-WIndows.html> > ) Perhaps I'm asking a naive question, but wouldn't it be easier if people had the opportunity to use the buildfarm without actually committing something? For example, perhaps the buildfarm could be made to run on a staging branch. Commits would actually be made to the staging branch. If and when the regression tests pass, the infrastructure then pushes the staging branch commit onto the master branch, and the commit is really committed - the -commiters list is now informed of this. If there is a problem with the buildfarm, the committer receives an e-mail informing them of this. The commit is non-destructively reverted on the staging branch. I don't know that it's worth worrying about, nor if the turnaround on having a commit not break the buildfarm would be generally acceptable in this situation. It would be nice to keep commit history cleaner, though. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Revert "commit_delay" change; just add comment that we don't hav
On 14 August 2012 21:26, Bruce Momjian wrote: > Revert "commit_delay" change; just add comment that we don't have > a microsecond specification. I think that if we eventually decide to change the name of commit_delay for 9.3 (you previously suggested that that might be revisited), it will be reasonable to have the new GUC in units of milliseconds. After all, we've already been switching to milliseconds across various statistics views, including pg_stat_statements. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Revert "commit_delay" change; just add comment that we don't hav
On 15 August 2012 05:15, Tom Lane wrote: > Peter Geoghegan writes: >> On 14 August 2012 21:26, Bruce Momjian wrote: >>> Revert "commit_delay" change; just add comment that we don't have >>> a microsecond specification. > >> I think that if we eventually decide to change the name of >> commit_delay for 9.3 (you previously suggested that that might be >> revisited), it will be reasonable to have the new GUC in units of >> milliseconds. > > Well, the reason why it's like that at all is the thought that values > of less than 1 millisecond might be useful. Are we prepared to suppose > that that is not and never will be true? I think that the need for sub-millisecond granularity had more to do with historic quirks of our implementation. Slight tweaks accidentally greatly improved throughput, if only for the synthetic benchmark in question. I personally have not seen any need for a sub-millisecond granularity when experimenting with the setting on 9.3-devel. However, I am not sure that sub-millisecond granularity could never be of any use, in squeezing the last small increase in throughput made possible by commit_delay. Importantly, feedback as the GUC is tuned is far more predictable than it was with the prior implementation, so this does seem quite unimportant. Why does commit_delay have to be an integer? Can't we devise a way of manipulating it in units of milliseconds, but have the internal representation be a double, as with pg_stat_statements' total_time column? That would allow very fine tuning of the delay. As I've outlined, I'm not sure that it's worth supporting such fine-tuning with the new implementation. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Disable _FORTIFY_SOURCE with ICC
On 30 September 2012 01:07, Peter Eisentraut wrote: > Disable _FORTIFY_SOURCE with ICC I'm having some problems of my own with this. Specifically: gcc -O0 -g -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -g -I../../src/port -DFRONTEND -I../../src/include -D_GNU_SOURCE -D_FORTIFY_SOURCE=2 -I/usr/include/libxml2 -c -o qsort.o qsort.c -MMD -MP -MF .deps/qsort.Po In file included from /usr/include/stdio.h:28:0, from ../../src/include/c.h:67, from qsort.c:46: /usr/include/features.h:314:4: warning: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Wcpp] ... [peter@peterlaptop port]$ gcc --version gcc (GCC) 4.7.2 20120921 (Red Hat 4.7.2-2) ... -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Add pg_xlogdump contrib program
On 22 February 2013 19:58, Alvaro Herrera wrote: > Add pg_xlogdump contrib program I feel slightly silly reporting this, but you probably should have updated the copyright years to 2013 before committing. -- Regards, Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Reorder 9.3 release note items
On Sun, Apr 21, 2013 at 12:57 AM, Simon Riggs wrote: > * Improve memory usage for in-memory sorts - IMHO this should read > "Allow in-memory sorts to use their full memory allocation", which > then explains why users may see higher memory usage and why they > should think about altering settings +1. As I believe you pointed out at the time of the development of this feature, it has the potential to greatly increase the amount of memory allocated by tuplesort, but only within the actual bounds specified by the DBA. There needs to be a clear warning of that - it's easy to imagine that DBAs are specifying a high value of work_mem to compensate for the previous implementation's tendency to under provision. > * "Require superuser privileges to set commit_delay" > Meaning of commit_delay has changed and users should review their > settings, if used +1 -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: pg_test_fsync: update output to show usecs/op clearer
On Thu, May 2, 2013 at 7:27 AM, Bruce Momjian wrote: > pg_test_fsync: update output to show usecs/op clearer I don't think that this is an improvement. Not everyone knows that usec is shorthand for microsecond. If I Google usec, I see something about the United States Enrichment Corporation, but that's about it. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: 9.3 release notes: move compatibility items into their own sect
On Fri, May 3, 2013 at 6:11 PM, Bruce Momjian wrote: > 9.3 release notes: move compatibility items into their own section The commit_delay improvements should stay under performance, where I previously indicated they should be - it's clearly a notable new feature (commit_delay is way more effective now), as opposed to a tweak to the semantics of an existing feature. The fact that the behavior was changed could be separately noted, alongside the fact that Simon made commit_delay PGC_POSTMASTER (this is already separately noted anyway). Similarly, I think that this is a new feature that needs a separate compatibility note: Allow in-memory sorts to use their full memory allocation (Jeff Janes) It's possible that people were previously over-allocating memory to compensate for the server's former unwillingness to make full use of work_mem. You should specifically warn against that. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Add support for ICU 4.2
On Sat, Aug 5, 2017 at 6:36 AM, Peter Eisentraut wrote: > In ICU 4.2, ucol_getKeywordsForLocale() sometimes returns values that > will not be accepted by uloc_toLanguageTag(). Skip loading keyword > variants in that version. This should be spelled ucol_getKeywordValuesForLocale() in both the commit message and the code comment. There is no ucol_getKeywordsForLocale() function. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix transient mdsync() errors of truncated relations due to 72a9
On Wed, May 4, 2016 at 1:59 AM, Andres Freund wrote: > The cleanest fix seems to be to allow the caller of _mdfd_getseg() to > specify whether checks for RELSEG_SIZE are performed. To allow doing so, > change the ExtensionBehavior enum into a bitmask. Besides allowing for > the addition of EXTENSION_DONT_CHECK_SIZE, this makes for a nicer > implementation of EXTENSION_REALLY_RETURN_NULL. You missed a remaining reference to EXTENSION_REALLY_RETURN_NULL within _mdfd_getseg(). -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
On Thu, Sep 28, 2017 at 7:47 AM, Alvaro Herrera wrote:
> Fix freezing of a dead HOT-updated tuple
If I run Dan Wood's test case again, the obvious symptom (spurious
duplicates) goes away. However, the enhanced amcheck, and thus CREATE
INDEX/REINDEX, still isn't happy about this:
postgres=# select bt_index_check('t_pkey', true);
DEBUG: 0: verifying presence of required tuples in index "t_pkey"
LOCATION: bt_check_every_level, verify_nbtree.c:424
ERROR: XX000: failed to find parent tuple for heap-only tuple at
(0,6) in table "t"
LOCATION: IndexBuildHeapRangeScan, index.c:2597
Time: 3.699 ms
--
Peter Geoghegan
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Extend & revamp pg_bswap.h infrastructure.
On Fri, Sep 29, 2017 at 5:29 PM, Andres Freund wrote:
> Extend & revamp pg_bswap.h infrastructure.
This looks wrong:
+static inline uint16
+pg_bswap64(uint16 x)
+{
+ return
+ ((x << 56) & UINT64CONST(0xff00)) |
+ ((x << 40) & UINT64CONST(0x00ff)) |
+ ((x << 24) & UINT64CONST(0xff00)) |
+ ((x << 8) & UINT64CONST(0x00ff)) |
+ ((x >> 8) & UINT64CONST(0xff00)) |
+ ((x >> 24) & UINT64CONST(0x00ff)) |
+ ((x >> 40) & UINT64CONST(0x0000ff00)) |
+ ((x >> 56) & UINT64CONST(0x00ff));
+}
--
Peter Geoghegan
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix traversal of half-frozen update chains
On Fri, Oct 6, 2017 at 8:29 AM, Alvaro Herrera wrote:
> Fix traversal of half-frozen update chains
I have a question about this (this is taken from the master branch):
> bool
> HeapTupleUpdateXmaxMatchesXmin(TransactionId xmax, HeapTupleHeader htup)
> {
> TransactionId xmin = HeapTupleHeaderGetXmin(htup);
>
> /*
> * If the xmax of the old tuple is identical to the xmin of the new one,
> * it's a match.
> */
> if (TransactionIdEquals(xmax, xmin))
> return true;
>
> /*
> * If the Xmin that was in effect prior to a freeze matches the Xmax,
> * it's good too.
> */
> if (HeapTupleHeaderXminFrozen(htup) &&
> TransactionIdEquals(HeapTupleHeaderGetRawXmin(htup), xmax))
> return true;
>
> /*
> * When a tuple is frozen, the original Xmin is lost, but we know it's a
> * committed transaction. So unless the Xmax is InvalidXid, we don't know
> * for certain that there is a match, but there may be one; and we must
> * return true so that a HOT chain that is half-frozen can be walked
> * correctly.
> *
> * We no longer freeze tuples this way, but we must keep this in order to
> * interpret pre-pg_upgrade pages correctly.
> */
> if (TransactionIdEquals(xmin, FrozenTransactionId) &&
> TransactionIdIsValid(xmax))
> return true;
>
> return false;
> }
Wouldn't this last "if" test, to cover the pg_upgrade case, be better
targeted by comparing *raw* xmin to FrozenTransactionId? You're using
the potentially distinct xmin value returned by
HeapTupleHeaderGetXmin() for the test here. I think we should be
directly targeting tuples frozen on or before 9.4 (prior to
pg_upgrade) instead.
Have I missed something?
--
Peter Geoghegan
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix traversal of half-frozen update chains
On Tue, Oct 17, 2017 at 3:40 AM, Alvaro Herrera wrote: > Peter Geoghegan wrote: > >> Wouldn't this last "if" test, to cover the pg_upgrade case, be better >> targeted by comparing *raw* xmin to FrozenTransactionId? You're using >> the potentially distinct xmin value returned by >> HeapTupleHeaderGetXmin() for the test here. I think we should be >> directly targeting tuples frozen on or before 9.4 (prior to >> pg_upgrade) instead. > > I also realized we can stop checking (i.e. don't compare xmin to > frozenxid) if the XMIN_FROZEN bits are set -- because in that case the > tuple cannot possibly come from 9.3 frozen. So I think this should do > it. > > (New HeapTupleUpdateXmaxMatchesXmin() implementation) Yeah, this is what I had in mind, too. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix traversal of half-frozen update chains
On Tue, Oct 17, 2017 at 12:23 PM, Peter Geoghegan wrote: > On Tue, Oct 17, 2017 at 3:40 AM, Alvaro Herrera > wrote: >> Peter Geoghegan wrote: >> >>> Wouldn't this last "if" test, to cover the pg_upgrade case, be better >>> targeted by comparing *raw* xmin to FrozenTransactionId? You're using >>> the potentially distinct xmin value returned by >>> HeapTupleHeaderGetXmin() for the test here. I think we should be >>> directly targeting tuples frozen on or before 9.4 (prior to >>> pg_upgrade) instead. >> >> I also realized we can stop checking (i.e. don't compare xmin to >> frozenxid) if the XMIN_FROZEN bits are set -- because in that case the >> tuple cannot possibly come from 9.3 frozen. So I think this should do >> it. >> >> (New HeapTupleUpdateXmaxMatchesXmin() implementation) > > Yeah, this is what I had in mind, too. BTW, seems worth fixing this old comment above heap_prepare_freeze_tuple() while you're at it: * NB: It is not enough to set hint bits to indicate something is * committed/invalid -- they might not be set on a standby, or after crash * recovery. We really need to remove old xids. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix traversal of half-frozen update chains
On Tue, Oct 17, 2017 at 3:40 AM, Alvaro Herrera wrote: > I also realized we can stop checking (i.e. don't compare xmin to > frozenxid) if the XMIN_FROZEN bits are set -- because in that case the > tuple cannot possibly come from 9.3 frozen. So I think this should do > it. When are you planning on committing this? -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
On Thu, Nov 2, 2017 at 4:20 AM, Andres Freund wrote: > I think the problem is on the pruning, rather than the freezing side. We > can't freeze a tuple if it has an alive predecessor - rather than > weakining this, we should be fixing the pruning to not have the alive > predecessor. Excellent catch. > If the update xmin is actually below the cutoff we can remove the tuple > even if there's live lockers - the lockers will also be present in the > newer version of the tuple. I verified that for me that fixes the > problem. Obviously that'd require some comment work and more careful > diagnosis. I didn't even know that that was safe. > I think a5736bf754c82d8b86674e199e232096c679201d might be dangerous in > the face of previously corrupted tuple chains and pg_upgraded clusters - > it can lead to tuples being considered related, even though they they're > from entirely independent hot chains. Especially when upgrading 9.3 post > your fix, to current releases. Frankly, I'm relieved that you got to this. I was highly suspicious of a5736bf754c82d8b86674e199e232096c679201d, even beyond my specific, actionable concern about how it failed to handle the 9.3/FrozenTransactionId xmin case as special. As I went into in the "heap/SLRU verification, relfrozenxid cut-off, and freeze-the-dead bug" thread, these commits left us with a situation where there didn't seem to be a reliable way of knowing whether or not it is safe to interrogate clog for a given heap tuple using a tool like amcheck. And, it wasn't obvious that you couldn't have a codepath that failed to account for pre-cutoff non-frozen tuples -- codepaths that call TransactionIdDidCommit() despite it actually being unsafe. If I'm not mistaken, your proposed fix restores sanity there. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
On Thu, Nov 2, 2017 at 9:44 AM, Robert Haas wrote: > The second commit (22576734b805fb0952f9be841ca8f643694ee868) is where > I think things get a lot more dangerous. The problem (as Andres > pointed out to me this afternoon) is that it seems possible to end up > with a situation where there should be two HOT chains on a page, and > because of the weakened xmin/xmax checking rules, we end up thinking > that the second one is a continuation of the first one, which will be > all kinds of bad news. That would be particularly likely to happen in > releases from before we invented HEAP_XMIN_FROZEN, when there's no > xmin/xmax matching at all, but could happen on later releases if we > are extraordinarily unlucky (i.e. xmin of the first tuple in the > second chain happens to be the same as the pre-freeze xmax in the old > chain, probably because the same XID was used to update the page in > two consecutive epochs). Fortunately, that commit is (I think) not > released anywhere. FWIW, if you look at the second commit (22576734b805fb0952f9be841ca8f643694ee868) carefully, you'll realize that it doesn't even treat those two cases differently. It was buggy even on its own terms. The FrozenTransactionId test used an xmin from HeapTupleHeaderGetXmin(), not HeapTupleHeaderGetRawXmin(). -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
Andres Freund wrote: Here's that patch. I've stared at this some, and Robert did too. Robert mentioned that the commit message might need some polish and I'm not 100% sure about the error message texts yet. The commit message should probably say that the bug involves the resurrection of previously dead tuples, which is different to there being duplicates because a constraint is not enforced because HOT chains are broken (that's a separate, arguably less serious problem). Staring at the vacuumlazy hunk I think I might have found a related bug: heap_update_tuple() just copies the old xmax to the new tuple's xmax if a multixact and still running. It does so without verifying liveliness of members. Isn't that buggy? Consider what happens if we have three blocks: 1 has free space, two is being vacuumed and is locked, three is full and has a tuple that's key share locked by a live tuple and is updated by a dead xmax from before the xmin horizon. In that case afaict the multi will be copied from the third page to the first one. Which is quite bad, because vacuum already processed it, and we'll set relfrozenxid accordingly. I hope I'm missing something here? Can you be more specific about what you mean here? I think that I understand where you're going with this, but I'm not sure. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
Alvaro Herrera wrote: He means that the tuple that heap_update moves to page 1 (which will no longer be processed by vacuum) will contain a multixact that's older than relminmxid -- because it is copied unchanged by heap_update instead of properly checking against age limit. I see. The problem is more or less with this heap_update() code: /* * And also prepare an Xmax value for the new copy of the tuple. If there * was no xmax previously, or there was one but all lockers are now gone, * then use InvalidXid; otherwise, get the xmax from the old tuple. (In * rare cases that might also be InvalidXid and yet not have the * HEAP_XMAX_INVALID bit set; that's fine.) */ if ((oldtup.t_data->t_infomask & HEAP_XMAX_INVALID) || HEAP_LOCKED_UPGRADED(oldtup.t_data->t_infomask) || (checked_lockers && !locker_remains)) xmax_new_tuple = InvalidTransactionId; else xmax_new_tuple = HeapTupleHeaderGetRawXmax(oldtup.t_data); My naive guess is that we have to create a new MultiXactId here in at least some cases, just like FreezeMultiXactId() sometimes does. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Stamp 9.2.24.
Tom Lane wrote: Stamp 9.2.24. Uh, I thought 9.2 was EOL. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Stamp 9.2.24.
On Mon, Nov 6, 2017 at 2:47 PM, Michael Paquier wrote: > On Tue, Nov 7, 2017 at 7:39 AM, Tom Lane wrote: >> Peter Geoghegan writes: >>> Tom Lane wrote: >>>> Stamp 9.2.24. > 9.2.24 is the last of the 9.2-series, November being the last minor > release after the 5-year community support window. We already removed 9.2 from the website, though. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
On Thu, Nov 9, 2017 at 2:24 PM, Andres Freund wrote: > Attached is a version of the already existing regression test that both > reproduces the broken hot chain (and thus failing index lookups) and > then also the tuple reviving. I don't see any need for letting this run > with arbitrary permutations. I thought that the use of every possible permutation was excessive, myself. It left us with an isolation test that didn't precisely describe the behavior that is tested. What you came up with seems far, far better, especially because of the comments you included. The mail message-id references seem to add a lot, too. > What I'm currently wondering about is how much we need to harden > postgres against such existing corruption. If e.g. the hot chains are > broken somebody might have reindexed thinking the problem is fixed - but > if they then later vacuum everything goes to shit again, with dead rows > reappearing. I don't follow you here. Why would REINDEXing make the rows that should be dead disappear again, even for a short period of time? It might do so for index scans, I suppose, but not for sequential scans. Are you concerned about a risk of somebody not noticing that sequential scans are still broken? Actually, on second thought, I take that back -- I don't think that REINDEXing will even finish once a HOT chain is broken by the bug. IndexBuildHeapScan() actually does quite a good job of making sure that HOT chains are sane, which is how the enhanced amcheck notices the bug here in practice. (Before this bug was discovered, I would have expected amcheck to catch problems like it slightly later, during the Bloom filter probe for that HOT chain...but, in fact, it never gets there with corruption from this bug in practice, AFAIK.) -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
On Thu, Nov 9, 2017 at 4:17 PM, Andres Freund wrote: >> I don't follow you here. Why would REINDEXing make the rows that >> should be dead disappear again, even for a short period of time? > > It's not the REINDEX that makes them reappear. Of course. I was just trying to make sense of what you said. > It's the second > vacuum. The reindex part was about $user trying to fix the problem... > As you need two vacuums with appropriate cutoffs to hit the "rows > revive" problem, that'll often in practice not happen immediately. This explanation clears things up, though. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
On Thu, Nov 9, 2017 at 4:17 PM, Andres Freund wrote: >> Actually, on second thought, I take that back -- I don't think that >> REINDEXing will even finish once a HOT chain is broken by the bug. >> IndexBuildHeapScan() actually does quite a good job of making sure >> that HOT chains are sane, which is how the enhanced amcheck notices >> the bug here in practice. > > I think that's too optimistic. Why? Because the "find the TID of the root" logic in IndexBuildHeapScan()/heap_get_root_tuples() won't reliably find the actual root (it might be some other HOT chain root following TID recycling by VACUUM)? Assuming that's what you meant: I would have thought that the xmin/xmax matching within heap_get_root_tuples() makes the sanity checking fairly reliable in practice. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
On Thu, Nov 9, 2017 at 4:53 PM, Andres Freund wrote: > Primarily because it's not an anti-corruption tool. I'd be surprised if > there weren't ways to corrupt the page using these corruptions that > aren't detected by it. It's very hard to assess the risk of missing something that's actually detectable with total confidence, but I think that the check is actually very thorough. > But even if it were, I don't think there's > enough information to do so in the general case. You very well can end > up with pages where subsequent hot pruning has removed a good bit of the > direct evidence of this bug. Sure, but maybe those are cases that can't get any worse anyway. So the question of avoiding making it worse doesn't arise. > But I'm not really sure why the error detection capabilities of matter > much for the principal point I raised, which is how much work we need to > do to not further worsen the corruption. You're right. Just trying to put the risk in context, and to understand the extent of the concern that you have. -- Peter Geoghegan
Re: [COMMITTERS] pgsql: Generate parallel sequential scan plans in simple cases.
On Wed, Nov 11, 2015 at 7:35 AM, Magnus Hagander wrote: >> Congratulations! >> > > +1. That's an exciting milestone! Congratulations, Robert. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Generate parallel sequential scan plans in simple cases.
On Wed, Nov 11, 2015 at 9:08 AM, Peter Geoghegan wrote: > Congratulations, Robert. I should certainly extend my congratulations to Amit too. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Support parallel joins, and make related improvements.
On Wed, Jan 20, 2016 at 11:41 AM, Robert Haas wrote: > Support parallel joins, and make related improvements. Congratulations to all involved. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Allow SetHintBits() to succeed if the buffer's LSN is new enough
On Mon, Feb 15, 2016 at 2:17 PM, Andres Freund wrote: > On 2016-02-15 22:01:12 +, Andres Freund wrote: >> Allow SetHintBits() to succeed if the buffer's LSN is new enough. > > Thanks, that works with my brand of annoying compiler... Wrong commit message. :-) -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Don't vacuum all-frozen pages.
On Thu, Mar 10, 2016 at 1:22 PM, Andres Freund wrote: > Yeha! Fantastic effort, particularly from Masahiko. Well done. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Load FK defs into relcache for use by planner
On Thu, Apr 7, 2016 at 4:09 AM, Simon Riggs wrote: > Load FK defs into relcache for use by planner I gather this is infrastructure for the "use foreign keys to improve join estimates" patch. A more worked out commit message would be nice, though. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: CREATE INDEX ... INCLUDING (column[, ...])
On Fri, Apr 8, 2016 at 11:31 AM, Robert Haas wrote: >> I assume that the problem here is larger than just failure to adhere to >> C89 comment style. Was this patch really ready to commit? I'm not very >> happy that such a large patch went from "Needs review" to "Committed" in >> the blink of an eye on the very last commitfest day ... and artifacts like >> this aren't doing anything to increase my confidence in it. > > +1. I wonder if this should be reverted entirely. I really wish I could have done more to help with this, but I didn't do enough soon enough. Regrettably, I think that the patch just isn't ready. For example, the way that expression indexes just aren't handled is a cause for concern, as is the general way in which high keys are modified during index builds. Interactions with logical decoding are also a concern; there could be significant issues there, but that analysis just didn't happen. I had significant misunderstandings about the patch as recently as this week. This should be reverted. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix assorted missing infrastructure for ON CONFLICT.
On Wed, May 11, 2016 at 1:20 PM, Tom Lane wrote: > Fix assorted missing infrastructure for ON CONFLICT. What does this mean? + /* XXX broken */ if (attno < 0) elog(ERROR, "system column in index"); -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix assorted missing infrastructure for ON CONFLICT.
On Wed, May 11, 2016 at 1:54 PM, Alvaro Herrera wrote: > My guess is that it means we do support indexes in system columns (oid > in particular) and that instead of an ugly error this should do > something else. Maybe silently ignore the index. Why ignore the index? Either they're not supported, and we should throw an error (granted, a less ugly one), or they are supported, and inference should succeed. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix building of large (bigger than shared_buffers) hash indexes.
On Mon, Jun 27, 2016 at 2:27 PM, Bruce Momjian wrote: > Do we have any way of helping people find out if they need to recreate > their hash indexes? No, but I don't think that it's especially needed. It ought to be completely obvious when the problem arises, because the resulting index is total garbage. This tells us a lot about how many people use hash indexes in production, of course. 9.5 has been out for months. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Improve parser's and planner's handling of set-returning functio
On Tue, Sep 13, 2016 at 10:54 AM, Tom Lane wrote: > There is one case the parser will now throw error for although previous > versions allowed it, which is SRFs in the tlist of an UPDATE. That never > behaved sensibly (since it's ill-defined which generated row should be > used to perform the update) and it's hard to see why it should not be > treated as an error. It's a release-note-worthy change though. There is a reference to this right at the end of the executor README that was missed. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Change the way pre-reading in external sort's merge phase works.
On Mon, Oct 3, 2016 at 3:38 AM, Heikki Linnakangas wrote: > Change the way pre-reading in external sort's merge phase works. I noticed a simple oversight in this patch. It looks like you missed one place where state->maxTapes ought to be replaced with numInputTapes -- the loop that calls LogicalTapeAssignReadBufferSize() needs that changed too, in order to continue to respect workMem as a budget. There is a lesson for me, here: Start running tests of sorting patches only after setting "sysctl -w vm.overcommit_memory=2", because over commit can obscure things. Doing that as standard procedure for testing would have allowed me to catch this immediately. Maybe you should do the same with the other loop that iterates based on a state->maxTapes invariant that was added to the end of mergeruns() by this commit (use numInputTapes there in place of state->maxTapes, too). And, I wonder if it's worth it to not actually rewind in that loop at all -- perhaps you could instead call a new logtape.c function that just frees preload buffer memory. You'd also call this new simple "free preload buffer" function in place of the LogicalTapeRewind() call within tuplesort_gettuple_common(), of course. I've found that I have to do this within the rebased parallel CREATE INDEX patch anyway, since LogicalTapeRewind() has various irrelevant pre and post conditions that don't interact well with tape unification (so you get assertion failures that are probably harmless, but not really fixable within LogicalTapeRewind() itself). Might be best to get ahead of that now; my problem with using LogicalTapeRewind() suggests to me that not using LogicalTapeRewind() to simply free preload memory could be good *general* future-proofing. Thanks -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Make getrusage() output a little more readable
On Wed, Oct 19, 2016 at 6:58 AM, Peter Eisentraut wrote: > Make getrusage() output a little more readable BTW, trace_sort is a user-visible, documented GUC, so I guess that means that this will need a release note item. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix accounting of memory needed for merge heap.
On Thu, Dec 8, 2016 at 12:20 AM, Heikki Linnakangas wrote: > While we're at it, add a safeguard for the case that we are already over > the limit when allocating the read buffers. That shouldn't happen, but > better safe than sorry. I think you should 's/Min/Max/' where the new limit is applied. Also suggest that the subsequent USEMEM() call have "state->read_buffer_size * numInputTapes" as its argument, rather than state->availMem, just to be neat. Thanks -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix accounting of memory needed for merge heap.
On Thu, Dec 8, 2016 at 1:07 PM, Heikki Linnakangas wrote: > D'oh, you're right, of course. Fixed, thanks for the vigilance! I've made exactly the same mistake myself before. :-) -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Simplify tape block format.
On Thu, Dec 22, 2016 at 8:45 AM, Heikki Linnakangas wrote: > Simplify tape block format. > > No more indirect blocks. The blocks form a linked list instead. There is still one remaining reference to indirect blocks that you missed in comments above LogicalTapeRewindForWrite(). -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: contrib/amcheck needs RecentGlobalXmin to be PGDLLIMPORT'ified.
On Fri, Mar 10, 2017 at 11:47 AM, Andres Freund wrote: >> Assert(TransactionIdIsValid(RecentGlobalXmin)); >> >> I agree: that is just about utterly useless. > > Well, it mirrors an existing Assert, that'd be hit when doing normal > index lookups. But I agree that a bug around this is exceedingly > unlikely at this point, so there's no coverage value in it. I was in favor of just removing the assertion myself, given the PGDLLIMPORT issue, but FWIW I am generally in favor of documenting assertions like this. I suppose that this assertion is less likely to ever break than most other assertions, but presumably no code ever gets committed without somebody being pretty confident that any assertions it happens to have will never fail to hold. It doesn't seem productive to worry about whether or not any trivial assertions are pulling their weight. They're justified as documentation. If the assertion ever does fail, preventing someone from pushing buggy code, then so much the better. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Faster expression evaluation and targetlist projection.
On Sat, Mar 25, 2017 at 3:55 PM, Thomas Munro wrote: > This is a huge achievement. Congratulations! I also think it's excellent. Well done to all involved. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Expose qurey ID in pg_stat_statements view.
I see now that references to "queryid" in the documentation say "querid" in one or two places. Whoops. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Keep pg_stat_statements' query texts in a file, not in shared me
On Mon, Jan 27, 2014 at 5:12 PM, KONDO Mitsumasa wrote: > This patch has security problem that root can easily see the statement file > in database cluster. By default, we always serialize statements along with their query texts to disk on shutdown. Until May of 2012, pg_stat_statements didn't bother unlinking on startup, and so the file with query texts was always on the PGDATA filesystem. What's the difference? -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Keep pg_stat_statements' query texts in a file, not in shared me
On Mon, Jan 27, 2014 at 5:23 PM, Tom Lane wrote: > Root can certainly also look at query texts in shared memory, or for that > matter in the local memory of any process. So can anybody else running as > the postgres userid. I think that the concern may have had something to do with a MAC-centric viewpoint (e.g. SELinux users), where bizarrely it doesn't necessarily follow that root would be able to do any of those things. But in that world, it is surely the security officer's responsibility to make a special effort to meet those strange requirements. It's totally orthogonal to our security model. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Keep pg_stat_statements' query texts in a file, not in shared me
On Mon, Jan 27, 2014 at 5:34 PM, Andrew Dunstan wrote: > The query texts are particularly uninteresting since I assume the data > values in the query have already been mostly dissolved away by > pg_stat_statements. Actually, it is possible for the query string to still have constants if things are timed just right. I do see it in the wild, albeit very infrequently. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Keep pg_stat_statements' query texts in a file, not in shared me
On Mon, Jan 27, 2014 at 6:04 PM, KONDO Mitsumasa wrote: > It is written in documents; "For security reasons, non-superusers are not > allowed to see the text of queries executed by other users." Is root user > superuser? And initdb user might change to non-superuser after creating > database cluster. In japan, database operation user isn't always database > admin. Because database admin's salary is expensive than system operator's. initdb will not run as a superuser to begin with. It flatly refuses. Why is your concern with pg_stat_statements after this patch in particular? You'll need to serialize the file at least once before seeing it, but then it's there for good (on old versions, before Magnus got annoyed that that affected basebackups). -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Keep pg_stat_statements' query texts in a file, not in shared me
On Mon, Jan 27, 2014 at 6:08 PM, Tom Lane wrote: > "Timed just right"? I could see it possibly happening due to queryid > collisions, but I'm not seeing how it would happen absent such a hash > collision. Consider what happens when there is a pg_stat_statements_reset() call query after another query's parse analysis, but before its execution finishes. That's one obvious way. But you don't even need a reset - a badly timed entry_dealloc() could do it too. I don't see what hash collisions have to do with it, though. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Keep pg_stat_statements' query texts in a file, not in shared me
On Mon, Jan 27, 2014 at 6:31 PM, KONDO Mitsumasa wrote: > No. I don't say root user is superuser. Executing initdb user will be > postgres superuser. But it can change non-superuser after creating database. Okay. I still don't understand what your point is, or how this patch makes any worse what you'd consider to be a general problem. It doesn't even differ from a security standpoint to the original pg_stat_statements from 2009. > I feel the sense of incongruity that is stored database data in text file. > I'd like to hear from other people... I think it's incongruous that you chose to make your opinion known at this time and in this way. You knew about this patch several months ago; are your surprised that it does what it was prominently advertised to do? -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Allow using huge TLB pages on Linux (MAP_HUGETLB)
On Wed, Jan 29, 2014 at 4:12 AM, Heikki Linnakangas wrote: > Allow using huge TLB pages on Linux (MAP_HUGETLB) The documentation says: +Remember that you will need at least shared_buffers / huge page size + +1 huge TLB pages. So for example for a system with 6GB shared buffers +and a hugepage size of 2kb of you will need at least 3156 huge pages. I think that this should say 2MiB rather than 2kb. Or 2M, if you prefer. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Include planning time in EXPLAIN ANALYZE output.
On Wed, Jan 29, 2014 at 1:10 PM, Robert Haas wrote: > Include planning time in EXPLAIN ANALYZE output. Isn't it perhaps a little confusing that "Planning time" may well exceed "Total runtime"? I'm aware that we aren't super clear on the accounting here generally, as with pg_stat_statements, which doesn't include planning time in total_time, while we aren't even explicit about that in the documentation. But even still, seeing the two side-by-side, with planning time > total runtime did give me pause. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Include planning time in EXPLAIN ANALYZE output.
On Sun, Feb 2, 2014 at 8:13 AM, Tom Lane wrote: > Perhaps s/Total runtime/Execution time/ ? +1 -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Add a GUC to report whether data page checksums are enabled.
On Tue, Feb 18, 2014 at 11:39 AM, Alvaro Herrera wrote: > Is there are reason this wasn't back-patched to 9.3? I think it should > be. +1. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [HACKERS] Re: [COMMITTERS] pgsql: Add a GUC to report whether data page checksums are enabled.
On Wed, Feb 19, 2014 at 4:49 PM, Michael Paquier wrote: >> +1 for back-patching. > Back-patching would be interesting for existing applications, but -1 > as it is a new feature :) I think that it rises to the level of an omission in 9.3 that now requires correction. Many of our users couldn't run pg_controldata even if they'd heard of it... -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Initial version of Postgres 9.4 release notes
On Sat, May 3, 2014 at 8:16 PM, Bruce Momjian wrote: > Initial version of Postgres 9.4 release notes Did you forget to "git add release-9.4.sgml"? -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Clean up jsonb code.
Thanks for cleaning this up. On Wed, May 7, 2014 at 1:18 PM, Heikki Linnakangas wrote: > The jsonb_exists_any and jsonb_exists_all functions no longer sort the input > array. That was a premature optimization, the idea being that if there are > duplicates in the input array, you only need to check them once. Also, > sorting the array saves some effort in the binary search used to find a key > within an object. But there were drawbacks too: the sorting and > deduplicating obviously isn't free, and in the typical case there are no > duplicates to remove, and the gain in the binary search was minimal. Remove > all that, which makes the code simpler too. This is not the reason why the code did that. De-duplication was not the point at all. findJsonbValueFromSuperHeader()'s lowbound argument previously served to establish a low bound for searching when searching for multiple keys (so the second and subsequent user-supplied key could skip much of the object). In the case of jsonb_exists_any(), say, if you only have a reasonable expectation that about 1 key exists, and that happens to be the last key that the user passed to the text[] argument (to the existence/? operator), then n - 1 calls to what is now findJsonbValueFromContainer() (which now does not accept a lowbound) are wasted. That's elem_count - 1 top-level binary searches of the entire jsonb. Or elem_count such calls rather than 1 call (plus 1 sort of the supplied array) in the common case where jsonb_exists_any() will return false. Granted, that might not be that bad right now, given that it's only ever (say) elem_count or elem_count - 1 wasted binary searches through the *top* level, but that might not always be true. And even today, sorting a presumably much smaller user-passed lookup array once has to be cheaper than searching through the entire jsonb perhaps elem_count times per call. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Add pinning_backends column to the pg_buffercache extension.
On Thu, Aug 21, 2014 at 3:29 PM, Andres Freund wrote: > Add pinning_backends column to the pg_buffercache extension. Minor point: "Existence" is misspelled. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix CreatePolicy, pg_dump -v; psql and doc updates
On Fri, Oct 3, 2014 at 3:06 PM, Tom Lane wrote: > It's not important enough to go back and change catversion.h now, but > please keep it in mind for the future. One of the main values of > catversion is to prevent developers from wasting time chasing regression > test failures that are just code-vs-data-skew issues. It would be nice to institutionalize the idea of "private OIDs", too, each represented by constants like PG_PRIVATE_OID_ONE (it would probably go up to TEN, or thereabouts). Fixing duplicates when rebasing a patch is something that we could thereby avoid - It would become the responsibility of the committer to adjust the OIDs to conventional "public OIDs" before pushing to the master branch, just like it is currently left to the commiter to bump catversion. That work could even be scripted, I think. I like picking pg_proc OIDs that are at the beginning of some unused range, and so do other people, so this happens more often than you might think. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Add support for INSERT ... ON CONFLICT DO NOTHING/UPDATE.
On Mon, May 11, 2015 at 1:49 AM, Thom Brown wrote: > I haven't had time for a proper read of this patch, but I did > immediately notice this: > > HINT: For example, ON CONFLICT ON CONFLICT (). > > This should perhaps either be: > > HINT: For example, ON CONFLICT (). > > or > > HINT: For example, ON CONFLICT ON CONSTRAINT . > > But at the moment it seems to be neither. What I'd intended here was the first suggestion of yours. Initially, it was actually a combination of Thom's two suggestions, but this was messed up at some point. What I suggest is that this be changed to match the first suggestion here (the intended message), since the "ON CONSTRAINT ... " variant is really just an escape hatch that I don't expect will see much use. I tried to encourage use of the conventional inference mechanism everywhere. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Additional functions and operators for jsonb
On Tue, May 12, 2015 at 12:55 PM, Andrew Dunstan wrote: > Additional functions and operators for jsonb I think that there should be examples of usage of some of these new operators within "8.14. JSON Types". In particular, I'd like to see an example of use of the concatenate operator to perform "field assignment" for JSON objects. The inability to do that is a complaint that people have with jsonb, and this ought to be positioned as the solution to that problem. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: release notes: Add entry for commit 5ea86e6e6.
On Fri, Jun 26, 2015 at 11:55 AM, Robert Haas wrote: > release notes: Add entry for commit 5ea86e6e6. s/Extend the infrastructure allow sorting/Extend the infrastructure that allows/ -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Improve 9.5 release notes.
On Tue, Jun 30, 2015 at 12:00 PM, Andres Freund wrote: > Improve 9.5 release notes. Noticed a typo: "compact and cheap to to update by" -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Use gender-neutral language in documentation
On Mon, Sep 21, 2015 at 9:32 PM, Erik Rijkers wrote: > I think this compulsive 'he'-avoiding is making the text worse. > > > - environment variable); any user can make such a change for his > session. > + environment variable); any user can make such a change for their > session. -1. It seems fine to me. > Yuck. even worse: > > - might not be the same as the database user he needs to connect as. > + might not be the same as the database user that is to be connect as. > > > It is not an improvement. I would like to see this change rolled back. I think that this should be reworded, since there is a grammatical error as things stand. I suggest the whole sentence be modified to read: When using an external authentication system such as Ident or GSSAPI, the name of the operating system user that initiated the connection might not be the same as the intended corresponding database user. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [HACKERS] [COMMITTERS] pgsql: Use gender-neutral language in documentation
On Tue, Sep 22, 2015 at 7:11 AM, Andrew Dunstan wrote: > You can think that if you like, but it's not even remotely true. It's a > deliberate choice to use a new, perfectly reasonable and now widely accepted > style of which you disapprove, but it's not lazy. It never occurred to me that this usage is even non-traditional. I am a native English speaker born in Ireland in the 1980s. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: doc: Update URLs of external projects
On Sat, Oct 3, 2015 at 1:37 AM, Magnus Hagander wrote: > Should we consider backpatching this further into the stable branches? > Arguably it's a bugfix given the state of pgfoundry. +1 -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Add CASCADE support for CREATE EXTENSION.
On Sat, Oct 3, 2015 at 9:48 AM, Andres Freund wrote: > Add CASCADE support for CREATE EXTENSION. As you may have noticed already, this patch fails to account for this change in earthdistance regression test output: + HINT: Use CREATE EXTENSION CASCADE to install required extensions too. -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Apply SELECT policies in INSERT/UPDATE+RETURNING
On Mon, Oct 5, 2015 at 4:55 AM, Stephen Frost wrote: > Apply SELECT policies in INSERT/UPDATE+RETURNING If we've changed this now, where does that leave the excluded.* pseudo-relation? -- Peter Geoghegan -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
