date:20110715

Hi!

Thank you for review. I've few questions about it.

On Fri, Jul 15, 2011 at 2:13 AM, Nathan Boley npbo...@gmail.com wrote:

 First, it makes me uncomfortable that you are using the MCV and histogram
 slot
 kinds in a way that is very different from other data types.

 I realize that tsvector uses MCV in the same way that you do but:

 1) I don't like that very much either.
 2) TS vector is different in that equality ( in the btree sense )
doesn't make sense, whereas it does for arrays.

 Using the histogram slot for the array lengths is also very surprising to
 me.

 Why not just use a new STA_KIND? It's not like we are running out of
 room, and this will be the second 'container' type that splits the
 container
 and stores stats about the elements.

Thus, do you think we should collect both btree and frequency/length
statistics for arrays?


 1) In calc_distr you go to some lengths to avoid round off errors. Since it
 is
   certainly just the order of the estimate that matters, why not just
   perform the calculation in log space?

It seems to me that I didn't anything to avoid round off errors there...

--
With best regards,
Alexander Korotkov.

Re: [HACKERS] pg_class.relistemp

2011-07-15 Thread Dave Page

On Thu, Jul 14, 2011 at 10:54 PM, Josh Berkus j...@agliodbs.com wrote:

 As one of said vendors, I completely disagree.

 I don't agree that you qualify as a vendor.  You're on the friggin' core
 team.

And I look after the development of the leading open source management
tool for PostgreSQL, as well as a number of tools at EnterpriseDB. I
might be on the core team, but I still build tools.

 I'm talking about vendors like DBVizualizer or TORA, for which
 PostgreSQL is just one of the databases they support.  If stuff breaks
 gratuitously, the reaction of some of them is always to either drop
 support or delay it for a year or more.  This doesn't benefit our community.

I'm not talking about gratuitously breaking things - just not masking
essential changes.

 There are a ton of
 things that change with each release, and all we do by putting in
 hacks for backwards compatibility is add bloat that needs to be
 maintained, and encourage vendors to be lazy.

 I don't agree that having comprehensive system views with multi-version
 stability would be a hack.

That isn't what was being suggested.

 Break compatibility is actually something that is important to us - it
 forces us to fix obvious issues, and makes it much harder to
 inadvertently miss important changes.

 What I'm hearing from you is: Breaking backwards compatibility is
 something we should do more of because it lets us know which vendors are
 paying attention and weeds out the unfit.   Is that what you meant to say?

No, I meant to say precisely what I did say. By not masking changes in
the catalogs, we draw attention to things that have changed for good
reason, and almost certainly need to be addressed.

 That seems like a way to ensure that PostgreSQL support continue to be
 considered optional, or based on outdated versions, for multi-database
 tools.

Whilst this particular case might be safe to just ignore in third part
tools, other changes to the catalogs are not, and masking them could
potentially hide bugs or issues that need to be fixed to actually work
properly with the newer version of the server.

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: 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] Small patch for GiST: move childoffnum to child

2011-07-15 Thread Heikki Linnakangas


On 14.07.2011 13:29, Alexander Korotkov wrote:

On Thu, Jul 14, 2011 at 12:56 PM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com  wrote:


First, notice that we're setting ptr-parent = top. 'top' is the current
node we're processing, and ptr represents the node to the right of the
current node. The current node is *not* the parent of the node to the right.
I believe that line should be ptr-parent = top-parent.


I think same.


Second, we're adding the entry for the right sibling to the end of the list
of nodes to visit. But when we process entries from the list, we exit
immediately when we see a leaf page. That means that the right sibling can
get queued up behind leaf pages, and thus never visited.


I think possible solution is to save right sibling immediatly after current
page . Thus, this code fragment should looks like this:



if (top-parent  XLByteLT(top-parent-lsn,
GistPageGetOpaque(page)-nsn)
GistPageGetOpaque(page)-**rightlink !=
InvalidBlockNumber /* sanity check */ )
{
/* page splited while we thinking of... */
ptr = (GISTInsertStack *) palloc0(sizeof(**
GISTInsertStack));
ptr-blkno = GistPageGetOpaque(page)-**rightlink;
ptr-childoffnum = InvalidOffsetNumber;
ptr-parent = top-parent;
ptr-next = top-next;
top-next = ptr;
if (tail == top);
tail = ptr;


 }


Agreed, committed. Thanks for verifying my thinking.

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch Review: Collect frequency statistics and selectivity estimation for arrays

On Fri, Jul 15, 2011 at 2:13 AM, Nathan Boley npbo...@gmail.com wrote:

 First, it makes me uncomfortable that you are using the MCV and histogram
 slot
 kinds in a way that is very different from other data types.

 I realize that tsvector uses MCV in the same way that you do but:

 1) I don't like that very much either.
 2) TS vector is different in that equality ( in the btree sense )
doesn't make sense, whereas it does for arrays.

Actually, not MCV slot is used but MCELEM. It is a different slot. ps_stats
view map both into same fileds. Surely, non-standard use of histogram slot
should be avoided.

--
With best regards,
Alexander Korotkov.

Re: [HACKERS] Small patch for GiST: move childoffnum to child

2011-07-15 Thread Heikki Linnakangas


On 13.07.2011 22:04, Heikki Linnakangas wrote:

On 13.07.2011 21:56, Alexander Korotkov wrote:

Thank you very much for detail explanation. But this line of modified
patch
seems strange for me:
*newchildoffnum = blkno;
I believe it should be:
*newchildoffnum = i;


Yes, you're right. It's scary that it worked during testing anyway.
Maybe the resulting tree was indeed broken but it didn't affect the
subsequent inserts so I didn't notice.


Ok, committed this now. I decided to rename the childoffnum field to 
downlinkoffnum. I figured it'd be dangerous that the field means 
something subtly different in different versions, if we need to 
backpatch bug fixes that use that field.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Small patch for GiST: move childoffnum to child

On Fri, Jul 15, 2011 at 1:27 PM, Heikki Linnakangas 
heikki.linnakan...@enterprisedb.com wrote:

 Ok, committed this now.

Thank you.


 I decided to rename the childoffnum field to downlinkoffnum. I figured
 it'd be dangerous that the field means something subtly different in
 different versions, if we need to backpatch bug fixes that use that field.

Yes, it seems very reasonable.

--
With best regards,
Alexander Korotkov.

Re: [HACKERS] WIP: Fast GiST index build

Fri, Jul 15, 2011 at 12:53 AM, Heikki Linnakangas 
heikki.linnakan...@enterprisedb.com wrote:

 On 14.07.2011 23:41, Alexander Korotkov wrote:

 Do you think using rightlink as pointer to parent page is possible
 during
 index build? It would allow to simplify code significantly, because of no
 more need to maintain in-memory structures for parents memorizing.


 I guess, but where do you store the rightlink, then? Would you need a final
 pass through the index to fix all the rightlinks?

 I think you could use the NSN field. It's used to detect concurrent page
 splits, but those can't happen during index build, so you don't need that
 field during index build. You just have to make it look like an otherwise
 illegal NSN, so that it won't be mistaken for a real NSN after the index is
 built. Maybe add a new flag to mean that the NSN is actually invalid.


Thank you for advice. But I didn't take into account that in this case I
need to update parent link in many pages(which might be not in cache) on
split. Seems that I still need to maintain some in-memory structures for
parent finding.

--
With best regards,
Alexander Korotkov.

Re: [HACKERS] Re: patch review : Add ability to constrain backend temporary file space

2011-07-15 Thread Mark Kirkwood


On 15/07/11 14:57, Tatsuo Ishii wrote:


Maybe we could add more info regarding current usage and requested
amount in addition to the temp file limit value. I mean something
like:

ERROR:  aborting due to exceeding temp file limit. Current usage 9000kB, 
requested size 1024kB, thus it will exceed temp file limit 1kB.



I modeled the original message on what happens when statement timeout is 
exceeded, which doesn't state its limit in the error message at all - 
actually I did wonder if there is was informal standard for *not* 
stating the value of the limit that is being exceeded! However, I agree 
with you and think it makes sense to include it here. I wonder if the 
additional detail you are suggesting above might be better added to a 
HINT - what do you think? If it is a better idea to just add it in the 
message as above I can certainly do that.


Cheers

Mark

--
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] Re: patch review : Add ability to constrain backend temporary file space

2011-07-15 Thread Cédric Villemain

2011/7/15 Mark Kirkwood mark.kirkw...@catalyst.net.nz:
 On 15/07/11 14:57, Tatsuo Ishii wrote:

 Maybe we could add more info regarding current usage and requested
 amount in addition to the temp file limit value. I mean something
 like:

 ERROR:  aborting due to exceeding temp file limit. Current usage 9000kB,
 requested size 1024kB, thus it will exceed temp file limit 1kB.


 I modeled the original message on what happens when statement timeout is
 exceeded, which doesn't state its limit in the error message at all -
 actually I did wonder if there is was informal standard for *not* stating
 the value of the limit that is being exceeded! However, I agree with you and
 think it makes sense to include it here. I wonder if the additional detail
 you are suggesting above might be better added to a HINT - what do you
 think? If it is a better idea to just add it in the message as above I can
 certainly do that.

Remember that what will happens is probably:

ERROR:  aborting due to exceeding temp file limit. Current usage 8000kB,
requested size 8008kB, thus it will exceed temp file limit 8kB.

because temp file are increased by 8kb at once, rarely more (and by
rare I mean that it can happens via an extension or in the future, not
with current core postgresql).

This kind of message can also trouble the DBA by suggesting that the
query doesn't need more than what was asking at this moment.

That's being said I have no strong opinion for the HINT message if the
documentation is clear enough to not confuse DBA.

-- 
Cédric Villemain               2ndQuadrant
http://2ndQuadrant.fr/     PostgreSQL : Expertise, Formation et Support

-- 
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] Single pass vacuum - take 1

2011-07-15 Thread Simon Riggs

On Fri, Jul 15, 2011 at 12:56 AM, Pavan Deolasee
pavan.deola...@gmail.com wrote:
 On Thu, Jul 14, 2011 at 6:22 PM, Simon Riggs si...@2ndquadrant.com wrote:

 This is a very rare issue, because of all the work yourself and Heikki
 have put in.


 I don't think its rare case since vacuum on any table, small or large, would
 take two passes today. Avoiding one of the two, would help the system in
 general. HOT and other things help to a great extent, but unfortunately they
 don't make vacuum completely go away.

I don't really believe that. To progress this I think we need a clear
test case that we can judge this patch against. Building that will
show quite how rarely this is a problem.

 It's only important when we have a (1) big table (hence long scan
 time), (2) a use case that avoids HOT *and* (3) we have dirtied a
 large enough section of table that the vacuum map is ineffective and
 we need to scan high % of table. That combination is pretty rare, so
 penalising everybody else with 8 bytes per block seems too much to me.


 The additional space is allocated only for those pages which has dead-vacuum
 line pointers and would stay only till the next HOT-prune operation on the
 page. So everybody does not pay the penalty, even if we assume its too much
 and if thats what worries you most.

OK, that's the winner. If you're only doing this when it really does
matter then I'm happy.

My additional requests would be that we can easily tell which blocks
have been modified like this, that we have a way to turn this off if
we get bugs for next few releases, that we check it all works with Hot
Standby just fine and that all the block inspection tools support it.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Reduced power consumption in WAL Writer process

On Jul 14, 2011, at 4:42 AM, Simon Riggs si...@2ndquadrant.com wrote:
 On Thu, Jul 14, 2011 at 9:57 AM, Fujii Masao masao.fu...@gmail.com wrote:
 
 Currently walwriter might write out the WAL before a transaction commits.
 IOW, walwriter tries to write out the WAL in wal_buffers in every wakeups.
 This might be useful for long transaction which generates lots of WAL
 records before commit. So we should call SetLatch() in XLogInsert() instead
 of RecordTransactionCommit()? Though I'm not sure how much walwriter
 improves the performance of synchronous commit case..
 
 Yeh, we did previously have a heuristic to write out the WAL when it
 was more than half full. Not sure I want to put exactly that code back
 into such a busy code path.
 
 I suggest that we set latch every time the wal buffers wrap.
 
 So at the bottom of AdvanceXLInsertBuffer(), if nextidx == 0 then
 SetLatch on the WALWriter.
 
 That's a simple test and we only check it if we're switch WAL buffer page.

Seems reasonable at first blush, but I worry that changing the algorithm here 
could change performance in somewhat unpredictable ways. Some of those might be 
improvements, but I think some careful measurement would be in order.

If the primary goal here is to reduce power consumption, another option would 
be to keep the regular wake-ups most of the time but have some mechanism for 
putting the process to sleep until wakened when no activity happens for a 
certain period of time - say, 10 cycles. I'm not at all sure that's better, but 
it would be less of a change to the existing behavior.

...Robert
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Reduced power consumption in WAL Writer process

2011-07-15 Thread Peter Geoghegan

My instrumentation wasn't that good. I was using powertop 1.13, which
apparently goes to great lengths to group processes by various
criteria (including process group), but doesn't actually offer the
option of seeing that instrumentation per process.

I'm using version 1.98 beta 1 as of now, which provides per-process
instrumentation - it only works with Kernel versions 2.6.36+ though.
The per process breakdown of wake-ups per second is:

248.3 us/s   5.0Process postgres: wal writer process
281.0 us/s   4.9Process postgres: writer process
82.3 us/s1.1Process postgres: autovacuum launcher 
process
82.3 us/s0.4Process postgres: stats collector 
process
442.8 us/s   0.15   Process postgres
23.6 us/s0.15   Process postgres -d1

The second column from the left is wake-ups per second. As previously
reported and as you see here, there are about 11.5 idle wake-ups per
second per cluster. Note that archiving was running when this
instrumentation was performed, but the recently-improved archiver
wasn't listed at all, presumably because powertop didn't detect any
wake-ups in the period of instrumentation, which was 10 or 20 seconds.

As you'd expect, the WAL writer is woken up (1 second /
wal_writer_delay milliseconds) times per second.

On 14 July 2011 11:00, Simon Riggs si...@2ndquadrant.com wrote:
 That was my first though too - but I wonder if that's too aggressive? A
 backend that does for example a large bulk load will cycle through the
 buffers real quick. It seems like a bad idea to wake up walwriter between
 each buffer in that case. Then again, setting a latch that's already set is
 cheap, so maybe it works fine in practice.

 Maybe it would be better to do it less frequently, say, every time you
 switch to new WAL segment. Or every 10 buffers or something like that.

 Yes, that roughly what I'm saying. When nextidx == 0 is just after we
 wrapped wal_buffers, i.e. we only wake up the WALWriter every
 wal_buffers pages.

I'll work towards that implementation.

-- 
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] Reduced power consumption in WAL Writer process

2011-07-15 Thread Simon Riggs

On Fri, Jul 15, 2011 at 2:29 PM, Robert Haas robertmh...@gmail.com wrote:

 If the primary goal here is to reduce power consumption, another option
 would be to keep the regular wake-ups most of the time but have some
 mechanism for putting the process to sleep until wakened when no activity
 happens for a certain period of time - say, 10 cycles. I'm not at all sure
 that's better, but it would be less of a change to the existing behavior.

Now we have them, latches seem the best approach because they (mostly)
avoid heuristics.

This proposal works same or better for async transactions.

The only difference is how bulk write operations are handled. As long
as we wake WALWriter before wal_buffers fills then we'll be good.
Wakeup once per wal buffer is too much. I agree we should measure this
to check how frequently wakeups are required for bulk ops.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Understanding GIN posting trees

2011-07-15 Thread Teodor Sigaev


Oh, I see. You essentially do a merge join of all the posting trees of
query keys.

Hmm, but we do need to scan all the posting trees of all the matched
keys in whole anyway. We could collect all TIDs in the posting lists of
all the keys into separate TIDBitmaps, and then combine the bitmaps,
calling consistentFn for each TID that was present in at least one
bitmap. I guess the performance characteristics of that would be
somewhat different from what we have now, and you'd need to keep a lot
of in-memory bitmaps if the query contains a lot of keys.
I hope to reimplement amgettuple interface someday and this interface is 
designed for small startup cost. With bitmaps per search key it will be impossible.



While we're at it, it just occurred to me that it if the number of query
keys is limited, say = 16, you could build a lookup table for each
combination of keys either occurring or not. You could use then use that
instead of calling consistentFn for each possible match. You could even
use the table to detect common cases like all/any keys must match,
perhaps opening some optimization opportunities elsewhere.

I'm afraid that it becomes looking as a separate optimizer/planner :)


--
Teodor Sigaev   E-mail: teo...@sigaev.ru
   WWW: http://www.sigaev.ru/

--
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 for distinguishing PG instances in event log

2011-07-15 Thread MauMau


Magnus,

Thank you for reviewing my patch. I'm going to modify the patch according to 
your comments and re-submit it. Before that, I'd like to discuss some points 
and get your agreement.



From: Magnus Hagander mag...@hagander.net

+para
+ On Windows, you need to register an event source
+ and its library with the operating system in order
+ to make use of the systemitemeventlog/systemitem option for
+ varnamelog_destination/.
+ See xref linkend=event-log-registration for details.
+/para

* This part is not strictly correct - you don't *need* to do that, it
just makes things look nicer, no?


How about the following statement? Is it better to say correctly instead 
of cleanly?


--
On Windows, when you use the eventlog option for log_destination, you need 
to register an event sourceand its library with the operating system so that 
the Windows Event Viewer can display event log messages cleanly.

--


* Also, what is the use for set_eventlog_parameters()? It's just a
string variable, it shuold work without it.


Yes, exactly. I'll follow your modification.


* We these days avoid #ifdef'ing gucs just because they are not on
that platform - so the list is consistent. The guc should be available
on non-windows platforms as well.


I see. When I saw syslog_ident not being #ifndef WIN32'ed, I thought that 
was because syslog might be available on Cygwin or MinGW. Anyway, I'll take 
your modification.




* The guc also needs to go in postgresql.conf.sample


I missed this completely.


* Are we really allowed to call MessageBox in DlLRegisterService?
Won't that break badly in cases like silent installs?


It seems that the only way to notify the error is MessageBox. Printing to 
stdout/stderr does not show any messages on the command prompt. I guess 
that's why the original author of pgevent.c used MessageBox.


We cannot know within the DLL if /s (silent) switch was specified to 
regsvr32.exe. If we want to suppress MessageBox during silent installs, it 
seems that we need to know the silent install by an environment variable or 
so. That is:


C:\ set PGSILENT=true
C:\ regsvr32.exe ...



* We never build in unicode mode, so all those checks are unnecessary.

Attached is an updated patch, which doesn't work yet. I believe the
changes to the backend are correct, but probably some of the cleanups
and changes in the dll are incorrect, because I seem to be unable to
register either the default or a custom handler so far.


The cause of the problem you are encountering is here:

+ if (pszCmdLine  *pszCmdLine != '\0')
+  strncpy(event_source, sizeof(event_source), pszCmdLine);
+ else
+  strcpy(event_source, PostgreSQL);

DllInstall() always receives the /i argument in UTF-16. str... functions 
like strncpy() cannot be used for handling UTF-16 strings. For example, when 
you run regsvr32.exe /i:abc ..., DllInstall's pszCmdLine value becomes 
a\0b\0c\0. So, strncpy() just copies a. This is the reason why you 
cannot register the custom event source.


The reason why you cannot register the default event source (i.e. running 
regsvr32 without /i) is that you memset()ed event_source in DllMain() and 
set PostgreSQL to it in DllInstall(). DllInstall() is not called when /i 
is not specified. So, event_source remains empty.


To solve the problem, we need to use wcstombs_s() instead of strncpy(), and 
initialize event_source to PostgreSQL when it is defined.


Regards
MauMau




--
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] Re: patch review : Add ability to constrain backend temporary file space

=?ISO-8859-1?Q?C=E9dric_Villemain?= cedric.villemain.deb...@gmail.com writes:
 On 15/07/11 14:57, Tatsuo Ishii wrote:
 Maybe we could add more info regarding current usage and requested
 amount in addition to the temp file limit value. I mean something
 like:
 
 ERROR:  aborting due to exceeding temp file limit. Current usage 9000kB,
 requested size 1024kB, thus it will exceed temp file limit 1kB.

 Remember that what will happens is probably:

 ERROR:  aborting due to exceeding temp file limit. Current usage 8000kB,
 requested size 8008kB, thus it will exceed temp file limit 8kB.

 because temp file are increased by 8kb at once, rarely more

Yes.  I think the extra detail Tatsuo proposes is useless and possibly
confusing.  I don't object to stating the current limit, but the other
numbers are not likely to be helpful to anyone.  (If we had some way of
knowing what the file would ultimately grow to if we allowed the query
to continue, that would be useful; but of course we don't know 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] ON COMMIT action not catalogued?

Peter Eisentraut pete...@gmx.net writes:
 (Going through some loose ends in the information schema ...)
 Is my understanding right that the ON COMMIT action of temporary tables
 is not recorded in the system catalogs anywhere?  Would make sense,
 wouldn't be a big problem, just want to make sure I didn't miss
 anything.

Yes, it's just remembered in local state within the owning backend.

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] Reduced power consumption in WAL Writer process

Excerpts from Simon Riggs's message of vie jul 15 09:55:40 -0400 2011:
 On Fri, Jul 15, 2011 at 2:29 PM, Robert Haas robertmh...@gmail.com wrote:
 
  If the primary goal here is to reduce power consumption, another option
  would be to keep the regular wake-ups most of the time but have some
  mechanism for putting the process to sleep until wakened when no activity
  happens for a certain period of time - say, 10 cycles. I'm not at all sure
  that's better, but it would be less of a change to the existing behavior.
 
 Now we have them, latches seem the best approach because they (mostly)
 avoid heuristics.

Yeah, there's no reason for less of a change to be a criterion to
determine the best way forward.  The new tech is clearly a better
solution overall, so lets just get rid of the cruft.

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] Is there a committer in the house?

Excerpts from Josh Berkus's message of jue jul 14 15:01:10 -0400 2011:
 All,
 
 Currently we have 8 patches Ready for Committer in the current CF.
 Some of them have been that status for some time.
 
 From traffic on this list, I'm getting the impression that nobody other
 than Robert, Heikki and Tom are committing other people's patches.  I
 know we have more committers than that.  Bruce?  Simon?  Itagaki?  Bueller?

It seems that by mentioning some people but not all, you offended both
the people you mentioned (at least some of them, because they are
already actively helping) and those that you didn't (at least some of
them, because they are already actively helping; those that are not
completely inactive in the project, that is).

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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_class.relistemp

Magnus Hagander mag...@hagander.net writes:
 On Thu, Jul 14, 2011 at 21:59, Josh Berkus j...@agliodbs.com wrote:
 So if we're going to break compatibility, then we could stand to make a
 little noise about it.

 We've broken the admin apps in pretty much every single release. And
 they generally don't complain.

Yeah.  Quite honestly, this thread is trying to turn a molehill into a
mountain.  I will confidently predict that the really big, nasty change
in 9.1 is the change of default standard_conforming_strings.  That's
going to break way more apps than anything else, and possibly in
security-critical ways.  Anybody who moves an app onto 9.1 without
testing it is going to suffer big-time.

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_class.relistemp

2011-07-15 Thread Magnus Hagander

On Fri, Jul 15, 2011 at 17:25, Tom Lane t...@sss.pgh.pa.us wrote:
 Magnus Hagander mag...@hagander.net writes:
 On Thu, Jul 14, 2011 at 21:59, Josh Berkus j...@agliodbs.com wrote:
 So if we're going to break compatibility, then we could stand to make a
 little noise about it.

 We've broken the admin apps in pretty much every single release. And
 they generally don't complain.

 Yeah.  Quite honestly, this thread is trying to turn a molehill into a
 mountain.  I will confidently predict that the really big, nasty change
 in 9.1 is the change of default standard_conforming_strings.  That's
 going to break way more apps than anything else, and possibly in
 security-critical ways.  Anybody who moves an app onto 9.1 without
 testing it is going to suffer big-time.

+(a lot)

in fact, I think we should definitely shout out more clearly about
that one in the release notes. Yes, it's the very first item. But I
think it deserves a big fat warning box as well.

-- 
 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] pg_class.relistemp

David E. Wheeler da...@kineticode.com writes:
 So pgTAP creates temporary tables to store result sets so that it can then 
 compare the results of two queries. The function in question was getting a 
 list of columns in such a temporary table in order to make sure that the 
 types were the same between two such tables before comparing results. It 
 checked relistemp to make sure it was looking at the temp table rather than 
 some other table that might happen to have the same name.

Well, actually, that code flat out doesn't work, so whether relistemp is
available in 9.1 is the least of your problems.  Consider what would
happen if two concurrent sessions did this with the same temp table
name.

How about doing this instead?

SELECT pg_catalog.format_type(a.atttypid, a.atttypmod)
  FROM pg_catalog.pg_attribute a
  JOIN pg_catalog.pg_class c ON a.attrelid = c.oid
 WHERE c.oid = 'pg_temp.tablenamehere'::pg_catalog.regclass
   AND attnum  0
   AND NOT attisdropped
 ORDER BY attnum

This would only work in releases that know the pg_temp abbreviation,
which includes any minor release later than March 2007.  But since
relistemp doesn't even exist before 8.4 (released in 2009), that's still
more backwards-portable than what you've got.  You could also just do
'tablenamehere'::pg_catalog.regclass and trust that the user didn't move
pg_temp to the back of the search path.

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] SSI error messages

Peter Eisentraut pete...@gmx.net wrote:
 Some of these new error messages from the SSI code are a mouthful:
 
 not enough elements in RWConflictPool to record a rw-conflict
 not enough elements in RWConflictPool to record a potential
   rw-conflict
 
 These are basically out of shared memory conditions that could
 be moved to a DETAIL message.
 
Yes and no.  The resource which is exhausted at this point *is* in
shared memory, but its allocation of space is fixed at startup --
it's not competing with other users of shared memory for the shared
memory slush space.  I have some concern that an out of shared
memory message might confuse people on that aspect of the issue.
 
That said, if people find it helps more than it hurts, I have no
objection to a wording change.
 
 Canceled on identification as a pivot, during conflict out
   checking.
 Canceled on conflict out to old pivot %u.
 Canceled on identification as a pivot, with conflict out to
   old committed transaction %u.
 Canceled on conflict out to old pivot.
 Canceled on identification as a pivot, during conflict in
   checking.
 Canceled on identification as a pivot, during write.
 Canceled on conflict out to pivot %u, during read.
 Canceled on identification as a pivot, during commit attempt.
 Canceled on commit attempt with conflict in from prepared
   pivot.
 
 These are DETAIL messages, with the rest of the report saying
 
 ERROR:  could not serialize access due to read/write
   dependencies among transactions
 HINT:  The transaction might succeed if retried.
 
 AFAICT, the documentation doesn't mention anything about this
 pivot that keeps coming up.
 
Good point.  It's in the README-SSI and the Wiki page, but not in
the user docs; so using it in the error detail seems likely to
confuse.  Also, some of the information is referring to phases of
processing which can only be understood by looking at the source
code, like during conflict out checking.  While it might not be
entirely unreasonable to document what a pivot is, documenting some
of the other language is really out of the question.
 
 Is it useful that have the user face this information?  Is there
 anything a user can derive from seeing one of these messages
 versus another, and in addition to the error and hint, that would
 help them address the situation?
 
A user with a firm grasp of SSI might be better able to figure out
mitigation techniques for frequently occurring rollback scenarios
with that detail.
 
We did have one thread where someone was hoping for as much
information as we could provide, but I don't know how typical that
will be:
 
http://archives.postgresql.org/pgsql-hackers/2010-09/msg01133.php
 
I find the differentiation of causes helpful when working through
test cases, but moving this information to DEBUG messages might
satisfy that need.  I've also wondered whether that HINT line is
worthwhile.
 
I have a suspicion that we might sometimes find the information
conveyed by the detail useful when responding to users with
questions; but the language as it stands seems confusing for users.
 
-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] Reduced power consumption in WAL Writer process

On Jul 15, 2011, at 8:55 AM, Simon Riggs si...@2ndquadrant.com wrote:
 On Fri, Jul 15, 2011 at 2:29 PM, Robert Haas robertmh...@gmail.com wrote:
 
 If the primary goal here is to reduce power consumption, another option
 would be to keep the regular wake-ups most of the time but have some
 mechanism for putting the process to sleep until wakened when no activity
 happens for a certain period of time - say, 10 cycles. I'm not at all sure
 that's better, but it would be less of a change to the existing behavior.
 
 Now we have them, latches seem the best approach because they (mostly)
 avoid heuristics.

That's my feeling as well.

 This proposal works same or better for async transactions.

Right. I would say probably better.  The potential for a reduction in latency 
here is very appealing.

 The only difference is how bulk write operations are handled. As long
 as we wake WALWriter before wal_buffers fills then we'll be good.
 Wakeup once per wal buffer is too much. I agree we should measure this
 to check how frequently wakeups are required for bulk ops.

Yeah. The trick is to get the wake-ups to be frequent enough without adding too 
much latency to the backends that have to perform them. Off-hand, I don't  have 
a good feeling for how hard that will be.

...Robert
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Need help understanding pg_locks

2011-07-15 Thread Bruce Momjian


Thanks, applied.

---

Florian Pflug wrote:
 On Jul14, 2011, at 22:18 , Bruce Momjian wrote:
  !OID of the database in which the lock target exists, or
  !zero if the lock is a shared object, or
  !null if the lock is on a transaction ID
 
 For consistency, I think it should say target in the second part
 of the sentence also now, instead of lock ... on.
 
 Updated patch attached. I tried to make the descriptions a
 bit more consistent, replaced object by target, and
 added targeted by after the phrase which describes the
 locked (or waited-for) object.
 
 best regards,
 Florian Pflug
 
 diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
 index d4a1d36..33be5d0 100644
 *** a/doc/src/sgml/catalogs.sgml
 --- b/doc/src/sgml/catalogs.sgml
 ***
 *** 6928,6936 
 entrytypeoid/type/entry
 entryliterallink 
 linkend=catalog-pg-databasestructnamepg_database/structname/link.oid/literal/entry
 entry
 !OID of the database in which the object exists, or
 !zero if the object is a shared object, or
 !null if the object is a transaction ID
 /entry
/row
row
 --- 6928,6936 
 entrytypeoid/type/entry
 entryliterallink 
 linkend=catalog-pg-databasestructnamepg_database/structname/link.oid/literal/entry
 entry
 !OID of the database in which the lock target exists, or
 !zero if the target is a shared object, or
 !null if the target is a transaction ID
 /entry
/row
row
 ***
 *** 6938,6944 
 entrytypeoid/type/entry
 entryliterallink 
 linkend=catalog-pg-classstructnamepg_class/structname/link.oid/literal/entry
 entry
 !OID of the relation, or null if the object is not
  a relation or part of a relation
 /entry
/row
 --- 6938,6944 
 entrytypeoid/type/entry
 entryliterallink 
 linkend=catalog-pg-classstructnamepg_class/structname/link.oid/literal/entry
 entry
 !OID of the relation targeted by the lock, or null if the target is 
 not
  a relation or part of a relation
 /entry
/row
 ***
 *** 6947,6954 
 entrytypeinteger/type/entry
 entry/entry
 entry
 !Page number within the relation, or null if the object
 !is not a tuple or relation page
 /entry
/row
row
 --- 6947,6954 
 entrytypeinteger/type/entry
 entry/entry
 entry
 !Page number targeted by the lock within the relation,
 !or null if the target is not a relation page or tuple
 /entry
/row
row
 ***
 *** 6956,6962 
 entrytypesmallint/type/entry
 entry/entry
 entry
 !Tuple number within the page, or null if the object is not a tuple
 /entry
/row
row
 --- 6956,6963 
 entrytypesmallint/type/entry
 entry/entry
 entry
 !Tuple number targeted by the lock within the page,
 !or null if the target is not a tuple
 /entry
/row
row
 ***
 *** 6964,6971 
 entrytypetext/type/entry
 entry/entry
 entry
 !Virtual ID of a transaction, or null if the object is not a
 !virtual transaction ID
 /entry
/row
row
 --- 6965,6972 
 entrytypetext/type/entry
 entry/entry
 entry
 !Virtual ID of the transaction targeted by the lock,
 !or null if the target is not a virtual transaction ID
 /entry
/row
row
 ***
 *** 6973,6979 
 entrytypexid/type/entry
 entry/entry
 entry
 !ID of a transaction, or null if the object is not a transaction ID
 /entry
/row
row
 --- 6974,6981 
 entrytypexid/type/entry
 entry/entry
 entry
 !ID of the transaction targeted by the lock,
 !or null if the target is not a transaction ID
 /entry
/row
row
 ***
 *** 6981,6988 
 entrytypeoid/type/entry
 entryliterallink 
 linkend=catalog-pg-classstructnamepg_class/structname/link.oid/literal/entry
 entry
 !OID of the system catalog containing the object, or null if the
 !object is not a general database object
 /entry
/row
row
 --- 6983,6990 
 entrytypeoid/type/entry
 entryliterallink 
 linkend=catalog-pg-classstructnamepg_class/structname/link.oid/literal/entry
 entry
 !OID of the system catalog containing the lock target, or null if the
 !target is not a general database object
 /entry
/row
row
 ***
 *** 6990,6997

Re: [HACKERS] Reduced power consumption in WAL Writer process

Robert Haas robertmh...@gmail.com writes:
 On Jul 15, 2011, at 8:55 AM, Simon Riggs si...@2ndquadrant.com wrote:
 The only difference is how bulk write operations are handled. As long
 as we wake WALWriter before wal_buffers fills then we'll be good.
 Wakeup once per wal buffer is too much. I agree we should measure this
 to check how frequently wakeups are required for bulk ops.

 Yeah. The trick is to get the wake-ups to be frequent enough without adding 
 too much latency to the backends that have to perform them. Off-hand, I don't 
  have a good feeling for how hard that will be.

I'd say send the signal when wal buffers are more than X% full (maybe
half).  The suggestion to send it when wrapping around at the end of the
array is not quite right, because that's an arbitrary condition that's
not related to how much work there is for the walwriter to do.  It
should be cheap to check for this while advancing to a new wal buffer.

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] SSI error messages

On Jul 15, 2011, at 12:06 PM, Kevin Grittner kevin.gritt...@wicourts.gov 
wrote:
 I have a suspicion that we might sometimes find the information
 conveyed by the detail useful when responding to users with
 questions; but the language as it stands seems confusing for users.

I think removing info here or making it harder to get would be a mistake.  SSI 
is a complicated technology and we are likely to find that we need more 
debugging and instrumentation, not less.

...Robert
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] SSI error messages

Robert Haas robertmh...@gmail.com writes:
 On Jul 15, 2011, at 12:06 PM, Kevin Grittner kevin.gritt...@wicourts.gov 
 wrote:
 I have a suspicion that we might sometimes find the information
 conveyed by the detail useful when responding to users with
 questions; but the language as it stands seems confusing for users.

 I think removing info here or making it harder to get would be a mistake.

Agreed.  If we can clarify the messages, and/or make sure the
terminology they use is documented, that'd be a good thing; but let's
not just remove them.

I think that Peter's real concern is whether these are worth
translating, and I share that doubt.  Perhaps we should invent an
errdetail_internal, parallel to errmsg_internal, that works like
errdetail but doesn't treat the message as a candidate for translation?

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] Three patches which desperately need reviewers

2011-07-15 Thread Yeb Havinga


On 2011-07-14 02:42, Josh Berkus wrote:

The first two are difficult patches, but the other two are not.  Please
volunteer to give these patches a review; we owe it to our contributors
to review everything before the end of the CF.
When is the end of the CF? (I'm strongly suspecting today, but haven't 
been able to find it with a quick search)


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] Three patches which desperately need reviewers

Yeb Havinga yebhavi...@gmail.com writes:
 On 2011-07-14 02:42, Josh Berkus wrote:
 The first two are difficult patches, but the other two are not.  Please
 volunteer to give these patches a review; we owe it to our contributors
 to review everything before the end of the CF.

 When is the end of the CF? (I'm strongly suspecting today, but haven't 
 been able to find it with a quick search)

The scheduled end date is today, but we're obviously not ready.

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] Three patches which desperately need reviewers

2011-07-15 Thread Magnus Hagander

On Thu, Jul 14, 2011 at 01:42, Josh Berkus j...@agliodbs.com wrote:

 Allow multiple Postgres clusters running on the same machine to
 distinguish themselves in the event log
 https://commitfest.postgresql.org/action/patch_view?id=562

I've reviewed this now, but I won't have time to take it across the
finishing line in time for this CF. Regrettable, since the author came
back with a response really quick. I propose we bump it to the next CF
and I'll try to work on it before we even enter into that CF - unless
somebody else feels comfortable taking it all the way. But given that
it's been pending so long, I doubt that, but I'l be happy to deal with
it in-between.

-- 
 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] SSI error messages

Excerpts from Tom Lane's message of vie jul 15 14:33:34 -0400 2011:

 I think that Peter's real concern is whether these are worth
 translating, and I share that doubt.  Perhaps we should invent an
 errdetail_internal, parallel to errmsg_internal, that works like
 errdetail but doesn't treat the message as a candidate for translation?

Yeah.  The other point is that translated versions of those phrases are
likely to introduce randomly diverging terms, which is not going to help
with debugging.  As long as the technology is new enough that a user is
going to need help from -hackers (or at least someone who read and
grokked README.SSI) to debug a problem, there's no benefit to
translating those errdetails.

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: Initial Review: JSON contrib modul was: Re: [HACKERS] Another swing at JSON

2011-07-15 Thread Joey Adams

On Mon, Jul 4, 2011 at 10:22 PM, Joseph Adams
joeyadams3.14...@gmail.com wrote:
 I'll try to submit a revised patch within the next couple days.

Sorry this is later than I said.

I addressed the issues covered in the review.  I also fixed a bug
where \u0022 would become , which is invalid JSON, causing an
assertion failure.

However, I want to put this back into WIP for a number of reasons:

 * The current code accepts invalid surrogate pairs (e.g.
\uD800\uD800).  The problem with accepting them is that it would be
inconsistent with PostgreSQL's Unicode support, and with the Unicode
standard itself.  In my opinion: as long as the server encoding is
universal (i.e. UTF-8), decoding a JSON-encoded string should not fail
(barring data corruption and resource limitations).

 * I'd like to go ahead with the parser rewrite I mentioned earlier.
The new parser will be able to construct a parse tree when needed, and
it won't use those overkill parsing macros.

 * I recently learned that not all supported server encodings can be
converted to Unicode losslessly.  The current code, on output,
converts non-ASCII characters to Unicode escapes under some
circumstances (see the comment above json_need_to_escape_unicode).

I'm having a really hard time figuring out how the JSON module should
handle non-Unicode character sets.  \u escapes in JSON literals
can be used to encode characters not available in the server encoding.
 On the other hand, the server encoding can encode characters not
present in Unicode (see the third bullet point above).  This means
JSON normalization and comparison (along with member lookup) are not
possible in general.

Even if I assume server - UTF-8 - server transcoding is lossless,
the situation is still ugly.  Normalization could be implemented a few
ways:

 * Convert from server encoding to UTF-8, and convert \u escapes
to UTF-8 characters.  This is space-efficient, but the resulting text
would not be compatible with the server encoding (which may or may not
matter).
 * Convert from server encoding to UTF-8, and convert all non-ASCII
characters to \u escapes, resulting in pure ASCII.  This bloats
the text by a factor of three, in the worst case.
 * Convert \u escapes to characters in the server encoding, but
only where possible.  This would be extremely inefficient.

The parse tree (for functions that need it) will need to store JSON
member names and strings either in UTF-8 or in normalized JSON (which
could be the same thing).

Help and advice would be appreciated.  Thanks!

- Joey


json-contrib-rev1-20110714.patch.gz
Description: GNU Zip compressed data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Is there a committer in the house?

Alvaro,

 It seems that by mentioning some people but not all, you offended both
 the people you mentioned (at least some of them, because they are
 already actively helping) and those that you didn't (at least some of
 them, because they are already actively helping; those that are not
 completely inactive in the project, that is).

Yeah, everybody's super-touchy this week.   Must be the weather.

Speaking of which, is there anything you could commit?  ;-)


-- 
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] lazy vxid locks, v2

Robert,

I should be able to do some performance testing on this, but not today.

Questions:

(1) can you re-link me to the pgbench and sysbench setup you used to
test this originally?  I'd like to implement those.

(2) the max machine I can test these on is 16 cores.  Is that adequate,
or do we need more cores for real testing?

-- 
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] patch: Allow \dd to show constraint comments

Josh,

 Fair enough. If the pg_comments patch does go down in flames, I can
 circle back and patch up the rest of the holes in \dd.

I am unable to figure out the status of the pg_comments patch from this
thread.  What's going on with it?

-- 
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

[HACKERS] Commitfest Status: Sudden Death Overtime

All,

As you can probably tell, we are not ready to end the commitfest.  (I
think Robert gave me this CF to show why even talking about a one-week
mini-fest is a fantasy.  If so, successful.).

I've booted the patches which obviously aren't going to be immediately
ready.  Nine patches are ready for committer.  The rest fall into the
following buckets:

KAGAI's PATCHES

These are complex and need review by advanced hackers.  They are also
often interdependant.  As such, I'd like to automatically move his
patches to the next CF and ask that hackers who are engaged in working
on them continue to do so between CFs.

PATCHES WHICH I MOVED TO THE NEXT CF 9-2011:

* Collect frequency statistics and selectivity estimation for arrays
* Allow multiple Postgres clusters running on the same machine to
distinguish themselves in the event log
* lazy vxid locks

PATCHES WHICH HAVE BEEN UPDATED AND NEED FURTHER REVIEW:

* Cascade Replication

PATCHES STILL UNDER ACTIVE DISCUSSION:

* Add ability to constrain backend temporary file space

PATCHES I CAN'T FIGURE OUT THE STATUS OF:

* pg_comments system view
* Bugfix for XPATH() if expression returns a scalar value

So, down to a fairly limited list, except that we need a lot of committing.

-- 
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] SSI error messages

Alvaro Herrera alvhe...@commandprompt.com wrote:
 Excerpts from Tom Lane's message:
 
 I think that Peter's real concern is whether these are worth
 translating, and I share that doubt.  Perhaps we should invent an
 errdetail_internal, parallel to errmsg_internal, that works like
 errdetail but doesn't treat the message as a candidate for
 translation?
 
 Yeah.  The other point is that translated versions of those
 phrases are likely to introduce randomly diverging terms, which is
 not going to help with debugging.  As long as the technology is
 new enough that a user is going to need help from -hackers (or at
 least someone who read and grokked README.SSI) to debug a problem,
 there's no benefit to translating those errdetails.
 
OK, since that seems to be the consensus, I put a patch together. 
Testing it now.  Will post once I've confirmed it doesn't break
anything.
 
-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] Is there a committer in the house?

Excerpts from Josh Berkus's message of vie jul 15 16:32:42 -0400 2011:
 Alvaro,
 
  It seems that by mentioning some people but not all, you offended both
  the people you mentioned (at least some of them, because they are
  already actively helping) and those that you didn't (at least some of
  them, because they are already actively helping; those that are not
  completely inactive in the project, that is).
 
 Yeah, everybody's super-touchy this week.   Must be the weather.

It's been regularly horrible here, so yeah, that might explain it.

 Speaking of which, is there anything you could commit?  ;-)

I intend to give the finishing touches to Pavel's plpgsql patch and
commit, so that's one item to take off the list.  Everything else seems
to be in somebody else's hands, or is larger than I can grab ATM.

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] Is there a committer in the house?

2011-07-15 Thread Bruce Momjian

Josh Berkus wrote:
 Alvaro,
 
  It seems that by mentioning some people but not all, you offended both
  the people you mentioned (at least some of them, because they are
  already actively helping) and those that you didn't (at least some of
  them, because they are already actively helping; those that are not
  completely inactive in the project, that is).
 
 Yeah, everybody's super-touchy this week.   Must be the weather.

Somehow blaming everyone else doesn't seem like the proper reaction.  :-(

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Mysterious server crashes

2011-07-15 Thread Žiga Kranjec


Hello!

Recently we have upgraded our debian system (sid),
which has since started crashing mysteriously.
We are still looking into that. It runs on 3ware RAID.
Postgres package is 8.4.8-2.

The database came back up apparently ok, except
for indexes. Running reindex produces this error on
one of the tables:

ERROR:  unexpected chunk number 1 (expected 0) for toast value 17539760 
in pg_toast_16992


Same with select.

I tried running reindex on toast table didn't help. Running:

select * from pg_toast.pg_toast_16992 where chunk_id = 17539760;

crashed postgres backend (and apparently the whole server).

Is there anything we can/should do to fix the problem, besides
restoring the whole database from backup?

Thanks!

Ziga


--
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] libpq SSL with non-blocking sockets

Robert Haas robertmh...@gmail.com writes:
 On Fri, Jul 8, 2011 at 9:36 AM, Martin Pihlak martin.pih...@gmail.com wrote:
 On 07/03/2011 05:08 AM, Steve Singer wrote:
 Since the original patch was submitted as a WIP patch and this version
 wasn't sent until well into the commit fest I am not sure if it
 qualifies for a committer during this commitfest or if it needs to wait
 until the next one.

 If possible I would like the fix to be backported as well. This is
 quite a nasty bug and difficult to track down. Especially as the
 actual SSL error messages are masked by the server closed the
 connection unexpectedly message.

 I would not be inclined to back-patch this straight away.  I agree
 it's an important fix, but I am a little worried about the chances of
 breaking something else...  then again, I don't have the only vote
 here.

I reviewed this patch a bit.  I agree that we have a bug here that
should be fixed and backported, but I don't think this patch is ready.
Some problems:

1. The behavior under low-memory conditions is pretty intolerable.
As coded, a malloc failure here:

+conn-outBufSize = Max(16 * 1024, remaining);
+conn-outBuffer = malloc(conn-outBufSize);
+if (conn-outBuffer == NULL)
+{
+printfPQExpBuffer(conn-errorMessage,
+  cannot allocate memory for output 
buffer\n);
+return -1;
+}

leaves libpq's internal state completely corrupt --- outBuffer is null,
which will result in instant coredump on any future access, but that's
just the tip of the iceberg because we've also lost buffered data and
messed up the none-too-clearly-defined-anyway state of which pending
data is in which buffer.

In general, I don't think it's a smart idea to proceed by duplicating
the buffer contents in this situation, particularly not if the most
obvious way to cause the problem is to have a very large buffer :-(.
I think the direction to move in ought to be to use the existing buffer
as-is, and have pqCheckOutBufferSpace refuse to enlarge the buffer while
we are in write frozen state.  It should be OK to append data to the
buffer, though, so long as we remember how much we're allowed to pass to
SSL_write when we next try to write.

2. According to the OpenSSL man pages, *both* SSL_read and SSL_write are
defined to need to be re-called with the identical arguments after a
WANT_READ or WANT_WRITE return.  This means that pqReadData() is also at
risk for this type of bug.  I believe it could only happen in code paths
where we call pqReadData when there is already some data in the buffer:
if the caller then consumes some of that data, the next call to
pqReadData would try to compact the buffer contents before making the
pqsecure_read call.  I think we probably need a read frozen state in
which we won't enlarge or compact the inBuffer until SSL_read succeeds.

3. As of 9.0, somebody's decided that the backend needs nonblock read
semantics in some cases.  I probably should have complained harder about
that before it went in, but since it's in, the backend is at risk for
this same type of issue in secure_read().

I will mark the patch Returned With Feedback.  I think that we need to
address all these issues before we consider applying it.  If we weren't
hard up against the end of the CommitFest I might have a go at it
myself, but I can't justify the time right now.

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] Is there a committer in the house?


 Yeah, everybody's super-touchy this week.   Must be the weather.
 
 Somehow blaming everyone else doesn't seem like the proper reaction.  :-(

Look, it wasn't meant to be a complete list, or even a representative
one.  That's why I tagged a Bueller? at the end.  Even for those who
don't get the reference, there should have been a Who the hell is Bueller?

You might notice that we don't publish a list of committers anywhere.
In fact, *I* don't have one.  So when I want to say hey, who could be
doing committing and isn't currently? I don't have any way to answer
that question.  Which is why I posted the e-mail.

So, I reiterate: who do we have, as committers, who are capable of
committing a fairly wide array of patches?  This would be good
information for every CF Manager to have, when things start getting
stuck in ready for committer.

-- 
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] Patch Review: Collect frequency statistics and selectivity estimation for arrays

2011-07-15 Thread Nathan Boley

 Actually, not MCV slot is used but MCELEM. It is a different slot. ps_stats
 view map both into same fileds.

Yes, you're right. I am sorry about the error.

 Surely, non-standard use of histogram slot
 should be avoided.

Agreed.

Best,
Nathan

-- 
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] Is there a committer in the house?

Josh Berkus j...@agliodbs.com writes:
 You might notice that we don't publish a list of committers anywhere.
 In fact, *I* don't have one.

http://wiki.postgresql.org/wiki/Committers

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] isolation tests broken for other than 'read committed'

It's been a few days since I've run through my usual builds and
tests, and I just discovered that part of my routine was broken by
this commit:
 
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=846af54dd5a77dc02feeb5e34283608012cfb217
 
The isolation tests are broken when run against a database with
default_transaction_isolation = 'repeatable read' or 'serializable'.
(Which is ironic, really.)
 
Adding the attached files to src/test/isolation/expected/ causes
those stricter isolation levels to work in my tests so far, but I
get random failures in 'read committed' due to apparent randomness
in which process is chosen as the deadlock victim.  I seem to
remember Noah mentioning this and a suggested fix, but the problem
in manifest in a current checkout of head.
 
Of course, another approach to this would be to set transaction
isolation level in the new tests.  If we do that, we might want to
create tests at all three levels, for completeness.
 
-Kevin



fk-deadlock_1.out
Description: Binary data


fk-deadlock2_1.out
Description: Binary data


fk-deadlock2_2.out
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] patch for distinguishing PG instances in event log

2011-07-15 Thread Magnus Hagander

On Fri, Jul 15, 2011 at 15:03, MauMau maumau...@gmail.com wrote:
 Magnus,

 Thank you for reviewing my patch. I'm going to modify the patch according to
 your comments and re-submit it. Before that, I'd like to discuss some points
 and get your agreement.

Ok, please do. If you want to, you can work off my git branch at
http://github.com/mhagander/postgres (branch multievent).


 From: Magnus Hagander mag...@hagander.net

 +        para
 +         On Windows, you need to register an event source
 +         and its library with the operating system in order
 +         to make use of the systemitemeventlog/systemitem option for
 +         varnamelog_destination/.
 +         See xref linkend=event-log-registration for details.
 +        /para

 * This part is not strictly correct - you don't *need* to do that, it
 just makes things look nicer, no?

 How about the following statement? Is it better to say correctly instead
 of cleanly?

 --
 On Windows, when you use the eventlog option for log_destination, you need
 to register an event sourceand its library with the operating system so that
 the Windows Event Viewer can display event log messages cleanly.
 --

Replace need with should and I'm happy with that.


 * Also, what is the use for set_eventlog_parameters()? It's just a
 string variable, it shuold work without it.

 Yes, exactly. I'll follow your modification.

 * We these days avoid #ifdef'ing gucs just because they are not on
 that platform - so the list is consistent. The guc should be available
 on non-windows platforms as well.

 I see. When I saw syslog_ident not being #ifndef WIN32'ed, I thought that
 was because syslog might be available on Cygwin or MinGW. Anyway, I'll take
 your modification.

Nope, it used to be #ifdefed on HAVE_SYSLOG, but we changed that a
while ago for consistency.



 * The guc also needs to go in postgresql.conf.sample

 I missed this completely.

 * Are we really allowed to call MessageBox in DlLRegisterService?
 Won't that break badly in cases like silent installs?

 It seems that the only way to notify the error is MessageBox. Printing to
 stdout/stderr does not show any messages on the command prompt. I guess
 that's why the original author of pgevent.c used MessageBox.

oh, we're already using messagebox.. I must've been confused, I
thought it was new. Heck, it might even be me who wrote that :O


 We cannot know within the DLL if /s (silent) switch was specified to
 regsvr32.exe. If we want to suppress MessageBox during silent installs, it
 seems that we need to know the silent install by an environment variable or
 so. That is:

 C:\ set PGSILENT=true
 C:\ regsvr32.exe ...

I think if we're not Ok with messagebox, then we should just write it
to the eventlog. However, given that we have been using messagebox
before and had no complaints, I should withdraw my complaint for now -
keep using messagebox like the surrounding code. We can attack the
potential issue of that in a separate patch later.


 * We never build in unicode mode, so all those checks are unnecessary.

 Attached is an updated patch, which doesn't work yet. I believe the
 changes to the backend are correct, but probably some of the cleanups
 and changes in the dll are incorrect, because I seem to be unable to
 register either the default or a custom handler so far.

 The cause of the problem you are encountering is here:

 + if (pszCmdLine  *pszCmdLine != '\0')
 +  strncpy(event_source, sizeof(event_source), pszCmdLine);
 + else
 +  strcpy(event_source, PostgreSQL);

 DllInstall() always receives the /i argument in UTF-16. str... functions
 like strncpy() cannot be used for handling UTF-16 strings. For example, when
 you run regsvr32.exe /i:abc ..., DllInstall's pszCmdLine value becomes
 a\0b\0c\0. So, strncpy() just copies a. This is the reason why you
 cannot register the custom event source.

Oh, it gets it as UTF-16 even when not in unicode mode. I see - yeah,
I didn't have time to read up on the calling conventions properly.


 The reason why you cannot register the default event source (i.e. running
 regsvr32 without /i) is that you memset()ed event_source in DllMain() and
 set PostgreSQL to it in DllInstall(). DllInstall() is not called when /i
 is not specified. So, event_source remains empty.

 To solve the problem, we need to use wcstombs_s() instead of strncpy(), and
 initialize event_source to PostgreSQL when it is defined.

Ok, seems reasonable.

-- 
 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] Is there a committer in the house?


 http://wiki.postgresql.org/wiki/Committers

Oh, thanks!  I didn't know that existed.

So, if any of the following people could possibly pick up even one patch
from the current commitfest and commit it, it would clear out our
pending commit list:

* Bruce Momjian
* Tatsuo Ishii
* Andrew Dunstan
* Magnus Hagander
* Greg Stark
* Itagaki Takahiro

... noting that I have already heard from Simon, Joe, Alvaro, Tom,
Robert, Peter and Heikki.  I'm basing the above list on (a) some
knowledge of which folks only seem to work on very specific areas of
code, and (b) looking at pgsql-committers for the past 3 weeks.

In any case, it seems like we have a pool of 13 people would could be
committing general patches every commitfest.  Not a huge number
considering the number of patches we get, but more than I see actually
committing stuff for most CFs.

-- 
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] proposal: a validator for configuration files

Excerpts from Alexey Kluykin's message of jue jul 14 09:18:15 -0400 2011:
 
 On Jul 14, 2011, at 4:38 AM, Alvaro Herrera wrote:
 

  On Jul14, 2011, at 01:38 , Alvaro Herrera wrote:

 This is happening because a check for total number of errors so far is 
 happening only after coming across at least one non-recognized configuration 
 option.  What about adding one more check right after ParseConfigFile, so we 
 can bail out early when overwhelmed with syntax errors? This would save a 
 line in translation :).

Actually I think it would make sense to do it completely the other way
around: reset the number of errors to zero before starting to apply the
values.  The rationale is that all the settings that made it past the
tokenisation are completely different lines for which the errors were
reported earlier.

  I know I'd feel more comfortable if you (and Alexey, and Selena) gave it
  another look :-)
 
 I have checked it here and don't see any more problems with it.

Thanks.

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] SSI error messages

Kevin Grittner kevin.gritt...@wicourts.gov wrote:
 Alvaro Herrera alvhe...@commandprompt.com wrote:
 Excerpts from Tom Lane's message:
 
 I think that Peter's real concern is whether these are worth
 translating, and I share that doubt.  Perhaps we should invent
 an errdetail_internal, parallel to errmsg_internal, that works
 like errdetail but doesn't treat the message as a candidate for
 translation?
 
 Yeah.  The other point is that translated versions of those
 phrases are likely to introduce randomly diverging terms, which
 is not going to help with debugging.  As long as the technology
 is new enough that a user is going to need help from -hackers (or
 at least someone who read and grokked README.SSI) to debug a
 problem, there's no benefit to translating those errdetails.
  
 OK, since that seems to be the consensus, I put a patch together. 
 Testing it now.  Will post once I've confirmed it doesn't break
 anything.
 
OK, after getting distracted by test failures caused by an unrelated
commit, I've confirmed that this passes my usual tests.  I don't
know anything about the tools used for extracting the text for the
translators, so if that needs any corresponding adjustment, someone
will need to point me in the right direction or cover that part
separately.
 
The leading whitespace changes in predicate.c are from pgindent.
 
-Kevin

*** a/src/backend/storage/lmgr/predicate.c
--- b/src/backend/storage/lmgr/predicate.c
***
*** 3776,3782  CheckForSerializableConflictOut(bool visible, Relation 
relation,
ereport(ERROR,
(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
 errmsg(could not serialize access due to 
read/write dependencies among transactions),
!errdetail(Canceled on identification as a 
pivot, during conflict out checking.),
 errhint(The transaction might succeed if 
retried.)));
}
  
--- 3776,3782 
ereport(ERROR,
(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
 errmsg(could not serialize access due to 
read/write dependencies among transactions),
!errdetail_internal(Canceled on identification 
as a pivot, during conflict out checking.),
 errhint(The transaction might succeed if 
retried.)));
}
  
***
*** 3865,3871  CheckForSerializableConflictOut(bool visible, Relation 
relation,
ereport(ERROR,

(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
 errmsg(could not serialize 
access due to read/write dependencies among transactions),
!   errdetail(Canceled on conflict out to old 
pivot %u., xid),
  errhint(The transaction might 
succeed if retried.)));
  
if (SxactHasSummaryConflictIn(MySerializableXact)
--- 3865,3871 
ereport(ERROR,

(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
 errmsg(could not serialize 
access due to read/write dependencies among transactions),
!errdetail_internal(Canceled 
on conflict out to old pivot %u., xid),
  errhint(The transaction might 
succeed if retried.)));
  
if (SxactHasSummaryConflictIn(MySerializableXact)
***
*** 3873,3879  CheckForSerializableConflictOut(bool visible, Relation 
relation,
ereport(ERROR,

(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
 errmsg(could not serialize 
access due to read/write dependencies among transactions),
!errdetail(Canceled on 
identification as a pivot, with conflict out to old committed transaction %u., 
xid),
  errhint(The transaction might 
succeed if retried.)));
  
MySerializableXact-flags |= 
SXACT_FLAG_SUMMARY_CONFLICT_OUT;
--- 3873,3879 
ereport(ERROR,

(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
 errmsg(could not serialize 
access due to read/write dependencies among transactions),
!errdetail_internal(Canceled 
on identification as a pivot, with conflict out to old committed transaction 
%u., xid),
  errhint(The transaction might 
succeed if retried.)));

Re: [HACKERS] Is there a committer in the house?

Excerpts from Josh Berkus's message of vie jul 15 18:33:14 -0400 2011:
 
  http://wiki.postgresql.org/wiki/Committers
 
 Oh, thanks!  I didn't know that existed.
 
 So, if any of the following people could possibly pick up even one patch
 from the current commitfest and commit it, it would clear out our
 pending commit list:

The ready for committer state does not mean that the committer can
grab the patch and apply it.  Last time I checked, one was still
expected to review it and take full responsibility for any breakage
caused by it.  Given this, I cannot responsibly grab more than a couple
patches, because then I'd be swamped when bug reports started coming in.

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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_comments system view

2011-07-15 Thread Josh Kupershmidt

On Fri, Jul 15, 2011 at 4:48 PM, Josh Berkus j...@agliodbs.com wrote:
I am unable to figure out the status of the pg_comments patch from this
thread. What's going on with it?

I don't blame you :-)

I think this thread got so confusing because two separate topics were
intertwined. (I'm going to try to change this thread subject to
reflect the fact that we're really talking about pg_comments here
now.)

First, the original post, with a small patch to fix one of the many
problems with psql's \dd command. That patch was rejected because it
only plugged one of the many problems with \dd. I have since started
another thread[1] with a plausible fix for \dd and several other
backslash commands, so that we will have working displays of all
comments with minimal duplication.

Second, we have the pg_comments view (latest version is the
v10.WIP). Despite its WIP tag, I think it is actually pretty close
to being complete at this point. The first concern which I raised a
concern in that thread[2]:

1.) For now, I'm just ignoring the issue of visibility checks;

is the only big issue I see as still outstanding. At first, I was
assuming that \dd should naturally read from pg_comments to fetch the
object comments it is interested in. But that would mean that we'd
need some way to duplicate those visibility checks \dd was doing,
either in \dd or in another is_visible column in pg_comments.

I haven't tried either of those options out yet, but I was worried
they'd both be tricky/ugly. Which leads me to think, maybe it's not so
bad if \dd stays the way I've suggested in thread[1], i.e. just
querying pg_[sh]description for the five object types it needs
directly. After all, \dd will IMO be close to useless/deprecated once
we have pg_comments; it'll be much easier to just query pg_comments
for what you're looking for directly, and \dd will only display five
funky object types, anyway.

How do folks feel about this issue?

The second concern I raised with the last pg_comments patch,

I think now might be a good time to
re-examine what types of objects are displayed by \dd.

should be handled by thread[1], and the third concern is just about
whitespace. Oh, and docs need some adjusting too, and it'd be nice if
someone sanity checked my guesses for the is_system column (or if
it's not needed, that's OK too).

So that's where the pg_comments patch stands, at least AIUI. Clear as
mud yet? :)

Josh

[1] http://archives.postgresql.org/pgsql-hackers/2011-07/msg00459.php
[2]
http://archives.postgresql.org/message-id/CAK3UJRGNwKq0c2VsSYV-Mg55Y_kvZE=8fmr_xt8rzp__1lo...@mail.gmail.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] patch: pg_comments system view

On 7/15/11 3:54 PM, Josh Kupershmidt wrote:
 So that's where the pg_comments patch stands, at least AIUI. Clear as
 mud yet? :)

Sounds like returned with feedback to 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] FOR KEY LOCK foreign keys

Excerpts from Noah Misch's message of mié jul 13 01:34:10 -0400 2011:

 coypu failed during the run of the test due to a different session being 
 chosen
 as the deadlock victim.  We can now vary deadlock_timeout to prevent this; see
 attached fklocks-tests-deadlock_timeout.patch.  This also makes the tests much
 faster on a default postgresql.conf.

I applied your patch, thanks.  I couldn't reproduce the failures without
it, even running only the three new tests in a loop a few dozen times.

 crake failed when it reported waiting on the first step of an existing 
 isolation
 test (two-ids.spec).  I will need to look into that further.

Actually, there are four failures in tests other than the two fixed by
your patch.  These are:

http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crakedt=2011-07-12%2022:32:02
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=nightjardt=2011-07-14%2016:27:00
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=pittadt=2011-07-15%2015:00:08
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crakedt=2011-07-15%2018:32:02

The last two are an identical failure in multiple-row-versions:
***
*** 1,11 
  Parsed test spec with 4 sessions
  
  starting permutation: rx1 wx2 c2 wx3 ry3 wy4 rz4 c4 c3 wz1 c1
! step rx1:  SELECT * FROM t WHERE id = 100; 
  id txt
  
  100   
- step wx2:  UPDATE t SET txt = 'b' WHERE id = 100; 
  step c2:  COMMIT; 
  step wx3:  UPDATE t SET txt = 'c' WHERE id = 100; 
  step ry3:  SELECT * FROM t WHERE id = 50; 
--- 1,12 
  Parsed test spec with 4 sessions
  
  starting permutation: rx1 wx2 c2 wx3 ry3 wy4 rz4 c4 c3 wz1 c1
! step rx1:  SELECT * FROM t WHERE id = 100;  waiting ...
! step wx2:  UPDATE t SET txt = 'b' WHERE id = 100; 
! step rx1: ... completed
  id txt
  
  100   
  step c2:  COMMIT; 
  step wx3:  UPDATE t SET txt = 'c' WHERE id = 100; 
  step ry3:  SELECT * FROM t WHERE id = 50; 


The other failure by crake in two-ids:

***
*** 440,447 
  step c3:  COMMIT; 
  
  starting permutation: rxwy2 wx1 ry3 c2 c3 c1
! step rxwy2:  update D2 set id = (select id+1 from D1); 
  step wx1:  update D1 set id = id + 1; 
  step ry3:  select id from D2; 
  id
  
--- 440,448 
  step c3:  COMMIT; 
  
  starting permutation: rxwy2 wx1 ry3 c2 c3 c1
! step rxwy2:  update D2 set id = (select id+1 from D1);  waiting ...
  step wx1:  update D1 set id = id + 1; 
+ step rxwy2: ... completed
  step ry3:  select id from D2; 
  id


And the most problematic one, in nightjar, is a failure to send two
async commands, which is not supported by the new code:

--- 255,260 
  ERROR:  could not serialize access due to read/write dependencies among 
transactions
  
  starting permutation: ry2 wx2 rx1 wy1 c2 c1
! step ry2:  SELECT count(*) FROM project WHERE project_manager = 1;  waiting 
...
! failed to send query: another command is already in progress
  



-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] Is there a committer in the house?

Alvaro,

 The ready for committer state does not mean that the committer can
 grab the patch and apply it.  Last time I checked, one was still
 expected to review it and take full responsibility for any breakage
 caused by it.

You're absolutely correct.  Which is why committer bandwidth is such a
choke point.

-- 
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] isolation tests broken for other than 'read committed'

Excerpts from Kevin Grittner's message of vie jul 15 18:23:10 -0400 2011:
It's been a few days since I've run through my usual builds and
tests, and I just discovered that part of my routine was broken by
this commit:

http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=846af54dd5a77dc02feeb5e34283608012cfb217

Sorry 'bout that.

The isolation tests are broken when run against a database with
default_transaction_isolation = 'repeatable read' or 'serializable'.
(Which is ironic, really.)

Adding the attached files to src/test/isolation/expected/ causes
those stricter isolation levels to work in my tests so far, but I
get random failures in 'read committed' due to apparent randomness
in which process is chosen as the deadlock victim. I seem to
remember Noah mentioning this and a suggested fix, but the problem
in manifest in a current checkout of head.

Yeah, the patch I committed from Noah should fix the issues in read
committed. It hadn't crossed my mind that I need to manually set the
level to serializable in order for the tests to be meaningful :-(
Shouldn't we be running the tests three times with the different useful
isolation levels?

Of course, another approach to this would be to set transaction
isolation level in the new tests. If we do that, we might want to
create tests at all three levels, for completeness.

I think your approach of adding alternative expected outputs makes more
sense.

--
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

--
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 for 9.2: enhanced errors

Pavel Stehule pavel.steh...@gmail.com writes:
 I am sending a updated patch

I looked over this patch a bit.  I guess my main concern about it
is that the set of items to be reported seems to have been made up on
a whim.  I think that we ought to follow the SQL standard, which has a
pretty clearly defined set of additional information items --- look at
the spec for the GET DIAGNOSTICS statement.  (In SQL:2008, this is
section 23.1 get diagnostics statement.)  I don't feel that we need to
implement every field the standard calls for, at least not right away,
but we ought to have their list in mind.  Conversely, implementing items
that *aren't* listed in the spec has to meet a considerably higher bar
than somebody just submitting a patch that does it.

The standard information items that seem reasonable for us to implement
in the near future are

COLUMN_NAME
CONSTRAINT_NAME
CONSTRAINT_SCHEMA
SCHEMA_NAME
TABLE_NAME
TRIGGER_NAME
TRIGGER_SCHEMA

So I'd like to see the patch revised to use this terminology.  We
probably also need to think a bit harder about the PG_DIAG_XXX code
letters to be used --- we're already just about at the limit of what
fields can have reasonably-mnemonic code letters, and not all of the
above have obvious choices, let alone the rest of what's in the spec
that we might someday want to implement.  What assignment rule should
we use when we can't choose a mnemonic letter?

Some other specific comments on the patch follow:

1. It's way short in the documentation department.  protocol.sgml
certainly needs additions (see Error and Notice Message Fields),
also libpq.sgml's discussion of PQresultErrorField(), also
sources.sgml's Reporting Errors Within the Server, and I'm not
sure where else.

2. I think you could drop the tuple-descriptor changes, because they're
only needed in service of an information item that is not found in the
standard and doesn't seem very necessary.  The standard says to report
the name of the constraint, not what columns it involves.

3. errrel() is extremely poorly considered.  The fact that it requires
utils/relcache.h to be #included by elog.h (and therefore by *every*
*single* *file* in the backend) is a symptom of that, but expecting
elog.c to do catalog lookups is as bad or worse from a modularity
standpoint.  I think all the added elog functions should not take
anything higher-level than a C string.

4. Actually, it would probably be a good idea to avoid inventing a new
elog API function for each individual new information item; something
along the lines of erritem(PG_DIAG_WHATEVER, string_value) would be
more appropriate to cover the inevitable future expansions.

5. I don't think IndexRelationGetParentRelation is very appropriate
either --- in the use cases you have, the parent table's OID is easily
accessible, as is its namespace (which'll be the same as the index's)
and so you could just have the callers do get_rel_name(tableoid).
Doing a relcache open in an error reporting path seems like overkill.

I'm going to mark this patch Returned With Feedback.

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] proposal: a validator for configuration files

Alvaro Herrera alvhe...@commandprompt.com writes:
 Excerpts from Alexey Kluykin's message of jue jul 14 09:18:15 -0400 2011:
 This is happening because a check for total number of errors so far
 is happening only after coming across at least one non-recognized
 configuration option.  What about adding one more check right after
 ParseConfigFile, so we can bail out early when overwhelmed with
 syntax errors? This would save a line in translation :).

 Actually I think it would make sense to do it completely the other way
 around: reset the number of errors to zero before starting to apply the
 values.  The rationale is that all the settings that made it past the
 tokenisation are completely different lines for which the errors were
 reported earlier.

I'd like to re-open the discussion from here
http://archives.postgresql.org/pgsql-hackers/2011-06/msg01741.php
http://archives.postgresql.org/pgsql-hackers/2011-04/msg00330.php
specifically about this concern:

 Does that mean that some backends might currently choose to ignore an
 updated config file wholesale on SIGUP (because some settings are invalid)
 while others happily apply it? Meaning that they'll afterwards disagree
 even on modified settings which *would* be valid for both backends?

I complained about exactly that point back in April, but at the time
people (quite reasonably) didn't want to touch the behavior.  Now that
we are entering a new development cycle, it's time to reconsider.
Particularly so if we're considering a patch that touches the behavior
already.

I think that it might be sensible to have the following behavior:

1. Parse the file, where parse means collect all the name = value
pairs.  Bail out if we find any syntax errors at that level of detail.
(With this patch, we could report some or all of the syntax errors
first.)

2. Tentatively apply the new custom_variable_classes setting if any.

3. Check to see whether all the names are valid.  If not, report
the ones that aren't and bail out.

4. Apply each value.  If some of them aren't valid, report that,
but continue, and apply all the ones that are valid.

We can expect that the postmaster and all backends will agree on the
results of steps 1 through 3.  They might differ as to the validity
of individual values in step 4 (as per my example of a setting that
depends on database_encoding), but we should never end up with a
situation where a globally correct value is not globally applied.

The original argument for the current behavior was to avoid applying
settings from a thoroughly munged config file, but I think that the
checks involved in steps 1-3 would be sufficient to reject files that
had major problems.  It's possible that step 1 is really sufficient to
cover the issue, in which case we could drop the separate step-3 pass
and just treat invalid GUC names as a reason to ignore the particular
line rather than the whole file.  That would make things simpler and
faster, and maybe less surprising too.

Comments?

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] SSI error messages

Kevin Grittner kevin.gritt...@wicourts.gov writes:
 OK, after getting distracted by test failures caused by an unrelated
 commit, I've confirmed that this passes my usual tests.  I don't
 know anything about the tools used for extracting the text for the
 translators, so if that needs any corresponding adjustment, someone
 will need to point me in the right direction or cover that part
 separately.

Well, the point is that this function *isn't* going to be known to the
NLS code, so AFAICS no adjustments should be needed there.  You did miss
some places that ought to be updated (mumble sources.sgml mumble) but
unless I hear objections to the whole idea, I'll fix and apply this
tomorrow.

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_class.relistemp

2011-07-15 Thread Jim Nasby

On Jul 13, 2011, at 2:23 PM, David E. Wheeler wrote:
 Wasn't newsysviews supposed to deal with these sorts of issues? Why were they 
 rejected?

Unless they recently came up again and got rejected again; the original 
complaint was that some of their conventions didn't follow information_schema 
conventions. The community wanted that changed and that never happened.
--
Jim C. Nasby, Database Architect   j...@nasby.net
512.569.9461 (cell) http://jim.nasby.net



-- 
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: pg_comments system view

2011-07-15 Thread Josh Kupershmidt

On Fri, Jul 15, 2011 at 7:01 PM, Josh Berkus j...@agliodbs.com wrote:
 On 7/15/11 3:54 PM, Josh Kupershmidt wrote:
 So that's where the pg_comments patch stands, at least AIUI. Clear as
 mud yet? :)

 Sounds like returned with feedback to me.

Yeah, that's fine for this CF (though I do welcome any
suggestions/feedback about the problems which need to be fixed). I'll
try to put together another version for the next CF soon.

Josh

-- 
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: Allow \dd to show constraint comments