Re: [HACKERS] [BUGS] BUG #4516: FOUND variable does not work after RETURN QUERY

2009-01-10 Thread Pavel Stehule
Hello

2009/1/10 Tom Lane t...@sss.pgh.pa.us:
 Bruce Momjian br...@momjian.us writes:
 Uh, is this ready to be applied?

 I don't think any consensus has been reached on changing this behavior.

regards, tom lane


I  thing, so this is bug - RETURN QUERY has to supply FOR SELECT LOOP
RETURN NEXT pattern.

My first patch expected so RETURN QUERY is final statement, so I don't
solve FOUND variable, but Heikki changed this behave.

Without correct FOUND behave we can't to use RETURN QUERY for following pattern

RETURN QUERY some;
IF FOUND THEN RETURN; END IF;
RETURN QUERY some_other;
RETURN;

regards
Pavel Stehule

-- 
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] Hot standby, slot ids and stuff

2009-01-10 Thread Simon Riggs

On Thu, 2009-01-08 at 20:30 +0200, Heikki Linnakangas wrote:

 Since I've been whining about that for some time, I figured I have to 
 put my money where my mouth is, so here's a patch based on v6a that 
 eliminates the concept of slotids, as well as the new xl_info2 field in 
 XLogRecord. This seems much simpler to me. I haven't given it much 
 testing, but seems to work. There's a whole bunch of comments marked 
 with XXX that need resolving, though.

There's a confusion in the patch between top level xid and parent xid.
The xl field is named parentxid but actually contains top level. That
distinction is important because the code now uses the top level xid to
locate the recovery proc, formerly the role of the slotid.

This leads to an error when we SubTransSetParent(child_xid, top_xid);
since this assumes that the top_xid is the parent, which it is not.
Mostly you wouldn't notice unless you were looking up the subtrans
status for an xid that had committed but was the child of an aborted
subtransaction, with the top level xid having  64 subtransactions. It's
possible the confusion leads to other bugs in UnobservedXid processing,
but I didn't look too hard at that.

AFAICS we need both parent and top xids. Or put another way, we need the
parent xid and other info that allows us to uniquely determine the proc
we need to update. Now the other info... could be top xid or it could
also be slotid, which then avoids later zig-zagging to look up the proc.

I'm wasn't looking for ways to reintroduce slotid, but it seems more
logical to keep slotid in light of the above. However, you will probably
view this as intransigence, so I will await your input.

I'm very happy that GetStandbyInfoForTransaction() and all the XLR2
flags have bitten the dust and will sleep for eternity.

For xl_rel_lock you add a field called xid and then ask
/* xid of the *parent* transaction. XXX why parent? */.
You've done this because it replaced slotid. But looking at that, I
think the 6a patch had a bug there: a subtransaction abort record would
release locks for the whole top level xact. So we need to pass both top
level xid (or slotid) and xid for each lock, then release using the
actual xid only.

You also ask: Shouldn't we call StartupSUBTRANS() and the other startup
functions like we do below, before letting anyone in?

My answer is that the current role of StartupSUBTRANS and friends is not
appropriate at that point, since they zero out those structures. I left
those routines in place thinking startup meant moving to normal
running. If we did have a startupsubtrans at the point you note, it
would currently be empty: we don't keep track of the latest page during
recovery. Perhaps we should, but then we'd need to do the equivalent of
ExtendSubtrans etc, which it seemed easier to avoid.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and 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] Synch Rep v5

2009-01-10 Thread Fujii Masao
Hi,

I attached the updated version of Synch Rep patch (v5) on wiki.
The description of User Overview is also already updated.
http://wiki.postgresql.org/wiki/NTT%27s_Development_Projects#Synch_Rep

On Thu, Dec 18, 2008 at 9:55 AM, Simon Riggs si...@2ndquadrant.com wrote:
 4. sleeping
 http://archives.postgresql.org/pgsql-hackers/2008-12/msg00438.php

 I'm looking for the better idea. How should we resolve that problem?
 Only reduce the timeout of pq_wait to 100ms? Get rid of
 SA_RESTART only during pq_wait as follows?

remove SA_RESTART
pq_wait()
add SA_RESTART

 Not sure, will consider. Ask others as well.

I've not got an idea yet. Now (v5), I only reduce the timeout of
pq_wait to 100ms. Is this sufficient? Do you have any good idea?

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

-- 
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: new border setting in psql

2009-01-10 Thread D'Arcy J.M. Cain
On Fri, 9 Jan 2009 22:46:06 -0500 (EST)
Greg Smith gsm...@gregsmith.com wrote:
 On Fri, 9 Jan 2009, D'Arcy J.M. Cain wrote:
 
  . is on the long list of characters to be escaped I sent out earlier.
 
  I tried escaping the '.' but it didn't change the behaviour.
 
 I did try that specific exapmle in Trac and it worked fine for me.  Using 
 your test rig (which you gave the wrong URL to: 
 http://www.druid.net/darcy/rest.py ), I see this:
 
 I. Test   -  1. Test
 I\. Test  -  I. Test

You are right.  I can't recall the actual test I did but somehow I
found a situation where escaping the period didn't DTRT.  I know that
it was in a table which is all we care about in this discussion but I
just retested and got the same result as you.

-- 
D'Arcy J.M. Cain da...@druid.net |  Democracy is three wolves
http://www.druid.net/darcy/|  and a sheep voting on
+1 416 425 1212 (DoD#0082)(eNTP)   |  what's for dinner.

-- 
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] Hot standby, slot ids and stuff

2009-01-10 Thread Simon Riggs

On Sat, 2009-01-10 at 09:40 +, Simon Riggs wrote:

 This leads to an error when we SubTransSetParent(child_xid, top_xid);
 since this assumes that the top_xid is the parent, which it is not.
 Mostly you wouldn't notice unless you were looking up the subtrans
 status for an xid that had committed but was the child of an aborted
 subtransaction, with the top level xid having  64 subtransactions.
 It's possible the confusion leads to other bugs in UnobservedXid
 processing, but I didn't look too hard at that.
 
 AFAICS we need both parent and top xids.

I wonder if its possible to derive the parent by looking at the
highest/most newly assigned xid? Abort records would remove aborted
subtransactions and AFAIK we currently assign a new subtransaction only
ever from the latest current subtransaction. (This wouldn't be
necessarily true if supported true branch-anywhere subtransactions, but
we don't). Sounds correct, but not really sure.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and 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] WIP: Automatic view update rules

2009-01-10 Thread Bernd Helmle
--On Freitag, Januar 09, 2009 17:53:31 +0100 Bernd Helmle 
maili...@oopsware.de wrote:



I've decided to check updatability of all involved views during view
creation. Please find attached a new version with all other open issues
adressed.


Oops, forgot to track some files in my new git branch and so the new files 
viewUpdate.c und viewUpdate.h are missing...please find attached a 
corrected patch file. Sorry for that.


--
 Thanks

   Bernd

view_update.patch.bz2
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] [BUGS] BUG #4516: FOUND variable does not work after RETURN QUERY

2009-01-10 Thread Robert Haas
 2009/1/10 Tom Lane t...@sss.pgh.pa.us:
 Bruce Momjian br...@momjian.us writes:
 Uh, is this ready to be applied?

 I don't think any consensus has been reached on changing this behavior.

 I  thing, so this is bug - RETURN QUERY has to supply FOR SELECT LOOP
 RETURN NEXT pattern.

 My first patch expected so RETURN QUERY is final statement, so I don't
 solve FOUND variable, but Heikki changed this behave.

 Without correct FOUND behave we can't to use RETURN QUERY for following 
 pattern

 RETURN QUERY some;
 IF FOUND THEN RETURN; END IF;
 RETURN QUERY some_other;
 RETURN;

+1.  I can't imagine it's good for this to be randomly inconsistent.

...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] Synch Rep v5

2009-01-10 Thread Simon Riggs

On Sat, 2009-01-10 at 19:16 +0900, Fujii Masao wrote:

 I attached the updated version of Synch Rep patch (v5) on wiki.
 The description of User Overview is also already updated.
 http://wiki.postgresql.org/wiki/NTT%27s_Development_Projects#Synch_Rep

Looks good on initial read of Wiki. Few minor comments:

The advantage of doing things this way is that the base backup can take
place any way the user chooses, so potentially much faster than using a
single session.

I notice we use the same settings for keepalives. We may need that to be
a second set of parameters.

Progress reporting will be easier with HS, so you are right to leave
that alone.

Don't understand: Completely automated catching up; User has to carry
out some procedure manually for making the standby catch up.

Multiple standby is still possible, but just using old file based
mechanisms. We would need to be careful about use of %R in that case.

I believe the max delay is 2* wal_sender_delay.

I like the way recovery_trigger_file avoids changing pg_standby, but I
guess we still need to plug that gap also one day. But does patch 10
also have the other mechanism?

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and 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] Synch Rep v5

2009-01-10 Thread Simon Riggs

On Sat, 2009-01-10 at 19:16 +0900, Fujii Masao wrote:

 On Thu, Dec 18, 2008 at 9:55 AM, Simon Riggs si...@2ndquadrant.com wrote:
  4. sleeping
  http://archives.postgresql.org/pgsql-hackers/2008-12/msg00438.php
 
  I'm looking for the better idea. How should we resolve that problem?
  Only reduce the timeout of pq_wait to 100ms? Get rid of
  SA_RESTART only during pq_wait as follows?
 
 remove SA_RESTART
 pq_wait()
 add SA_RESTART
 
  Not sure, will consider. Ask others as well.
 
 I've not got an idea yet. Now (v5), I only reduce the timeout of
 pq_wait to 100ms. Is this sufficient? Do you have any good idea?

To be honest I didn't follow that part of the discussion.

My preferred approach, mentioned earlier in the summer, was to use a
mechanism very similar to LWlocks. A proc queue with semaphores. Minimum
delay, no need for signals. The process doing the wakeup can walk up the
queue until it finds somebody whose wait-for-LSN is higher than has just
been sent/written. Doing it this way also gives us group commit when
synch rep is not enabled.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and 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] Re: [COMMITTERS] pgsql: Explicitly bind gettext() to the UTF8 locale when in use.

2009-01-10 Thread Hiroshi Saito

Hi.

My swift attack test was the MinGW environment.
But, Inoue-san suggestion.

1. MinGW+gcc build
HIROSHI=# set LC_TIME=Ja;
SET
HIROSHI=# select to_char(now(),'TMDay');
to_char
-
日曜日
(1 行)
HIROSHI=# set LC_TIME='Japan';
SET
HIROSHI=# select to_char(Now(),'TMDay');
to_char
-
日曜日
(1 行)
HIROSHI=# set LC_TIME='Japanese';
SET
HIROSHI=# select to_char(Now(),'TMDay');
to_char
-
日曜日
(1 行)

However,  A setup of 'Ja' was strange.?_?
http://msdn.microsoft.com/en-us/library/aa246450(VS.60).aspx

2. MSVC build
HIROSHI=# set LC_TIME='Ja';
ERROR:  invalid value for parameter lc_time: Ja
STATEMENT:  set LC_TIME='Ja';
ERROR:  invalid value for parameter lc_time: Ja
HIROSHI=# set LC_TIME='Japan';
ERROR:  invalid value for parameter lc_time: Japan
STATEMENT:  set LC_TIME='Japan';
ERROR:  invalid value for parameter lc_time: Japan
HIROSHI=# set LC_TIME=Japanese;
SET
HIROSHI=# select to_char(Now(),'TMDay');
to_char
-
日曜日
(1 行)

Umm, Re-investigation is required for this. :-(
However, If reasonable clear, it will be good for a document at suggestion.

Regards,
Hiroshi Saito 



--
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] [PATCHES] updated hash functions for postgresql v1

2009-01-10 Thread Kenneth Marshall
On Fri, Jan 09, 2009 at 02:00:39PM -0800, Jeff Davis wrote:
 On Fri, 2009-01-09 at 14:29 -0600, Kenneth Marshall wrote:
  Jeff,
  
  Thanks for the review. I would not really expect any differences in hash
  index build times other than normal noise variances. The most definitive
  benchmark that I have seen was done with my original patch submission
  in March posted by Luke of Greenplum:
  
We just applied this and saw a 5 percent speedup on a hash aggregation
 query with four columns in a 'group by' clause run against a single
 TPC-H table.
  
  I wonder if they would be willing to re-run their test? Thanks again.
 
 Separating mix() and final() should have some performance benefit,
 right?
 
 Regards,
   Jeff Davis
 
 
Yes, it does but the results can be swamped by other latencies in the
code path. Tests such as Tom's benchmark of the underlying functions is
needed to isolate the timings effectively or a benchmark like Greenplum's
that will benefit from a more efficient function.

Ken

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


Re: [HACKERS] [PATCHES] updated hash functions for postgresql v1

2009-01-10 Thread Jeff Davis
On Sat, 2009-01-10 at 11:06 -0600, Kenneth Marshall wrote:
  Separating mix() and final() should have some performance benefit,
  right?
  
 Yes, it does but the results can be swamped by other latencies in the
 code path. Tests such as Tom's benchmark of the underlying functions is
 needed to isolate the timings effectively or a benchmark like Greenplum's
 that will benefit from a more efficient function.
 

Ok. I isolated the function itself by just doing:

-- 10 million rows of random()::text
EXPLAIN ANALYZE SELECT hashtext(t) FROM randomtext;

I ran 5 times on both old and new code, eliminating the top and bottom
and taking the average of the remaining 3, and I got a 6.9% performance
improvement with the new code.

I tried quickly with a few other data types and got similar results.
It's obviously a small microbenchmark, but that's good enough for me. 

Thanks!

Regards,
Jeff Davis


-- 
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] [BUGS] BUG #4516: FOUND variable does not work after RETURN QUERY

2009-01-10 Thread Heikki Linnakangas

Pavel Stehule wrote:

My first patch expected so RETURN QUERY is final statement, so I don't
solve FOUND variable, but Heikki changed this behave.


Me? I don't recall doing anything related to this.

--
  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] [PATCHES] updated hash functions for postgresql v1

2009-01-10 Thread Tom Lane
Jeff Davis pg...@j-davis.com writes:
 I ran 5 times on both old and new code, eliminating the top and bottom
 and taking the average of the remaining 3, and I got a 6.9% performance
 improvement with the new code.

The question that has been carefully evaded throughout the discussion
of this patch is whether the randomness of the hash result is decreased,
and if so what is that likely to cost us in performance of the various
hash-dependent algorithms.  I would still like to have an answer to that
before we make a change to gain marginal performance improvement in
the hash function itself (which is already shown to be barely measurable
in the total context of a hash-dependent operation...)

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: [SPAM] Re: [HACKERS] posix_fadvise v22

2009-01-10 Thread Tom Lane
Peter Eisentraut pete...@gmx.net writes:
 The way I read this is that this was a temporary kernel/libc mismatch in 
 a development version of Debian 3 years ago that was fixed within 2 
 months of being reported and was never released to the general public. 
 So it would be on the same level as any of a million temporary breakages 
 in Linux distributions under development.

This is incorrect, as the problem was in fact present on Red Hat and
presumably all other distros as well.

 Unless there are other reports of this problem, I wouldn't bother 
 testing or working around this at all.  If people are running PostgreSQL 
   8.4+ on Debian unstable June 2005 with kernel 2.4, they cannot be helped.

While I would like to agree with that position, I can't help noticing
lines 2438-2461 of xlog.c, which represent the still-smoking wreckage of
our last attempt to do something with posix_fadvise.  It's not that old
either --- we gave up on it less than three years ago:
http://archives.postgresql.org/pgsql-hackers/2006-06/msg01481.php

I think at a minimum there should be a manual configuration switch
(ie something in pg_config_manual.h) to allow the builder to disable
use of posix_fadvise, even if configure thinks it's there.  Depending
on buildfarm results we may have to do more than that.

BTW, I intend to un-disable the xlog change when committing the fadvise
patch.  In for a penny, in for a pound ...

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] [PATCHES] updated hash functions for postgresql v1

2009-01-10 Thread Jeff Davis
On Sat, 2009-01-10 at 13:36 -0500, Tom Lane wrote:
 Jeff Davis pg...@j-davis.com writes:
  I ran 5 times on both old and new code, eliminating the top and bottom
  and taking the average of the remaining 3, and I got a 6.9% performance
  improvement with the new code.
 
 The question that has been carefully evaded throughout the discussion
 of this patch is whether the randomness of the hash result is decreased,
 and if so what is that likely to cost us in performance of the various
 hash-dependent algorithms.  I would still like to have an answer to that
 before we make a change to gain marginal performance improvement in
 the hash function itself (which is already shown to be barely measurable
 in the total context of a hash-dependent operation...)
 

In:
http://archives.postgresql.org/message-id/20081104202655.gp18...@it.is.rice.edu

Ken showed that the number of hash collisions is the same in the old and
the new code for his test data. Not a direct measurement of randomness,
but it's some kind of indication.

I'm not an authority on either hash functions or statistics, so I'll
have to defer to Ken or someone else to actually prove that the
randomness is maintained.

Regards,
Jeff Davis


-- 
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] [PATCHES] updated hash functions for postgresql v1

2009-01-10 Thread Gregory Stark
Tom Lane t...@sss.pgh.pa.us writes:

 Jeff Davis pg...@j-davis.com writes:
 I ran 5 times on both old and new code, eliminating the top and bottom
 and taking the average of the remaining 3, and I got a 6.9% performance
 improvement with the new code.

 The question that has been carefully evaded throughout the discussion
 of this patch is whether the randomness of the hash result is decreased,

In fairness that doesn't seem to be the case. The original patch was posted
with such an analysis using cracklib and raw binary data:

http://article.gmane.org/gmane.comp.db.postgresql.devel.general/105675

  marginal performance improvement in the hash function itself (which is
 already shown to be barely measurable in the total context of a
 hash-dependent operation...)

If it's a 6% gain in the speed of Hash Join or HashAggregate it would be very
interesting. But I gather it's a 6% speedup in the time spent actually in the
hash function. Is that really where much of our time is going? If it's 10% of
the total time to execute one of these nodes then we're talking about a 0.6%
optimization...

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Ask me about EnterpriseDB's PostGIS 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: [SPAM] Re: [HACKERS] posix_fadvise v22

2009-01-10 Thread Bruce Momjian
Tom Lane wrote:
 Peter Eisentraut pete...@gmx.net writes:
  The way I read this is that this was a temporary kernel/libc mismatch in 
  a development version of Debian 3 years ago that was fixed within 2 
  months of being reported and was never released to the general public. 
  So it would be on the same level as any of a million temporary breakages 
  in Linux distributions under development.
 
 This is incorrect, as the problem was in fact present on Red Hat and
 presumably all other distros as well.
 
  Unless there are other reports of this problem, I wouldn't bother 
  testing or working around this at all.  If people are running PostgreSQL 
8.4+ on Debian unstable June 2005 with kernel 2.4, they cannot be helped.
 
 While I would like to agree with that position, I can't help noticing
 lines 2438-2461 of xlog.c, which represent the still-smoking wreckage of
 our last attempt to do something with posix_fadvise.  It's not that old
 either --- we gave up on it less than three years ago:
 http://archives.postgresql.org/pgsql-hackers/2006-06/msg01481.php
 
 I think at a minimum there should be a manual configuration switch
 (ie something in pg_config_manual.h) to allow the builder to disable
 use of posix_fadvise, even if configure thinks it's there.  Depending
 on buildfarm results we may have to do more than that.
 
 BTW, I intend to un-disable the xlog change when committing the fadvise
 patch.  In for a penny, in for a pound ...

I assumed if effective_io_concurrency  2 that no posix_fadvise() calls
would be made.

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

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [SPAM] Re: [HACKERS] posix_fadvise v22

2009-01-10 Thread Robert Haas
 I think at a minimum there should be a manual configuration switch
 (ie something in pg_config_manual.h) to allow the builder to disable
 use of posix_fadvise, even if configure thinks it's there.  Depending
 on buildfarm results we may have to do more than that.

This seems pretty pointless, since there is already a GUC to control
the behavior, and the default is off.  The only value here would be if
you could demonstrate that even with effective_io_concurrency set to 1
there is a significant performance dropoff.  But that would probably
be an argument for rejecting the patch outright, not adding a
compile-time switch.

...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] [PATCHES] updated hash functions for postgresql v1

2009-01-10 Thread Jeff Davis
On Sat, 2009-01-10 at 13:57 -0500, Gregory Stark wrote:
 But I gather it's a 6% speedup in the time spent actually in the
 hash function.

That's correct. I was not able to detect any difference at all between
the old and new code using HashAggregate, which is where most of my
testing effort went:

http://archives.postgresql.org/message-id/1231531455.25019.75.ca...@jdavis

(see test results attached to that message, if you're interested)

I tested with two data distributions and 6 data types.

The 6-10% speedup I saw was a super-micro benchmark testing the hash
function, and I didn't spend much time with that.

Regards,
Jeff Davis


-- 
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] New patch for Column-level privileges

2009-01-10 Thread Stephen Frost
Tom, et al,

* Stephen Frost (sfr...@snowman.net) wrote:
  applyColumnPrivs is misnamed as it's not applying any privileges nor
  indeed doing much of anything directly to do with privileges.  It should
  probably be named something more like findReferencedColumns.  And what's
  with the special exception for SortGroupClause!?
 
 I'm not sure what the story with SortGroupClause is..  Might just have
 been a bit more difficult to do.  KaiGai?

This should be resolved now, since..

  Actually, though, you probably shouldn't have applyColumnPrivsWalker at all.
  It requires an additional traversal of the parse tree, and an additional RTE
  search for each var, for no good reason.  Can't we just mark the column
  as referenced in make_var() and maybe a couple other places that already 
  have
  their hands on the RTE?
 
 I certainly like this idea and I'll look into it, but it might take me a
 bit longer to find the appropriate places beyond make_var().

I've implemented and tested these suggested changes, and have removed
applyColumnPrivs, etc.  It passes all the regression tests, which had a
variety of tests, so I'm reasonably happy with this.

  pg_attribute_aclmask seems to need a rethink.  I don't like the amount
  of policy copied-and-pasted from pg_class_aclmask, nor the fact that
  it will fail for system attributes (attnum  0), nor the fact that it
  insists on looping over the attributes even if the relation privileges
  would provide what's needed.  (Indeed, you shouldn't need that merge
  ACLs operation at all -- you could be ORing a couple of bitmasks
  instead, no?)
 
 I'll have to think of the entry points for pg_attribute_aclmask.  In
 general, we shouldn't ever get to it if the relation privileges are
 sufficient but I think it's exposed to the user at some level, where
 we would be wrong to say a user doesn't have rights on the column
 when they do have the appropriate table-level rights.  Unfortunately,
 aclmask() uses information beyond the bitmasks (who granted them,
 specifically) so I don't think we can just OR the bitmasks.
 
 With regard to looping through the attributes, I could split it up into
 two functions, would that be better?  A function that handles
 all-attribute cases (either 'AND'd or 'OR'd together depending on what
 the caller wants) could be added pretty easily and then
 pg_attribute_aclmask could be reverted to just handling a single
 attribute at a time (which would fix it for system attributes as
 well..).

I've modified the code to handle system attributes but otherwise kept it
pretty much the same (with the loop and the special values to indicate
how to handle it).  I looked at creating a seperate function and it
really seemed like it would be alot of code duplication..  It might
still be possible to refactor it if you'd really like.  I'm open to
other thoughts or suggestions you have based on my comments above.

Updated patch attached.

Thanks!

Stephen


colprivs_2009011001.diff.gz
Description: Binary data


signature.asc
Description: Digital signature


Re: [HACKERS] Duplicated docs on libpq parameters

2009-01-10 Thread Bruce Momjian
Magnus Hagander wrote:
 Tom Lane wrote:
  Magnus Hagander mag...@hagander.net writes:
  Any reason not to just have the environment variable documentation refer
  back to the connection option documentation?
  
  Well, not all of 'em are connection options.
  
  I could see documenting the connection options more like the
  GUC-variable options, ie a few words and a reference to the underlying
  option.  Or is that what you meant?
 
 Well, I was mostly thinking like:
 PGSSLMODE
   This corresponds to the linksslmode/link connection option.
 
 Then for those that aren't connection options, we'd keep the full
 description. I'm not advocating we remove the whole chapter, just the
 duplicated stuff. (I assume we can make the link above go directly to
 the description for the option?)

Done, with attached, applied patch.

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

  + If your life is a hard drive, Christ can be your backup. +
Index: doc/src/sgml/libpq.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.274
diff -c -c -r1.274 libpq.sgml
*** doc/src/sgml/libpq.sgml	15 Dec 2008 10:28:21 -	1.274
--- doc/src/sgml/libpq.sgml	10 Jan 2009 20:09:58 -
***
*** 101,107 
 The currently recognized parameter key words are:
  
 variablelist
! varlistentry
   termliteralhost/literal/term
   listitem
para
--- 101,107 
 The currently recognized parameter key words are:
  
 variablelist
! varlistentry id=libpq-connect-host xreflabel=host
   termliteralhost/literal/term
   listitem
para
***
*** 119,125 
   /listitem
  /varlistentry
  
! varlistentry
   termliteralhostaddr/literal/term
   listitem
para
--- 119,125 
   /listitem
  /varlistentry
  
! varlistentry id=libpq-connect-hostaddr xreflabel=hostaddr
   termliteralhostaddr/literal/term
   listitem
para
***
*** 160,166 
/listitem
   /varlistentry
  
!  varlistentry
termliteralport/literal/term
listitem
para
--- 160,166 
/listitem
   /varlistentry
  
!  varlistentry id=libpq-connect-port xreflabel=port
termliteralport/literal/term
listitem
para
***
*** 171,177 
   /listitem
  /varlistentry
  
! varlistentry
   termliteraldbname/literal/term
   listitem
   para
--- 171,177 
   /listitem
  /varlistentry
  
! varlistentry id=libpq-connect-dbname xreflabel=dbname
   termliteraldbname/literal/term
   listitem
   para
***
*** 180,186 
   /listitem
  /varlistentry
  
! varlistentry
   termliteraluser/literal/term
   listitem
   para
--- 180,186 
   /listitem
  /varlistentry
  
! varlistentry id=libpq-connect-user xreflabel=user
   termliteraluser/literal/term
   listitem
   para
***
*** 191,197 
   /listitem
  /varlistentry
  
! varlistentry
   termliteralpassword/literal/term
   listitem
   para
--- 191,197 
   /listitem
  /varlistentry
  
! varlistentry id=libpq-connect-password xreflabel=password
   termliteralpassword/literal/term
   listitem
   para
***
*** 200,206 
   /listitem
  /varlistentry
  
! varlistentry
   termliteralconnect_timeout/literal/term
   listitem
   para
--- 200,206 
   /listitem
  /varlistentry
  
! varlistentry id=libpq-connect-connect-timeout xreflabel=connect_timeout
   termliteralconnect_timeout/literal/term
   listitem
   para
***
*** 211,226 
   /listitem
  /varlistentry
  
! varlistentry
   termliteraloptions/literal/term
   listitem
para
!Command-line options to be sent to the server.
/para
   /listitem
  /varlistentry
  
! varlistentry
   termliteraltty/literal/term
   listitem
   para
--- 211,230 
   /listitem
  /varlistentry
  
! varlistentry id=libpq-connect-options xreflabel=options
   termliteraloptions/literal/term
   listitem
para
!Adds command-line options to send to the server at run-time.
!For example, setting this to literal-c geqo=off/ sets the
!session's value of the varnamegeqo/ 

[HACKERS] Documentation on SGML linking

2009-01-10 Thread Bruce Momjian
I often get confused about how to do linking in SGML so I wrote up
/doc/src/sgml/README.links and added the file to CVS.  Peter, would you
review it and make any improvements?  Thanks.

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

  + If your life is a hard drive, Christ can be your backup. +

-- 
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] [BUGS] BUG #4516: FOUND variable does not work after RETURN QUERY

2009-01-10 Thread Pavel Stehule
2009/1/10 Heikki Linnakangas heikki.linnakan...@enterprisedb.com:
 Pavel Stehule wrote:

 My first patch expected so RETURN QUERY is final statement, so I don't
 solve FOUND variable, but Heikki changed this behave.

 Me? I don't recall doing anything related to this.


I have to look to archive, maybe Peter.

Pavel

 --
  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] [PATCHES] updated hash functions for postgresql v1

2009-01-10 Thread Kenneth Marshall
On Sat, Jan 10, 2009 at 10:56:15AM -0800, Jeff Davis wrote:
 On Sat, 2009-01-10 at 13:36 -0500, Tom Lane wrote:
  Jeff Davis pg...@j-davis.com writes:
   I ran 5 times on both old and new code, eliminating the top and bottom
   and taking the average of the remaining 3, and I got a 6.9% performance
   improvement with the new code.
  
  The question that has been carefully evaded throughout the discussion
  of this patch is whether the randomness of the hash result is decreased,
  and if so what is that likely to cost us in performance of the various
  hash-dependent algorithms.  I would still like to have an answer to that
  before we make a change to gain marginal performance improvement in
  the hash function itself (which is already shown to be barely measurable
  in the total context of a hash-dependent operation...)
  
 
 In:
 http://archives.postgresql.org/message-id/20081104202655.gp18...@it.is.rice.edu
 
 Ken showed that the number of hash collisions is the same in the old and
 the new code for his test data. Not a direct measurement of randomness,
 but it's some kind of indication.
 
 I'm not an authority on either hash functions or statistics, so I'll
 have to defer to Ken or someone else to actually prove that the
 randomness is maintained.
 
 Regards,
   Jeff Davis
 
First, while I am not an expert by any means, basic statistics will give you the
probability of a collision when packing N items into M buckets chosen at random.
In all of my tests, I used 1.6M items into 2^32 buckets. If the bits mixing is
complete and random, the number of collisions should be close to the same no
matter what arrangement of bits are used for inputs. I think my test cases
indicate quite clearly that the new hash function is as independent of bit-
order as the older functions, in that the number of bucket collisions is
basically the same for all layouts. I have the test harness and can test
any other input that you would like me to. Second, the author of the basic
hash function (old and new) has performed more extensive testing and has
shown that the new functions are better in randomizing bits than his original
function. I will try and run a micro-benchmark of my original patch in March
and the result of the incremental approach that is the result of my Novermber
patch tomorrow.

Cheers,
Ken




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


Re: [HACKERS] [PATCHES] updated hash functions for postgresql v1

2009-01-10 Thread Kenneth Marshall
On Sat, Jan 10, 2009 at 01:57:27PM -0500, Gregory Stark wrote:
 Tom Lane t...@sss.pgh.pa.us writes:
 
  Jeff Davis pg...@j-davis.com writes:
  I ran 5 times on both old and new code, eliminating the top and bottom
  and taking the average of the remaining 3, and I got a 6.9% performance
  improvement with the new code.
 
  The question that has been carefully evaded throughout the discussion
  of this patch is whether the randomness of the hash result is decreased,
 
 In fairness that doesn't seem to be the case. The original patch was posted
 with such an analysis using cracklib and raw binary data:
 
 http://article.gmane.org/gmane.comp.db.postgresql.devel.general/105675
 
   marginal performance improvement in the hash function itself (which is
  already shown to be barely measurable in the total context of a
  hash-dependent operation...)
 
 If it's a 6% gain in the speed of Hash Join or HashAggregate it would be very
 interesting. But I gather it's a 6% speedup in the time spent actually in the
 hash function. Is that really where much of our time is going? If it's 10% of
 the total time to execute one of these nodes then we're talking about a 0.6%
 optimization...
 

The Greenplum test did show a 5% increase in performance with the replacement
functions in March.

Regards,
Ken

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


[HACKERS] Documenting pglesslog

2009-01-10 Thread Bruce Momjian
In thinking about how to communicate to users about reducing continuous
archiving storage requirements, I realized we don't mention pglesslog in
our official documentation.

The attached patch documents how to use pglesslog and gzip/gunzip to
reduce storage requirements.  Comments?

Also, I assume pg_lesslog removes the padding we use to make all WAL
files 16MB, effectively doing the function of clearxlogtail too, right?

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

  + If your life is a hard drive, Christ can be your backup. +
Index: doc/src/sgml/backup.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/backup.sgml,v
retrieving revision 2.121
diff -c -c -r2.121 backup.sgml
*** doc/src/sgml/backup.sgml	9 Nov 2008 17:51:15 -	2.121
--- doc/src/sgml/backup.sgml	11 Jan 2009 01:41:12 -
***
*** 1337,1342 
--- 1337,1359 
WAL files are part of the same applicationtar/ file.
Please remember to add error handling to your backup scripts.
   /para
+ 
+  para
+   If archive storage size is a concern, use applicationpg_compresslog/,
+   ulink url=http://pglesslog.projects.postgresql.org;/ulink, to
+   remove unnecessary xref linkend=guc-full-page-writes and trailing
+   space from the WAL files.  You can then use
+   applicationgzip/application to further compress the output of
+   applicationpg_compresslog/:
+ programlisting
+ archive_command = 'pg_compresslog %p - | gzip gt; /var/lib/pgsql/archive/%f'
+ /programlisting
+   You will then need to use applicationgunzip/ and
+   applicationpg_decompresslog/ during recovery:
+ programlisting
+ restore_command = 'gunzip lt; /mnt/server/archivedir/%f | pg_decompresslog - %p'
+ /programlisting
+  /para
  /sect3
  
  sect3 id=backup-scripts

-- 
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] autovacuum and reloptions

2009-01-10 Thread Robert Haas
 Here is the patch that replace pg_autovaccum catalog with reloptions. I
 refactored the reloptions.c to support multiple parameters and made the
 action of adding a new option an easy task. I'm testing it yet, so don't
 expect it to work properly. I'll prepare docs as soon as I finish the
 tests. Do i have to prepare some regression tests?

 I don't provide a pg_autovacuum view as suggested by Itagari-san [1] but
 if others agree that we need it, I will work on it. I don't if we need a
 function (wrapper around getRelOption()) to get an option from
 reloptions array.

 I add an ugly-hack to \d+ foo. IMHO, it'll be good to know what options
 are used by table/index foo (we already do it for oids) but I'm not
 happy with my suggestion.

 I move RelationGet*() functions from rel.h. That's because we need some
 knowledge that's only in reloptions.c (getRelOptions). But I want to
 avoid including reloptions.h at some files.

 Comments?

 PS don't forget to remove include/catalog/pg_autovacuum.h

Several things related to this patch have been committed:

http://archives.postgresql.org/message-id/20081219143958.6f2dd756...@cvs.postgresql.org
http://archives.postgresql.org/message-id/20090105171428.77b29754...@cvs.postgresql.org
(and several follow-on commits)

What still remains to be done for 8.4?

...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] Hot standby, slot ids and stuff

2009-01-10 Thread Simon Riggs

On Sat, 2009-01-10 at 11:45 +, Simon Riggs wrote:
 On Sat, 2009-01-10 at 09:40 +, Simon Riggs wrote:
 
  This leads to an error when we SubTransSetParent(child_xid, top_xid);
  since this assumes that the top_xid is the parent, which it is not.
  Mostly you wouldn't notice unless you were looking up the subtrans
  status for an xid that had committed but was the child of an aborted
  subtransaction, with the top level xid having  64 subtransactions.
  It's possible the confusion leads to other bugs in UnobservedXid
  processing, but I didn't look too hard at that.
  
  AFAICS we need both parent and top xids.
 
 I wonder if its possible to derive the parent by looking at the
 highest/most newly assigned xid? Abort records would remove aborted
 subtransactions and AFAIK we currently assign a new subtransaction only
 ever from the latest current subtransaction. (This wouldn't be
 necessarily true if supported true branch-anywhere subtransactions, but
 we don't). Sounds correct, but not really sure.

Starting to sound like a do-me-later-if-ever optimisation and certainly
nothing I want to rely on in court.

I'm progressing with parent_xid added to the xlog record header, for
now.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and 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] Documenting pglesslog

2009-01-10 Thread Simon Riggs

On Sat, 2009-01-10 at 21:09 -0500, Bruce Momjian wrote:

 Comments?

If this is for backpatching, it makes sense. We should at least wait
until sync rep is accepted or rejected and docs written.

In general I don't think we should refer/link to other companies'
copyrighted materials in our documentation. That could cause
difficulties.

If you're going to do this, then I think you should go through the docs
and refer directly to many other commonly used tools that are also on
pg_foundry. 

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and 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] re-merged posix_fadvise patch

2009-01-10 Thread Robert Haas
Tom Lane committed some parts of the posix_fadvise patch today; here
is my attempt at a re-merged version for the benefit of anyone
attempting to do performance testing, etc.  It passes regression tests
but I haven't tested it much beyond that.

...Robert


posix_fadvise_v23_rh2.diff.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] Documenting pglesslog

2009-01-10 Thread Bruce Momjian
Simon Riggs wrote:
 
 On Sat, 2009-01-10 at 21:09 -0500, Bruce Momjian wrote:
 
  Comments?
 
 If this is for backpatching, it makes sense. We should at least wait
 until sync rep is accepted or rejected and docs written.

No, it is not for backpatching.

 In general I don't think we should refer/link to other companies'
 copyrighted materials in our documentation. That could cause
 difficulties.

It is BSD licensed.  I don't see any copyright issues:

http://pglesslog.projects.postgresql.org/

 If you're going to do this, then I think you should go through the docs
 and refer directly to many other commonly used tools that are also on
 pg_foundry. 

I add items where they fit logically.

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

  + If your life is a hard drive, Christ can be your backup. +

-- 
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] Synch Rep v5

2009-01-10 Thread Fujii Masao
Hi,

On Sat, Jan 10, 2009 at 10:42 PM, Simon Riggs si...@2ndquadrant.com wrote:

 On Sat, 2009-01-10 at 19:16 +0900, Fujii Masao wrote:

 On Thu, Dec 18, 2008 at 9:55 AM, Simon Riggs si...@2ndquadrant.com wrote:
  4. sleeping
  http://archives.postgresql.org/pgsql-hackers/2008-12/msg00438.php
 
  I'm looking for the better idea. How should we resolve that problem?
  Only reduce the timeout of pq_wait to 100ms? Get rid of
  SA_RESTART only during pq_wait as follows?
 
 remove SA_RESTART
 pq_wait()
 add SA_RESTART
 
  Not sure, will consider. Ask others as well.

 I've not got an idea yet. Now (v5), I only reduce the timeout of
 pq_wait to 100ms. Is this sufficient? Do you have any good idea?

 To be honest I didn't follow that part of the discussion.

 My preferred approach, mentioned earlier in the summer, was to use a
 mechanism very similar to LWlocks. A proc queue with semaphores. Minimum
 delay, no need for signals. The process doing the wakeup can walk up the
 queue until it finds somebody whose wait-for-LSN is higher than has just
 been sent/written. Doing it this way also gives us group commit when
 synch rep is not enabled.

Yes, using semaphores for the communication is also my first approach.
The problem of this approach is that walsender cannot wait for both signal
from backends and the response from walreceiver concurrently, because
wait-for-semaphore is blocking at least. So, I use signal for the communication.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

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