date:20100311

2010/3/10 David Fetter da...@fetter.org:
  --disable-shared, as previously mentioned.

 Oh.  Well, we don't really support that, and there is a proposal on
 the table to remove it altogether from the configure script.  I
 don't think we're going to contort our source code in order to make
 a marginal improvement in the ability to coexist with random other
 code that is also polluting the link-time namespace.

 +1 for de-supporting this option.

I would be sad about this. It seems likely there are platforms where
it's important. But I'm not really about to spend the effort to fix it
up myself and I agree it wouldn't be worth hacking the source to get
it to work. I'm a bit puzzled why the symbol conflicts occur only with
static linking though -- it seems like static linking would give more
opportunity to isolate symbols than dynamic linking, not less. Perhaps
our static linking rules are broken?



-- 
greg

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

Re: [HACKERS] [patch] build issues on Win32

2010-03-11 Thread Dag-Erling Smørgrav

Greg Stark gsst...@mit.edu writes:
 I would be sad about this. It seems likely there are platforms where
 it's important. But I'm not really about to spend the effort to fix it
 up myself and I agree it wouldn't be worth hacking the source to get
 it to work. I'm a bit puzzled why the symbol conflicts occur only with
 static linking though -- it seems like static linking would give more
 opportunity to isolate symbols than dynamic linking, not less. Perhaps
 our static linking rules are broken?

A dynamic library is a single entity with a certain degree of isolation.
You can hide symbols so they are only visible within that library.

A static library is basically just a tarball of individual relocatable
objects.

The GNU toolchain (and probably others too) allows you to combine
several relocatable objects together into one and hide some of the
symbols, as if the entire thing had been a single C file with some of
the functions declared as static.  However, when you do that, you lose
an important advantage of static libraries: the ability to link only
what you need.

DES
-- 
Dag-Erling Smørgrav - d...@des.no

-- 
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] operator exclusion constraints

On Thu, Mar 11, 2010 at 5:29 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Indexes:
     foo_pkey PRIMARY KEY, btree (f1), tablespace ts1
     foo_f2_exclusion btree (f2), tablespace ts1
     foo_f3_exclusion btree (f3) DEFERRABLE INITIALLY DEFERRED
 Exclusion constraints:
     foo_f2_exclusion EXCLUDE USING btree (f2 WITH =)
     foo_f3_exclusion EXCLUDE USING btree (f3 WITH =) DEFERRABLE INITIALLY 
 DEFERRED

 This might have been defensible back when the idea was to keep constraints
 decoupled from indexes, but now it just looks bizarre.

The only really bizarre part is the DEFERRABLE INITIALLY DEFERRED on
the index.

  We should either
 get rid of the Exclusion constraints: display and attach the info to
 the index entries, or hide indexes that are attached to exclusion
 constraints.  I lean to the former on the grounds of the precedent for
 unique/pkey indexes --- which is not totally arbitrary, since an index
 is usable as a query index regardless of its function as a constraint.
 It's probably a debatable point though.

There is a third option -- print PRIMARY keys twice, once as a btree
index and again as a constraint where it says somehting like USING
index foo_pkey
I think in the long term that would be best -- especially if we
combine it with a patch to be able to create a new primary key
constraint using an existing index. That's something people have been
asking for anyways and I think it's a somewhat important property that
these lines can be copy pasted and run nearly as-is to recreate the
objects.

I definitely agree that your other proposed way to go is worse. I
think people need a list of indexes in one place.

So given the current syntax for creating these I think your proposed
change is the least worst alternative.

-- 
greg

-- 
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] Dyamic updates of NEW with pl/pgsql

2010-03-11 Thread strk

On Wed, Mar 10, 2010 at 07:50:16AM -0500, Andrew Dunstan wrote:

 Using an hstore in 9.0 it's not too bad,

Does it still have a limit of 65535 bytes per field ?

--strk;

  ()   Free GIS  Flash consultant/developer
  /\   http://strk.keybit.net/services.html

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

[HACKERS] Can we still trust plperl?



Last night my attention was drawn to this:

http://search.cpan.org/~timb/PostgreSQL-PLPerl-Injector-1.002/lib/PostgreSQL/PLPerl/Injector.pm

I'm wondering if we can reasonably continue to support plperl as a 
trusted language, or at least redefine what trusted actually means. 
Does it mean can't do untrusted operations or does it mean can't do 
untrusted operations unless the DBA and/or possibly the user decide to 
subvert the mechanism? To me, the latter doesn't sound much like it's 
worth having. Is it?


There are a few places where plperl has an advantage over plpgsql, e.g. 
code that uses lots of regexes and use of variable to access records 
dynamically, so losing it might be a bit of a pain. Of course, there 
would still be plperlu, with the downside that the functions have to be 
installed by a superuser. One of my PGExperts colleagues told me his 
reaction was Well, I might just as well use plperlu, and that pretty 
well sums up my reaction.


Of course, another thing is that it might spur either building of some 
of the missing stuff into plpgsql, or addition of another language that 
is both safe and which supports them, like say PL/JavaScript.


Thoughts?

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] operator exclusion constraints

Greg Stark gsst...@mit.edu writes:
 There is a third option -- print PRIMARY keys twice, once as a btree
 index and again as a constraint where it says somehting like USING
 index foo_pkey

No, that's exactly what I hate about the current behavior for exclusion
constraints, and I'd like it even less for more-common options like
primary or unique constraints.  \d is too d*mn verbose already; there is
no percentage in making it even longer by printing redundant entries for
many indexes.

One thing I did think about was converting PK/UNIQUE indexes to be
printed by pg_get_constraintdef() too, rather than assembling an ad-hoc
output for them as we do now.  This would make the code a bit simpler
but would involve some small changes in the output --- in particular,
you wouldn't see any indication that they were btrees, since there's
no place for that in standard constraint syntax.  On balance it didn't
seem like an improvement, although it would partially respond to your
desire to have the output be cut-and-pasteable.

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] Can we still trust plperl?

2010-03-11 Thread Kenneth Marshall

On Thu, Mar 11, 2010 at 09:31:46AM -0500, Andrew Dunstan wrote:

Last night my attention was drawn to this:

http://search.cpan.org/~timb/PostgreSQL-PLPerl-Injector-1.002/lib/PostgreSQL/PLPerl/Injector.pm

I'm wondering if we can reasonably continue to support plperl as a trusted
language, or at least redefine what trusted actually means. Does it mean
can't do untrusted operations or does it mean can't do untrusted
operations unless the DBA and/or possibly the user decide to subvert the
mechanism? To me, the latter doesn't sound much like it's worth having. Is
it?

There are a few places where plperl has an advantage over plpgsql, e.g.
code that uses lots of regexes and use of variable to access records
dynamically, so losing it might be a bit of a pain. Of course, there would
still be plperlu, with the downside that the functions have to be installed
by a superuser. One of my PGExperts colleagues told me his reaction was
Well, I might just as well use plperlu, and that pretty well sums up my
reaction.

Of course, another thing is that it might spur either building of some of
the missing stuff into plpgsql, or addition of another language that is
both safe and which supports them, like say PL/JavaScript.

Thoughts?

cheers

andrew

The DBA can do what ever he wants to do to subvert the system up to
installing hacked versions of any other trusted language so I do
not see much of a distinction. We already provide many other foot-guns
that may be used by the DBA. pl/perl is very useful as a trusted
language but I am certainly for fleshing out the features in other
pl-s.

Regards,
Ken

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

Re: [HACKERS] Can we still trust plperl?

Andrew Dunstan and...@dunslane.net wrote:
 
 I'm wondering if we can reasonably continue to support plperl as
 a trusted language
 
 would still be plperlu, with the downside that the functions have
 to be installed by a superuser. One of my PGExperts colleagues
 told me his reaction was Well, I might just as well use plperlu,
 and that pretty well sums up my reaction.
 
Well, I can see where running plperl with this module would be no
more safe than running plperlu, so I don't really understand the
purpose of the module; however, to install this module you need to:
 
| Set the PERL5OPT before starting postgres, to something like this:
| PERL5OPT='-e require q{plperlinit.pl}'
| and create a plperlinit.pl file in the same directory as your
| postgres.conf file.
| In the plperlinit.pl file write the code to load this module, plus
| any others you want to load and share subroutines from. 
 
I don't see where plperl is unsafe unless you do those things.  A
user who can do those things can likely subvert your database in
other ways, no?
 
-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] renameatt() can rename attribute of index, sequence, ...

2010-03-11 Thread Robert Haas

2010/3/10 KaiGai Kohei kai...@ak.jp.nec.com:
 Indeed, it is useful to allow renaming attribute of composite types.

 However, it is also useless to allow to rename attribute of sequences,
 but harmless, like renames on indexes. It seems to me it is fair enough
 to allow renaming attributes of tables, views and composite types...

I don't agree.  I think users should be allowed to rename things they
had a hand in naming in the first place (and index columns fall into
that category, since the names are derived from table column names).
But changing system-assigned column names for sequences or toast
tables is just weird.

...Robert

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

Re: [HACKERS] Server crash with older tzload library

Jeevan Chalke jeevan.cha...@enterprisedb.com writes:
 While setting timezone using SET command (say GMT+3:30), postgres sometimes
 crashes randomly.

I can't reproduce that:

regression=# SET TimeZone = 'GMT+3:30';
SET
regression=# SELECT '1969-12-31 20:30:00'::timestamptz;
timestamptz
---
 1969-12-31 20:30:00-03:30
(1 row)


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] Can we still trust plperl?

Andrew Dunstan and...@dunslane.net writes:
Last night my attention was drawn to this:

http://search.cpan.org/~timb/PostgreSQL-PLPerl-Injector-1.002/lib/PostgreSQL/PLPerl/Injector.pm

I'm wondering if we can reasonably continue to support plperl as a
trusted language, or at least redefine what trusted actually means.
Does it mean can't do untrusted operations or does it mean can't do
untrusted operations unless the DBA and/or possibly the user decide to
subvert the mechanism? To me, the latter doesn't sound much like it's
worth having. Is it?

AFAICS the DBA has to participate in setting up that module, so it's
no different from any other PL language. You can insert stuff into the
trusted interpreter in pltcl too. It's on the DBA's head to not insert
stuff that's insecure --- so what? To my mind it's a feature not a
bug that this is possible. It's just like the on_init work that you've
been doing; it's about letting the DBA have control over what users of
the trusted language can get at.

What bothers me more is the fact that genuine holes are beginning to
show up in Safe. I wonder if we aren't seeing the first stages of what
happened to trusted plpython. Building a secure sandbox feature into
a language that wasn't designed for it is hard. However, I'm not going
to panic until there's reason for panic, and this doesn't look like a
reason.

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] Dyamic updates of NEW with pl/pgsql

2010-03-11 Thread David Fetter

On Thu, Mar 11, 2010 at 03:27:23PM +0100, strk wrote:
 On Wed, Mar 10, 2010 at 07:50:16AM -0500, Andrew Dunstan wrote:
 
  Using an hstore in 9.0 it's not too bad,
 
 Does it still have a limit of 65535 bytes per field ?

No. :)

Cheers,
David.
-- 
David Fetter da...@fetter.org http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

-- 
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] gothic_moth, codlin_moth failures on REL8_2_STABLE

On Wed, Mar 10, 2010 at 11:37 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 My conclusion is that this is probably a compiler bug.  Both buildfarm
 animals appear to be using Sun Studio, although on different
 architectures which weakens the compiler-bug theory a bit.  Even though
 we are not seeing the failure in later PG branches, it would probably be
 worth looking into more closely, because if it's biting 8.2 it could
 start biting newer code as well.  But without access to a machine
 showing the problem it is difficult to do much.


Could be this:

http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6750087

It's fixed in patch 124861-11 which came out Feb 23, 2009. Is this
patch missing on both gothic-moth and codlin-moth?

I suppose it's possible to have a configure test to check for whether
this patch is present but I'm not sure how much it's worthwhile given
that it'll only help people who happen to recompile their 8.2 server
after the next Postgres patch. And I'm not sure we can check for
patches without assuming the CC is the OS-shipped cc. Does cc itself
have an option to list which patches it has applied to it?

-- 
greg

-- 
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] gothic_moth, codlin_moth failures on REL8_2_STABLE

Incidentally Zdenek came to the same conclusion that it was a compiler
bug in 4aa775a9.80...@sun.com

-- 
greg

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

[HACKERS] tsearch profiling - czech environment - take 55MB

Hello

There are some wrong in our implementation NISortDictionary. After
initialisation is ts_cache memory context 55MB long and pg takes
190MB.

dispell_init
cspell: 1024 total in 1 blocks; 136 free (1 chunks); 888 used
After dictionary loading
cspell: 3072 total in 2 blocks; 568 free (5 chunks); 2504 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
free (12 chunks); 19904424 used
After AffFile loading
cspell: 816952 total in 78 blocks; 18072 free (18 chunks); 798880 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
free (20 chunks); 19904424 used
After stop words loading
cspell: 816952 total in 78 blocks; 13360 free (13 chunks); 803592 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
free (20 chunks); 19904424 used
After dictionary sort
cspell: 55706480 total in 6775 blocks; 140200 free (1728 chunks); 55566280 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
free (20 chunks); 19904424 used
After Affixes sort
cspell: 55853736 total in 6789 blocks; 130208 free (1553 chunks); 55723528 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
free (34 chunks); 19904424 used
final
cspell: 55853736 total in 6789 blocks; 130208 free (1553 chunks); 55723528 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
free (34 chunks); 19904424 used

Regards
Pavel Stehule

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

Re: [HACKERS] tsearch profiling - czech environment - take 55MB

2010/3/11 Pavel Stehule pavel.steh...@gmail.com:
 Hello

 There are some wrong in our implementation NISortDictionary. After
 initialisation is ts_cache memory context 55MB long and pg takes
 190MB.

 dispell_init
 cspell: 1024 total in 1 blocks; 136 free (1 chunks); 888 used
 After dictionary loading
 cspell: 3072 total in 2 blocks; 568 free (5 chunks); 2504 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
 free (12 chunks); 19904424 used
 After AffFile loading
 cspell: 816952 total in 78 blocks; 18072 free (18 chunks); 798880 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
 free (20 chunks); 19904424 used
 After stop words loading
 cspell: 816952 total in 78 blocks; 13360 free (13 chunks); 803592 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
 free (20 chunks); 19904424 used
 After dictionary sort
 cspell: 55706480 total in 6775 blocks; 140200 free (1728 chunks); 55566280 
 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
 free (20 chunks); 19904424 used
 After Affixes sort
 cspell: 55853736 total in 6789 blocks; 130208 free (1553 chunks); 55723528 
 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
 free (34 chunks); 19904424 used
 final
 cspell: 55853736 total in 6789 blocks; 130208 free (1553 chunks); 55723528 
 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
 free (34 chunks); 19904424 used


the mkSPNode takes 45MB

Conf-Dictionary = mkSPNode(Conf, 0, Conf-nspell, 0);

 Regards
 Pavel Stehule


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

Re: [HACKERS] gothic_moth, codlin_moth failures on REL8_2_STABLE

Greg Stark gsst...@mit.edu writes:
 Incidentally Zdenek came to the same conclusion that it was a compiler
 bug in 4aa775a9.80...@sun.com

Drat, I had forgotten that exchange.  I reconstructed Teodor's advice
the hard 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] gothic_moth, codlin_moth failures on REL8_2_STABLE

Greg Stark gsst...@mit.edu writes:
 On Wed, Mar 10, 2010 at 11:37 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 My conclusion is that this is probably a compiler bug.

 Could be this:
 http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6750087

Hmmm ... that doesn't seem to be quite an exact match, because the
setting and testing of the bitfield is in different functions in
different files in our case.  Still, it seems related.  It would
be useful to verify whether these two buildfarm animals are fully
up-to-date on compiler patches.

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 profiling - czech environment - take 55MB

Pavel Stehule pavel.steh...@gmail.com writes:
 There are some wrong in our implementation NISortDictionary. After
 initialisation is ts_cache memory context 55MB long and pg takes
 190MB.

What's your tsearch configuration exactly?

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] Can we still trust plperl?




Tom Lane wrote:

AFAICS the DBA has to participate in setting up that module, so it's
no different from any other PL language.  You can insert stuff into the
trusted interpreter in pltcl too.  It's on the DBA's head to not insert
stuff that's insecure --- so what?  To my mind it's a feature not a
bug that this is possible.  It's just like the on_init work that you've
been doing; it's about letting the DBA have control over what users of
the trusted language can get at.
  


Fair enough, but I think we need to be clearer in the docs about what 
trusted actually means.


The docs say:

   TRUSTED specifies that the language is safe, that is, it does not
   offer an unprivileged user any functionality to bypass access
   restrictions.

Perhaps we need to add some words to that to indicate that the DBA can 
inject extra functionality into some trusted PLs, which might or might 
not be able to access system resources.



What bothers me more is the fact that genuine holes are beginning to
show up in Safe.  I wonder if we aren't seeing the first stages of what
happened to trusted plpython.  Building a secure sandbox feature into
a language that wasn't designed for it is hard.  However, I'm not going
to panic until there's reason for panic, and this doesn't look like a
reason.


  


Yes, Tim was saying something about how Safe was a failed experiment to 
me the other day. I suspect we might be fairly close to having to do 
something radical about that.


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] tsearch profiling - czech environment - take 55MB

2010/3/11 Tom Lane t...@sss.pgh.pa.us:
 Pavel Stehule pavel.steh...@gmail.com writes:
 There are some wrong in our implementation NISortDictionary. After
 initialisation is ts_cache memory context 55MB long and pg takes
 190MB.

 What's your tsearch configuration exactly?


files: http://www.pgsql.cz/data/czech.tar.gz

configuration:

CREATE TEXT SEARCH DICTIONARY cspell
   (template=ispell, dictfile = czech, afffile=czech, stopwords=czech);
CREATE TEXT SEARCH CONFIGURATION cs (copy=english);
ALTER TEXT SEARCH CONFIGURATION cs
   ALTER MAPPING FOR word, asciiword WITH cspell, simple;

then try: select * from ts_debug('cs','Příliš žluťoučký kůň se napil
žluté vody');

with some time (used fce clock())

cspell: 1024 total in 1 blocks; 136 free (1 chunks); 888 used
After dictionary loading 32
cspell: 3072 total in 2 blocks; 568 free (5 chunks); 2504 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
free (12 chunks); 19904424 used
After AffFile loading 33
cspell: 816952 total in 78 blocks; 18072 free (18 chunks); 798880 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
free (20 chunks); 19904424 used
After stop words loading 33
cspell: 816952 total in 78 blocks; 13360 free (13 chunks); 803592 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
free (20 chunks); 19904424 used
** 1 **
cspell: 816952 total in 78 blocks; 9240 free (12 chunks); 807712 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
free (20 chunks); 19904424 used
** 2 ** 38
cspell: 825144 total in 79 blocks; 8440 free (10 chunks); 816704 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
free (20 chunks); 19904424 used
** 2.5 ** 49
// mkSPNode
cspell: 825144 total in 79 blocks; 8440 free (10 chunks); 816704 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
free (20 chunks); 19904424 used
** 3 ** 58
cspell: 55706480 total in 6775 blocks; 140200 free (1728 chunks); 55566280 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
free (20 chunks); 19904424 used
After dictionary sort 58
cspell: 55706480 total in 6775 blocks; 140200 free (1728 chunks); 55566280 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
free (20 chunks); 19904424 used
After Affixes sort 58
cspell: 55853736 total in 6789 blocks; 130208 free (1553 chunks); 55723528 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
free (34 chunks); 19904424 used
final 58
cspell: 55853736 total in 6789 blocks; 130208 free (1553 chunks); 55723528 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
free (34 chunks); 19904424 used
executor start



                        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] gothic_moth, codlin_moth failures on REL8_2_STABLE


Hi Tom,

I'm sorry that I did not look on it early. I played with it and there 
are some facts. gothic(sparc) and codlin(x86) uses Sun Studio 12 nad I 
setup them to use very high optimization.


Gothic:
---
-xalias_level=basic -xarch=native -xdepend -xmemalign=8s -xO5 
-xprefetch=auto,explicit


Codlin:
---
-xalias_level=basic -xarch=native -xdepend -xO4 -xprefetch=auto,explicit

-xO5 is highest optimization, -xO4 is little bit worse

A play with flags and found that

-xO4 -xalias_level=basic generates problem.

-xO3 -xalias_level=basic works fine

-xO5 works fine


As documentation say:

Cite from Sun studio compiler guide:
http://docs.sun.com/app/docs/doc/819-5265/bjapp?a=view


xalias_level=basic
--
If you use the -xalias_level=basic option, the compiler assumes that 
memory references that involve different C basic types do not alias each 
other. The compiler also assumes that references to all other types can 
alias each other as well as any C basic type. The compiler assumes that 
references using char * can alias any other type.


For example, at the -xalias_level=basic level, the compiler assumes that 
a pointer variable of type int * is not going to access a float object. 
Therefore it is safe for the compiler to perform optimizations that 
assume a pointer of type float * will not alias the same memory that is 
referenced with a pointer of type int *.


-x04
-
Preforms automatic inlining of functions contained in the same file in 
addition to performing -xO3 optimizations. This automatic inlining 
usually improves execution speed, but sometimes makes it worse. In 
general, this level results in increased code size.





I redefined  bitfields to char in  HLWORD and it works. Your guess is 
correct. But question still where is the place when bitfields works bad. 
Any idea where I should look?


IIRC, I had this problem also on head, when I tried to fix tsearch 
regression test for Czech locale. This problem appears and disappears.


Zdenek




Dne 11.03.10 00:37, Tom Lane napsal(a):

Since the buildfarm is mostly green these days, I took some time to look
into the few remaining consistent failures.  One is that gothic_moth and
codlin_moth fail on contrib/tsearch2 in the 8.2 branch, with a
regression diff like this:

*** 2453,2459 
body
bSea/b  view wowubfoo/b  bar/u  iqq/i
a href=http://www.google.com/foo.bar.html; target=_blankYESnbsp;/a
!   ff-bg
script
   document.write(15);
/script
--- 2453,2459 
body
bSea/b  view wowubfoo/b  bar/u  iqq/i
a href=http://www.google.com/foo.bar.html; target=_blankYESnbsp;/a
!  ff-bgff-bg
script
   document.write(15);
/script

These animals are not testing any branches older than 8.2.  The same
test appears in newer branches and passes, but the code involved got
migrated to core and probably changed around a bit.

I traced through this test on my own machine and determined that the
way it's supposed to work is like this: the tsearch parser breaks the
string into a series of tokens that include these:

ff-bg   compound word
ff  compound word element
-   punctuation
bg  compound word element

The highlight function is then supposed to set skip = 1 on the compound
word, causing it to be skipped when genhl() reconstructs the text.
The failure looks to me like the compound word is not getting skipped.
Both the setting and the testing of the flag seem to be absolutely
straightforward portable code; although the skip struct field is a
bitfield, which is a C feature we don't use very heavily.

My conclusion is that this is probably a compiler bug.  Both buildfarm
animals appear to be using Sun Studio, although on different
architectures which weakens the compiler-bug theory a bit.  Even though
we are not seeing the failure in later PG branches, it would probably be
worth looking into more closely, because if it's biting 8.2 it could
start biting newer code as well.  But without access to a machine
showing the problem it is difficult to do much.

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] gothic_moth, codlin_moth failures on REL8_2_STABLE

Zdenek Kotala zdenek.kot...@sun.com writes:
 -xO4 -xalias_level=basic generates problem.
 -xO3 -xalias_level=basic works fine
 -xO5 works fine

 As documentation say:

 Cite from Sun studio compiler guide:
 http://docs.sun.com/app/docs/doc/819-5265/bjapp?a=view

 xalias_level=basic
 --
 If you use the -xalias_level=basic option, the compiler assumes that 
 memory references that involve different C basic types do not alias each 
 other. The compiler also assumes that references to all other types can 
 alias each other as well as any C basic type. The compiler assumes that 
 references using char * can alias any other type.

 For example, at the -xalias_level=basic level, the compiler assumes that 
 a pointer variable of type int * is not going to access a float object. 
 Therefore it is safe for the compiler to perform optimizations that 
 assume a pointer of type float * will not alias the same memory that is 
 referenced with a pointer of type int *.

I think you need to turn that off.  On gcc we use -fno-strict-aliasing
which disables the type of compiler assumption that this is talking about.
I'm not sure exactly how that might create the specific failure we are
seeing here, but I can point you to lots and lots of places in the
sources where such an assumption would break things.

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] gothic_moth, codlin_moth failures on REL8_2_STABLE


Dne 11.03.10 16:24, Greg Stark napsal(a):

On Wed, Mar 10, 2010 at 11:37 PM, Tom Lanet...@sss.pgh.pa.us  wrote:

My conclusion is that this is probably a compiler bug.  Both buildfarm
animals appear to be using Sun Studio, although on different
architectures which weakens the compiler-bug theory a bit.  Even though
we are not seeing the failure in later PG branches, it would probably be
worth looking into more closely, because if it's biting 8.2 it could
start biting newer code as well.  But without access to a machine
showing the problem it is difficult to do much.



Could be this:

http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6750087

It's fixed in patch 124861-11 which came out Feb 23, 2009. Is this
patch missing on both gothic-moth and codlin-moth?


It seems as a our case. See compiler versions:

Ghost:
-bash-3.2$ cc -V
cc: Sun C 5.9 SunOS_sparc Patch 124867-09 2008/11/25

Codlin
-bash-4.0$ cc -V
cc: Sun C 5.9 SunOS_i386 Patch 124868-10 2009/04/30


I should apply patch on Ghost, but Codlin have to wait, because I don't 
have a control on compiler version. I try to find update SS12 somewhere 
on the disk/network.


The patch which you refer does not fix cc itself but some others 
binaries/libs which cc uses.


I try to update Ghost and we will see what happen.


I suppose it's possible to have a configure test to check for whether
this patch is present but I'm not sure how much it's worthwhile given
that it'll only help people who happen to recompile their 8.2 server
after the next Postgres patch. And I'm not sure we can check for
patches without assuming the CC is the OS-shipped cc. Does cc itself
have an option to list which patches it has applied to it?



cc is not shipped with solaris you have to install it separately. And 
bug appear only when you use high optimization (see my email). You can 
see patch version when you run cc -V but you see only compiler version.


Zdenek


--
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] gothic_moth, codlin_moth failures on REL8_2_STABLE


Dne 11.03.10 17:37, Tom Lane napsal(a):

Zdenek Kotalazdenek.kot...@sun.com  writes:

-xO4 -xalias_level=basic generates problem.
-xO3 -xalias_level=basic works fine
-xO5 works fine



As documentation say:



Cite from Sun studio compiler guide:
http://docs.sun.com/app/docs/doc/819-5265/bjapp?a=view



xalias_level=basic
--
If you use the -xalias_level=basic option, the compiler assumes that
memory references that involve different C basic types do not alias each
other. The compiler also assumes that references to all other types can
alias each other as well as any C basic type. The compiler assumes that
references using char * can alias any other type.



For example, at the -xalias_level=basic level, the compiler assumes that
a pointer variable of type int * is not going to access a float object.
Therefore it is safe for the compiler to perform optimizations that
assume a pointer of type float * will not alias the same memory that is
referenced with a pointer of type int *.


I think you need to turn that off.  On gcc we use -fno-strict-aliasing
which disables the type of compiler assumption that this is talking about.
I'm not sure exactly how that might create the specific failure we are
seeing here, but I can point you to lots and lots of places in the
sources where such an assumption would break things.


OK. I first try to update compiler to latest version to see if it helps 
and finally I will remove aliasing.


Thanks Zdenek

--
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 profiling - czech environment - take 55MB

2010/3/11 Pavel Stehule pavel.steh...@gmail.com:
 2010/3/11 Tom Lane t...@sss.pgh.pa.us:
 Pavel Stehule pavel.steh...@gmail.com writes:
 There are some wrong in our implementation NISortDictionary. After
 initialisation is ts_cache memory context 55MB long and pg takes
 190MB.

 What's your tsearch configuration exactly?


I have a 64bit Linux.

The problem is in very large small allocations - there are 853215 nodes.

The memory can be minimalized with some block allocations

static void.
binit(void)
{
--data = NULL;
--allocated = 0;
}


static char *
balloc(size_t size)
{
--char *result;
--
--if (data == NULL || size  allocated )
--{
data = palloc(1024 * 100);
allocated = 1024 * 100;
--}
--
--result = data;
--data += size;
--allocated -= size;
--memset(result, 0, size);
--
--return result;
}

I replaced palloc0 inside mkSPnode by balloc

cspell: 25626352 total in 349 blocks; 11048 free (2 chunks); 25615304 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
free (34 chunks); 19904424 used

versus

cspell: 55853736 total in 6789 blocks; 130208 free (1553 chunks); 55723528 used
  Ispell dictionary init context: 27615288 total in 13 blocks; 7710864
free (34 chunks); 19904424 used

Regards
Pavel

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

Re: [HACKERS] tsearch profiling - czech environment - take 55MB

Pavel Stehule pavel.steh...@gmail.com writes:
 The problem is in very large small allocations - there are 853215 nodes.
 I replaced palloc0 inside mkSPnode by balloc

This goes back to the idea we've discussed from time to time of having a
variant memory context type in which pfree() is a no-op and we dispense
with all the per-chunk overhead.  I guess that if there really isn't any
overhead there then pfree/repalloc would actually crash :-( but for the
particular case of dictionaries that would probably be OK because
there's so little code that touches them.

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 profiling - czech environment - take 55MB

2010/3/11 Tom Lane t...@sss.pgh.pa.us:
 Pavel Stehule pavel.steh...@gmail.com writes:
 The problem is in very large small allocations - there are 853215 nodes.
 I replaced palloc0 inside mkSPnode by balloc

 This goes back to the idea we've discussed from time to time of having a
 variant memory context type in which pfree() is a no-op and we dispense
 with all the per-chunk overhead.  I guess that if there really isn't any
 overhead there then pfree/repalloc would actually crash :-( but for the
 particular case of dictionaries that would probably be OK because
 there's so little code that touches them.

it has a sense. I was surprised how much memory is necessary :(. Some
smarter allocation save 50% - 2.5G for 100 users, what is important,
but I thing, so these data has to be shared. I believed to preloading,
but it is problematic - there are no data in shared preload time, and
the allocated size is too big.

Pavel



                        regards, tom lane


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

Re: [HACKERS] [patch] build issues on Win32

2010-03-11 Thread Magnus Hagander

2010/3/11 Dag-Erling Smørgrav d...@des.no:
 Greg Stark gsst...@mit.edu writes:
 I would be sad about this. It seems likely there are platforms where
 it's important. But I'm not really about to spend the effort to fix it
 up myself and I agree it wouldn't be worth hacking the source to get
 it to work. I'm a bit puzzled why the symbol conflicts occur only with
 static linking though -- it seems like static linking would give more
 opportunity to isolate symbols than dynamic linking, not less. Perhaps
 our static linking rules are broken?

 A dynamic library is a single entity with a certain degree of isolation.
 You can hide symbols so they are only visible within that library.

In particular, libpq only exports a fixed subset of symbols on any
platform that supports that (which includes Windows).

AFAIK, there is no way to make that restriction on static libraries,
at least not on Windows.




-- 
 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] [patch] build issues on Win32

Magnus Hagander mag...@hagander.net writes:
 In particular, libpq only exports a fixed subset of symbols on any
 platform that supports that (which includes Windows).

 AFAIK, there is no way to make that restriction on static libraries,
 at least not on Windows.

Right, and not anyplace else either except by pre-linking the contained
.o files, which eliminates one of the claimed advantages of a .a library.

So one of the reasons not to support a static version of libpq is
precisely this one: it means that every non-static symbol inside libpq
is subject to conflicts against calling applications and other libraries
that they choose to link with.  I don't particularly care to deal with
that, and I especially don't choose to accept the position that any such
conflicts are *our* problem to fix.

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] tsearch - using a transformed data files

Hello

we are able to go down load time of ispell dictionaries if we can
remove upper to lower transformations (and some flag check).

these operations take about 40% of dictionary initialisation time.

so maybe we can thinking about some flag for ispell_dictionary - like
clean, or some similar?

Regards
Pavel Stehule

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

Re: [HACKERS] Server crash with older tzload library

Jeevan Chalke jeevan.cha...@enterprisedb.com writes:
 The upstream zoneinfo project just released a new tzcode version, 2010c.
 After syncing the code to this version does not lead to server crash. The
 new release is now initializing the tzstate variable with zeros to avoid any
 garbage values.

 PFA, patch which will bring us up-to date to 2010c.

I've applied the update to 2010c since that apparently fixes some
misbehaviors in obscure time zones (where is America/Godthab???).
However, the proposed addition of explicit clears of the tzstate
struct doesn't match any upstream change that I can see.  I inserted
explicit initializations to random data instead and still couldn't
provoke a crash.  While it seems harmless enough to explicitly zero
it, I'd like to see an instance of the reported crash, because I have
a feeling that the real problem you're dealing with is elsewhere.
If you can't provoke it reliably, maybe the zeroing didn't really
fix it.

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] operator exclusion constraints

2010-03-11 Thread Jeff Davis

On Thu, 2010-03-11 at 00:29 -0500, Tom Lane wrote:

Patch changes:

  Indexes:
  foo_pkey PRIMARY KEY, btree (f1), tablespace ts1
  foo_f2_exclusion btree (f2), tablespace ts1
  foo_f3_exclusion btree (f3) DEFERRABLE INITIALLY DEFERRED
  Exclusion constraints:
  foo_f2_exclusion EXCLUDE USING btree (f2 WITH =)
  foo_f3_exclusion EXCLUDE USING btree (f3 WITH =) DEFERRABLE INITIALLY 
  DEFERRED

To:

 Indexes:
 foo_pkey PRIMARY KEY, btree (f1), tablespace ts1
 foo_f2_exclusion EXCLUDE USING btree (f2 WITH =), tablespace ts1
 foo_f3_exclusion EXCLUDE USING btree (f3 WITH =) DEFERRABLE INITIALLY 
 DEFERRED
 
 Any objections?

Looks good to me.

Regards,
Jeff Davis


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

Re: [HACKERS] Server crash with older tzload library

2010-03-11 Thread Robert Haas

On Thu, Mar 11, 2010 at 1:49 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Jeevan Chalke jeevan.cha...@enterprisedb.com writes:
 The upstream zoneinfo project just released a new tzcode version, 2010c.
 After syncing the code to this version does not lead to server crash. The
 new release is now initializing the tzstate variable with zeros to avoid any
 garbage values.

 PFA, patch which will bring us up-to date to 2010c.

 I've applied the update to 2010c since that apparently fixes some
 misbehaviors in obscure time zones (where is America/Godthab???).

Greenland, apparently.

http://www.travelmath.com/time-zone/America/Godthab

...Robert

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

Re: [HACKERS] tsearch profiling - czech environment - take 55MB

Pavel Stehule escribió:
 2010/3/11 Tom Lane t...@sss.pgh.pa.us:
  Pavel Stehule pavel.steh...@gmail.com writes:
  The problem is in very large small allocations - there are 853215 nodes.
  I replaced palloc0 inside mkSPnode by balloc
 
  This goes back to the idea we've discussed from time to time of having a
  variant memory context type in which pfree() is a no-op and we dispense
  with all the per-chunk overhead.  I guess that if there really isn't any
  overhead there then pfree/repalloc would actually crash :-( but for the
  particular case of dictionaries that would probably be OK because
  there's so little code that touches them.
 
 it has a sense. I was surprised how much memory is necessary :(. Some
 smarter allocation save 50% - 2.5G for 100 users, what is important,
 but I thing, so these data has to be shared. I believed to preloading,
 but it is problematic - there are no data in shared preload time, and
 the allocated size is too big.

Could it be mmapped and shared that way?

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

-- 
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 profiling - czech environment - take 55MB

2010/3/11 Alvaro Herrera alvhe...@commandprompt.com:
 Pavel Stehule escribió:
 2010/3/11 Tom Lane t...@sss.pgh.pa.us:
  Pavel Stehule pavel.steh...@gmail.com writes:
  The problem is in very large small allocations - there are 853215 nodes.
  I replaced palloc0 inside mkSPnode by balloc
 
  This goes back to the idea we've discussed from time to time of having a
  variant memory context type in which pfree() is a no-op and we dispense
  with all the per-chunk overhead.  I guess that if there really isn't any
  overhead there then pfree/repalloc would actually crash :-( but for the
  particular case of dictionaries that would probably be OK because
  there's so little code that touches them.

 it has a sense. I was surprised how much memory is necessary :(. Some
 smarter allocation save 50% - 2.5G for 100 users, what is important,
 but I thing, so these data has to be shared. I believed to preloading,
 but it is problematic - there are no data in shared preload time, and
 the allocated size is too big.

 Could it be mmapped and shared that way?

I don't know - I newer worked with mmap.

Pavel


 --
 Alvaro Herrera                                http://www.CommandPrompt.com/
 The PostgreSQL Company - Command Prompt, Inc.


-- 
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] Server crash with older tzload library

Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
 No, Jeevan is talking about tzcode, not tzdata. The zoneinfo library is
 split into two parts, we update the data part at each release, but we
 don't sync up our code with upstream code changes regularly.

It strikes me that maybe we are putting ourselves at risk by blithely
pushing tzdata updates into back branches without also pushing tzcode
updates.  However, doing this would mean updating the back branches for
64bit tzdata, which is not a small change.  Heikki, do you remember how
much that patch affected stuff outside the tzcode files proper?

In any case it would be madness to try to get that into the releases due
to be wrapped today, but maybe it should be on the to-do list to make it
happen soon (in particular, before we desupport 8.0, because this could
matter for the long-term usability of 8.0).

regards, tom lane

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

Re: [HACKERS] [patch] build issues on Win32

Greg Stark gsst...@mit.edu writes:
 2010/3/10 David Fetter da...@fetter.org:
 --disable-shared, as previously mentioned.
 
 +1 for de-supporting this option.

 I would be sad about this. It seems likely there are platforms where
 it's important.

Any such platform would already be contending with plpgsql not working,
encoding conversion not working, etc etc.  It's barely conceivable that
a client-only installation would be useful.  But given that nobody has
actually proposed supporting such a platform in the past ten years,
I don't think one's likely to come out of the woodwork now.  AFAICT
the only case where anyone tries to do this is they have a personal
preference to avoid shared libraries, for generally-pretty-darn-dubious
reasons.

Red Hat has developed a pretty strict policy against even shipping
static libraries, because it's such a PITA to deal with updates.
Let me give you a fresh-in-mind example: there is an open security bug
against libpng (which I package for Red Hat in my copious spare time).
I was distressed to find that fixing the bug left firefox still failing
against a test web page.  Investigation disclosed that the reason for
this is that firefox is using a private static copy of libpng.  That was
a stupid decision on their part, as it means they're going to have to be
involved in fixing this issue, not to mention past and future issues.

Now libpq doesn't often have critical security bugs filed against it,
but it certainly has bugs.  Do you really want to have to remember to
rebuild every piece of dependent software when you update it?  The OP's
case apparently involves multiple independent libraries that he wants to
link statically, which makes the problem multiple times worse.

So my position is that static linking has enough negatives that you
need a lot more than a hypothetical use-case to justify it.

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] HeapTupleData.t_self garbage values

According to htup.h:
 
 * t_self and t_tableOid should be valid if the HeapTupleData points
 * to a disk buffer, or if it represents a copy of a tuple on disk.
 * They should be explicitly set invalid in manufactured tuples.
 
In the heap_hot_search_buffer function of heapam.c this is not true.
I can't find a clear explanation of why that is.  I'm assuming it
just doesn't matter here, but at a minimum it seems worth a
comment.  It's not immediately obvious to me what the random garbage
from the stack would be replaced with if we were to try to make the
above comment true.
 
Quick brain dump on the topic, anyone?
 
-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] HeapTupleData.t_self garbage values

Kevin Grittner kevin.gritt...@wicourts.gov writes:
 According to htup.h:
  * t_self and t_tableOid should be valid if the HeapTupleData points
  * to a disk buffer, or if it represents a copy of a tuple on disk.
  * They should be explicitly set invalid in manufactured tuples.
 
 In the heap_hot_search_buffer function of heapam.c this is not true.
 I can't find a clear explanation of why that is.  I'm assuming it
 just doesn't matter here, but at a minimum it seems worth a
 comment.  It's not immediately obvious to me what the random garbage
 from the stack would be replaced with if we were to try to make the
 above comment true.

What that comment is recommending is

ItemPointerSetInvalid((tuple.t_self));

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] HeapTupleData.t_self garbage values

Tom Lane t...@sss.pgh.pa.us wrote:
 Kevin Grittner kevin.gritt...@wicourts.gov writes:
 According to htup.h:
  * t_self and t_tableOid should be valid if the HeapTupleData
  * points to a disk buffer, or if it represents a copy of a tuple
  * on disk. They should be explicitly set invalid in manufactured
  * tuples.
  
 In the heap_hot_search_buffer function of heapam.c this is not
 true.
 
 What that comment is recommending is
 
   ItemPointerSetInvalid((tuple.t_self));
 
Aren't those tuples pointing to a disk buffer?  I know how to set an
invalid pointer, but I thought that was only for manufactured
tuples?
 
-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] [patch] build issues on Win32

2010/3/11 Tom Lane t...@sss.pgh.pa.us:
 Now libpq doesn't often have critical security bugs filed against it,
 but it certainly has bugs.  Do you really want to have to remember to
 rebuild every piece of dependent software when you update it?

I absolutely agree that linking statically is a terrible idea for
distributions. Debian has a similar prohibition against linking
statically against any other package.

But it does ship static libraries because commercial binary-only
packages often require them. They have to be packaged to work on
multiple OS distributions including some which don't provide
especially smooth dependency systems. You don't want to get calls from
customers running your software against random library versions you've
never tested.

The two (separate) goals which are useful are 1) Provide a library
others can link against to produce a binary which has no run-time
dependency on your library. In this case the only library they might
want to link against would be libpq. The encoding libraries etc aren't
things they're going to link agains. And 2) build binaries which have
no dependencies on system libraries so someone can ship them and
expect them to run on any system regardless of the run-time
environment.

I agree that these are both over-used but they are sometimes the least
bad option. On the other hand, the third goal avoid using the shared
library facilities is pointless, I see no reason to avoid building
and loading the encoding code or the contrib modules. They're using
the same technology as shared libraries but they're not really shared
libraries in the sense of being shipped separately from the binaries
using them.

-- 
greg

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

Re: [HACKERS] [patch] build issues on Win32




Tom Lane wrote:

Now libpq doesn't often have critical security bugs filed against it,
but it certainly has bugs.  Do you really want to have to remember to
rebuild every piece of dependent software when you update it?  The OP's
case apparently involves multiple independent libraries that he wants to
link statically, which makes the problem multiple times worse.

So my position is that static linking has enough negatives that you
need a lot more than a hypothetical use-case to justify it.

  


+1.

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] HeapTupleData.t_self garbage values

Kevin Grittner kevin.gritt...@wicourts.gov writes:
 Tom Lane t...@sss.pgh.pa.us wrote:
 ItemPointerSetInvalid((tuple.t_self));
 
 Aren't those tuples pointing to a disk buffer?

Oh, I should have looked at the code before commenting ;-).

Yeah, the correct TID value would be ItemPointerGetBlockNumber(tid)
plus the current offnum.  However we don't have enough information
in this function to set t_tableOid correctly, so maybe it would be
best to just set both fields invalid.  Or do nothing --- AFAICS none
of the uses of the heapTuple look at those fields.  Is it worth a few
extra cycles to initialize unused fields of a short-lived heapTuple?

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] HeapTupleData.t_self garbage values

Tom Lane t...@sss.pgh.pa.us wrote:
 
 Yeah, the correct TID value would be
 ItemPointerGetBlockNumber(tid) plus the current offnum.
 
Thanks!
 
 However we don't have enough information in this function to set
 t_tableOid correctly, so maybe it would be best to just set both
 fields invalid.  Or do nothing --- AFAICS none of the uses of the
 heapTuple look at those fields.  Is it worth a few extra cycles to
 initialize unused fields of a short-lived heapTuple?
 
At a minimum, it might be good to qualify the comment in htup.h and
add a comment where there is an exception.  This can be startling in
a debugger if you don't know that the comment isn't really true. 
(And I've found another place where t_tableOid isn't set, but it is
apparently benign; that's without an exhaustive search.)
 
I could put forward a comment-only patch per the above if there are
no objections.
 
-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] [patch] build issues on Win32

Greg Stark gsst...@mit.edu writes:
 The two (separate) goals which are useful are 1) Provide a library
 others can link against to produce a binary which has no run-time
 dependency on your library. In this case the only library they might
 want to link against would be libpq. The encoding libraries etc aren't
 things they're going to link agains. And 2) build binaries which have
 no dependencies on system libraries so someone can ship them and
 expect them to run on any system regardless of the run-time
 environment.

 I agree that these are both over-used but they are sometimes the least
 bad option. On the other hand, the third goal avoid using the shared
 library facilities is pointless, I see no reason to avoid building
 and loading the encoding code or the contrib modules. They're using
 the same technology as shared libraries but they're not really shared
 libraries in the sense of being shipped separately from the binaries
 using them.

True.  That still makes the current --disable-shared configure option
useless, but it doesn't go as far as suggesting that we ought to
implement --disable-static, which would be the logical conclusion from
my position.

I don't actually want to do --disable-static, but my feeling about it
is if it breaks you get to keep both pieces.  I'm not interested in
providing workarounds to let a static libpq be used in combination
with any-random-other-static-library.  That's up to the user of the
thing to deal with.

BTW, I'm not sure I buy the argument that commercial software requires
static linking.  Red Hat would be as interested in that market as
anybody, and as I said, they don't think it's necessary to ship static
libraries (with a *very* short list of exceptions).

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] HeapTupleData.t_self garbage values

Kevin Grittner kevin.gritt...@wicourts.gov writes:
 At a minimum, it might be good to qualify the comment in htup.h and
 add a comment where there is an exception.  This can be startling in
 a debugger if you don't know that the comment isn't really true. 
 (And I've found another place where t_tableOid isn't set, but it is
 apparently benign; that's without an exhaustive search.)
 
 I could put forward a comment-only patch per the above if there are
 no objections.

I don't want to put weasel wording into the comment.  If you're bothered
by the discrepancy then we should fix the code to initialize all the
struct fields.

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] Warning about invalid .pgpass passwords

Bruce Momjian wrote:
 Tom Lane wrote:
  Bruce Momjian br...@momjian.us writes:
   Tom Lane wrote:
   I'm inclined to think that maybe we should make the server return a
   distinct SQLSTATE for bad password, and have libpq check for that
   rather than just assuming that the failure must be bad password.
  
   Modifying the backend to issue this hint seems like overkill, unless we
   have some other use for it.
  
  I wouldn't suggest it if I thought it were only helpful for this
  particular message.  It seems to me that we've spent a lot of time
  kluging around the lack of certainty about whether a connection failure
  is a password issue.  Admittedly a lot of that was between libpq and its
  client, but the state of affairs on the wire isn't great either.
 
 Yes, I have seen that myself in psql.
 
  I'm not convinced we have to do it that way, but now is definitely
  the time to think about it before we implement yet another
  sort-of-good-enough kluge.  Which is what this is.
 
 True.  Should we just hold this all for 9.1 or should I code it and
 let's look at the size of the patch?

With no one replying, I decide to code up a patch that adds a new
SQLSTATE (28001) to report invalid/missing passwords.  With this code,
the warning will only appear when connecting to 9.0 servers.  The output
still looks the same, but will only appear for a password failure:

$ sql -h localhost test
psql: FATAL:  password authentication failed for user postgres
password retrieved from .pgpass

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

  PG East:  http://www.enterprisedb.com/community/nav-pg-east-2010.do
Index: doc/src/sgml/errcodes.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/errcodes.sgml,v
retrieving revision 1.28
diff -c -c -r1.28 errcodes.sgml
*** doc/src/sgml/errcodes.sgml	7 Dec 2009 05:22:21 -	1.28
--- doc/src/sgml/errcodes.sgml	11 Mar 2010 21:07:09 -
***
*** 761,766 
--- 761,772 
  entryinvalid_authorization_specification/entry
  /row
  
+ row
+ entryliteral28001/literal/entry
+ entryINVALID PASSWORD SPECIFICATION/entry
+ entryinvalid_password_specification/entry
+ /row
+ 
  
  row
  entry spanname=span13emphasis role=boldClass 2B mdash; Dependent Privilege Descriptors Still Exist//entry
Index: src/backend/libpq/auth.c
===
RCS file: /cvsroot/pgsql/src/backend/libpq/auth.c,v
retrieving revision 1.195
diff -c -c -r1.195 auth.c
*** src/backend/libpq/auth.c	26 Feb 2010 02:00:42 -	1.195
--- src/backend/libpq/auth.c	11 Mar 2010 21:07:16 -
***
*** 232,238 
  auth_failed(Port *port, int status)
  {
  	const char *errstr;
! 
  	/*
  	 * If we failed due to EOF from client, just quit; there's no point in
  	 * trying to send a message to the client, and not much point in logging
--- 232,239 
  auth_failed(Port *port, int status)
  {
  	const char *errstr;
! 	int		errcode_return = ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION;
! 	
  	/*
  	 * If we failed due to EOF from client, just quit; there's no point in
  	 * trying to send a message to the client, and not much point in logging
***
*** 269,274 
--- 270,280 
  		case uaMD5:
  		case uaPassword:
  			errstr = gettext_noop(password authentication failed for user \%s\);
+ 			/*
+ 			 *	This might require the client to prompt for a password, so we use
+ 			 *	a distinct error code.
+ 			 */
+ 			errcode_return = ERRCODE_INVALID_PASSWORD_SPECIFICATION;
  			break;
  		case uaPAM:
  			errstr = gettext_noop(PAM authentication failed for user \%s\);
***
*** 285,291 
  	}
  
  	ereport(FATAL,
! 			(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
  			 errmsg(errstr, port-user_name)));
  	/* doesn't return */
  }
--- 291,297 
  	}
  
  	ereport(FATAL,
! 			(errcode(errcode_return),
  			 errmsg(errstr, port-user_name)));
  	/* doesn't return */
  }
Index: src/include/utils/elog.h
===
RCS file: /cvsroot/pgsql/src/include/utils/elog.h,v
retrieving revision 1.102
diff -c -c -r1.102 elog.h
*** src/include/utils/elog.h	2 Jan 2010 16:58:10 -	1.102
--- src/include/utils/elog.h	11 Mar 2010 21:07:16 -
***
*** 61,66 
--- 61,73 
  	(PGSIXBIT(ch1) + (PGSIXBIT(ch2)  6) + (PGSIXBIT(ch3)  12) + \
  	 (PGSIXBIT(ch4)  18) + (PGSIXBIT(ch5)  24))
  
+ #define SQLSTATE_TO_BASE10(state)	\
+ 	(((state) 0x3F) * 1 + \
+ 	(((state)   6)  0x3F) *  1000 + \
+ 	(((state)  12)  0x3F) *   100 + \
+ 	(((state)  18)  0x3F) *10 + \
+ 	(((state)  24)  0x3F))
+ 
  /* These macros depend on the fact that '0' becomes a zero in SIXBIT */
  #define ERRCODE_TO_CATEGORY(ec)  ((ec)  ((1  12) - 1))
  #define ERRCODE_IS_CATEGORY(ec)  (((ec)  ~((1  12) - 1)) == 0)
Index:

Re: [HACKERS] Warning about invalid .pgpass passwords

Bruce Momjian wrote:

 + static void
 + dot_pg_pass_warning(PGconn *conn)
 + {
 + /* If it was 'invalid authorization', add .pgpass mention */
 + if (conn-dot_pgpass_used  conn-password_needed  conn-result 
 + /* only works with = 9.0 servers */
 + atoi(PQresultErrorField(conn-result, PG_DIAG_SQLSTATE)) ==
 + 
 SQLSTATE_TO_BASE10(ERRCODE_INVALID_PASSWORD_SPECIFICATION))
 + appendPQExpBufferStr(conn-errorMessage,
 + libpq_gettext(password retrieved from .pgpass\n));
 + }

This bit seems strange ...  I think we just do strcmp() to compare sqlstates?

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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

Re: [HACKERS] [patch] build issues on Win32

2010-03-11 Thread Steve Atkins


On Mar 11, 2010, at 1:06 PM, Tom Lane wrote:
 
 
 BTW, I'm not sure I buy the argument that commercial software requires
 static linking.  Red Hat would be as interested in that market as
 anybody, and as I said, they don't think it's necessary to ship static
 libraries (with a *very* short list of exceptions).

Trying to support a binary distributed[1] application on a dozen or more
different distributions is fairly painful (I'm building on four Debians,
eight Ubuntus, several RHELs, three Solarises and a few others
right now), and the biggest problem is the different distributions
don't package all the libraries you need, and when they do they
include a wide range of versions, often woefully obsolete.

It's a serious pain. The woefully obsolete bit also means that you
have to code to the lowest common denominator - you cannot
use any feature more recent than the obsolete library that the 
oldest version of RHEL you support has, nor any feature that's
been deprecated in the latest bleeding edge Ubuntu release

(There are some APIs where an old feature has been replaced
by a new feature, and the old one deprecated and removed
over a timeframe short enough that conservative distributions
only have the old one and more bleeding edge distributions
only have the new one. libpq is stable enough and backwards
compatible enough that it's never been an issue there).

Building applications mostly statically linked avoids the vast
majority of those problems. It's not the only way to do it, and
I don't think it's ever the best way to do it as it can lead to
a bunch of other problems but it's often the easiest fix for
getting cross-platform support working right now, and I
can understand why people want to be able to do it.

Cheers,
  Steve

[1] Binary distributed is not the same as commercial, though
a lot of commercial code is distributed as binaries so it's
one place where the problem comes up. Commercial
software is also more limited in it's ability to just blame
the OS vendor and ignore the issue.

-- 
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] Warning about invalid .pgpass passwords

Alvaro Herrera wrote:
 Bruce Momjian wrote:
 
  + static void
  + dot_pg_pass_warning(PGconn *conn)
  + {
  +   /* If it was 'invalid authorization', add .pgpass mention */
  +   if (conn-dot_pgpass_used  conn-password_needed  conn-result 
  +   /* only works with = 9.0 servers */
  +   atoi(PQresultErrorField(conn-result, PG_DIAG_SQLSTATE)) ==
  +   
  SQLSTATE_TO_BASE10(ERRCODE_INVALID_PASSWORD_SPECIFICATION))
  +   appendPQExpBufferStr(conn-errorMessage,
  +   libpq_gettext(password retrieved from .pgpass\n));
  + }
 
 This bit seems strange ...  I think we just do strcmp() to compare sqlstates?

Uh, the PQresultErrorField is a string, while
ERRCODE_INVALID_PASSWORD_SPECIFICATI is a 4-byte value in base-64.  
Yea, it's true.  For excitement, see the MAKE_SQLSTATE macro.

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

  PG East:  http://www.enterprisedb.com/community/nav-pg-east-2010.do

-- 
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] HeapTupleData.t_self garbage values

Tom Lane t...@sss.pgh.pa.us wrote:
 
 I don't want to put weasel wording into the comment.  If you're
 bothered by the discrepancy then we should fix the code to
 initialize all the struct fields.
 
Maybe for 9.1.  At this point, unless it's breaking something I
can't see that anything beyond a comment would be justified.
 
Longer term, it's hard enough getting one's head around a million
lines of code without false statements in the comments.
 
Thanks again.
 
-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] Warning about invalid .pgpass passwords

Bruce Momjian br...@momjian.us writes:
 Alvaro Herrera wrote:
 This bit seems strange ...  I think we just do strcmp() to compare sqlstates?

 Uh, the PQresultErrorField is a string, while
 ERRCODE_INVALID_PASSWORD_SPECIFICATI is a 4-byte value in base-64.  
 Yea, it's true.  For excitement, see the MAKE_SQLSTATE macro.

I don't particularly believe in doing things this way: I think that
libpq should just hardwire the string the same as it's doing for the
other specific sqlstate it's checking for.  If we do this at all,
then we are effectively embedding that sqlstate value in the protocol.
We are not going to be able to change it later.  Doing it like this
opens us up to randomly using errcodes in the frontend without realizing
that we've thereby lost the freedom to change them.

Even if you insist on including errcodes.h into the frontend code
despite that, this is an ugly brute-force way of solving the problem.
The intended way of solving the problem was to redefine MAKE_SQLSTATE
before including the file, in .c files where you need a different
representation.

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] plperl db access documentation enhancement

Bruce Momjian wrote:
 Alvaro Herrera wrote:
  Bruce Momjian wrote:
   
   Patch applied for 9.0.  We don't normally backpatch such documentation
   improvements unless we receive multiple reports of confusion.
  
  I think that's a mistake in this case.  The documentation wasn't
  confusing -- it was bogus.  (Actually, the bug fixing is a smaller
  change than the whole of this patch, so we could provide the smaller
  patch if desired to apply to 8.4.  To be honest I think it is better to
  just apply the larger patch verbatim.)
  
  I could do the backpatch if you want.  I just wanted some more peer
  review on the changes.
 
 Sure, go ahead if you are sure. I wasn't clear enough to risk it, and
 documentation churn in back branches has its own downsides, which is why
 I avoid it, especially for larger patches.

Just applied it.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
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] Warning about invalid .pgpass passwords

Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  Alvaro Herrera wrote:
  This bit seems strange ...  I think we just do strcmp() to compare 
  sqlstates?
 
  Uh, the PQresultErrorField is a string, while
  ERRCODE_INVALID_PASSWORD_SPECIFICATION is a 4-byte value in base-64.  
  Yea, it's true.  For excitement, see the MAKE_SQLSTATE macro.
 
 I don't particularly believe in doing things this way: I think that
 libpq should just hardwire the string the same as it's doing for the
 other specific sqlstate it's checking for.  If we do this at all,
 then we are effectively embedding that sqlstate value in the protocol.
 We are not going to be able to change it later.  Doing it like this
 opens us up to randomly using errcodes in the frontend without realizing
 that we've thereby lost the freedom to change them.
 
 Even if you insist on including errcodes.h into the frontend code
 despite that, this is an ugly brute-force way of solving the problem.
 The intended way of solving the problem was to redefine MAKE_SQLSTATE
 before including the file, in .c files where you need a different
 representation.

OK, just defined it as a constant string. I am not a big fan of
redefining macros, especially ones that are referenced in later include
files.

Is this going to cause problems for client applications that only test
for ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION?  I doubt many of them
are testing for just the first two digits.

Is there anywhere else we should be testing for this new sqlstate value?

Updated patch attached.

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

  PG East:  http://www.enterprisedb.com/community/nav-pg-east-2010.do
Index: doc/src/sgml/errcodes.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/errcodes.sgml,v
retrieving revision 1.28
diff -c -c -r1.28 errcodes.sgml
*** doc/src/sgml/errcodes.sgml	7 Dec 2009 05:22:21 -	1.28
--- doc/src/sgml/errcodes.sgml	11 Mar 2010 22:40:24 -
***
*** 761,766 
--- 761,772 
  entryinvalid_authorization_specification/entry
  /row
  
+ row
+ entryliteral28001/literal/entry
+ entryINVALID PASSWORD SPECIFICATION/entry
+ entryinvalid_password_specification/entry
+ /row
+ 
  
  row
  entry spanname=span13emphasis role=boldClass 2B mdash; Dependent Privilege Descriptors Still Exist//entry
Index: src/backend/libpq/auth.c
===
RCS file: /cvsroot/pgsql/src/backend/libpq/auth.c,v
retrieving revision 1.195
diff -c -c -r1.195 auth.c
*** src/backend/libpq/auth.c	26 Feb 2010 02:00:42 -	1.195
--- src/backend/libpq/auth.c	11 Mar 2010 22:40:24 -
***
*** 232,238 
  auth_failed(Port *port, int status)
  {
  	const char *errstr;
! 
  	/*
  	 * If we failed due to EOF from client, just quit; there's no point in
  	 * trying to send a message to the client, and not much point in logging
--- 232,239 
  auth_failed(Port *port, int status)
  {
  	const char *errstr;
! 	int		errcode_return = ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION;
! 	
  	/*
  	 * If we failed due to EOF from client, just quit; there's no point in
  	 * trying to send a message to the client, and not much point in logging
***
*** 269,274 
--- 270,277 
  		case uaMD5:
  		case uaPassword:
  			errstr = gettext_noop(password authentication failed for user \%s\);
+ 			/* We use it to indicate if a .pgpass password failed. */
+ 			errcode_return = ERRCODE_INVALID_PASSWORD_SPECIFICATION;
  			break;
  		case uaPAM:
  			errstr = gettext_noop(PAM authentication failed for user \%s\);
***
*** 285,291 
  	}
  
  	ereport(FATAL,
! 			(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
  			 errmsg(errstr, port-user_name)));
  	/* doesn't return */
  }
--- 288,294 
  	}
  
  	ereport(FATAL,
! 			(errcode(errcode_return),
  			 errmsg(errstr, port-user_name)));
  	/* doesn't return */
  }
Index: src/include/utils/errcodes.h
===
RCS file: /cvsroot/pgsql/src/include/utils/errcodes.h,v
retrieving revision 1.31
diff -c -c -r1.31 errcodes.h
*** src/include/utils/errcodes.h	2 Jan 2010 16:58:10 -	1.31
--- src/include/utils/errcodes.h	11 Mar 2010 22:40:25 -
***
*** 194,199 
--- 194,200 
  
  /* Class 28 - Invalid Authorization Specification */
  #define ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION MAKE_SQLSTATE('2','8', '0','0','0')
+ #define ERRCODE_INVALID_PASSWORD_SPECIFICATION MAKE_SQLSTATE('2','8', '0','0','1')
  
  /* Class 2B - Dependent Privilege Descriptors Still Exist */
  #define ERRCODE_DEPENDENT_PRIVILEGE_DESCRIPTORS_STILL_EXIST		MAKE_SQLSTATE('2','B', '0','0','0')
Index: src/interfaces/libpq/fe-connect.c
===
RCS file:

Re: [HACKERS] Warning about invalid .pgpass passwords

Bruce Momjian br...@momjian.us writes:
 ERRCODE_INVALID_PASSWORD_SPECIFICATION

BTW, why not just ERRCODE_INVALID_PASSWORD?  The extra word doesn't
seem to promote anything except carpal tunnel syndrome.

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] Warning about invalid .pgpass passwords

Bruce Momjian br...@momjian.us writes:
 + #define ERRCODE_INVALID_PASSWORD_SPECIFICATION MAKE_SQLSTATE('2','8', 
 '0','0','1')

Oh, another thought: you're infringing on SQL-committee-controlled code
space there.  Probably 28P01 would be a safer choice.

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] buildfarm logging versus embedded nulls

I was looking at this recent nonrepeatable buildfarm failure:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=polecatdt=2010-03-11%2021:45:10
which has several instances of the known pgstat wait timeout problem
during the parallel regression tests.

I was disappointed to see that the postmaster log part of the report
is truncated near the start of the run, so there's no way to see if
anything interesting got logged near the point of the failure.

When I run make check on my own OS X machine, I notice that the
postmaster.log file usually has some runs of a few dozen null bytes in
it.  I suspect this is an artifact of Apple's stdio buffering
implementation when several backends are writing to the same log file.
I suppose that what happened in the above case is that some nulls got
embedded in postmaster.log, and then the file got truncated at the first
null, perhaps during the upload to the buildfarm server, or maybe it's
intact on the server but the web page is failing to display anything
past that point.

There's probably not much we can do about Apple's stdio (and I would bet
that they inherited this behavior from the BSDen anyway).  What we
*could* do is

(1) encourage buildfarm owners to run the tests with logging_collector
turned on, and/or

(2) fix the buildfarm reporting mechanisms to not be fazed by nulls in
the log files.

I have no clear idea how hard either of these things is.

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] renameatt() can rename attribute of index, sequence, ...

2010-03-11 Thread KaiGai Kohei

(2010/03/11 23:55), Robert Haas wrote:
 2010/3/10 KaiGai Koheikai...@ak.jp.nec.com:
 Indeed, it is useful to allow renaming attribute of composite types.

 However, it is also useless to allow to rename attribute of sequences,
 but harmless, like renames on indexes. It seems to me it is fair enough
 to allow renaming attributes of tables, views and composite types...
 
 I don't agree.  I think users should be allowed to rename things they
 had a hand in naming in the first place (and index columns fall into
 that category, since the names are derived from table column names).
 But changing system-assigned column names for sequences or toast
 tables is just weird.

OK, the attached patch forbid to rename an attribute of relations except
for tables, views, composite types and indexes.

Thanks,
-- 
KaiGai Kohei kai...@ak.jp.nec.com


pgsql-renameatt-check-relkind.2.patch
Description: application/octect-stream

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

Re: [HACKERS] [BUGS] log : bad file dscriptor????

2010-03-11 Thread Gurjeet Singh

On Wed, Mar 18, 2009 at 3:47 AM, Heikki Linnakangas 
heikki.linnakan...@enterprisedb.com wrote:

 Gurjeet Singh wrote:

On Windows, the write to log file is done by a thread (whose main
 function is pipeThread() ), and since it works completely independent of
 the
 SysLoggerMain() ( which is responsible for calling logfile_rotate()
 periodically, which in turn changes the global variable syslogFile) this
 is
 causing a race condition due to an error in the way we are using the
 related
 critical section.


 Thanks, committed. I hope this helps with Ati's problems.


It seems this commit never made it to the release notes. A customer came
asking for the fix to this very problem, and although we know that the issue
has been fixed, we could not refer him to the release notes. All we could
suggest was to do the minor upgrade.

I could not find any more reports of this bug after the fix was committed.
So what would it take to make release notes say that it has been fixed?

Best regards,
-- 
gurjeet.singh
@ EnterpriseDB - The Enterprise Postgres Company
http://www.enterprisedb.com

singh.gurj...@{ gmail | yahoo }.com
Twitter/Skype: singh_gurjeet

Mail sent from my BlackLaptop device

Re: [HACKERS] Warning about invalid .pgpass passwords

Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  + #define ERRCODE_INVALID_PASSWORD_SPECIFICATION MAKE_SQLSTATE('2','8', 
  '0','0','1')
 
 Oh, another thought: you're infringing on SQL-committee-controlled code
 space there.  Probably 28P01 would be a safer choice.

OK, updated patch with code 28P01, and I shorted the name (now
ERRCODE_INVALID_PASSWORD).

I thought carpal tunnel was the goal of these error codes.  ;-)

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

  PG East:  http://www.enterprisedb.com/community/nav-pg-east-2010.do
Index: doc/src/sgml/errcodes.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/errcodes.sgml,v
retrieving revision 1.28
diff -c -c -r1.28 errcodes.sgml
*** doc/src/sgml/errcodes.sgml	7 Dec 2009 05:22:21 -	1.28
--- doc/src/sgml/errcodes.sgml	12 Mar 2010 00:37:47 -
***
*** 761,766 
--- 761,772 
  entryinvalid_authorization_specification/entry
  /row
  
+ row
+ entryliteral28P01/literal/entry
+ entryINVALID PASSWORD/entry
+ entryinvalid_password/entry
+ /row
+ 
  
  row
  entry spanname=span13emphasis role=boldClass 2B mdash; Dependent Privilege Descriptors Still Exist//entry
Index: src/backend/libpq/auth.c
===
RCS file: /cvsroot/pgsql/src/backend/libpq/auth.c,v
retrieving revision 1.195
diff -c -c -r1.195 auth.c
*** src/backend/libpq/auth.c	26 Feb 2010 02:00:42 -	1.195
--- src/backend/libpq/auth.c	12 Mar 2010 00:37:49 -
***
*** 232,238 
  auth_failed(Port *port, int status)
  {
  	const char *errstr;
! 
  	/*
  	 * If we failed due to EOF from client, just quit; there's no point in
  	 * trying to send a message to the client, and not much point in logging
--- 232,239 
  auth_failed(Port *port, int status)
  {
  	const char *errstr;
! 	int		errcode_return = ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION;
! 	
  	/*
  	 * If we failed due to EOF from client, just quit; there's no point in
  	 * trying to send a message to the client, and not much point in logging
***
*** 269,274 
--- 270,277 
  		case uaMD5:
  		case uaPassword:
  			errstr = gettext_noop(password authentication failed for user \%s\);
+ 			/* We use it to indicate if a .pgpass password failed. */
+ 			errcode_return = ERRCODE_INVALID_PASSWORD;
  			break;
  		case uaPAM:
  			errstr = gettext_noop(PAM authentication failed for user \%s\);
***
*** 285,291 
  	}
  
  	ereport(FATAL,
! 			(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
  			 errmsg(errstr, port-user_name)));
  	/* doesn't return */
  }
--- 288,294 
  	}
  
  	ereport(FATAL,
! 			(errcode(errcode_return),
  			 errmsg(errstr, port-user_name)));
  	/* doesn't return */
  }
Index: src/include/utils/errcodes.h
===
RCS file: /cvsroot/pgsql/src/include/utils/errcodes.h,v
retrieving revision 1.31
diff -c -c -r1.31 errcodes.h
*** src/include/utils/errcodes.h	2 Jan 2010 16:58:10 -	1.31
--- src/include/utils/errcodes.h	12 Mar 2010 00:37:50 -
***
*** 194,199 
--- 194,200 
  
  /* Class 28 - Invalid Authorization Specification */
  #define ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION MAKE_SQLSTATE('2','8', '0','0','0')
+ #define ERRCODE_INVALID_PASSWORD MAKE_SQLSTATE('2','8', 'P','0','1')
  
  /* Class 2B - Dependent Privilege Descriptors Still Exist */
  #define ERRCODE_DEPENDENT_PRIVILEGE_DESCRIPTORS_STILL_EXIST		MAKE_SQLSTATE('2','B', '0','0','0')
Index: src/interfaces/libpq/fe-connect.c
===
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.389
diff -c -c -r1.389 fe-connect.c
*** src/interfaces/libpq/fe-connect.c	3 Mar 2010 20:31:09 -	1.389
--- src/interfaces/libpq/fe-connect.c	12 Mar 2010 00:37:51 -
***
*** 91,96 
--- 91,99 
   */
  #define ERRCODE_APPNAME_UNKNOWN 42704
  
+ /* This is part of the protocol so just define it */
+ #define ERRCODE_INVALID_PASSWORD 28P01
+ 
  /*
   * fall back options if they are not specified by arguments or defined
   * by environment variables
***
*** 284,289 
--- 287,293 
  static char *pwdfMatchesString(char *buf, char *token);
  static char *PasswordFromFile(char *hostname, char *port, char *dbname,
   char *username);
+ static void dot_pg_pass_warning(PGconn *conn);
  static void default_threadlock(int acquire);
  
  
***
*** 652,657 
--- 656,663 
  		conn-dbName, conn-pguser);
  		if (conn-pgpass == NULL)
  			conn-pgpass = strdup(DefaultPassword);
+ 		else
+ 			conn-dot_pgpass_used = true;
  	}
  
  	/*
***
*** 2133,2138 
--- 2139,2146 
  
  error_return:
  
+ 	dot_pg_pass_warning(conn);
+ 	
  	/*
  	 * We used to close the

Re: [HACKERS] [patch] build issues on Win32

2010-03-11 Thread David Fetter

On Thu, Mar 11, 2010 at 01:34:34PM -0800, Steve Atkins wrote:
 On Mar 11, 2010, at 1:06 PM, Tom Lane wrote:
  
  BTW, I'm not sure I buy the argument that commercial software
  requires static linking.  Red Hat would be as interested in that
  market as anybody, and as I said, they don't think it's necessary
  to ship static libraries (with a *very* short list of exceptions).
 
 Trying to support a binary distributed[1] application on a dozen or
 more different distributions is fairly painful (I'm building on four
 Debians, eight Ubuntus, several RHELs, three Solarises and a few
 others right now), and the biggest problem is the different
 distributions don't package all the libraries you need, and when
 they do they include a wide range of versions, often woefully
 obsolete.

I've found it easiest to ship all of PostgreSQL rather than deal with
this kind of stuff.

Cheers,
David.
-- 
David Fetter da...@fetter.org http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

-- 
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] buildfarm logging versus embedded nulls




Tom Lane wrote:

I was looking at this recent nonrepeatable buildfarm failure:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=polecatdt=2010-03-11%2021:45:10
which has several instances of the known pgstat wait timeout problem
during the parallel regression tests.

I was disappointed to see that the postmaster log part of the report
is truncated near the start of the run, so there's no way to see if
anything interesting got logged near the point of the failure.

When I run make check on my own OS X machine, I notice that the
postmaster.log file usually has some runs of a few dozen null bytes in
it.  I suspect this is an artifact of Apple's stdio buffering
implementation when several backends are writing to the same log file.
I suppose that what happened in the above case is that some nulls got
embedded in postmaster.log, and then the file got truncated at the first
null, perhaps during the upload to the buildfarm server, or maybe it's
intact on the server but the web page is failing to display anything
past that point.

There's probably not much we can do about Apple's stdio (and I would bet
that they inherited this behavior from the BSDen anyway).  What we
*could* do is

(1) encourage buildfarm owners to run the tests with logging_collector
turned on, and/or

(2) fix the buildfarm reporting mechanisms to not be fazed by nulls in
the log files.

I have no clear idea how hard either of these things is.


  


Well, the good news is that we actually have the data on the server, in 
a temp file that will be cleaned up, but hasn't been yet. I have placed 
a copy at http://buildfarm.postgresql.org/polecat-check.log.gz. And 
thus we know that the client does exactly what you want, and the problem 
is entirely on the server. That's more good news.


Now, the log_text field in our build_status_log table is text, so it's 
on insertion to the database that it gets truncated. I'm thinking that I 
should just escape nulls with a regex ( 's/\x00/\\0/g' or similar) 
before inserting the data.


Anyone got any better ideas?

(BTW, your idea of using logging_collector won't work without 
significant changes in the buildfarm client. Nice idea, though.)


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] Warning about invalid .pgpass passwords

2010-03-11 Thread Robert Haas

On Thu, Mar 11, 2010 at 4:19 PM, Bruce Momjian br...@momjian.us wrote:
        $ sql -h localhost test
        psql: FATAL:  password authentication failed for user postgres
        password retrieved from .pgpass

I find this not quite explicit enough for my taste.  Maybe something like this?

the failing password was retrieved from .pgpass

...Robert

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

Re: [HACKERS] Warning about invalid .pgpass passwords

Robert Haas wrote:
 On Thu, Mar 11, 2010 at 4:19 PM, Bruce Momjian br...@momjian.us wrote:
  ? ? ? ?$ sql -h localhost test
  ? ? ? ?psql: FATAL: ?password authentication failed for user postgres
  ? ? ? ?password retrieved from .pgpass
 
 I find this not quite explicit enough for my taste.  Maybe something like 
 this?
 
 the failing password was retrieved from .pgpass

Uh, not sure.  That doesn't match the style of many of our other
messages.

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

  PG East:  http://www.enterprisedb.com/community/nav-pg-east-2010.do

-- 
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] Warning about invalid .pgpass passwords




Bruce Momjian wrote:

+   /* If it was 'invalid authorization', add .pgpass mention */
+   if (conn-dot_pgpass_used  conn-password_needed  conn-result 
+   /* only works with = 9.0 servers */
+   strcmp(PQresultErrorField(conn-result, PG_DIAG_SQLSTATE),
+   ERRCODE_INVALID_PASSWORD_SPECIFICATION) == 0)
+   appendPQExpBufferStr(conn-errorMessage,
+   libpq_gettext(password retrieved from .pgpass\n));
  


Surely we should use the name of the actual file from which the password 
was retrieved here, which could be quite different from .pgpass (see 
PGPASSFILE environment setting) and is different by default on Windows 
anyway. Using a hardcoded .pgpass in those situations could be quite 
confusing.


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] Dyamic updates of NEW with pl/pgsql

2010-03-11 Thread Merlin Moncure

On Wed, Mar 10, 2010 at 7:50 AM, Andrew Dunstan and...@dunslane.net wrote:
 2010/3/9 strk s...@keybit.net:
 How can a pl/pgsql trigger change the
 values of dynamic fields in NEW record ?

 By dynamic I mean that the field name
 is a variable in the trigger context.

 I've been told it's easy to do with pl/perl but
 I'd like to delive a pl/pgsql solution to have
 less dependencies.

 Using an hstore in 9.0 it's not too bad, Try something like:


Agree 100%.  The new hstore going to completely nail a broad class of
issues that have historically been awkward in plpgsql functions.
(small aside: the other biggie would be able to push a composite type
in to an update statement...something like 'update foo set foo =
new').  This is really great...some variant of this question is
continually asked it seems.

merlin

-- 
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] Dyamic updates of NEW with pl/pgsql

Merlin Moncure escribió:


 (small aside: the other biggie would be able to push a composite type
 in to an update statement...something like 'update foo set foo =
 new').  This is really great...some variant of this question is
 continually asked it seems.

Can't you already do that with EXECUTE ... USING NEW?  hmm, ah, but you
have to specify the columns in NEW, so it doesn't really work for you,
does it?

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

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

[HACKERS] Reposnse from backend when wrong user/database request send

2010-03-11 Thread Tatsuo Ishii

It seems between 8.4 and CVS HEAD backend responses 'E' packet
(error/fatal message) if a startup packet sent with wrong user and/or
database. Before backend responses 'R' packet first followd by 'E'
packet.

Does anybody know why this change made? I do not againt this change
because this is correct behavior. Just Pgpool has to live with
previous behavior (it was wrong IMO) and recent change not to break
existing pgpool applications.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp

-- 
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] [GENERAL] trouble with to_char('L')

2010-03-11 Thread Takahiro Itagaki


Bruce Momjian br...@momjian.us wrote:

 OK, I have created a new function, win32_wchar_to_db_encoding(), to
 share the conversion from wide characters to the database encoding.
 New patch attached.

Since 9.0 has GetPlatformEncoding() for the purpose, we could simplify
db_encoding_strdup() with the function. Like this:

static char *
db_encoding_strdup(const char *str)
{
char   *pstr;
char   *mstr;

/* convert the string to the database encoding */
pstr = (char *) pg_do_encoding_conversion(
(unsigned char *) str, 
strlen(str),
GetPlatformEncoding(), 
GetDatabaseEncoding());
mstr = strdup(pstr);
if (pstr != str)
pfree(pstr);

return mstr;
}

I beleive the code is harmless on all platforms and we can use it
instead of strdup() without any #ifdef WIN32 quotes.


BTW, I found we'd better to add ANSI_X3.4-1968 as an alias for
PG_SQL_ASCII. My Fedora 12 returns the name when --no-locale is used.

Regards,
---
Takahiro Itagaki
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] Server crash with older tzload library

2010-03-11 Thread Heikki Linnakangas

Tom Lane wrote:
 Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
 No, Jeevan is talking about tzcode, not tzdata. The zoneinfo library is
 split into two parts, we update the data part at each release, but we
 don't sync up our code with upstream code changes regularly.
 
 It strikes me that maybe we are putting ourselves at risk by blithely
 pushing tzdata updates into back branches without also pushing tzcode
 updates.

I believe they're designed to be compatible both ways, I remember that
the 64-bit changes in particular were done in such a way that new tzdata
files work with older tzcode versions. I don't know if anyone else is
testing various combinations, though, so it probably would be good to
update tzcode anyway.

  However, doing this would mean updating the back branches for
 64bit tzdata, which is not a small change.  Heikki, do you remember how
 much that patch affected stuff outside the tzcode files proper?

There was no changes outside tzcode files.

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

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

Re: [HACKERS] gothic_moth, codlin_moth failures on REL8_2_STABLE