Re: [HACKERS] tsearch Parser Hacking

2011-02-14 Thread Oleg Bartunov

On Mon, 14 Feb 2011, David E. Wheeler wrote:


On Feb 14, 2011, at 11:37 PM, Oleg Bartunov wrote:


it's not easy to hack tsearch parser, sorry. You can preparse your input
before to_tsquery,to_tsvector.


Yeah, I was thinking about s{/}{-}g before passing the values in. Might be the 
only way to do it for now?


actually, it's not so difficult to *hack* parser to treat '/' as '-'.
I thought about overriding some default parser behaviour, but didn't come
to any useful solution. 
btw, some users already wrote their own parsers and even I have little

tutorial:
http://www.sai.msu.su/~megera/postgres/gist/tsearch/V2/docs/HOWTO-parser-tsearch2.html
I wonder if it's worth to add it to 
http://www.postgresql.org/docs/8.4/static/test-parser.html


Probably, good paper/presentation along with improving code docs would be 
enough for now, until someone got very bright idea about parser and 
time to implement it.


Regards,
Oleg
_
Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru),
Sternberg Astronomical Institute, Moscow University, Russia
Internet: o...@sai.msu.su, http://www.sai.msu.su/~megera/
phone: +007(495)939-16-83, +007(495)939-23-83

--
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] tsearch Parser Hacking

2011-02-14 Thread Oleg Bartunov

On Mon, 14 Feb 2011, Tom Lane wrote:


"David E. Wheeler"  writes:

Is it possible to modify the default tsearch parser so that / doesn't get lexed as a 
"file" token?


There is zero, none, nada, provision for modifying the behavior of the
default parser, other than by changing its compiled-in state transition
tables.

It doesn't help any that said tables are baroquely designed and utterly
undocumented.


what do you mean 'baroquely' ? Do you know 'gothic' design :?



IMO, sooner or later we need to trash that code and replace it with
something a bit more modification-friendly.


We thought about configurable parser, but AFAIR, we didn't get any support 
for this at that time.


Regards,
Oleg
_
Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru),
Sternberg Astronomical Institute, Moscow University, Russia
Internet: o...@sai.msu.su, http://www.sai.msu.su/~megera/
phone: +007(495)939-16-83, +007(495)939-23-83

--
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] tsearch Parser Hacking

2011-02-14 Thread David E. Wheeler
On Feb 14, 2011, at 11:37 PM, Oleg Bartunov wrote:

> it's not easy to hack tsearch parser, sorry. You can preparse your input
> before to_tsquery,to_tsvector.

Yeah, I was thinking about s{/}{-}g before passing the values in. Might be the 
only way to do it for now…

Thanks,

David


-- 
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] tsearch Parser Hacking

2011-02-14 Thread Oleg Bartunov

David,

it's not easy to hack tsearch parser, sorry. You can preparse your input
before to_tsquery,to_tsvector.

Oleg
On Mon, 14 Feb 2011, David E. Wheeler wrote:


Hackers,

Is it possible to modify the default tsearch parser so that / doesn't get lexed as a 
"file" token? That is, instead of this:

try=# select * from ts_debug('simple'::regconfig, 'w/d');
alias │description│ token │ dictionaries │ dictionary │ lexemes
───┼───┼───┼──┼┼─
file  │ File or path name │ w/d   │ {simple} │ simple │ {w/d}

Ideally it'd think that / was the same as -:

try=# select * from ts_debug('simple'::regconfig, 'w-d');
 alias  │   description   │ token │ dictionaries │ 
dictionary │ lexemes
─┼─┼───┼──┼┼─
asciihword  │ Hyphenated word, all ASCII  │ w-d   │ {simple} │ 
simple │ {w-d}
hword_asciipart │ Hyphenated word part, all ASCII │ w │ {simple} │ 
simple │ {w}
blank   │ Space symbols   │ - │ {}   │ 
[null] │ [null]
hword_asciipart │ Hyphenated word part, all ASCII │ d │ {simple} │ 
simple │ {d}
(4 rows)

Possible? Or would I have to write a completely new parser just to change this 
bit?

Thanks,

David





Regards,
Oleg
_
Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru),
Sternberg Astronomical Institute, Moscow University, Russia
Internet: o...@sai.msu.su, http://www.sai.msu.su/~megera/
phone: +007(495)939-16-83, +007(495)939-23-83
--
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] Sync Rep for 2011CF1

2011-02-14 Thread Jaime Casanova
On Sat, Jan 15, 2011 at 4:40 PM, Simon Riggs  wrote:
>
> Here's the latest patch for sync rep.
>

I was looking at this code and found something in SyncRepWaitOnQueue
we declare a timeout variable that is a long and another that is a
boolean (this last one in the else part of the "if
(!IsOnSyncRepQueue())"), and then use the boolean one as if it were
the long one

+   else
+   {
+   bool release = false;
+   bool timeout = false;
+
+   SpinLockAcquire(&queue->qlock);
+
+   /*
+* Check the LSN on our queue and if its moved far enough then
+* remove us from the queue. First time through this is
+* unlikely to be far enough, yet is possible. Next time we are
+* woken we should be more lucky.
+*/
+   if (XLByteLE(XactCommitLSN, queue->lsn))
+   release = true;
+   else if (timeout > 0 &&
+   TimestampDifferenceExceeds(GetCurrentTransactionStopTimestamp(),
+   now,
+   timeout))
+   {
+   release = true;
+   timeout = true;
+   }

the other two things are on postgresql.conf.sample:
- we have two replication_timeout_client, obviously one of them should
be replication_timeout_server
- synchronous_replication_feedback is off by default, but docs says otherwise

i also have been testing this, until now the only issue i have found
is that if i set allow_standalone_primary to off and there isn't a
standby connected i need to stop the server with -m immediate which is
at least surprising

-- 
Jaime Casanova         www.2ndQuadrant.com
Professional PostgreSQL: Soporte y capacitación de PostgreSQL

-- 
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] Debian readline/libedit breakage

2011-02-14 Thread Stefan Kaltenbrunner

On 02/14/2011 02:26 PM, Marko Kreen wrote:

On Mon, Feb 14, 2011 at 3:08 PM, Martin Pitt  wrote:

thanks Markus for CC'ing me, I'm not on -hackers@.

Markus Wanner [2011-02-14 13:37 +0100]:

On 02/10/2011 11:34 PM, Joshua D. Drake wrote:

http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=607109


Note that the recent discussions happened on bug 608442, in particular

  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=608442#30

and the following comments.


Personally, I'm a bit suspicious about that solution (technically as
well as from a licensing perspective), [...]


For the record, so am I (see comment 30 in the link above), as it uses
the very same ld.so in both cases. However, Andreas Barth pointed out
that with LD_PRELOAD it's guaranteed that we do not "import" any code
from the libreadline header files, which guarantees that psql doesn't
become something that can be considered a "derived work".

Technically, this is a bit fragile, of course, as there might be some
subtle ABI differences which lead to crashes. However, the preloading
workaround already makes the situation so much better than before, so
IMHO it's better than the previous status quo.

I don't really like this situation, and personally I'd rather move
back to libreadline until OpenSSL or readline or PostgreSQL threatens
Debian with a legal case for license violation (I daresay that the
chances of this happening are very close to zero..). But oh well..


I think it would be better to revert to readline and make note
that conversion depends on libedit's readiness for unicode.
I doubt anybody in Debian is that gung-ho to veto current state...

Informing libedit about relevant problem would
be good too.  I don't see any bugs about that in Debian's
bugtracker, did you send them to upstream?


from what I can see upstream libedit actually has utf8 support for a 
while now (as well as some other fixes) but the debian libedit version 
(and also the one of other distributions) is way too old for that so 
maybe most of the issues would be mood if debian updated to a newer 
libedit version...



Stefan

--
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] Sync Rep for 2011CF1

2011-02-14 Thread Fujii Masao
On Mon, Feb 14, 2011 at 2:08 PM, Fujii Masao  wrote:
> On Fri, Feb 11, 2011 at 4:06 AM, Heikki Linnakangas
>  wrote:
>> I committed the patch with those changes, and some minor comment tweaks and
>> other kibitzing.

I have another comment:

The description of wal_receiver_status_interval is in "18.5.4.
Streaming Replication".
But I think that it should be moved to "18.5.5. Standby Servers" since
it's a parameter
to control the behavior of the standby server rather than that of the master.

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] tsearch Parser Hacking

2011-02-14 Thread Sushant Sinha
I agree that it will be a good idea to rewrite the entire thing. However, in
the mean time, I sent a proposal earlier

http://archives.postgresql.org/pgsql-hackers/2010-08/msg00019.php

And a patch later:

http://archives.postgresql.org/pgsql-hackers/2010-09/msg00476.php

Tom asked me to look into Compound Word support but I found it not usable.
Here was my response:
http://archives.postgresql.org/pgsql-hackers/2011-01/msg00419.php

I have not got any response since then,

-Sushant.


On Tue, Feb 15, 2011 at 9:33 AM, David E. Wheeler wrote:

> On Feb 14, 2011, at 3:57 PM, Tom Lane wrote:
>
> > There is zero, none, nada, provision for modifying the behavior of the
> > default parser, other than by changing its compiled-in state transition
> > tables.
> >
> > It doesn't help any that said tables are baroquely designed and utterly
> > undocumented.
> >
> > IMO, sooner or later we need to trash that code and replace it with
> > something a bit more modification-friendly.
>
> I was afraid you'd say that. Thanks.
>
> David
>
> --
> 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] Add support for logging the current role

2011-02-14 Thread Itagaki Takahiro
On Mon, Feb 14, 2011 at 23:30, Stephen Frost  wrote:
> > * In assign_csvlog_fields(), we need to cleanup memory and memory context
> > before return on error.
> Fixed this and a couple of similar issues.

Not yet fixed. Switched memory context is not restored on error.

> Updated patch attached, git log below.

Now I mark the patch to "Ready for Committer",
because I don't have suggestions any more.

For reference, I note my previous questions. Some of them might be TODO
items, or might not. We can add the basic feature in 9.1, and improve it
9.2 or later versions.

* csvlog_fields and csvlog_header won't work with non-default log_filename
  when it doesn't contain seconds in the format. They expect they can always
  open empty log files.

* The long default value for csvlog_fields leads long text line in
  postgresql.conf, SHOW ALL, pg_settings view, but there were no better
  alternative solutions in the past discussion.

* csvlog_fields is marked as PGC_POSTMASTER. It can protect mixed formats
  in a csv file on default log_filename, but other similar GUC variables
  are usually marked AS PGC_SIGHUP.

-- 
Itagaki Takahiro

-- 
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] Change pg_last_xlog_receive_location not to move backwards

2011-02-14 Thread Fujii Masao
On Sat, Feb 12, 2011 at 11:32 PM, Robert Haas  wrote:
> So, what if we did some renaming?  I'd be inclined to start by
> renaming "receivedUpTo" to Flush, and add a new position called
> Stream.  When walreciever is started, we set Stream to the position at
> which streaming is going to begin (which can rewind) and leave Flush
> alone (so it never rewinds). We then change the walreceiver feedback
> mechanism to use the term stream_location rather than write_location;
> and we could consider replacing pg_last_xlog_receive_location() with
> pg_last_xlog_stream_location() and pg_last_xlog_flush_location().

You suggest that the shared variable Stream tracks the WAL write location,
after it's set to the replication starting position? I don't think
that the write
location needs to be tracked in the shmem because other processes than
walreceiver don't use it.

What I proposed is:

Walreceiver-local variables
==
1. LogstreamResult.Write
 - Indicates the location of recently written WAL record
 - Can rewind
 - pg_stat_replication.write_location returns this

2. LogstreamResult.Flush
 - Indicates the location of recently flushed WAL record
 - Can rewind
 - pg_stat_replication.flush_location returns this

Shmem variables
===
3. WalRcv->receiveStart
 - Indicates the replication starting location
 - Updated only when walreceiver is started
 - Doesn't exist at the moment, so I propose to add this

4. WalRcv->receivedUpto
 - Indicates the latest location of all the flushed WAL records
 - Never rewinds
(Can rewind at the moment, so I propose to prevent the rewind)
 - pg_last_xlog_receive_location returns this

You propose to rename LogstreamResult.Write to .Stream, and
merge it and receiveStart?

> I'd also be inclined to go to the walreceiver code and and rename the
> apply_location to replay_location, so that it matches
> pg_last_xlog_replay_location().  The latter is in 9.0, but the former
> is new to 9.1, so we can still fix it to be consistent without a
> backward compatibility break.

+1

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] pg_upgrade seems a tad broken

2011-02-14 Thread Tom Lane
I wrote:
> I tried to do a pg_upgrade from 9.0.x to HEAD today.  The pg_upgrade run
> went through without complaint, and I could start the postmaster, but
> every connection attempt fails with 

> psql: FATAL:  could not read block 0 in file "base/11964/11683": read only 0 
> of 8192 bytes

> The database OID varies depending on which database I try to connect to,
> but the filenode doesn't.  In the source 9.0 database, this relfilenode
> belongs to pg_largeobject_metadata.  I'm not sure whether pg_upgrade
> would've preserved relfilenode numbering, so that may or may not be a
> useful hint as to where the problem is.  But in any case it's busted.

Closer investigation shows that in the new database, relfilenode 11683
belongs to pg_class_oid_index, which explains why it's being touched
during backend startup.  It is indeed of zero length, and surely should
not be.  I can't resist the guess that something about the recently
added hacks for pg_largeobject_metadata is not right.

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] pg_upgrade seems a tad broken

2011-02-14 Thread Tom Lane
I tried to do a pg_upgrade from 9.0.x to HEAD today.  The pg_upgrade run
went through without complaint, and I could start the postmaster, but
every connection attempt fails with 

psql: FATAL:  could not read block 0 in file "base/11964/11683": read only 0 of 
8192 bytes

The database OID varies depending on which database I try to connect to,
but the filenode doesn't.  In the source 9.0 database, this relfilenode
belongs to pg_largeobject_metadata.  I'm not sure whether pg_upgrade
would've preserved relfilenode numbering, so that may or may not be a
useful hint as to where the problem is.  But in any case it's busted.

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] CommitFest 2011-01 as of 2011-02-04

2011-02-14 Thread Peter Eisentraut
On mån, 2011-02-14 at 11:49 -0500, Stephen Frost wrote:
> Perhaps a thought for next time would be to offset things a bit.  eg:
> 
> CF 2011-03 (or whatever):
> 2011-02-14: Patches should all be submitted
> 2011-02-14: Reviewers start
> 2011-03-01: Committers start w/ 'Ready for Committer' patches
> 2011-03-14: Patches not marked 'Ready for Committer' get bounced
> 2011-03-31: All patches committed
> 
> I'm not against the 'waiting on author' approach, but I do feel like
> if we're going to continue to have it, we need to spread it out a bit
> more.

I don't think it is realistic to add even more dates and bounds and
guidelines.  People are already widely ignoring the current ones.

If you want to have the ability the bounce things more aggressively, I'd
argue for shorter and more frequent commitfests.  Say, one week per
month.


-- 
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] pl/python do not delete function arguments

2011-02-14 Thread Peter Eisentraut
On mån, 2011-02-14 at 22:22 +0100, Jan Urbański wrote:
> The problem is that every *second* call to the function fails,
> regardless of the number. The first execution succeeds, but then
> PLy_delete_args deletes the argument from the globals, and when the
> next execution tries to fetch "n" from it, it raises a KeyError.

This isn't quite right either, because it obviously depends on the
recursion somehow.  So in

SELECT recursion_test(5);
SELECT recursion_test(4);

it is the first recursive invocation of the (4) call that fails.  If you
just do

SELECT recursion_test(1);
SELECT recursion_test(1);
SELECT recursion_test(1);

nothing fails.  (We'd have noticed that sooner, obviously. ;-) )

But in

SELECT recursion_test(1);
SELECT recursion_test(4);
SELECT recursion_test(1);

it's the last (1) call, which is not recursive, that fails.

Your patch obviously fixes all that, but I'm wondering if we have
another problem with the procedure caching somehow.  And I'm also
wondering what to put into the commit message. :-/



-- 
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] why two dashes in extension load files

2011-02-14 Thread Robert Haas
On Mon, Feb 14, 2011 at 8:03 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> Are we deparsing the names of the SQL files to infer the set of
>> version numbers we have to worry about?  It seems to me that if
>> there's a list of known version numbers somewhere, we can use dash as
>> the separator without any special restricton.
>
> The list of known version numbers is inferred from the available files,
> not vice versa.  IMO that's a feature not a bug.  A manually maintained
> list would just be one more thing to forget to update.

I could go either way on that one; I was just throwing it up against
the wall to see whether it would stick.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] ALTER TYPE 2: skip already-provable no-work rewrites

2011-02-14 Thread Robert Haas
On Mon, Feb 14, 2011 at 7:52 AM, Noah Misch  wrote:
>> I'm half-tempted to put that part off to
>> 9.2, in the hopes of getting a more substantial solution that can also
>> handle things like text -> xml which we don't have time to re-engineer
>> right now.
>
> I see.

After sleeping on it, I think this route makes most sense.  The
ability to downgrade a rewrite to a scan is really a separate feature,
and I'd like to see us implement that in a more complete way when/if
we're going to do it; and I'd rather commit it at the beginning of a
development cycle when we have more time to find any lurking bugs.  So
I've committed a change that just handles the unconstrained domain
case.  I think for 9.2 we should revisit the following areas:

1. Downgrading rewrites to scans (vs. skipping them altogether).  One
idea is that we might modify CREATE CAST so that you can do this:

CREATE CAST (source_type AS target_type) WITH [ CHECK ] FUNCTION
function_name (argument_type [, ...])
  [ AS ASSIGNMENT | AS IMPLICIT ];

The inclusion of the keyword "check" there would inform the system
that the binary representation can't change, but (as distinguished
from WITHOUT FUNCTION) an error might be thrown.  Of course, I'm not
quite sure how to get this information over to the alter table
machinery cleanly.

2. Detecting binary-coercible cases that involve typemods, rather than
just type OIDs.

3. Avoiding index rebuilds.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] why two dashes in extension load files

2011-02-14 Thread David E. Wheeler
On Feb 14, 2011, at 8:18 PM, Tom Lane wrote:

>> 
>> Yes, but the truth is that the extension name, at least, is known from the 
>> control file.
> 
> Yeah, I think it's true in the current code base that we always know the
> extension name we are interested in.  However, that's no protection if
> we allow extensions to contain the separator substring.  Consider
>   foo--bar--baz.sql
> Is this an update script for foo (from version bar to version baz),
> or is it an install script for some other extension named foo--bar?
> 
> Also, I think it'd be better if we didn't assume that we will always
> know the extension name when trying to make sense of a script name.
> That's the sort of assumption that will bite you on the rear eventually.

Works for me.

Best,

David


-- 
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] why two dashes in extension load files

2011-02-14 Thread Tom Lane
"David E. Wheeler"  writes:
> On Feb 14, 2011, at 5:03 PM, Tom Lane wrote:
>>> Are we deparsing the names of the SQL files to infer the set of
>>> version numbers we have to worry about?  It seems to me that if
>>> there's a list of known version numbers somewhere, we can use dash as
>>> the separator without any special restricton.

>> The list of known version numbers is inferred from the available files,
>> not vice versa.  IMO that's a feature not a bug.  A manually maintained
>> list would just be one more thing to forget to update.

> Yes, but the truth is that the extension name, at least, is known from the 
> control file.

Yeah, I think it's true in the current code base that we always know the
extension name we are interested in.  However, that's no protection if
we allow extensions to contain the separator substring.  Consider
foo--bar--baz.sql
Is this an update script for foo (from version bar to version baz),
or is it an install script for some other extension named foo--bar?

Also, I think it'd be better if we didn't assume that we will always
know the extension name when trying to make sense of a script name.
That's the sort of assumption that will bite you on the rear eventually.

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] tsearch Parser Hacking

2011-02-14 Thread David E. Wheeler
On Feb 14, 2011, at 3:57 PM, Tom Lane wrote:

> There is zero, none, nada, provision for modifying the behavior of the
> default parser, other than by changing its compiled-in state transition
> tables.
> 
> It doesn't help any that said tables are baroquely designed and utterly
> undocumented.
> 
> IMO, sooner or later we need to trash that code and replace it with
> something a bit more modification-friendly.

I was afraid you'd say that. Thanks.

David

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


Re: [DOCS] [HACKERS] "Extension" versus "module"

2011-02-14 Thread David E. Wheeler
On Feb 14, 2011, at 5:42 PM, Tom Lane wrote:

>> Remember also that not all modules out there on the net will have been
>> updated either, so we must be able to discuss "extension-izing a
>> module". (??)
> 
> Right.  So it seems like we ought to stick with more or less the
> existing terminology: those various components under contrib/ are
> modules.  Some of them are also extensions, but not all.

The similarity of the meaning of the words "extension" and "module" is 
unfortunate, as it might be hard for one to remember which is which. But given 
the precedent of the word "module," I don't suppose there's much choice.

Best,

David


-- 
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] why two dashes in extension load files

2011-02-14 Thread David E. Wheeler
On Feb 14, 2011, at 5:03 PM, Tom Lane wrote:

>> Are we deparsing the names of the SQL files to infer the set of
>> version numbers we have to worry about?  It seems to me that if
>> there's a list of known version numbers somewhere, we can use dash as
>> the separator without any special restricton.
> 
> The list of known version numbers is inferred from the available files,
> not vice versa.  IMO that's a feature not a bug.  A manually maintained
> list would just be one more thing to forget to update.

Yes, but the truth is that the extension name, at least, is known from the 
control file.

Best,

David


-- 
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] .gitignore patch for coverage builds

2011-02-14 Thread Robert Haas
On Mon, Feb 14, 2011 at 7:38 PM, Jeff Janes  wrote:
> On Wed, Jan 26, 2011 at 2:41 PM, Alvaro Herrera
>  wrote:
>> Excerpts from Tom Lane's message of mié ene 26 19:20:52 -0300 2011:
>>> Robert Haas  writes:
>>> > On Wed, Jan 26, 2011 at 4:44 PM, Tom Lane  wrote:
>>> >> Ick. That's an awful lot of stuff to have global ignores for.
>>>
>>> > The "coverage" directory ignore seems a little icky, but the rest
>>> > seems unlikely to pick up anything incidental.
>>>
>>> Tying /coverage to the root as in his V2 makes that better,
>>
>> Hmm, I don't think that works, because you can run "make coverage" in
>> any subdir and it will create a "coverage" subdir there.
>
> I like being told that I have a coverage directory outstanding when I
> run "git status".
>
> The hundreds of other files, not so much.
>
>
>>> but I'm
>>> still unexcited about the thesis that we should auto-ignore the results
>>> of any random tool somebody wants to run in their source tree.
>>
>> Well, in this case it's not any random tool, because it's integrated
>> into our makefiles.
>
> I agree.  Should this be added to commit-fest 2011-Next?

I think there's little reason not to go ahead and commit this now.
It's a trivial patch, Tom is the only one objecting, and there are at
least four votes on the other side.  The only question in my mind is
whether we ought to try to ignore the coverage directories, or just
the other glob patterns.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] pg_ctl failover Re: Latches, signals, and waiting

2011-02-14 Thread Fujii Masao
On Tue, Feb 15, 2011 at 2:10 AM, Stephen Frost  wrote:
> Fujii,
>
> * Fujii Masao (masao.fu...@gmail.com) wrote:
>> Yeah, I rebased the patch to the current git master and attached it.
>
> Reviewing this, I just had a couple of comments and questions.  Overall,
> I think it looks good and hence will be marking it 'Ready for
> Committer'.

Thanks for the review!

>  * You removed trigger_file from the list in
>   doc/src/sgml/high-availability.sgml and I'm not sure I agree with
>   that.  It's still perfectly valid and could be used by someone
>   instead of pg_ctl promote.  I'd recommend two things:
>   - Adding comments into this recovery.conf snippet

Adding the following is enough?

+# NOTE that if you plan to use "pg_ctl promote" command to promote
+# the standby, no trigger file needs to be specified.

>   - Adding a comment indicationg that trigger_file is only needed if
>         you're not using pg_ctl promote.

Where should I add such a comment?

>  * I'm not happy that pg_ctl.c doesn't #include something which defines
>   all the file names which are used, couldn't we use a header which
>   makes sense and is pulled in by pg_ctl.c and xlog.c to #define all of
>   these?  Still, that's not really the fault of this patch.

That would make sense. But I'm not sure that's possible. As a trial,
I added '#include "access/xlog.h"' into pg_ctl.c and compiled the source,
but I got many compilation errors. So probably hacking Makefiles is
required to do that. Do you know where should be changed?

>  * I'm a bit worried that there's just only so many USR signals that we
>   can send and it looks like we're burning another one here.  Should we
>   be considering a better way to do this?

You're worried about the case where users wrongly send the SIGUSR2
to the startup process, and then the standby is brought up to the master
unexpectedly?

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] sepgsql contrib module

2011-02-14 Thread Tom Lane
Andrew Dunstan  writes:
> On 02/14/2011 08:36 PM, Tom Lane wrote:
>> It looks to me like /selinux/mls is some weird phony-filesystem file,
>> because "cat" prints one character (a "1") while "ls" claims the file is
>> of zero length.  So it's probably something consed up by the kernel,
>> like /proc/.  Do you have selinux enabled on your machine?

> Np, but that really shouldn't be a build requirement, ISTM, even if it 
> is a test requirement.

[ A few reboots later... ]  Yeah, I've confirmed that /selinux/mls isn't
there at all when SELinux is disabled.  When it is there, it reflects
the setting of SELINUXTYPE ("targeted" or "mls").  So that explains what
/usr/share/selinux/devel/Makefile is doing, but it doesn't make it a
good idea.

>> (BTW, testing what seems to be a kernel-configuration-reporting flag at
>> build time strikes me as pretty awful design.)

> Yeah, I agree.

Yup, this is just broken by design.

It looks like /usr/share/selinux/devel/Makefile basically just sets NAME
and TYPE and then calls /usr/share/selinux/devel/include/Makefile, so we
could avoid the dependence on the build machine's current state if we
did that for ourselves.  Of course that just begs the question of what
we should set these variables *to*.  Since the file being built is only
used for regression testing, it wouldn't be unreasonable to pick some
values, but it's not clear to me whether things would go blooey if the
eventual test machine had different settings.

On the whole, I don't think that sepgsql-regtest.pp should be built or
installed at all during the build phase.  It ought to be generated
during regression test startup, instead.

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] tsearch Parser Hacking

2011-02-14 Thread David Blewett
On Mon, Feb 14, 2011 at 6:57 PM, Tom Lane  wrote:
> "David E. Wheeler"  writes:
>> Is it possible to modify the default tsearch parser so that / doesn't get 
>> lexed as a "file" token?
>
> There is zero, none, nada, provision for modifying the behavior of the
> default parser, other than by changing its compiled-in state transition
> tables.
>
> It doesn't help any that said tables are baroquely designed and utterly
> undocumented.
>
> IMO, sooner or later we need to trash that code and replace it with
> something a bit more modification-friendly.

I added this to the TODO as something that can be tackled in the
future. I've been wishing it would be possible to add other tokens as
well (Python dotted path 'foo.bar.baz', Perl namespace path
'Foo::Bar', more flexible version number parsing, etc).

David Blewett

-- 
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] ALTER TYPE 2: skip already-provable no-work rewrites

2011-02-14 Thread Noah Misch
On Mon, Feb 14, 2011 at 04:06:59PM -0500, Stephen Frost wrote:
> * Noah Misch (n...@leadboat.com) wrote:
> > On Mon, Feb 14, 2011 at 01:12:21PM -0500, Stephen Frost wrote:
> > > In ATColumnChangeSetWorkLevel(), I'm really not a huge fan of using a
> > > passed-in argument to move through a list with..  I'd suggest using a
> > > local variable that is set from what's passed in.  I do see that's
> > > inheirited, but still, you've pretty much redefined that entire
> > > function anyway..
> > 
> > Could do that.  However, the function would reference the original argument 
> > just
> > once, to make that copy.  I'm not seeing a win.
> 
> Perhaps I'm just deficient in this area, but I think of arguments,
> unless specifically intended otherwise, to be 'read-only' and based on
> that assumption, seeing it come up as an lvalue hits me as wrong.
> 
> I'm happy enough to let someone else decide if they agree with me or you
> though. :)

Same here.  If there's a general project tendency either way, I'll comply.  I
haven't noticed one, but I haven't been looking.


I've attached a new version of the patch that attempts to flesh out the comments
based on your feedback.  Does it improve things?

> > The validity of this optimization does not
> > rely on any user-settable property of a data type, but it does lean heavily 
> > on
> > the execution semantics of specific nodes.
> 
> After thinking through this and diving into coerce_to_target_type() a
> bit, I'm finally coming to understand how this is working.  The gist of
> it, if I follow correctly, is that if the planner doesn't think we have
> to do anything but copy this value to make it the new data type, then
> we're good to go.  That makes sense, when you think about it, but boy
> does it go the long way around to get there.

Essentially.  The planner is not yet involved.  We're looking at an expression
tree after parse analysis, before planning.

> Essentially, coerce_to_target_type() is returning an expression tree and
> ATColumnChangeSetWorkLevel() is checking to see if that expression tree
> is "copy the value".  Maybe this is a bit much, but if
> coerce_to_target_type() knows the expression given to it is a
> straight-up copy, perhaps it could pass that information along in an
> easier to use format than an expression tree?  That would obviate the
> need for ATColumnChangeSetWorkLevel() entirely, if I understand
> correctly.

PostgreSQL has a strong tradition of passing around expression trees and walking
them to (re-)discover facts.  See the various clauses.h functions.  Also, when
you have a USING clause, coerce_to_target_type() doesn't know the whole picture.
We could teach it to discover the whole picture, but that would amount to a very
similar tree walk in a different place.

> Of course, coerce_to_target_type() is used by lots of other places,
> almost all of which probably have to have an expression tree to stuff
> into a plan, so maybe a simpler function could be defined which operates
> at the level that ATColumnChangeSetWorkLevel() needs?

Offhand, I can't think of any concrete implementation along those lines that
would simplify the overall task.  Did you have something more specific in mind?

> Or perhaps other
> places would benefit from knowing that a given conversion is an actual
> no-op rather than copying the value?

RelabelType itself does not cause a copy; the existing datum passes through.  In
the case of ALTER TABLE, we do eventually copy the datum via heap_form_tuple.

There may be other places that would benefit from similar analysis.  For that
reason, I originally had ATColumnChangeSetWorkLevel() in parse_coerce.c with the
name GetCoerceExemptions().  I'm not aware of any specific applications, though.
For now it seemed like premature abstraction.

Thanks again,
nm
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 452ced6..466be25 100644
diff --git a/src/backend/commands/index 1db42d0..ab0bcda 100644
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***
*** 71,76 
--- 71,77 
  #include "storage/smgr.h"
  #include "utils/acl.h"
  #include "utils/builtins.h"
+ #include "utils/datum.h"
  #include "utils/fmgroids.h"
  #include "utils/inval.h"
  #include "utils/lsyscache.h"
***
*** 142,147  typedef struct AlteredTableInfo
--- 143,149 
List   *newvals;/* List of NewColumnValue */
boolnew_notnull;/* T if we added new NOT NULL 
constraints */
boolrewrite;/* T if we a rewrite is forced 
*/
+   boolverify; /* T if we shall verify 
constraints */
Oid newTableSpace;  /* new tablespace; 0 means no 
change */
/* Objects to rebuild after completing ALTER TYPE operations */
List   *changedConstraintOids;  /* OIDs of constraints to 
rebuild */
***
*** 336,342  static void ATPr

Re: [HACKERS] sepgsql contrib module

2011-02-14 Thread Andrew Dunstan



On 02/14/2011 08:36 PM, Tom Lane wrote:

Andrew Dunstan  writes:

Yeah. The next thing I hit was this:
 [andrew@aurelia sepgsql]$ make -f /usr/share/selinux/devel/Makefile
 sepgsql-regtest.pp
 cat: /selinux/mls: No such file or directory
 make: *** No rule to make target `sepgsql-regtest.pp'.  Stop.
 [andrew@aurelia sepgsql]$

Hmph.  A build with --with-selinux goes through for me on a
pretty-vanilla Fedora 13 installation (at least the build and install
steps, I dunno how to test it).

It looks to me like /selinux/mls is some weird phony-filesystem file,
because "cat" prints one character (a "1") while "ls" claims the file is
of zero length.  So it's probably something consed up by the kernel,
like /proc/.  Do you have selinux enabled on your machine?


Np, but that really shouldn't be a build requirement, ISTM, even if it 
is a test requirement.




(BTW, testing what seems to be a kernel-configuration-reporting flag at
build time strikes me as pretty awful design.)




Yeah, I agree.

cheers

andrew

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


Re: [HACKERS] Re: [COMMITTERS] pgsql: Basic Recovery Control functions for use in Hot Standby. Pause,

2011-02-14 Thread Fujii Masao
On Wed, Feb 9, 2011 at 5:12 PM, Magnus Hagander  wrote:
> I was also worried about the non-hot-standby case, but I see that the
> patch makes sure you can't enable pause when not in hot standby mode.
> Which in itself might be surprising - perhaps we need a NOTICE for
> when that happens as well?
>
> And it definitely needs to be mentioned in the docs for
> pause_at_recovery_target that it only works in hot standby.

+1 with all the above.

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


[HACKERS] Re: [COMMITTERS] pgsql: Basic Recovery Control functions for use in Hot Standby. Pause,

2011-02-14 Thread Fujii Masao
On Tue, Feb 15, 2011 at 9:21 AM, Simon Riggs  wrote:
> On Wed, 2011-02-09 at 15:22 +0900, Fujii Masao wrote:
>
>> On the second thought, I think it's useful to emit the NOTICE message when
>> recovery reaches the pause point, as follows.
>>
>>     NOTICE: Recovery will not complete until pg_xlog_replay_resume() is 
>> called.
>
> I'm OK with adding a message, but NOTICE is the wrong level.
>
> My proposal is this message
>
> LOG:  Recovery has paused. Execute pg_xlog_replay_resume() to continue.

I agree to use LOG level. But "Execute pg_xlog_replay_resume() to continue."
looks like a hint rather than log. So what about:

LOG:  Recovery has paused.
HINT: Execute pg_xlog_replay_resume() to continue.

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: [DOCS] [HACKERS] "Extension" versus "module"

2011-02-14 Thread Tom Lane
Simon Riggs  writes:
> I would say that some modules are extensions, but not all. A standalone
> executable might be part of a module, but would not be an extension. 

> Remember also that not all modules out there on the net will have been
> updated either, so we must be able to discuss "extension-izing a
> module". (??)

Right.  So it seems like we ought to stick with more or less the
existing terminology: those various components under contrib/ are
modules.  Some of them are also extensions, but not all.

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] "Extension" versus "module"

2011-02-14 Thread Simon Riggs
On Mon, 2011-02-14 at 12:48 +0100, Dimitri Fontaine wrote:
> Tom Lane  writes:
> > Appendix F (contrib.sgml and its subsidiary files) is pretty consistent
> > about using "module" to refer to a contrib, uh, module.
> 
> I'm now thinking in those terms: the module is the shared object library
> that the backend needs to dlopen().  The extension is the SQL level
> object that wraps all its components.

I would say that some modules are extensions, but not all. A standalone
executable might be part of a module, but would not be an extension. 

Remember also that not all modules out there on the net will have been
updated either, so we must be able to discuss "extension-izing a
module". (??)

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 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] sepgsql contrib module

2011-02-14 Thread Tom Lane
Andrew Dunstan  writes:
> Yeah. The next thing I hit was this:

> [andrew@aurelia sepgsql]$ make -f /usr/share/selinux/devel/Makefile
> sepgsql-regtest.pp
> cat: /selinux/mls: No such file or directory
> make: *** No rule to make target `sepgsql-regtest.pp'.  Stop.
> [andrew@aurelia sepgsql]$

Hmph.  A build with --with-selinux goes through for me on a
pretty-vanilla Fedora 13 installation (at least the build and install
steps, I dunno how to test it).

It looks to me like /selinux/mls is some weird phony-filesystem file,
because "cat" prints one character (a "1") while "ls" claims the file is
of zero length.  So it's probably something consed up by the kernel,
like /proc/.  Do you have selinux enabled on your machine?

(BTW, testing what seems to be a kernel-configuration-reporting flag at
build time strikes me as pretty awful design.)

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] Replication server timeout patch

2011-02-14 Thread Simon Riggs
On Mon, 2011-02-14 at 14:13 -0800, Daniel Farina wrote:
> On Mon, Feb 14, 2011 at 12:48 AM, Fujii Masao  wrote:
> > On Sat, Feb 12, 2011 at 8:58 AM, Daniel Farina  wrote:
> >> Context diff equivalent attached.
> >
> > Thanks for the patch!
> >
> > As I said before, the timeout which this patch provides doesn't work well
> > when the walsender gets blocked in sending WAL. At first, we would
> > need to implement a non-blocking write function as an infrastructure
> > of the replication timeout, I think.
> > http://archives.postgresql.org/message-id/AANLkTi%3DPu2ne%3DVO-%2BCLMXLQh9y85qumLCbBP15CjnyUS%40mail.gmail.com

I wasn't aware that had been raised before. Thanks for noting it again.

I guess that's why you thought "wait forever" was a good idea ;-)

> Interesting point...if that's accepted as required-for-commit, what
> are the perceptions of the odds that, presuming I can write the code
> quickly enough, that there's enough infrastructure/ports already in
> postgres to allow for a non-blocking write on all our supported
> platforms?

I'd like to see what you come up with. I would rate that as important,
though not essential for sync replication.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 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] CommitFest 2011-01 as of 2011-02-04

2011-02-14 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote:
> But the
> trickiest part of this whole process is that, on the one hand, it's
> not fair for committers to ignore other people's patches, but on the
> other hand, it's not fair to expect committers to sacrifice getting
> their own projects done to get other people's projects done.

It wasn't my intent to ask the committers to do more but rather to
change the timings a bit so that when committers begin their "commitfest
month", the patches are already in better quality and more apt to be
ready for committer.

Anyhow, was just a thought.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] why two dashes in extension load files

2011-02-14 Thread Tom Lane
Robert Haas  writes:
> Are we deparsing the names of the SQL files to infer the set of
> version numbers we have to worry about?  It seems to me that if
> there's a list of known version numbers somewhere, we can use dash as
> the separator without any special restricton.

The list of known version numbers is inferred from the available files,
not vice versa.  IMO that's a feature not a bug.  A manually maintained
list would just be one more thing to forget to update.

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] .gitignore patch for coverage builds

2011-02-14 Thread Jeff Janes
On Wed, Jan 26, 2011 at 2:41 PM, Alvaro Herrera
 wrote:
> Excerpts from Tom Lane's message of mié ene 26 19:20:52 -0300 2011:
>> Robert Haas  writes:
>> > On Wed, Jan 26, 2011 at 4:44 PM, Tom Lane  wrote:
>> >> Ick. That's an awful lot of stuff to have global ignores for.
>>
>> > The "coverage" directory ignore seems a little icky, but the rest
>> > seems unlikely to pick up anything incidental.
>>
>> Tying /coverage to the root as in his V2 makes that better,
>
> Hmm, I don't think that works, because you can run "make coverage" in
> any subdir and it will create a "coverage" subdir there.

I like being told that I have a coverage directory outstanding when I
run "git status".

The hundreds of other files, not so much.


>> but I'm
>> still unexcited about the thesis that we should auto-ignore the results
>> of any random tool somebody wants to run in their source tree.
>
> Well, in this case it's not any random tool, because it's integrated
> into our makefiles.

I agree.  Should this be added to commit-fest 2011-Next?

Also, should "make clean-coverage" be changed to remove all of those
files from the entire tree and not just root?

Cheers,

Jeff

-- 
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: [COMMITTERS] pgsql: Basic Recovery Control functions for use in Hot Standby. Pause,

2011-02-14 Thread Simon Riggs
On Wed, 2011-02-09 at 15:22 +0900, Fujii Masao wrote:

> On the second thought, I think it's useful to emit the NOTICE message when
> recovery reaches the pause point, as follows.
> 
> NOTICE: Recovery will not complete until pg_xlog_replay_resume() is 
> called.

I'm OK with adding a message, but NOTICE is the wrong level.

My proposal is this message

LOG:  Recovery has paused. Execute pg_xlog_replay_resume() to continue.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 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] tsearch Parser Hacking

2011-02-14 Thread Thom Brown
On 14 February 2011 23:57, Tom Lane  wrote:
> "David E. Wheeler"  writes:
>> Is it possible to modify the default tsearch parser so that / doesn't get 
>> lexed as a "file" token?
>
> There is zero, none, nada, provision for modifying the behavior of the
> default parser, other than by changing its compiled-in state transition
> tables.
>
> It doesn't help any that said tables are baroquely designed and utterly
> undocumented.

This is very true. I intended to look into adding new tokens, but gave
up when I couldn't see how those transition tables worked.

> IMO, sooner or later we need to trash that code and replace it with
> something a bit more modification-friendly.

+1 for annihilating the existing code at some point.

-- 
Thom Brown
Twitter: @darkixion
IRC (freenode): dark_ixion
Registered Linux user: #516935

-- 
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] why two dashes in extension load files

2011-02-14 Thread Robert Haas
On Mon, Feb 14, 2011 at 10:13 AM, Tom Lane  wrote:
> Peter Eisentraut  writes:
>> Why do the extension load files need two dashes, like xml2--1.0.sql?
>> Why isn't one enough?
>
> Because we'd have to forbid dashes in extension name and version
> strings.  This was judged to be a less annoying solution.  See
> yesterday's discussion.

Are we deparsing the names of the SQL files to infer the set of
version numbers we have to worry about?  It seems to me that if
there's a list of known version numbers somewhere, we can use dash as
the separator without any special restricton.  For example
foo-bar-baz-bletch.sql is either an upgrade script from version
bar-baz to version bletch, or else it's an upgrade script from bar to
baz-bletch.  But presumably no real-world cases will actually be
ambiguous, assuming any sort of half-way sane version numbering
scheme.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] tsearch Parser Hacking

2011-02-14 Thread Tom Lane
"David E. Wheeler"  writes:
> Is it possible to modify the default tsearch parser so that / doesn't get 
> lexed as a "file" token?

There is zero, none, nada, provision for modifying the behavior of the
default parser, other than by changing its compiled-in state transition
tables.

It doesn't help any that said tables are baroquely designed and utterly
undocumented.

IMO, sooner or later we need to trash that code and replace it with
something a bit more modification-friendly.

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] CommitFest 2011-01 as of 2011-02-04

2011-02-14 Thread Robert Haas
Sorry for the previous, content-free reply.

On Mon, Feb 14, 2011 at 11:49 AM, Stephen Frost  wrote:
> * Robert Haas (robertmh...@gmail.com) wrote:
>> Here's where I think we are with this CommitFest.
>
>  Subject: Re: [HACKERS] CommitFest 2011-01 as of 2011-02-04
>
> I'm gonna go out on a limb and hope you meant '2011-02-14' there. :)

Yeah, sorry.

>> So there are two basic difficulties with wrapping the CommitFest up.
>
> I have to say that I've always been a bit suprised by the idea that the
> CommitFest is intended to be done and all patches *committed* at the end
> of the month.  It's been working really rather well, which is due in
> great part to the excellent CF managers (thanks again for being that,
> again).  That said, we have quite a few non-committer reviewers who
> provide good feedback and move the patch back to 'waiting for author'
> and that whole process takes a while.

It does, but frankly I don't see much reason to change it, since it's
been working pretty well on the whole.  Andrew was on point when he
mentioned that it's not obvious what committers get out of working on
other people's patches.  Obviously, the answer is, well, they get a
better PostgreSQL, and that's ultimately good for all of us.  But the
trickiest part of this whole process is that, on the one hand, it's
not fair for committers to ignore other people's patches, but on the
other hand, it's not fair to expect committers to sacrifice getting
their own projects done to get other people's projects done.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] FOR KEY LOCK foreign keys

2011-02-14 Thread Alvaro Herrera
Excerpts from Marti Raudsepp's message of lun feb 14 19:39:25 -0300 2011:
> On Fri, Feb 11, 2011 at 09:13, Noah Misch  wrote:
> > The patch had a trivial conflict in planner.c, plus plenty of offsets.  I've
> > attached the rebased patch that I used for review.  For anyone following 
> > along,
> > all the interesting hunks touch heapam.c; the rest is largely mechanical.  A
> > "diff -w" patch is also considerably easier to follow.
> 
> Here's a simple patch for the RelationGetIndexAttrBitmap() function,
> as explained in my last post. I don't know if it's any help to you,
> but since I wrote it I might as well send it up. This applies on top
> of Noah's rebased patch.

Got it, thanks.

> I did some tests and it seems to work, although I also hit the same
> visibility bug as Noah.

Yeah, that bug is fixed with the attached, though I am rethinking this
bit.

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


0001-Fix-visibility-bug-and-poorly-worded-comment.patch
Description: Binary data

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


Re: [HACKERS] CommitFest 2011-01 as of 2011-02-04

2011-02-14 Thread Robert Haas
On Mon, Feb 14, 2011 at 11:49 AM, Stephen Frost  wrote:
> I have to say that I've always been a bit suprised by the idea that the
> CommitFest is intended to be done and all patches *committed* at the end
> of the month.  It's been working really rather well, which is due in
> great part to the excellent CF managers (thanks again for being that,
> again).  That said, we have quite a few non-committer reviewers who
> provide good feedback and move the patch back to 'waiting for author'
> and that whole process takes a while.


>
> Perhaps a thought for next time would be to offset things a bit.  eg:
>
> CF 2011-03 (or whatever):
> 2011-02-14: Patches should all be submitted
> 2011-02-14: Reviewers start
> 2011-03-01: Committers start w/ 'Ready for Committer' patches
> 2011-03-14: Patches not marked 'Ready for Committer' get bounced
> 2011-03-31: All patches committed
>
> I'm not against the 'waiting on author' approach, but I do feel like if
> we're going to continue to have it, we need to spread it out a bit more.
> I do think this would place more work on the CF manager, unfortunately,
> but I'd hope that they would primairly be focused on managing the
> reviews and not be as busy during the last 2 weeks.  Maybe one day I'll
> be brave enough to offer to manage one and see. :)
>
>        Thanks again, Robert, you've done an excellent job managing the CF.
>
>                Stephen
>
> -BEGIN PGP SIGNATURE-
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iEYEARECAAYFAk1ZXTQACgkQrzgMPqB3kiiLAQCfUVusKmhcQW1KNGQwZSFpdONx
> G4oAnjzPLSQpyaounlTMrumdoQe58yA/
> =zuCX
> -END PGP SIGNATURE-
>
>



-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] FOR KEY LOCK foreign keys

2011-02-14 Thread Marti Raudsepp
On Fri, Feb 11, 2011 at 09:13, Noah Misch  wrote:
> The patch had a trivial conflict in planner.c, plus plenty of offsets.  I've
> attached the rebased patch that I used for review.  For anyone following 
> along,
> all the interesting hunks touch heapam.c; the rest is largely mechanical.  A
> "diff -w" patch is also considerably easier to follow.

Here's a simple patch for the RelationGetIndexAttrBitmap() function,
as explained in my last post. I don't know if it's any help to you,
but since I wrote it I might as well send it up. This applies on top
of Noah's rebased patch.

I did some tests and it seems to work, although I also hit the same
visibility bug as Noah.

Test case I used:

THREAD A:
create table foo (pk int primary key, ak int);
create unique index on foo (ak) where ak != 0;
create unique index on foo ((-ak));

create table bar (foo_pk int references foo (pk));
insert into foo values(1,1);
begin; insert into bar values(1);

THREAD B:
begin; update foo set ak=2 where ak=1;


Regards,
Marti
From e069cef91c686aa87e220336198267e5a5a2aeac Mon Sep 17 00:00:00 2001
From: Marti Raudsepp 
Date: Tue, 15 Feb 2011 00:33:35 +0200
Subject: [PATCH] Only acquire KEY LOCK for colums that can be referenced by foreign keys

Don't consider columns in unique indexes that have expressions or WHERE
predicates.
---
 src/backend/utils/cache/relcache.c |   23 +--
 src/include/utils/rel.h|2 +-
 src/include/utils/relcache.h   |2 +-
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 4d37e8e..5119288 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -3608,7 +3608,8 @@ RelationGetIndexPredicate(Relation relation)
  * simple index keys, but attributes used in expressions and partial-index
  * predicates.)
  *
- * If "unique" is true, only attributes of unique indexes are considered.
+ * If "keyAttrs" is true, only attributes that can be referenced by foreign
+ * keys are considered.
  *
  * Attribute numbers are offset by FirstLowInvalidHeapAttributeNumber so that
  * we can include system attributes (e.g., OID) in the bitmap representation.
@@ -3617,7 +3618,7 @@ RelationGetIndexPredicate(Relation relation)
  * be bms_free'd when not needed anymore.
  */
 Bitmapset *
-RelationGetIndexAttrBitmap(Relation relation, bool unique)
+RelationGetIndexAttrBitmap(Relation relation, bool keyAttrs)
 {
 	Bitmapset  *indexattrs;
 	Bitmapset  *uindexattrs;
@@ -3627,7 +3628,7 @@ RelationGetIndexAttrBitmap(Relation relation, bool unique)
 
 	/* Quick exit if we already computed the result. */
 	if (relation->rd_indexattr != NULL)
-		return bms_copy(unique ? relation->rd_uindexattr : relation->rd_indexattr);
+		return bms_copy(keyAttrs ? relation->rd_keyattr : relation->rd_indexattr);
 
 	/* Fast path if definitely no indexes */
 	if (!RelationGetForm(relation)->relhasindex)
@@ -3653,12 +3654,18 @@ RelationGetIndexAttrBitmap(Relation relation, bool unique)
 		Relation	indexDesc;
 		IndexInfo  *indexInfo;
 		int			i;
+		bool		isKey;
 
 		indexDesc = index_open(indexOid, AccessShareLock);
 
 		/* Extract index key information from the index's pg_index row */
 		indexInfo = BuildIndexInfo(indexDesc);
 
+		/* Can this index be referenced by a foreign key? */
+		isKey = indexInfo->ii_Unique &&
+indexInfo->ii_Expressions == NIL &&
+indexInfo->ii_Predicate == NIL;
+
 		/* Collect simple attribute references */
 		for (i = 0; i < indexInfo->ii_NumIndexAttrs; i++)
 		{
@@ -3668,7 +3675,7 @@ RelationGetIndexAttrBitmap(Relation relation, bool unique)
 			{
 indexattrs = bms_add_member(indexattrs,
 			   attrnum - FirstLowInvalidHeapAttributeNumber);
-if (indexInfo->ii_Unique)
+if (isKey)
 	uindexattrs = bms_add_member(uindexattrs,
 			   	 attrnum - FirstLowInvalidHeapAttributeNumber);
 			}
@@ -3676,13 +3683,9 @@ RelationGetIndexAttrBitmap(Relation relation, bool unique)
 
 		/* Collect all attributes used in expressions, too */
 		pull_varattnos((Node *) indexInfo->ii_Expressions, &indexattrs);
-		if (indexInfo->ii_Unique)
-			pull_varattnos((Node *) indexInfo->ii_Expressions, &uindexattrs);
 
 		/* Collect all attributes in the index predicate, too */
 		pull_varattnos((Node *) indexInfo->ii_Predicate, &indexattrs);
-		if (indexInfo->ii_Unique)
-			pull_varattnos((Node *) indexInfo->ii_Predicate, &uindexattrs);
 
 		index_close(indexDesc, AccessShareLock);
 	}
@@ -3692,11 +3695,11 @@ RelationGetIndexAttrBitmap(Relation relation, bool unique)
 	/* Now save a copy of the bitmap in the relcache entry. */
 	oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
 	relation->rd_indexattr = bms_copy(indexattrs);
-	relation->rd_uindexattr = bms_copy(uindexattrs);
+	relation->rd_keyattr = bms_copy(uindexattrs);
 	MemoryContextSwitchTo(oldcxt);
 
 	/* We return our original working copy for caller to play with */
-	return unique ? uindexattrs : indexat

[HACKERS] tsearch Parser Hacking

2011-02-14 Thread David E. Wheeler
Hackers,

Is it possible to modify the default tsearch parser so that / doesn't get lexed 
as a "file" token? That is, instead of this:

try=# select * from ts_debug('simple'::regconfig, 'w/d');
 alias │description│ token │ dictionaries │ dictionary │ lexemes 
───┼───┼───┼──┼┼─
 file  │ File or path name │ w/d   │ {simple} │ simple │ {w/d}

Ideally it'd think that / was the same as -:

try=# select * from ts_debug('simple'::regconfig, 'w-d');
  alias  │   description   │ token │ dictionaries │ 
dictionary │ lexemes 
─┼─┼───┼──┼┼─
 asciihword  │ Hyphenated word, all ASCII  │ w-d   │ {simple} │ 
simple │ {w-d}
 hword_asciipart │ Hyphenated word part, all ASCII │ w │ {simple} │ 
simple │ {w}
 blank   │ Space symbols   │ - │ {}   │ 
[null] │ [null]
 hword_asciipart │ Hyphenated word part, all ASCII │ d │ {simple} │ 
simple │ {d}
(4 rows)

Possible? Or would I have to write a completely new parser just to change this 
bit?

Thanks,

David


-- 
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] sepgsql contrib module

2011-02-14 Thread Andrew Dunstan



On 02/14/2011 04:21 PM, Tom Lane wrote:

Andrew Dunstan  writes:

Thew makefile still has this bogosity:
 sepgsql-regtest.pp: sepgsql-regtest.te
  $(MAKE) -f $(DESTDIR)/usr/share/selinux/devel/Makefile $@
We need to fix that up before we even think of trying to get buildfarm
coverage. The presence and location of this makefile should be
determined at configure time, ISTM.

I'd suggest just getting rid of the $(DESTDIR), which is flat out wrong,
and leaving it as-is otherwise.  The portability level of this code is
somewhere at the bad-joke level anyway; if you're trying to get it to
run anywhere but a recent Fedora/RHEL system, I really doubt that path
is the first or biggest problem you'll hit.



Yeah. The next thing I hit was this:

   [andrew@aurelia sepgsql]$ make -f /usr/share/selinux/devel/Makefile
   sepgsql-regtest.pp
   cat: /selinux/mls: No such file or directory
   make: *** No rule to make target `sepgsql-regtest.pp'.  Stop.
   [andrew@aurelia sepgsql]$


That's not very nice.

cheers

andrew

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


Re: [HACKERS] Replication server timeout patch

2011-02-14 Thread Daniel Farina
On Mon, Feb 14, 2011 at 12:48 AM, Fujii Masao  wrote:
> On Sat, Feb 12, 2011 at 8:58 AM, Daniel Farina  wrote:
>> Context diff equivalent attached.
>
> Thanks for the patch!
>
> As I said before, the timeout which this patch provides doesn't work well
> when the walsender gets blocked in sending WAL. At first, we would
> need to implement a non-blocking write function as an infrastructure
> of the replication timeout, I think.
> http://archives.postgresql.org/message-id/AANLkTi%3DPu2ne%3DVO-%2BCLMXLQh9y85qumLCbBP15CjnyUS%40mail.gmail.com

Interesting point...if that's accepted as required-for-commit, what
are the perceptions of the odds that, presuming I can write the code
quickly enough, that there's enough infrastructure/ports already in
postgres to allow for a non-blocking write on all our supported
platforms?

--
fdr

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


Re: [HACKERS] Scheduled maintenance affecting gitmaster

2011-02-14 Thread Magnus Hagander
On Mon, Feb 14, 2011 at 16:15, Magnus Hagander  wrote:
> On Mon, Feb 14, 2011 at 10:39, Stefan Kaltenbrunner
>  wrote:
>> On 02/14/2011 10:09 AM, Magnus Hagander wrote:
>>> On Mon, Feb 14, 2011 at 07:13, Stefan Kaltenbrunner
>>>  wrote:
 On 02/14/2011 01:27 AM, Tom Lane wrote:
>
> Magnus Hagander  writes:
>>
>> Unfortunately, one of the worst-case scenarios appears to have
>> happened - a machine did not come back up after a reboot.
>> ...
>> We'll get back to you with more information as soon as we have it.
>
> I didn't see any followup to this?

 yeah - the hosting company managed to reboot the box for us which brought 
 it
 back to life in the middle of the night (with both magnus and me asleep).
>>>
>>> Indeed. But the good news is that once it came back up, the VM with
>>> the git server started ok :-)
>>>
>>>
> gitmaster seems to be responding as of now, is it safe to push?

 yes it is - however we will need to schedule another maintenance window 
 soon
 to finish the stuff we actually wanted to do.
>>>
>>> So, after some discussion with Stefan, we (well, I guess I) decided we
>>> should just go ahead and declare the maintenance window not closed
>>> yet, and finish off the upgrade right now :-) Given that the majority
>>> of our commits don't happen now, we'll hopefully have it done by the
>>> time the US folks wake up again.
>>>
>>> So, maintenance window again, starting now, and we'll let you know as
>>> soon as we're done. And we're definitely hoping for the machine to
>>> come back up properly this time :-)
>>
>> and it did not... We are trying to figure out what the actual problem
>> here really is because it seems to boot just fine when powercycled just
>> not with a software initiated reboot.
>> We will notify once we have more information...
>
> Status update on this - Stefan is currently working with the
> datacenter people on getting this fixed (they are now available
> on-site), since we are now having an actual issue with the machine
> (GRUB failure on boot) rather than just a failure to shut down.

We are still having issues with this box. For that reason, we have
moved the gitmaster server over to another box, where it's now up and
running. DNS has been updated, but it will take some time for it to
sync out. For those of you who want access now, you ca nreach the new
master server at 98.129.198.116.

Note, however, that mail relaying from this machine does not currently
work, so commit messages will be out for a bit longer while we work on
clearing things up.

-- 
 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] sepgsql contrib module

2011-02-14 Thread Tom Lane
Andrew Dunstan  writes:
> Thew makefile still has this bogosity:

> sepgsql-regtest.pp: sepgsql-regtest.te
>  $(MAKE) -f $(DESTDIR)/usr/share/selinux/devel/Makefile $@

> We need to fix that up before we even think of trying to get buildfarm 
> coverage. The presence and location of this makefile should be 
> determined at configure time, ISTM.

I'd suggest just getting rid of the $(DESTDIR), which is flat out wrong,
and leaving it as-is otherwise.  The portability level of this code is
somewhere at the bad-joke level anyway; if you're trying to get it to
run anywhere but a recent Fedora/RHEL system, I really doubt that path
is the first or biggest problem you'll hit.

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] pl/python do not delete function arguments

2011-02-14 Thread Jan Urbański
On 14/02/11 22:13, Jan Urbański wrote:
> On 14/02/11 21:06, Peter Eisentraut wrote:
>> On ons, 2011-02-09 at 10:02 +0100, Jan Urbański wrote:
>>> On 09/02/11 04:52, Hitoshi Harada wrote:
 2010/12/31 Jan Urbański :
> (continuing the flurry of patches)
>
> Here's a patch that stops PL/Python from removing the function's
> arguments from its globals dict after calling it. It's
> an incremental patch on top of the plpython-refactor patch sent in
> http://archives.postgresql.org/message-id/4d135170.3080...@wulczer.org.
>
> Git branch for this patch:
> https://github.com/wulczer/postgres/tree/dont-remove-arguments
>
> Apart from being useless, as the whole dict is unreffed and thus freed
> in PLy_procedure_delete, removing args actively breaks things for
> recursive invocation of the same function. The recursive callee after
> returning will remove the args from globals, and subsequent access to
> the arguments in the caller will cause a NameError (see new regression
> test in patch).

 I've reviewed this. The patch is old enough to be rejected by patch
 command, but I manged to apply it by hand.
 It compiles clean. Added tests pass.
 I created fibonacci function similar to recursion_test in the patch
 and confirmed the recursion raises error on 9.0 but not on 9.1.
 Doc is not with the patch since this change is to remove unnecessary
 optimization internally.

 "Ready for Committer"
>>>
>>> Thanks,
>>>
>>> patch merged with HEAD attached.
>>
>> Curiously, without the patch the recursion_test(4) call fails but
>> recursion_test(5) passes.  Anyone know why?

Baah, damn, did not read the "without patch" part.

The problem is that every *second* call to the function fails,
regardless of the number. The first execution succeeds, but then
PLy_delete_args deletes the argument from the globals, and when the next
execution tries to fetch "n" from it, it raises a KeyError.

Jan

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


Re: [HACKERS] pl/python do not delete function arguments

2011-02-14 Thread Jan Urbański
On 14/02/11 21:06, Peter Eisentraut wrote:
> On ons, 2011-02-09 at 10:02 +0100, Jan Urbański wrote:
>> On 09/02/11 04:52, Hitoshi Harada wrote:
>>> 2010/12/31 Jan Urbański :
 (continuing the flurry of patches)

 Here's a patch that stops PL/Python from removing the function's
 arguments from its globals dict after calling it. It's
 an incremental patch on top of the plpython-refactor patch sent in
 http://archives.postgresql.org/message-id/4d135170.3080...@wulczer.org.

 Git branch for this patch:
 https://github.com/wulczer/postgres/tree/dont-remove-arguments

 Apart from being useless, as the whole dict is unreffed and thus freed
 in PLy_procedure_delete, removing args actively breaks things for
 recursive invocation of the same function. The recursive callee after
 returning will remove the args from globals, and subsequent access to
 the arguments in the caller will cause a NameError (see new regression
 test in patch).
>>>
>>> I've reviewed this. The patch is old enough to be rejected by patch
>>> command, but I manged to apply it by hand.
>>> It compiles clean. Added tests pass.
>>> I created fibonacci function similar to recursion_test in the patch
>>> and confirmed the recursion raises error on 9.0 but not on 9.1.
>>> Doc is not with the patch since this change is to remove unnecessary
>>> optimization internally.
>>>
>>> "Ready for Committer"
>>
>> Thanks,
>>
>> patch merged with HEAD attached.
> 
> Curiously, without the patch the recursion_test(4) call fails but
> recursion_test(5) passes.  Anyone know why?

Damn, I remember that bug and thought I fixed it. I think calls with
even numbers passed and calls with odd numbers failed... I'll try to see
if a merge error did not introduce that bug back.

> Btw., I get a KeyError, not a NameError as you say above.

Will look into that too.

Cheers,
Jan

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


Re: [HACKERS] ALTER TYPE 2: skip already-provable no-work rewrites

2011-02-14 Thread Stephen Frost
* Noah Misch (n...@leadboat.com) wrote:
> On Mon, Feb 14, 2011 at 01:12:21PM -0500, Stephen Frost wrote:
> > First question is- why do you use #ifdef USE_ASSERT_CHECKING ..?
> 
> The other six code sites checking assert_enabled directly do the same.

Wow, I could have sworn that I looked at what others were doing and
didn't see that.  Sorry for the noise.

> > In ATColumnChangeSetWorkLevel(), I'm really not a huge fan of using a
> > passed-in argument to move through a list with..  I'd suggest using a
> > local variable that is set from what's passed in.  I do see that's
> > inheirited, but still, you've pretty much redefined that entire
> > function anyway..
> 
> Could do that.  However, the function would reference the original argument 
> just
> once, to make that copy.  I'm not seeing a win.

Perhaps I'm just deficient in this area, but I think of arguments,
unless specifically intended otherwise, to be 'read-only' and based on
that assumption, seeing it come up as an lvalue hits me as wrong.

I'm happy enough to let someone else decide if they agree with me or you
though. :)

> The way I like to think about it is that we're recursively walking an 
> expression
> tree to determine which of three categories it falls into: always produces the
> same value without error; always produces the same value on success, but may
> throw an error; neither of the above.  We have a whitelist of node types that
> are acceptable, and anything else makes us assume the worst.  (The nodes we
> accept are simple enough that the recursion degenerates to iteration.)  

It's that degeneration that definitely hits me as making the whole thing
look a bit 'funny'.  When I first looked at the loop, I was looking for
a tree structure and trying to figure out how it could work with just a
simple while().

> http://archives.postgresql.org/message-id/20101231013534.ga7...@tornado.leadboat.com
> 
> Should I restore some of that?  Any other particular text that would have 
> helped?

I definitely think the examples given, enumerating the types of nodes
that matter for this (and why they're the ones we look for), and the
rules that are followed would help a great deal.  Anyone else who comes
across this code may be wondering "do I need to do something here for
this new node type that I just added".

> > It also seems like it'd make more sense to me to
> > be a while() controlled by (IsA(expr, Var) && ((Var *) expr)->varattno
> > == varattno), since that's really the normal "stopping point".
> 
> If we can optimize to some extent, that is the stopping point.  If not,
> tab->rewrite is the stopping point.  I picked the no-optimization case as
> "normal" and used that as the loop condition, but one could go either way.

I would think you could still short-circuit the loop when you've
discovered a case where we have to rewrite the table anyway.  Having the
comments updated to reflect what's going on and why stopping on
tab->rewrite, in particular, makes sense is more important though.

> The validity of this optimization does not
> rely on any user-settable property of a data type, but it does lean heavily on
> the execution semantics of specific nodes.

After thinking through this and diving into coerce_to_target_type() a
bit, I'm finally coming to understand how this is working.  The gist of
it, if I follow correctly, is that if the planner doesn't think we have
to do anything but copy this value to make it the new data type, then
we're good to go.  That makes sense, when you think about it, but boy
does it go the long way around to get there.

Essentially, coerce_to_target_type() is returning an expression tree and
ATColumnChangeSetWorkLevel() is checking to see if that expression tree
is "copy the value".  Maybe this is a bit much, but if
coerce_to_target_type() knows the expression given to it is a
straight-up copy, perhaps it could pass that information along in an
easier to use format than an expression tree?  That would obviate the
need for ATColumnChangeSetWorkLevel() entirely, if I understand
correctly.

Of course, coerce_to_target_type() is used by lots of other places,
almost all of which probably have to have an expression tree to stuff
into a plan, so maybe a simpler function could be defined which operates
at the level that ATColumnChangeSetWorkLevel() needs?  Or perhaps other
places would benefit from knowing that a given conversion is an actual
no-op rather than copying the value?

Just my 2c, I don't believe the patch could cause problems now that I'm
understanding it better, but it sure does seem excessive to use an
expression tree to figure out when something is a no-op.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] FOR KEY LOCK foreign keys

2011-02-14 Thread Alvaro Herrera
Excerpts from Noah Misch's message of vie feb 11 04:13:22 -0300 2011:

> I observe visibility breakage with this test case:
>
>  [ ... ]
>
> The problem seems to be that funny t_cid (2249).  Tracing through heap_update,
> the new code is not setting t_cid during this test case.

So I can fix this problem by simply adding a call to
HeapTupleHeaderSetCmin when the stuff about ComboCid does not hold, but
seeing that screenful plus the subsequent call to
HeapTupleHeaderAdjustCmax feels wrong.  I think this needs to be
rethought ...

-- 
Álvaro Herrera 
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] why two dashes in extension load files

2011-02-14 Thread Chris Browne
t...@sss.pgh.pa.us (Tom Lane) writes:
> Peter Eisentraut  writes:
>> On mån, 2011-02-14 at 10:13 -0500, Tom Lane wrote:
>>> Peter Eisentraut  writes:
 Why do the extension load files need two dashes, like xml2--1.0.sql?
 Why isn't one enough?
>
>>> Because we'd have to forbid dashes in extension name and version
>>> strings.  This was judged to be a less annoying solution.  See
>>> yesterday's discussion.
>
>> I'm not convinced.  There was nothing in that discussion why any
>> particular character would have to be allowed in a version number.
>
> Well, there's already a counterexample in the current contrib stuff:
> uuid-ossp.  We could rename that to uuid_ossp of course, but it's
> not clear to me that there's consensus for forbidding dashes here.

I suspect that "_" might be troublesome.

Let me observe on Debian policy... It requires that package names
consist as follows:

   Package names (both source and binary, see Package, Section 5.6.7)
   must consist only of lower case letters (a-z), digits (0-9), plus (+)
   and minus (-) signs, and periods (.). They must be at least two
   characters long and must start with an alphanumeric character.

   http://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-Source

I suspect that we'll need to have a policy analagous to that.

Also worth observing: Debian package files are of the form:
   "${package}_${version}-${dversion}_${arch}.deb"

where package and version have fairly obvious interpretation, and...
  - dversion indicates a sequence handled by Debian
  - arch indicates CPU architecture (i386, amd64, ...)

Probably the dversion/arch bits aren't of interest to us, but the
remainder of the notation used by Debian seems not inapplicable for us.
-- 
let name="cbbrowne" and tld="gmail.com" in String.concat "@" [name;tld];;
http://linuxdatabases.info/info/languages.html
Signs  of  a Klingon  Programmer  -  4. "You  cannot really appreciate
Dilbert unless you've read it in the original Klingon."

-- 
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] why two dashes in extension load files

2011-02-14 Thread Cédric Villemain
2011/2/14 Tom Lane :
> =?ISO-8859-1?Q?C=E9dric_Villemain?=  
> writes:
>> why do we care if there is a dash in the middle of a text where there
>> are no numbers ?
>
> Umm ... we are not requiring version names to be numbers.

good point  I was believing we had something like
multi-name-1.2.3-5.6.7 at a maximum.

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



-- 
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] why two dashes in extension load files

2011-02-14 Thread Tom Lane
=?ISO-8859-1?Q?C=E9dric_Villemain?=  writes:
> why do we care if there is a dash in the middle of a text where there
> are no numbers ?

Umm ... we are not requiring version names to be numbers.

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] pl/python do not delete function arguments

2011-02-14 Thread Peter Eisentraut
On ons, 2011-02-09 at 10:02 +0100, Jan Urbański wrote:
> On 09/02/11 04:52, Hitoshi Harada wrote:
> > 2010/12/31 Jan Urbański :
> >> (continuing the flurry of patches)
> >>
> >> Here's a patch that stops PL/Python from removing the function's
> >> arguments from its globals dict after calling it. It's
> >> an incremental patch on top of the plpython-refactor patch sent in
> >> http://archives.postgresql.org/message-id/4d135170.3080...@wulczer.org.
> >>
> >> Git branch for this patch:
> >> https://github.com/wulczer/postgres/tree/dont-remove-arguments
> >>
> >> Apart from being useless, as the whole dict is unreffed and thus freed
> >> in PLy_procedure_delete, removing args actively breaks things for
> >> recursive invocation of the same function. The recursive callee after
> >> returning will remove the args from globals, and subsequent access to
> >> the arguments in the caller will cause a NameError (see new regression
> >> test in patch).
> > 
> > I've reviewed this. The patch is old enough to be rejected by patch
> > command, but I manged to apply it by hand.
> > It compiles clean. Added tests pass.
> > I created fibonacci function similar to recursion_test in the patch
> > and confirmed the recursion raises error on 9.0 but not on 9.1.
> > Doc is not with the patch since this change is to remove unnecessary
> > optimization internally.
> > 
> > "Ready for Committer"
> 
> Thanks,
> 
> patch merged with HEAD attached.

Curiously, without the patch the recursion_test(4) call fails but
recursion_test(5) passes.  Anyone know why?

Btw., I get a KeyError, not a NameError as you say above.




-- 
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] CommitFest 2011-01 as of 2011-02-04

2011-02-14 Thread Dimitri Fontaine
Robert Haas  writes:
> We have committed 45 patches and returned with feedback or rejected
> 23.  There are 30 remaining patches, every single one of which has
> been reviewed.  20 of those are marked Ready for Committer; 5 are
> marked Waiting on Author; 5 are marked Needs Review.  However, again,

I just took the liberty to mark the 2 extension patches as commited,
even if we have some more "details" to fix.  Well if we include the PL
stuff, it's detail and a half, but I though marking them commited would
help us following here.  I hope it does.  Will sleep on that.  Now. :)

I would like to see this commit fest extended by at least a week, maybe
two, like has been proposed before.  The overall rhythm has been quite
impressive, and I'm not seeing such a slowdown that it's useless to try
continuing.

Regards,
-- 
Dimitri Fontaine
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


[HACKERS] pageinspect's infomask and infomask2 as smallint

2011-02-14 Thread Alvaro Herrera
Thanks to Noah Misch's review of the keylock patch I noticed that
pageinspect's heap_page_items(bytea) function returns infomask and
infomask2 as smallint (signed).  But the fields in the tuple header are
16 bits unsigned, so if the high (16th) bit is set, it returns negative
values which seem hard to handle.  Not a problem for infomask, because
the high bit is used for a VACUUM FULL-era flag; but in infomask2 it is
used.

This seems hard to fix for existing installations with the unpackaged
module already loaded -- IIRC it's not acceptable to drop a function,
which is what would need to be done here.

I report this because it seems the first case to test upgradability of a
module.

-- 
Álvaro Herrera 

-- 
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] why two dashes in extension load files

2011-02-14 Thread Cédric Villemain
2011/2/14 Tom Lane :
> "David E. Wheeler"  writes:
>> On Feb 14, 2011, at 8:54 AM, Tom Lane wrote:
 I'm not convinced.  There was nothing in that discussion why any
 particular character would have to be allowed in a version number.
>
>>> Well, there's already a counterexample in the current contrib stuff:
>>> uuid-ossp.  We could rename that to uuid_ossp of course, but it's
>>> not clear to me that there's consensus for forbidding dashes here.

why do we care if there is a dash in the middle of a text where there
are no numbers ?

>
>> I'd be fine if commas were used instead.
>
> Commas do not seem like an improvement to me at all --- they are widely
> used as list separators.
>
> I guess the real question is what's Peter's concrete objection to the
> double-dash method?

I have to admit that I am a bit surprised by this -- stuff too.
An objection might be completely non-technical, but advocacy :

"what this funny new name convention those PostgreSQL folks did invent ?!"


-- 
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] ALTER TYPE 2: skip already-provable no-work rewrites

2011-02-14 Thread Noah Misch
Hi Stephen,

Thanks for jumping in on this one.

On Mon, Feb 14, 2011 at 01:12:21PM -0500, Stephen Frost wrote:
> First question is- why do you use #ifdef USE_ASSERT_CHECKING ..?

The other six code sites checking assert_enabled directly do the same.

> assert_enabled exists and will work the way you expect regardless, no?

Yes.  

> Strikes me as unlikely that the checks would be a real performance
> problem..

Agreed.

> In ATColumnChangeSetWorkLevel(), I'm really not a huge fan of using a
> passed-in argument to move through a list with..  I'd suggest using a
> local variable that is set from what's passed in.  I do see that's
> inheirited, but still, you've pretty much redefined that entire
> function anyway..

Could do that.  However, the function would reference the original argument just
once, to make that copy.  I'm not seeing a win.

> Also, I feel like that while(!tab->rewrite) really deserves more
> explanation of what's happening than the function-level comment above
> gives it.  I'd prefer to see a comment above the while() explaining
> that we're moving through a list to see if there's any level at which
> expr is something complicated or is referring to a domain which has
> constraints on it (presuming that I've followed what's going on
> correctly, that is..).

The way I like to think about it is that we're recursively walking an expression
tree to determine which of three categories it falls into: always produces the
same value without error; always produces the same value on success, but may
throw an error; neither of the above.  We have a whitelist of node types that
are acceptable, and anything else makes us assume the worst.  (The nodes we
accept are simple enough that the recursion degenerates to iteration.)  Here's
the comment explaining the algorithm, from an earlier version of the patch:

+ /* GetCoerceExemptions()
+  *Assess invariants of a coercion expression.
+  *
+  * Various common expressions arising from type coercion are subject to
+  * optimizations.  For example, a simple varchar -> text cast will never 
change
+  * the underlying data (COERCE_EXEMPT_NOCHANGE) and never yield an error
+  * (COERCE_EXEMPT_NOERROR).  A varchar(8) -> varchar(4) will never change the
+  * data, but it may yield an error.  Given a varno and varattno denoting "the"
+  * source datum, determine which invariants hold for an expression by walking 
it
+  * per these rules:
+  *
+  *1. A Var with the varno/varattno in question has both invariants.
+  *2. A RelabelType node inherits the invariants of its sole argument.
+  *3. A CoerceToDomain node inherits any COERCE_EXEMPT_NOCHANGE invariant 
from
+  *its sole argument.  When GetDomainConstraints() == NIL, it also 
inherits
+  *COERCE_EXEMPT_NOERROR.  Otherwise, COERCE_EXEMPT_NOERROR 
becomes false.
+  *4. All other nodes have neither invariant.
+  *
+  * Returns a bit string that may contain the following bits:
+  *COERCE_EXEMPT_NOCHANGE: expression result will always have the same 
binary
+  *representation as a Var expression having the 
given varno and
+  *varattno
+  *COERCE_EXEMPT_NOERROR: expression will never throw an error
+  */

The return value changed, but the rest remains accurate.  Here's a similar
explanation from the design thread; it covers a more general case:

http://archives.postgresql.org/message-id/20101231013534.ga7...@tornado.leadboat.com

Should I restore some of that?  Any other particular text that would have 
helped?

> It also seems like it'd make more sense to me to
> be a while() controlled by (IsA(expr, Var) && ((Var *) expr)->varattno
> == varattno), since that's really the normal "stopping point".

If we can optimize to some extent, that is the stopping point.  If not,
tab->rewrite is the stopping point.  I picked the no-optimization case as
"normal" and used that as the loop condition, but one could go either way.

> These are all more stylistic issues than anything else.  Last, but not
> least, I do worry about how this may impact contrib modules, external
> projects, or user-added data types, such as PostGIS, hstore, and ip4r.
> Could we/should we limit this to only PG data types that we 'know' are
> binary compatible?  Is there any way or reason such external modules
> could be fouled up by this?

External modules are safe.  If a binary coercion cast (CREATE CAST ... WITHOUT
FUNCTION) implements the type conversion for an ALTER TABLE ... SET DATA TYPE,
coerce_to_target_type() will inject a RelabelType node, and this code will pick
up on that and avoid the rewrite.  If such a cast were defined erroneously, the
user is no less in trouble.  He'd get a table rewrite, but the rewrite would
just transfer the bits unchanged.  The validity of this optimization does not
rely on any user-settable property of a data type, but it does lean heavily on
the execution semantics of specific nodes.

Thanks,
nm

-- 

Re: [HACKERS] sepgsql contrib module

2011-02-14 Thread Andrew Dunstan



On 02/14/2011 11:47 AM, Kohei Kaigai wrote:

We really need to get a buildfarm which is building with this.  To that
end, would you mind providing directions so someone else could set up a
buildfarm member to test it..?

It seems to me not difficult to describe a direction to build, install and
run regression test.
Do we have any Fedora 14 environment in the buildfarm?
It is the most suitable distribution to set up sepgsql module, because the
default installation already has selinux configurations.




Thew makefile still has this bogosity:

   sepgsql-regtest.pp: sepgsql-regtest.te
$(MAKE) -f $(DESTDIR)/usr/share/selinux/devel/Makefile $@


We need to fix that up before we even think of trying to get buildfarm 
coverage. The presence and location of this makefile should be 
determined at configure time, ISTM.


cheers

andrew



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


Re: [HACKERS] SSI bug?

2011-02-14 Thread Kevin Grittner
YAMAMOTO Takashi  wrote:
 
>> Did you notice whether the loop involved multiple tuples within a
>> single page?
> 
> if i understand correctly, yes.
> 
> the following is a snippet of my debug code (dump targets when
> triggerCheckTargetForConflictsIn loops >1000 times) and its
> output.the same locktag_field3 value means the same page, right?
 
Right.
 
> the table seems mostly hot-updated, if it matters.
 
> idx_scan  | 53681
> idx_tup_fetch | 52253
> n_tup_ins | 569
> n_tup_upd | 12054
> n_tup_del | 476
> n_tup_hot_upd | 12041
> n_live_tup| 93
> n_dead_tup| 559
 
That probably matters a lot.
 
> analyze_count | 4922528128875102208
> autoanalyze_count | 7598807461784802080
> 
> (values in the last two columns seems bogus.
> i don't know if it's related or not.)
 
That seems unlikely to be related to this problem.  It sure does
look odd, though.  Maybe post that in a separate thread?
 
Thanks for all the additional info.  I'll keep digging.
 
-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] sepgsql contrib module

2011-02-14 Thread Alvaro Herrera
Excerpts from Kohei Kaigai's message of lun feb 14 13:47:58 -0300 2011:
> > We really need to get a buildfarm which is building with this.  To that
> > end, would you mind providing directions so someone else could set up a
> > buildfarm member to test it..?
> 
> It seems to me not difficult to describe a direction to build, install and
> run regression test.
> Do we have any Fedora 14 environment in the buildfarm?
> It is the most suitable distribution to set up sepgsql module, because the
> default installation already has selinux configurations.

It would be good to fix the Makefile problem that prevents the code to
build elsewhere.  There's another thread about a hardcoded path to a
system Makefile in contrib/sepgsql that causes that module to FTBFS.  Is
there a way to fix that?  I had a look some days ago and it didn't seem
like there was a way to query the system for the proper path to use.

-- 
Álvaro Herrera 
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_terminate_backend and pg_cancel_backend by not administrator user

2011-02-14 Thread Kevin Grittner
Torello Querci  wrote:
 
> I attach a path for this
 
It's too late in the release cycle to consider this for version 9.1.
Please add it to the open CommitFest for consideration for 9.2:
 
https://commitfest.postgresql.org/action/commitfest_view/open
 
-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] pika failing since the per-column collation patch

2011-02-14 Thread Rémi Zara

Le 12 févr. 2011 à 18:51, Peter Eisentraut a écrit :

> On lör, 2011-02-12 at 13:34 +0100, Rémi Zara wrote:
>> Since the per-column collation patch went in, pika (NetBSD 5.1/mips) started 
>> failing consistently with this diff:
>> 
>> *** 
>> /home/pgbuildfarm/workdir/HEAD/pgsql.15101/src/test/regress/expected/polymorphism.out
>> Sat Feb 12 02:16:07 2011
>> --- 
>> /home/pgbuildfarm/workdir/HEAD/pgsql.15101/src/test/regress/results/polymorphism.out
>>  Sat Feb 12 09:10:21 2011
>> ***
>> *** 624,630 
>> 
>>  -- such functions must protect themselves if varying element type isn't OK
>>  select max(histogram_bounds) from pg_stats;
>> ! ERROR:  cannot compare arrays of different element types
>>  -- test variadic polymorphic functions
>>  create function myleast(variadic anyarray) returns anyelement as $$
>>select min($1[i]) from generate_subscripts($1,1) g(i)
>> --- 624,630 
>> 
>>  -- such functions must protect themselves if varying element type isn't OK
>>  select max(histogram_bounds) from pg_stats;
>> ! ERROR:  locale operation to be invoked, but no collation was derived
>>  -- test variadic polymorphic functions
>>  create function myleast(variadic anyarray) returns anyelement as $$
>>select min($1[i]) from generate_subscripts($1,1) g(i)
>> 
>> Is there something I can do to help investigate this ?
> 
> It's only failing on this one machine, but there isn't anything
> platform-specific in this code, so I'd look for memory management faults
> on the code or a compiler problem.  Try with lower optimization for a
> start.
> 


Same failure with -O0 (and more shared memory).

Regards,

Rémi Zara
-- 
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] ALTER TYPE 2: skip already-provable no-work rewrites

2011-02-14 Thread Stephen Frost
Noah,

I'm even less familiar w/ this code than Robert, but figured I'd give a
shot at reviewing this anyway.  I definitely like avoiding table
rewrites if I can get away with it. :)

First question is- why do you use #ifdef USE_ASSERT_CHECKING ..?
assert_enabled exists and will work the way you expect regardless, no?
Strikes me as unlikely that the checks would be a real performance
problem..

In ATColumnChangeSetWorkLevel(), I'm really not a huge fan of using a
passed-in argument to move through a list with..  I'd suggest using a
local variable that is set from what's passed in.  I do see that's
inheirited, but still, you've pretty much redefined that entire
function anyway..

Also, I feel like that while(!tab->rewrite) really deserves more
explanation of what's happening than the function-level comment above
gives it.  I'd prefer to see a comment above the while() explaining
that we're moving through a list to see if there's any level at which
expr is something complicated or is referring to a domain which has
constraints on it (presuming that I've followed what's going on
correctly, that is..).  It also seems like it'd make more sense to me to
be a while() controlled by (IsA(expr, Var) && ((Var *) expr)->varattno
== varattno), since that's really the normal "stopping point".

These are all more stylistic issues than anything else.  Last, but not
least, I do worry about how this may impact contrib modules, external
projects, or user-added data types, such as PostGIS, hstore, and ip4r.
Could we/should we limit this to only PG data types that we 'know' are
binary compatible?  Is there any way or reason such external modules
could be fouled up by this?

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] SSI bug?

2011-02-14 Thread Kevin Grittner
Heikki Linnakangas  wrote:
 
> Looking at the prior/next version chaining, aside from the
> looping issue, isn't it broken by lock promotion too? There's a
> check in RemoveTargetIfNoLongerUsed() so that we don't release a
> lock target if its priorVersionOfRow is set, but what if the tuple
> lock is promoted to a page level lock first, and
> PredicateLockTupleRowVersionLink() is called only after that? Or
> can that not happen because of something else that I'm missing?
 
I had to ponder that a while.  Here's my thinking.
 
Predicate locks only matter when there is a write.  Predicate locks
on heap tuples only matter when there is an UPDATE or DELETE of a
locked tuple.  The problem these links are addressing is that an
intervening transaction might UPDATE the transaction between the
read of the tuple and a later UPDATE or DELETE.  We want the second
UPDATE to see that it conflicts with a read from before the first
UPDATE.  The first UPDATE creates the link from the "before" tuple
ID the "after" tuple ID at the target level.  What predicate locks
exist on the second target are irrelevant when it comes to seeing
the conflict between the second UPDATE (or DELETE) and the initial
read.  So I don't see where granularity promotion for locks on the
second target is a problem as long as the target itself doesn't get
deleted because of the link to the prior version of the tuple. 
 
Promotion of the lock granularity on the prior tuple is where we
have problems. If the two tuple versions are in separate pages then
the second UPDATE could miss the conflict.  My first thought was to
fix that by requiring promotion of a predicate lock on a tuple to
jump straight to the relation level if nextVersionOfRow is set for
the lock target and it points to a tuple in a different page.  But
that doesn't cover a situation where we have a heap tuple predicate
lock which gets promoted to page granularity before the tuple is
updated.  To handle that we would need to say that an UPDATE to a
tuple on a page which is predicate locked by the transaction would
need to be promoted to relation granularity if the new version of
the tuple wasn't on the same page as the old version.
 
That's all doable without too much trouble, but more than I'm
likely to get done today.  It would be good if someone can confirm
my thinking on this first, too.
 
That said, the above is about eliminating false negatives from some
corner cases which escaped notice until now.  I don't think the
changes described above will do anything to prevent the problems
reported by YAMAMOTO Takashi.  Unless I'm missing something, it
sounds like tuple IDs are being changed or reused while predicate
locks are held on the tuples.  That's probably not going to be
overwhelmingly hard to fix if we can identify how that can happen. 
I tried to cover HOT issues, but it seems likely I missed something.
 :-(  I will be looking at it.
 
-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] sepgsql contrib module

2011-02-14 Thread Kohei Kaigai
> We really need to get a buildfarm which is building with this.  To that
> end, would you mind providing directions so someone else could set up a
> buildfarm member to test it..?

It seems to me not difficult to describe a direction to build, install and
run regression test.
Do we have any Fedora 14 environment in the buildfarm?
It is the most suitable distribution to set up sepgsql module, because the
default installation already has selinux configurations.

Thanks,

> -Original Message-
> From: Stephen Frost [mailto:sfr...@snowman.net]
> Sent: 14 February 2011 16:29
> To: Kohei Kaigai
> Cc: Robert Haas; KaiGai Kohei; PgHacker
> Subject: Re: [HACKERS] sepgsql contrib module
> 
> KaiGai,
> 
> * Kohei Kaigai (kohei.kai...@eu.nec.com) wrote:
> > > It would be good to have some buildfarm coverage of this code.  Can
> > > we find anyone brave enough to set up a buildfarm critter using
> > > --with-selinux?
> > >
> > Although I don't have an account on the buildfarm, I'll set up an
> > environment for daily build with --with-selinux.
> > Because it needs kernel with selinux, libselinux and security policy,
> > I think it is good idea to set up a virtual machine for the purpose.
> 
> We really need to get a buildfarm which is building with this.  To that
> end, would you mind providing directions so someone else could set up a
> buildfarm member to test it..?
> 
>   Thanks,
> 
>   Stephen

-- 
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] Extensions vs PGXS' MODULE_PATHNAME handling

2011-02-14 Thread Dimitri Fontaine
Tom Lane  writes:
> I don't really think that's a behavior we want to encourage.  ISTM the
> cases that are going to be trouble are paths you failed to think about,
> and therefore what you want to do is look over the whole output set to
> see if there are any surprising paths...

Mmm, yes.  Ok.

-- 
Dimitri Fontaine
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] Debian readline/libedit breakage

2011-02-14 Thread Stephen Frost
* Florian Weimer (fwei...@bfk.de) wrote:
> Source?  I've only seen GPLed copies.  We wouldn't face this issue
> with LGPL code.

Yeah, Greg corrected me on this already.

So we have both FSF folks *and* OpenSSL people being foolish.

Sigh.

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Debian readline/libedit breakage

2011-02-14 Thread Florian Weimer
* Stephen Frost:

> * Greg Smith (g...@2ndquadrant.com) wrote:
>> -GNU libreadine is certainly never going to add an OpenSSL exemption
>
> I really wish they would, that's just them being obnoxious- it's already
> LGPL, after all..

Source?  I've only seen GPLed copies.  We wouldn't face this issue
with LGPL code.

-- 
Florian Weimer
BFK edv-consulting GmbH   http://www.bfk.de/
Kriegsstraße 100  tel: +49-721-96201-1
D-76133 Karlsruhe fax: +49-721-96201-99

-- 
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] Extensions vs PGXS' MODULE_PATHNAME handling

2011-02-14 Thread Tom Lane
Dimitri Fontaine  writes:
> Tom Lane  writes:
>>> [ about omitting rows for which there is no update path ]

>> Yeah, possibly.  I'm a bit concerned about cases where the author meant
>> to provide an update path and forgot: it would be fairly obvious in this
>> representation but maybe you could keep making the same oversight if the
>> row's not there at all.  Also, it's easy enough to write "where path is
>> not null" if you want to filter the rows that way.

> I would expect the author to check with something like
>   WHERE installed = '1.0' and available = '1.2'

I don't really think that's a behavior we want to encourage.  ISTM the
cases that are going to be trouble are paths you failed to think about,
and therefore what you want to do is look over the whole output set to
see if there are any surprising paths...

regards, tom lane

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


Re: [HACKERS] why two dashes in extension load files

2011-02-14 Thread David E. Wheeler
On Feb 14, 2011, at 9:14 AM, Tom Lane wrote:

> Commas do not seem like an improvement to me at all --- they are widely
> used as list separators.

Fair enough.

> I guess the real question is what's Peter's concrete objection to the
> double-dash method?

Hey, I know, a double-dash between the extension name and first version, and -> 
between the first and second version:

foo--1.2->1.4.sql

;-P

David


-- 
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] why two dashes in extension load files

2011-02-14 Thread Tom Lane
"David E. Wheeler"  writes:
> On Feb 14, 2011, at 8:54 AM, Tom Lane wrote:
>>> I'm not convinced.  There was nothing in that discussion why any
>>> particular character would have to be allowed in a version number.

>> Well, there's already a counterexample in the current contrib stuff:
>> uuid-ossp.  We could rename that to uuid_ossp of course, but it's
>> not clear to me that there's consensus for forbidding dashes here.

> I'd be fine if commas were used instead.

Commas do not seem like an improvement to me at all --- they are widely
used as list separators.

I guess the real question is what's Peter's concrete objection to the
double-dash method?

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_ctl failover Re: Latches, signals, and waiting

2011-02-14 Thread Stephen Frost
Fujii,

* Fujii Masao (masao.fu...@gmail.com) wrote:
> Yeah, I rebased the patch to the current git master and attached it.

Reviewing this, I just had a couple of comments and questions.  Overall,
I think it looks good and hence will be marking it 'Ready for
Committer'.

 * You removed trigger_file from the list in
   doc/src/sgml/high-availability.sgml and I'm not sure I agree with
   that.  It's still perfectly valid and could be used by someone
   instead of pg_ctl promote.  I'd recommend two things:
   - Adding comments into this recovery.conf snippet
   - Adding a comment indicationg that trigger_file is only needed if
 you're not using pg_ctl promote.

 * I'm not happy that pg_ctl.c doesn't #include something which defines
   all the file names which are used, couldn't we use a header which
   makes sense and is pulled in by pg_ctl.c and xlog.c to #define all of
   these?  Still, that's not really the fault of this patch.

 * I'm a bit worried that there's just only so many USR signals that we
   can send and it looks like we're burning another one here.  Should we
   be considering a better way to do this?

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] why two dashes in extension load files

2011-02-14 Thread David E. Wheeler
On Feb 14, 2011, at 8:54 AM, Tom Lane wrote:

>> I'm not convinced.  There was nothing in that discussion why any
>> particular character would have to be allowed in a version number.
> 
> Well, there's already a counterexample in the current contrib stuff:
> uuid-ossp.  We could rename that to uuid_ossp of course, but it's
> not clear to me that there's consensus for forbidding dashes here.

I'd be fine if commas were used instead.

Best,

David

-- 
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] Extensions vs PGXS' MODULE_PATHNAME handling

2011-02-14 Thread Dimitri Fontaine
Tom Lane  writes:
> I intentionally left out columns that seem like extension implementation
> details rather than things users of the extension need to know.  Hence,
> no directory, encoding, or module_pathname.  There's no fundamental
> reason not to include these, I guess, although maybe there could be some
> security objection to showing directory.  But do we need 'em?

I share your view on the directory and module_pathname, but though that
maybe encoding could be the source of subtle errors and that users would
be happy to know what PostgreSQL is using.  But well, that's not holding
enough water now that I think some more about it.

> I was thinking the other way --- you can split it with
> regexp_split_to_array (or regexp_split_to_table) if you want to, but
> having a compact human-readable form is probably the most important
> case.  It's not a big deal either way though.  Anyone else want to
> vote?

I'm not set one way or the other and won't share another opinion on
that :)

> Sorry, I only meant that in this example I put the rows coming from
> single scripts first.  I didn't mean to suggest that the function would
> guarantee any particular output ordering.

Ok.

> Yeah, possibly.  I'm a bit concerned about cases where the author meant
> to provide an update path and forgot: it would be fairly obvious in this
> representation but maybe you could keep making the same oversight if the
> row's not there at all.  Also, it's easy enough to write "where path is
> not null" if you want to filter the rows that way.

I would expect the author to check with something like

  WHERE installed = '1.0' and available = '1.2'

But again, the preference here is about either "cluttering" the default
output more than necessary or having to type a WHERE clause to double
check your setup.  No strong opinion here, just a preference…

Regards,
-- 
Dimitri Fontaine
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] why two dashes in extension load files

2011-02-14 Thread Marko Kreen
On Mon, Feb 14, 2011 at 6:49 PM, Peter Eisentraut  wrote:
> On mån, 2011-02-14 at 10:13 -0500, Tom Lane wrote:
>> Peter Eisentraut  writes:
>> > Why do the extension load files need two dashes, like xml2--1.0.sql?
>> > Why isn't one enough?
>>
>> Because we'd have to forbid dashes in extension name and version
>> strings.  This was judged to be a less annoying solution.  See
>> yesterday's discussion.
>
> I'm not convinced.  There was nothing in that discussion why any
> particular character would have to be allowed in a version number.  I'd
> propose that dashes should be prohibited in version names anyway,
> because downstream packaging will want to use that to separate packaging
> revisions.  It might be better to discuss that explicitly rather than
> hiding it in some thread of another title.

I think the question is more - what do we disallow in package name?

Eg. Debian disallows '_' and uses it as magic separator.  It works,
but it not as obvious as '-' vs '--', and '--' allows both '_' and '-' in
package name.  Unlikely anyone will want '--' in package name.

I would vote for current '--' and keeping version name simple,
no '_' and '-' there.  As we want to do some logic on that.

-- 
marko

-- 
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] why two dashes in extension load files

2011-02-14 Thread Tom Lane
Peter Eisentraut  writes:
> On mån, 2011-02-14 at 10:13 -0500, Tom Lane wrote:
>> Peter Eisentraut  writes:
>>> Why do the extension load files need two dashes, like xml2--1.0.sql?
>>> Why isn't one enough?

>> Because we'd have to forbid dashes in extension name and version
>> strings.  This was judged to be a less annoying solution.  See
>> yesterday's discussion.

> I'm not convinced.  There was nothing in that discussion why any
> particular character would have to be allowed in a version number.

Well, there's already a counterexample in the current contrib stuff:
uuid-ossp.  We could rename that to uuid_ossp of course, but it's
not clear to me that there's consensus for forbidding dashes here.

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] Extensions vs PGXS' MODULE_PATHNAME handling

2011-02-14 Thread Tom Lane
Dimitri Fontaine  writes:
> Tom Lane  writes:
>> and pg_available_extension_versions that produces a row per install
>> script, with columns
>> 
>> name
>> version  ((name, version) is primary key)
>> comment
>> requires
>> relocatable
>> schema
>> 
>> where the last four columns can vary across versions due to secondary
>> control files.

> I like this primary key because that's also the one for debian stable
> distributions :)  Joking apart, aren't we missing the encoding somewhere?

I intentionally left out columns that seem like extension implementation
details rather than things users of the extension need to know.  Hence,
no directory, encoding, or module_pathname.  There's no fundamental
reason not to include these, I guess, although maybe there could be some
security objection to showing directory.  But do we need 'em?

>> The output might look like this:
>> 
>> 1.0  1.1 1.0--1.1
>> 1.1  1.2 1.1--1.2
>> unpackaged   1.0 unpackaged--1.0
>> 1.0  1.2 1.0--1.1--1.2
>> 1.0  unpackaged
>> 1.1  1.0
>> 1.1  unpackaged
>> 1.2  1.1
>> 1.2  1.0
>> 1.2  unpackaged
>> unpackaged   1.1 unpackaged--1.0--1.1
>> unpackaged   1.2 unpackaged--1.0--1.1--1.2

> What about having this chain column be an array of version strings?  If
> you want to see it this way, use array_to_string(path, '--')…

I was thinking the other way --- you can split it with
regexp_split_to_array (or regexp_split_to_table) if you want to, but
having a compact human-readable form is probably the most important
case.  It's not a big deal either way though.  Anyone else want to
vote?

>> where the first three rows correspond to available update scripts and
>> the rest are synthesized.

> The ordering is not clearly apparent, but I don't think it matters.

Sorry, I only meant that in this example I put the rows coming from
single scripts first.  I didn't mean to suggest that the function would
guarantee any particular output ordering.

>> (Looking at this, it looks like it could get pretty bulky pretty
>> quickly.  Maybe we should eliminate all rows in which the path would be
>> NULL?  Or just eliminate rows in which the target doesn't have an
>> install script, which would remove the three rows with target =
>> unpackaged in the above example?)

> Removing NULL path rows seems the best option to me.

Yeah, possibly.  I'm a bit concerned about cases where the author meant
to provide an update path and forgot: it would be fairly obvious in this
representation but maybe you could keep making the same oversight if the
row's not there at all.  Also, it's easy enough to write "where path is
not null" if you want to filter the rows that way.

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] CommitFest 2011-01 as of 2011-02-04

2011-02-14 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote:
> Here's where I think we are with this CommitFest.

  Subject: Re: [HACKERS] CommitFest 2011-01 as of 2011-02-04

I'm gonna go out on a limb and hope you meant '2011-02-14' there. :)

> So there are two basic difficulties with wrapping the CommitFest up.

I have to say that I've always been a bit suprised by the idea that the
CommitFest is intended to be done and all patches *committed* at the end
of the month.  It's been working really rather well, which is due in
great part to the excellent CF managers (thanks again for being that,
again).  That said, we have quite a few non-committer reviewers who
provide good feedback and move the patch back to 'waiting for author'
and that whole process takes a while.

Perhaps a thought for next time would be to offset things a bit.  eg:

CF 2011-03 (or whatever):
2011-02-14: Patches should all be submitted
2011-02-14: Reviewers start
2011-03-01: Committers start w/ 'Ready for Committer' patches
2011-03-14: Patches not marked 'Ready for Committer' get bounced
2011-03-31: All patches committed

I'm not against the 'waiting on author' approach, but I do feel like if
we're going to continue to have it, we need to spread it out a bit more.
I do think this would place more work on the CF manager, unfortunately,
but I'd hope that they would primairly be focused on managing the
reviews and not be as busy during the last 2 weeks.  Maybe one day I'll
be brave enough to offer to manage one and see. :)

Thanks again, Robert, you've done an excellent job managing the CF.

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] why two dashes in extension load files

2011-02-14 Thread Peter Eisentraut
On mån, 2011-02-14 at 10:13 -0500, Tom Lane wrote:
> Peter Eisentraut  writes:
> > Why do the extension load files need two dashes, like xml2--1.0.sql?
> > Why isn't one enough?
> 
> Because we'd have to forbid dashes in extension name and version
> strings.  This was judged to be a less annoying solution.  See
> yesterday's discussion.

I'm not convinced.  There was nothing in that discussion why any
particular character would have to be allowed in a version number.  I'd
propose that dashes should be prohibited in version names anyway,
because downstream packaging will want to use that to separate packaging
revisions.  It might be better to discuss that explicitly rather than
hiding it in some thread of another title.

Other comments?



-- 
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] using a lot of maintenance_work_mem

2011-02-14 Thread Kevin Grittner
Frederik Ramm  wrote:
 
> I am (ab)using a PostgreSQL database (with PostGIS extension) in
> a large data processing job - each day, I load several GB of data,
> run a lot of analyses on it, and then throw everything away again.
> Loading, running, and dumping the results takes about 18 hours
> every day.
> 
> The job involves a lot of index building and sorting, and is run
> on a 64-bit machine with 96 GB of RAM.
> 
> Naturally I would like the system to use as much RAM as possible
> before resorting to disk-based operations, but no amount of 
> maintenance_work_mem setting seems to make it do my bidding.
 
If you can tolerate some risk that for a given day you might fail to
generate the analysis, or you might need to push the schedule back
to get it, you could increase performance by compromising
recoverability.  You seem to be willing to consider such risk based
on your mention of a RAM disk.
 
 - If a single session can be maintained for loading and using the
data, you might be able to use temporary tables and a large
temp_buffers size.  Of course, when the connection closes, the
tables are gone.
 
 - You could turn off fsync and full_page_writes, but on a crash
your database might be corrupted beyond usability.
 
 - You could turn off synchronous_commit.
 
 - Make sure you have archiving turned off.
 
 - If you are not already doing so, load the data into each table
within the same database transaction which does CREATE TABLE or
TRUNCATE TABLE.
 
Other than the possibility that the temp table might keep things in
RAM, these suggestions don't directly address your question, but I
thought they might be helpful.
 
-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] Extensions vs PGXS' MODULE_PATHNAME handling

2011-02-14 Thread Dimitri Fontaine
Tom Lane  writes:
> Thinking about this some more ... it seems like we now need two separate
> views, because there is some information that could change per-version,
> and some that really only makes sense at the per-extension level.

Makes sense.

> For instance, we could have pg_available_extensions that produces a row
> per primary control file, with columns
>
>   name(view's effective primary key)
>   default_version
>   installed_version   (NULL if not installed)
>   comment (if one is present in primary control file)

Check.

> and pg_available_extension_versions that produces a row per install
> script, with columns
>
>   name
>   version ((name, version) is primary key)
>   comment
>   requires
>   relocatable
>   schema
>
> where the last four columns can vary across versions due to secondary
> control files.

I like this primary key because that's also the one for debian stable
distributions :)  Joking apart, aren't we missing the encoding somewhere?

> Or we could combine these into just one view with pkey (name, version),
> but then the default_version and installed_version columns would be the
> same across all rows with the same extension name, which seems confusing
> and unnormalized.

Let's go with two views.  Once we have that it's easy enough to LEFT
JOIN if we want a summarized view.  Maybe we could even revive \dX.
Without pattern it would show the short form (pg_available_extension)
and given a pattern pg_available_extension_versions.

> I suggest instead that we invent a SRF, say
> pg_extension_update_paths(extension_name text) returns setof record,
> that returns a row for each pair of distinct version names found in
> the extension's install and update scripts, with columns

Agreed.

>   source  version name
>   target  other version name
>   pathupdate path from source to target, or NULL if none
>
> The output might look like this:
>
>   1.0 1.1 1.0--1.1
>   1.1 1.2 1.1--1.2
>   unpackaged  1.0 unpackaged--1.0
>   1.0 1.2 1.0--1.1--1.2
>   1.0 unpackaged
>   1.1 1.0
>   1.1 unpackaged
>   1.2 1.1
>   1.2 1.0
>   1.2 unpackaged
>   unpackaged  1.1 unpackaged--1.0--1.1
>   unpackaged  1.2 unpackaged--1.0--1.1--1.2

What about having this chain column be an array of version strings?  If
you want to see it this way, use array_to_string(path, '--')…

> where the first three rows correspond to available update scripts and
> the rest are synthesized.

The ordering is not clearly apparent, but I don't think it matters.

> (Looking at this, it looks like it could get pretty bulky pretty
> quickly.  Maybe we should eliminate all rows in which the path would be
> NULL?  Or just eliminate rows in which the target doesn't have an
> install script, which would remove the three rows with target =
> unpackaged in the above example?)

Removing NULL path rows seems the best option to me.

Regards,
-- 
Dimitri Fontaine
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] "Extension" versus "module"

2011-02-14 Thread Tom Lane
Dimitri Fontaine  writes:
> Another concern has to do with PLs.  We said that with the dependency
> mechanism it would be good to have PLs be EXTENSIONs.  But those are
> core provided extensions, one of them installed by default.

> If we make PLs extensions, we might also want to have CREATE LANGUAGE
> either ERROR out or silently do the CREATE EXTENSION instead, meaning
> that CREATE LANGUAGE behavior would depend on creating_extension.
> Sounds like a crock but ensures compatibility.

Yeah.  I was sort of wondering whether we could get rid of pg_pltemplate
altogether, and instead rely on the extension mechanism to package up
the correct parameters for installing a language.  However, one thing
that'd have to be solved before going very far in this direction is the
question of allowing CREATE EXTENSION to non-superusers.  We'd at least
need to be able to duplicate the current functionality of allowing
CREATE LANGUAGE to database owners (with an override available to the
DBA).

This seems like a matter for a separate thread though, and not on
pgsql-docs.

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] sepgsql contrib module

2011-02-14 Thread Stephen Frost
KaiGai,

* Kohei Kaigai (kohei.kai...@eu.nec.com) wrote:
> > It would be good to have some buildfarm coverage of this code.  Can we
> > find anyone brave enough to set up a buildfarm critter using
> > --with-selinux?
> >
> Although I don't have an account on the buildfarm, I'll set up an environment
> for daily build with --with-selinux.
> Because it needs kernel with selinux, libselinux and security policy, I think
> it is good idea to set up a virtual machine for the purpose.

We really need to get a buildfarm which is building with this.  To that
end, would you mind providing directions so someone else could set up a
buildfarm member to test it..?

Thanks,

Stephen


signature.asc
Description: Digital signature


[HACKERS] CommitFest 2011-01 as of 2011-02-04

2011-02-14 Thread Robert Haas
Here's where I think we are with this CommitFest.

We have committed 45 patches and returned with feedback or rejected
23.  There are 30 remaining patches, every single one of which has
been reviewed.  20 of those are marked Ready for Committer; 5 are
marked Waiting on Author; 5 are marked Needs Review.  However, again,
even the ones that are marked as Needs Review have in fact been
reviewed multiple times, in many cases by multiple people.  Thanks to
all who have contributed to the reviewing effort, especially Stephen
Frost, Hitoshi Hirada, Alex Hunsaker, Noah Misch, and Itagaki
Takahiro.

I respectfully submit that every patch which is not yet Ready for
Committer is in that state not because of any neglect on the part of
anyone in the community, but because it's got problems.  It isn't
done; it turned out to be more complicated than anticipated; it wasn't
updated in a timely fashion; and/or we had difficulty reaching
agreement on the best way forward (and still haven't).  Most of these
patches should probably be deferred to 9.2.

So there are two basic difficulties with wrapping the CommitFest up.
One is that the 20 patches which are marked Ready for Committer really
deserve to be looked at by a committer, and hopefully committed.
Unfortunately, my ability to help in this area will be somewhat
limited, because at least half of the remaining patches are in areas
that I know nothing about (e.g. 7 PL/python patches).

The second problem is that the patches that are in serious trouble
including synchronous replication and SQL/MED, which are key features
I know we're all hoping to have for 9.2.   However, we have to face
the fact that getting these features in may involve quite a bit of
delay.  With respect to SQL/MED, Heikki tells me that he is working on
the core patch, and I am hopeful that will be committed soon.
However, file_fdw is in pretty serious trouble because (1) the copy
API patch that it depends on still isn't committed and (2) it's going
to be utterly broken if we don't do something about the
client_encoding vs. file_encoding problem; there was a patch to do
that in this CF, but we gave up on it.  And postgresql_fdw was hacked
up by Heikki but he didn't seem to have much confidence in it and no
one else has reviewed his hacked-up version.  With respect to
synchronous replication, Dan Farina and I have extracted some of the
less-intrusive bits of that patch.  One of those changes (standby
replies) is committed, though it seems to need a bit of fixup, and the
other two are awaiting review.  Simon also reported that he is working
on this, but I'm not certain of the specifics.  Based on the looking
at that patch that I did so far, it certainly needs cleanup, and there
may be some more serious architectural issues that need to be
addressed also.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] "Extension" versus "module"

2011-02-14 Thread Dimitri Fontaine
Tom Lane  writes:
> Hmm ... but what of contrib "modules" that don't build shared libraries
> at all --- pgbench and pg_upgrade for example?
>
> I think "shared library" is a perfectly fine term for that kind of
> object, and we don't need an alias for it anyway.

In my view, if there's no script, that is no SQL object to install in a
database, then it's not an extension.  I would think that we keep the
directory named contrib for them.  Then, an extension can be implemented
partly with a module, coded in C, installed as a shared library.

Another concern has to do with PLs.  We said that with the dependency
mechanism it would be good to have PLs be EXTENSIONs.  But those are
core provided extensions, one of them installed by default.

If we make PLs extensions, we might also want to have CREATE LANGUAGE
either ERROR out or silently do the CREATE EXTENSION instead, meaning
that CREATE LANGUAGE behavior would depend on creating_extension.
Sounds like a crock but ensures compatibility.

Regards,
-- 
Dimitri Fontaine
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] Range Types: empty ranges

2011-02-14 Thread Kevin Grittner
Jan Wieck  wrote:
 
> Does ['15:15:00','15:15:00') make any more sense? Doesn't this 
> essentially mean
> 
>  >= '15:15:00' && < '15:15:00'
> 
> which again doesn't include a single point on the time line?
 
It defines a position in time with zero duration.
 
Some of the graphics programming I've done in the past was based on
a system where, at the pixel level, the coordinates referred to the
boundaries *between* the pixels.  If you were to have a number of
horizontal bars, for example, the size of which you would adjust to
represent fluctuating data, you might have an x coordinate of 20 for
the left edge, and [20,30) would paint 10 pixels.  I guess you
*could* destroy and recreate the object when the number dropped to
zero and came off it again; but the concept of [20,20) to draw zero
pixels but maintain the positional anchor can be convenient.  I see
parallel concepts for some data domains in a database.
 
-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] Extensions vs PGXS' MODULE_PATHNAME handling

2011-02-14 Thread Tom Lane
Dimitri Fontaine  writes:
> Tom Lane  writes:
>> Also, I've been looking at the pg_available_extensions issue a bit.
>> I don't yet have a proposal for exactly how we ought to redefine it,
>> but I did notice that the existing code is terribly confused by
>> secondary control files: it doesn't realize that they're not primary
>> control files, so you get e.g. hstore and hstore-1.0 as separate
>> listings.

> I'd think that's it's a good idea if dealt with "correctly" because now
> that ALTER EXTENSION UPDATE can deal with more than one target VERSION
> I expect the view to show each available update here.

Thinking about this some more ... it seems like we now need two separate
views, because there is some information that could change per-version,
and some that really only makes sense at the per-extension level.

For instance, we could have pg_available_extensions that produces a row
per primary control file, with columns

name(view's effective primary key)
default_version
installed_version   (NULL if not installed)
comment (if one is present in primary control file)

and pg_available_extension_versions that produces a row per install
script, with columns

name
version ((name, version) is primary key)
comment
requires
relocatable
schema

where the last four columns can vary across versions due to secondary
control files.

Or we could combine these into just one view with pkey (name, version),
but then the default_version and installed_version columns would be the
same across all rows with the same extension name, which seems confusing
and unnormalized.

> If possible adding the "update chain sequence" information as computed
> in the code would be great.  Because we can't ask people to figure that
> out all by themselves, the best way to check your upgrading setup is
> fine would be to run SELECT * FROM pg_available_extensions; and read the
> result.

I think this is probably a good thing to provide but it shouldn't go in
either of the above views, on two grounds: (1) it's going to be
relatively expensive to compute, and most people won't need it; (2)
the views could only sensibly cover paths from current version to listed
version, which isn't good enough.  What an extension author actually
wants to know is "have I introduced any undesirable update paths
anywhere?"  I suggest instead that we invent a SRF, say
pg_extension_update_paths(extension_name text) returns setof record,
that returns a row for each pair of distinct version names found in
the extension's install and update scripts, with columns

source  version name
target  other version name
pathupdate path from source to target, or NULL if none

The output might look like this:

1.0 1.1 1.0--1.1
1.1 1.2 1.1--1.2
unpackaged  1.0 unpackaged--1.0
1.0 1.2 1.0--1.1--1.2
1.0 unpackaged
1.1 1.0
1.1 unpackaged
1.2 1.1
1.2 1.0
1.2 unpackaged
unpackaged  1.1 unpackaged--1.0--1.1
unpackaged  1.2 unpackaged--1.0--1.1--1.2

where the first three rows correspond to available update scripts and
the rest are synthesized.

(Looking at this, it looks like it could get pretty bulky pretty
quickly.  Maybe we should eliminate all rows in which the path would be
NULL?  Or just eliminate rows in which the target doesn't have an
install script, which would remove the three rows with target =
unpackaged in the above example?)

Thoughts?

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] Range Types: empty ranges

2011-02-14 Thread Jan Wieck

On 2/11/2011 1:50 PM, Kevin Grittner wrote:

Josh Berkus  wrote:


 if I, in one of my applications, accidentally defined something
 as having the range '('15:15:00','15:15:00')', I would *want* the
 database to through an error and not accept it.


I can agree with that, but I think that range '[15:15:00,15:15:00)'
should be valid as a zero-length range between, for example,
'[15:00:00,15:15:00)' and '[15:15:00,15:30:00)'.


Does ['15:15:00','15:15:00') make any more sense? Doesn't this 
essentially mean


>= '15:15:00' && < '15:15:00'

which again doesn't include a single point on the time line?


Jan

--
Anyone who trades liberty for security deserves neither
liberty nor security. -- Benjamin Franklin

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

2011-02-14 Thread Peter Rosin
Den 2011-02-12 11:10 skrev Ralf Wildenhues:
> Hello, and sorry for the delay,
> 
> * Peter Rosin wrote on Sat, Jan 29, 2011 at 02:26:24PM CET:
>> Or is plain 'ar' used somewhere instead of 'x86_64-w64-mingw32-ar'?
> 
> Automake outputs 'AR = ar' in Makefile.in for rules creating old
> libraries iff neither AC_PROG_LIBTOOL nor another method to define
> AR correctly is used in configure.ac.
> 
> So this issue concerns packages using Automake but not using Libtool.
> 
> I figured with AM_PROG_AR eventually being needed anyway that would fix
> this in one go ...
> 
> A good workaround, as already mentioned, is to use this in configure.ac:
>   AC_CHECK_TOOL([AR], [ar], [false])

I just cannot understand why the workaround isn't always working in
this case.

There was a log posted with this in it
(in http://archives.postgresql.org/pgsql-hackers/2011-01/msg02697.php):

...
configure:5962: checking for x86_64-w64-mingw32-ranlib
configure:5978: found /mingw/bin/x86_64-w64-mingw32-ranlib
configure:5989: result: x86_64-w64-mingw32-ranlib
configure:6055: checking for x86_64-w64-mingw32-strip
configure:6071: found /mingw/bin/x86_64-w64-mingw32-strip
configure:6082: result: x86_64-w64-mingw32-strip
configure:6145: checking whether it is possible to strip libraries
configure:6150: result: yes
configure:6164: checking for x86_64-w64-mingw32-ar
configure:6180: found /mingw/bin/x86_64-w64-mingw32-ar
configure:6191: result: x86_64-w64-mingw32-ar
configure:6257: checking for x86_64-w64-mingw32-dlltool
configure:6273: found /mingw/bin/x86_64-w64-mingw32-dlltool
configure:6284: result: x86_64-w64-mingw32-dlltool
configure:6349: checking for x86_64-w64-mingw32-dllwrap
configure:6365: found /mingw/bin/x86_64-w64-mingw32-dllwrap
configure:6376: result: x86_64-w64-mingw32-dllwrap
configure:6441: checking for x86_64-w64-mingw32-windres
configure:6457: found /mingw/bin/x86_64-w64-mingw32-windres
configure:6468: result: x86_64-w64-mingw32-windres
...

Which seem to match this snippet from configure.in:

...
AC_PROG_RANLIB
PGAC_CHECK_STRIP
AC_CHECK_TOOL(AR, ar, ar)
if test "$PORTNAME" = "win32"; then
  AC_CHECK_TOOL(DLLTOOL, dlltool, dlltool)
  AC_CHECK_TOOL(DLLWRAP, dllwrap, dllwrap)
  AC_CHECK_TOOL(WINDRES, windres, windres)
fi
...

Sure, AC_CHECK_TOOL has under-quoted arguments and the last argument is
'ar' instead of 'false'.  But that shouldn't really matter here.  (Or
does it?)

Still, elsewhere in the thread there's a report about the wrong ar being
used.
(in http://archives.postgresql.org/pgsql-hackers/2011-01/msg02713.php)

Sure, the configure log and the "wrong ar"-report are not from the same
person, but the configure script should be the same for everybody (git
log hints that this part of configure has been stable for a couple of
years).

It just doesn't add up.

Cheers,
Peter

-- 
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] sepgsql contrib module

2011-02-14 Thread Kohei Kaigai
Sorry for the late responding, because of my relocation.

> It would be good to have some buildfarm coverage of this code.  Can we
> find anyone brave enough to set up a buildfarm critter using
> --with-selinux?
>
Although I don't have an account on the buildfarm, I'll set up an environment
for daily build with --with-selinux.
Because it needs kernel with selinux, libselinux and security policy, I think
it is good idea to set up a virtual machine for the purpose.

Thanks,

> -Original Message-
> From: Robert Haas [mailto:robertmh...@gmail.com]
> Sent: 03 February 2011 05:27
> To: KaiGai Kohei
> Cc: KaiGai Kohei; PgHacker
> Subject: Re: [HACKERS] sepgsql contrib module
> 
> On Wed, Feb 2, 2011 at 11:43 PM, Robert Haas  wrote:
> > Committed.
> 
> I did some more polishing of the documentation as well.
> 
> It would be good to have some buildfarm coverage of this code.  Can we
> find anyone brave enough to set up a buildfarm critter using
> --with-selinux?
> 
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company

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


Re: [HACKERS] "Extension" versus "module"

2011-02-14 Thread Tom Lane
Dimitri Fontaine  writes:
> Tom Lane  writes:
>> Appendix F (contrib.sgml and its subsidiary files) is pretty consistent
>> about using "module" to refer to a contrib, uh, module.

> I'm now thinking in those terms: the module is the shared object library
> that the backend needs to dlopen().  The extension is the SQL level
> object that wraps all its components.

Hmm ... but what of contrib "modules" that don't build shared libraries
at all --- pgbench and pg_upgrade for example?

I think "shared library" is a perfectly fine term for that kind of
object, and we don't need an alias for it anyway.

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] Scheduled maintenance affecting gitmaster

2011-02-14 Thread Magnus Hagander
On Mon, Feb 14, 2011 at 10:39, Stefan Kaltenbrunner
 wrote:
> On 02/14/2011 10:09 AM, Magnus Hagander wrote:
>> On Mon, Feb 14, 2011 at 07:13, Stefan Kaltenbrunner
>>  wrote:
>>> On 02/14/2011 01:27 AM, Tom Lane wrote:

 Magnus Hagander  writes:
>
> Unfortunately, one of the worst-case scenarios appears to have
> happened - a machine did not come back up after a reboot.
> ...
> We'll get back to you with more information as soon as we have it.

 I didn't see any followup to this?
>>>
>>> yeah - the hosting company managed to reboot the box for us which brought it
>>> back to life in the middle of the night (with both magnus and me asleep).
>>
>> Indeed. But the good news is that once it came back up, the VM with
>> the git server started ok :-)
>>
>>
 gitmaster seems to be responding as of now, is it safe to push?
>>>
>>> yes it is - however we will need to schedule another maintenance window soon
>>> to finish the stuff we actually wanted to do.
>>
>> So, after some discussion with Stefan, we (well, I guess I) decided we
>> should just go ahead and declare the maintenance window not closed
>> yet, and finish off the upgrade right now :-) Given that the majority
>> of our commits don't happen now, we'll hopefully have it done by the
>> time the US folks wake up again.
>>
>> So, maintenance window again, starting now, and we'll let you know as
>> soon as we're done. And we're definitely hoping for the machine to
>> come back up properly this time :-)
>
> and it did not... We are trying to figure out what the actual problem
> here really is because it seems to boot just fine when powercycled just
> not with a software initiated reboot.
> We will notify once we have more information...

Status update on this - Stefan is currently working with the
datacenter people on getting this fixed (they are now available
on-site), since we are now having an actual issue with the machine
(GRUB failure on boot) rather than just a failure to shut down.

-- 
 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] why two dashes in extension load files

2011-02-14 Thread Tom Lane
Peter Eisentraut  writes:
> Why do the extension load files need two dashes, like xml2--1.0.sql?
> Why isn't one enough?

Because we'd have to forbid dashes in extension name and version
strings.  This was judged to be a less annoying solution.  See
yesterday's discussion.

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] using a lot of maintenance_work_mem

2011-02-14 Thread Tom Lane
Frederik Ramm  writes:
> Now I assume that there are reasons that you're doing this. memutils.h 
> has the (for me) cryptic comment about MaxAllocSize: "XXX This is 
> deliberately chosen to correspond to the limiting size of varlena 
> objects under TOAST. See VARATT_MASK_SIZE in postgres.h.", but 
> VARATT_MASK_SIZE has zero other occurences in the source code.

Hm, I guess that comment needs updated then.

> If I were to either (a) increase MaxAllocSize to, say, 48 GB instead of 
> 1 GB, or (b) hack tuplesort.c to ignore MaxAllocSize, just for my local 
> setup - would that likely be viable in my situation, or would I break 
> countless things?

You would break countless things.  It might be okay anyway in a trusted
environment, ie, one without users trying to crash the system, but there
are a lot of security-critical implications of that test.

If we were actually trying to support such large allocations,
what I'd be inclined to do is introduce a separate call along the lines
of MemoryContextAllocLarge() that lacks the safety check.  But before
expending time on that, I'd want to see some evidence that it's actually
helpful for production situations.  I'm a bit dubious that you're going
to gain much here.

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] using a lot of maintenance_work_mem

2011-02-14 Thread Frederik Ramm

Hi,

I am (ab)using a PostgreSQL database (with PostGIS extension) in a 
large data processing job - each day, I load several GB of data, run a 
lot of analyses on it, and then throw everything away again. Loading, 
running, and dumping the results takes about 18 hours every day.


The job involves a lot of index building and sorting, and is run on a 
64-bit machine with 96 GB of RAM.


Naturally I would like the system to use as much RAM as possible before 
resorting to disk-based operations, but no amount of 
maintenance_work_mem setting seems to make it do my bidding.


I'm using PostgreSQL 8.3 but would be willing and able to upgrade to any 
later version.


Some googling has unearthed the issue - which is likely known to all of 
you, just repeating it to prove I've done my homework - that tuplesort.c 
always tries to double its memory allocation, and will refuse to do so 
if that results in an allocation greater than MaxAllocSize:


 if ((Size) (state->memtupsize * 2) >= MaxAllocSize / sizeof(SortTuple))
 return false;

And MaxAllocSize is hardcoded to 1 GB in memutils.h.

(All this based on Postgres 9.1alpha source - I didn't want to bring 
something up that has been fixed already.)


Now I assume that there are reasons that you're doing this. memutils.h 
has the (for me) cryptic comment about MaxAllocSize: "XXX This is 
deliberately chosen to correspond to the limiting size of varlena 
objects under TOAST. See VARATT_MASK_SIZE in postgres.h.", but 
VARATT_MASK_SIZE has zero other occurences in the source code.


If I were to either (a) increase MaxAllocSize to, say, 48 GB instead of 
1 GB, or (b) hack tuplesort.c to ignore MaxAllocSize, just for my local 
setup - would that likely be viable in my situation, or would I break 
countless things?


I can afford some experimentation; as I said, I'm throwing away the 
database every day anyway. I just thought I'd solicit your advice before 
I do anything super stupid. - If I can use my setup to somehow 
contribute to further PostgreSQL development by trying out some things, 
I'll be more than happy to do so. I do C/C++ but apart from building 
packages for several platforms, I haven't worked with the PostgreSQL 
source code.


Of course the cop-out solution would be to just create a huge RAM disk 
and instruct PostgreSQL to use that for disk-based sorting. I'll do that 
if all of you say "OMG don't touch MaxAllocSize" ;)


Bye
Frederik

--
Frederik Ramm  ##  eMail frede...@remote.org  ##  N49°00'09" E008°23'33"

--
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] Add support for logging the current role

2011-02-14 Thread Stephen Frost
Itagaki,

* Itagaki Takahiro (itagaki.takah...@gmail.com) wrote:
> We need to design csvlog_header more carefully. csvlog_header won't work
> if log_filename is low-resolution, ex. log-per-day.

This isn't any different a problem to the issue of someone changing the
csvlog_fields GUC but not checking if the log file already exists on
restart.  I've suggested a number of different options, but none of them
are terribly good, and I havn't heard anyone supporting any solution to
this issue.

> It's still useful when
> a DBA reads the file manually, but documentation would better.

Eh?  If you mean that we should add documentation to make users aware of
the possible issue of changing these values without making sure the log
file gets rotated- sure, I'd be happy to do that.

> Or, should we skip writing headers when the open log file is not
> empty?

This doesn't help the csvlog_fields issue, unfortunately.  I don't think
it'd be hard to implement this to help with the header issue, I'm just
not sure if it makes sense to do so when the actual list of fields could
change...

> * It might be my misunderstanding, but there was a short description for %U
> for in log_line_prefix in postgresql.conf, right? Did you remove it in the
> latest version?

No, and I don't see where I ever added it..  I've fixed it.

> * In assign_csvlog_fields(), we need to cleanup memory and memory context
> before return on error.
> + /* check if no option matched, and if so, return error */
> + if (curr_option == MAX_CSVLOG_OPTS)
> + return NULL;

Fixed this and a couple of similar issues.

> * An added needless "return" should be removed.

Meh, I like explicit returns, but since it generated a hunk all by
itself, I'll clear it out.

Updated patch attached, git log below.

Thanks,

Stephen

commit 304e35ebb74f68da69163ed9dd1dd453b67181e7
Author: Stephen Frost 
Date:   Mon Feb 14 09:26:03 2011 -0500

csvlog_fields: fix leak, other cleanup

Fix a couple of potential memory leaks in assign_csvlog_fields().
Also added a few comments, removed an extra 'return;', and added
%U to the sample postgresql.conf.

commit 592c2564ffde77fc29ff28fdedd2c9f2dafd
Merge: 33639eb cebbaa1
Author: Stephen Frost 
Date:   Sun Feb 13 21:11:44 2011 -0500

Merge branch 'master' of git://git.postgresql.org/git/postgresql into 
log_csv_options

commit 33639ebfe67b0dd58a0a89161e9f0d5237830ed4
Author: Stephen Frost 
Date:   Sun Feb 13 21:08:08 2011 -0500

Add csvlog_header GUC, other cleanup

This patch adds a csvlog_header option which will start each CSV
log file with a header which matches the GUC (and hence the format
of the CSV log file generated).

Numerous other whitespace clean-ups, removed build_default_csvlog_list(),
since it wasn't actually necessary or useful.  Added an array which
lists the text strings of the various CSVLOG options to simplify
assign_csvlog_fields().

commit 6bd2b9f1d2bc3b166a3e5598ee590e25159c61a5
Author: Stephen Frost 
Date:   Fri Feb 11 11:16:17 2011 -0500

Rename log_csv_fields GUC to csvlog_fields

This patch renames the log_csv_fileds GUC to csvlog_fields, to better
match the other csvlog_* options.

Also cleaned up the CSV generation code a bit by moving the comma-adding
code out of the switch() statement.

commit a281ca611e6181339e92b488c815e0cb8c1298d2
Merge: d81 183d3cf
Author: Stephen Frost 
Date:   Fri Feb 11 08:37:27 2011 -0500

Merge branch 'master' of git://git.postgresql.org/git/postgresql into 
log_csv_options

commit d81c425a4c320540769084ceeb7d23bc3662
Author: Stephen Frost 
Date:   Sun Feb 6 14:02:05 2011 -0500

Change log_csv_options listing to a table

This patch changes the listing of field options available to
log_csv_options into a table, which will hopefully both look
better and be clearer.

commit f9851cdfaeb931f01c015f5651b72d16957c7114
Merge: 3e71e33 5ed45ac
Author: Stephen Frost 
Date:   Sun Feb 6 13:26:17 2011 -0500

Merge branch 'master' of git://git.postgresql.org/git/postgresql into 
log_csv_options

commit 3e71e338a2b9352d730f59a989027e33d99bea50
Author: Stephen Frost 
Date:   Fri Jan 28 22:44:33 2011 -0500

Cleanup log_csv_options patch

Clean up of various function declarations to hopefully be correct
and clean and matching PG conventions.  Also move TopMemoryContext
usage to later, since the local variables don't need to be in
TopMemoryContext.  Lastly, ensure that a comma is not produced
after the last CSV field, and that one is produced if
application_name is not the last field.

Review by Itagaki Takahiro, thanks!

commit 1825def11badd661d219fa4c516f06e0ad423443
Merge: ff249ae 847e8c7
Author: Stephen Frost 
Date:   Wed Jan 19 06:50:03 2011 -0500

Merge branch 'master' of git://git.postgresql.org/git/postgresql into 
log_csv_options

commit ff249aeac7216da623

  1   2   >