Re: [HACKERS] spinlocks on HP-UX

2011-10-18 Thread Pavan Deolasee
On Tue, Oct 18, 2011 at 10:04 AM, Robert Haas robertmh...@gmail.com wrote:

 Hmm, so you added the non-locked test in TAS()?  Did you try adding it
 just to TAS_SPIN()?  On Itanium, I found that it was slightly better
 to do it only in TAS_SPIN() - i.e. in the contended case.


Would it be a good change for S_LOCK() to use TAS_SPIN()  as well ?

Thanks,
Pavan

-- 
Pavan Deolasee
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] Online base backup from the hot-standby

2011-10-18 Thread Jun Ishiduka

 + /*
 +  * The backend writes WAL of FPW at checkpoint. However, The 
 backend do
 +  * not need to write WAL of FPW at checkpoint shutdown because 
 it
 +  * performs when startup finishes.
 +  */
 + XLogReportParameters(REPORT_ON_BACKEND);
 
 I'm still unclear why that WAL doesn't need to be written at shutdown
 checkpoint.
 Anyway, the first sentence in the above comments is not right. Not a backend 
 but
 a bgwriter writes that WAL at checkpoint.
 
 The second also seems not to be right. It implies that a shutdown checkpoint 
 is
 performed only at end of startup. But it may be done when smart or fast 
 shutdown
 is requested.


Okay. 
I change to the following messages.

/* 
 * The bgwriter writes WAL of FPW at checkpoint. But does not at shutdown.
 * Because XLogReportParameters() is always called at the end of startup
 * process, it does not need to be called at shutdown.
 */


In addition, I change macro name.

REPORT_ON_BACKEND - REPORT_ON_BGWRITER


Regards.


Jun Ishizuka
NTT Software Corporation
TEL:045-317-7018
E-Mail: ishizuka@po.ntts.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] BUG or strange behaviour of update on primary key

2011-10-18 Thread desmodemone
Hi there,
  I could workaround the behavior with deferred constraint, and
it's ok, but as I show, I have different behavior for constraint with the
same definition in two rdbms and Postgresql depends on the physical order of
row (with the same definition of constraint NOT DEFERRABLE INITIALLY
IMMEDIATE) , or better Postgresql seems to check for every row, even if the
command is one (I am doing one update on all of rows) , right?  .

Moreover , in documentation the definition says that a not deferrable
constraints will check after every command , not after every row of the
command:

http://www.postgresql.org/docs/9.1/static/sql-createtable.html

DEFERRABLE
NOT DEFERRABLE

This controls whether the constraint can be deferred.* A constraint that is
not deferrable will be checked immediately after every command*. Checking of
constraints that are deferrable can be postponed until the end of the
transaction (using the SET
CONSTRAINTShttp://www.postgresql.org/docs/9.0/static/sql-set-constraints.html
 command). NOT DEFERRABLE is the default. Currently, only UNIQUE, PRIMARY
KEY, EXCLUDE, and REFERENCES (foreign key) constraints accept this clause. NOT
NULL and CHECK constraints are not deferrable.
---

If this is historical buggy behavior for performance , I think we have to
change  the definition of NOT DEFERRABLE in documentation,
because Postgresql is not checking at end of a dml, but for every row
modified by the command or   there is something needs a patch.


Regards, Mat

2011/10/18 Robert Haas robertmh...@gmail.com

 On Mon, Oct 17, 2011 at 7:30 PM, desmodemone desmodem...@gmail.com
 wrote:
  Seems an Oracle bug not Postgresql one!

 I don't think it's a bug for it to work.  It'd probably work in
 PostgreSQL too, if you inserted (2) first and then (1).  It's just
 that, as Tom says, if you want it to be certain to work (rather than
 depending on the order in which the rows are inserted), you need the
 checks to be deferred.

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



Re: [HACKERS] WIP: Collecting statistics on CSV file data

2011-10-18 Thread Leonardo Francalanci
 New API AnalyzeForeignTable



I didn't look at the patch, but I'm using CSV foreign tables with named pipes
to get near-realtime KPI calculated by postgresql. Of course, pipes can be
read just once, so I wouldn't want an automatic analyze of foreign tables...

-- 
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) regression diffs on collate.linux.utf8 test

2011-10-18 Thread Jeff Davis
On Sun, 2011-10-16 at 16:00 -0400, Tom Lane wrote:
 Jeff Davis pg...@j-davis.com writes:
  On master, I see a minor test error (at least on my machine) as well as
  a diff. Patch attached.
 
 Hmm, yeah, I forgot to fix this regression test when I added that DETAIL
 line.  However, I don't see the need for fooling with the lc_time value?
 
   regards, tom lane

Here is the diff that I'm seeing on master right now with:

  make -s check EXTRA_TESTS=collate.linux.utf8

If I qualify it as tr_TR.UTF-8 it works. Perhaps I have something
misconfigured on my system (Ubuntu 11.10)? I just installed:
  language-pack-de
  language-pack-tr
  language-pack-sv

in an attempt to make the test work, and it works all except for that
lc_time settng.

Regards,
Jeff Davis
*** /home/jdavis/wd/git/postgresql/src/test/regress/expected/collate.linux.utf8.out	2011-10-18 00:47:06.817223853 -0700
--- /home/jdavis/wd/git/postgresql/src/test/regress/results/collate.linux.utf8.out	2011-10-18 01:02:06.509206748 -0700
***
*** 396,411 
  
  -- to_char
  SET lc_time TO 'tr_TR';
  SELECT to_char(date '2010-04-01', 'DD TMMON ');
 to_char   
  -
!  01 NIS 2010
  (1 row)
  
  SELECT to_char(date '2010-04-01', 'DD TMMON ' COLLATE tr_TR);
 to_char   
  -
!  01 NİS 2010
  (1 row)
  
  -- backwards parsing
--- 396,412 
  
  -- to_char
  SET lc_time TO 'tr_TR';
+ ERROR:  invalid value for parameter lc_time: tr_TR
  SELECT to_char(date '2010-04-01', 'DD TMMON ');
 to_char   
  -
!  01 APR 2010
  (1 row)
  
  SELECT to_char(date '2010-04-01', 'DD TMMON ' COLLATE tr_TR);
 to_char   
  -
!  01 APR 2010
  (1 row)
  
  -- backwards parsing

==


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


[HACKERS] new compiler warnings

2011-10-18 Thread Jeff Davis
I'm not sure if these can/should be fixed or not, but here are the
compiler warnings I'm getting on gcc and clang on ubuntu 11.10 with -O2.
The gcc ones are mostly new.

 GCC 

$ gcc --version
gcc (Ubuntu/Linaro 4.6.1-9ubuntu3) 4.6.1
Copyright (C) 2011 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is
NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
PURPOSE.

warnings generated:

execQual.c: In function ‘GetAttributeByNum’:
execQual.c:1112:11: warning: the comparison will always evaluate as
‘true’ for the address of ‘tmptup’ will never be NULL [-Waddress]
execQual.c: In function ‘GetAttributeByName’:
execQual.c:1173:11: warning: the comparison will always evaluate as
‘true’ for the address of ‘tmptup’ will never be NULL [-Waddress]
execQual.c: In function ‘ExecEvalFieldSelect’:
execQual.c:3922:11: warning: the comparison will always evaluate as
‘true’ for the address of ‘tmptup’ will never be NULL [-Waddress]
In file included from gram.y:12962:0:
scan.c: In function ‘yy_try_NUL_trans’:
scan.c:16257:23: warning: unused variable ‘yyg’ [-Wunused-variable]
elog.c: In function ‘write_pipe_chunks’:
elog.c:2479:8: warning: ignoring return value of ‘write’, declared with
attribute warn_unused_result [-Wunused-result]
elog.c:2488:7: warning: ignoring return value of ‘write’, declared with
attribute warn_unused_result [-Wunused-result]
elog.c: In function ‘write_console’:
elog.c:1797:7: warning: ignoring return value of ‘write’, declared with
attribute warn_unused_result [-Wunused-result]
tuplesort.c: In function ‘comparetup_heap’:
tuplesort.c:2751:12: warning: the comparison will always evaluate as
‘true’ for the address of ‘ltup’ will never be NULL [-Waddress]
tuplesort.c:2752:12: warning: the comparison will always evaluate as
‘true’ for the address of ‘rtup’ will never be NULL [-Waddress]
tuplesort.c: In function ‘copytup_heap’:
tuplesort.c:2783:17: warning: the comparison will always evaluate as
‘true’ for the address of ‘htup’ will never be NULL [-Waddress]
tuplesort.c: In function ‘readtup_heap’:
tuplesort.c:2835:17: warning: the comparison will always evaluate as
‘true’ for the address of ‘htup’ will never be NULL [-Waddress]
fe-connect.c: In function ‘PQconndefaults’:
fe-connect.c:832:6: warning: the comparison will always evaluate as
‘false’ for the address of ‘errorBuf’ will never be NULL [-Waddress]
fe-connect.c: In function ‘PQconninfoParse’:
fe-connect.c:3970:6: warning: the comparison will always evaluate as
‘false’ for the address of ‘errorBuf’ will never be NULL [-Waddress]
common.c: In function ‘handle_sigint’:
common.c:247:4: warning: ignoring return value of ‘write’, declared with
attribute warn_unused_result [-Wunused-result]
common.c:250:4: warning: ignoring return value of ‘write’, declared with
attribute warn_unused_result [-Wunused-result]
common.c:251:4: warning: ignoring return value of ‘write’, declared with
attribute warn_unused_result [-Wunused-result]
In file included from mainloop.c:425:0:
psqlscan.l: In function ‘evaluate_backtick’:
psqlscan.l:1677:6: warning: the comparison will always evaluate as
‘false’ for the address of ‘cmd_output’ will never be NULL [-Waddress]

 CLANG 

$ clang --version
clang version 2.9 (tags/RELEASE_29/final)
Target: x86_64-pc-linux-gnu
Thread model: posix

A lot of warnings of the form:

ruleutils.c:698:11: warning: array index of '1' indexes past the end of
an array (that contains 1 elements) [-Warray-bounds]
value = fastgetattr(ht_trig, Anum_pg_trigger_tgargs,
^~~~
In file included from ruleutils.c:23:
In file included from ../../../../src/include/catalog/indexing.h:18:
../../../../src/include/access/htup.h:808:3: note: instantiated from:
att_isnull((attnum)-1, (tup)-t_data-t_bits) ?
\
^
In file included from ruleutils.c:23:
In file included from ../../../../src/include/catalog/indexing.h:18:
In file included from ../../../../src/include/access/htup.h:18:
../../../../src/include/access/tupmacs.h:21:34: note: instantiated from:
#define att_isnull(ATT, BITS) (!((BITS)[(ATT)  3]  (1  ((ATT) 
0x07
 ^  ~~
In file included from ruleutils.c:23:
In file included from ../../../../src/include/catalog/indexing.h:18:
../../../../src/include/access/htup.h:153:9: note: array 't_bits'
declared here
bits8   t_bits[1];  /* bitmap of NULLs --
VARIABLE LENGTH */

==

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] HeapTupleHeaderAdvanceLatestRemovedXid doing the wrong thing with multixacts

2011-10-18 Thread Simon Riggs
On Tue, Oct 18, 2011 at 3:14 AM, Alvaro Herrera alvhe...@alvh.no-ip.org wrote:

 I'll add an assert to check this and a comment to explain.

 This means I'll have to hack it up further in my FK locks patch.  No problem 
 with that.

OK, I'll hold back to avoid interfering with your patch.

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

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


Re: [HACKERS] BUG or strange behaviour of update on primary key

2011-10-18 Thread desmodemone
 2011/10/18 Robert Haas robertmh...@gmail.com

 On Mon, Oct 17, 2011 at 7:30 PM, desmodemone desmodem...@gmail.com
 wrote:
  Seems an Oracle bug not Postgresql one!

 I don't think it's a bug for it to work.  It'd probably work in
 PostgreSQL too, if you inserted (2) first and then (1).  It's just
 that, as Tom says, if you want it to be certain to work (rather than
 depending on the order in which the rows are inserted), you need the
 checks to be deferred.

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



 2011/10/18 desmodemone desmodem...@gmail.com
 Hi there,
   I could workaround the behavior with deferred constraint, and
 it's ok, but as I show, I have different behavior for constraint with the
 same definition in two rdbms and Postgresql depends on the physical order of
 row (with the same definition of constraint NOT DEFERRABLE INITIALLY
 IMMEDIATE) , or better Postgresql seems to check for every row, even if
 the command is one (I am doing one update on all of rows) , right?  .

 Moreover , in documentation the definition says that a not deferrable
 constraints will check after every command , not after every row of the
 command:

 http://www.postgresql.org/docs/9.1/static/sql-createtable.html

 DEFERRABLE
 NOT DEFERRABLE

 This controls whether the constraint can be deferred.* A constraint that
 is not deferrable will be checked immediately after every command*.
 Checking of constraints that are deferrable can be postponed until the end
 of the transaction (using the SET 
 CONSTRAINTShttp://www.postgresql.org/docs/9.0/static/sql-set-constraints.html
  command). NOT DEFERRABLE is the default. Currently, only UNIQUE, PRIMARY
 KEY, EXCLUDE, and REFERENCES (foreign key) constraints accept this clause.
  NOT NULL and CHECK constraints are not deferrable.
 ---

 If this is historical buggy behavior for performance , I think we have to
 change  the definition of NOT DEFERRABLE in documentation,
 because Postgresql is not checking at end of a dml, but for every row
 modified by the command or   there is something needs a patch.


 Regards, Mat




Hello there,

I think I have find a limit of this workaround. Imagine I have two tables in
Oracle or other rdbms with a foreign key between them :

testup3 ( a int)  primary key on a NOT DEFERRABLE INITIALLY IMMEDIATE ;

testup4 ( a int)  foreign key on a references testup3(a)  ;

For first table I could create this (to have a normal sql standard
behavior on update with multiple rows) :

testup3 ( a int)  primary key on a DEFERRABLE INITIALLY IMMEDIATE ;

By the way I could not create a   foreign key on a DEFERRABLE constraint ,
in fact I obtain an  error like this :

ERROR:  cannot use a deferrable unique constraint for referenced table


So if I  have a normal ERD schema with FK , I could not use the workaround
of DEFERRABLE constraints .
I found an old discussion on this  :
http://archives.postgresql.org/pgsql-hackers/2010-06/msg00151.php
http://archives.postgresql.org/pgsql-hackers/2010-06/msg00168.php

In my opinion it could be a big limitation for who want  migrate
applications or is developing  applications on different db.

Any suggest or idea ?

Regards, Mat


[HACKERS] Silent failure with invalid hba_file setting

2011-10-18 Thread Thom Brown
Hi,

I noticed that if the hba_file setting in the config is uncommented
and set to a directory instead of the full path to the file, no error
occurs when the service starts.

For example:

hba_file = '/home/thom/Development/data'

The problem with this is you cannot get into the database as it acts
as if it did find the hba file but found it empty.

Shouldn't a check be in place to ensure that the parameter resolves to
a file rather than anything else?  And even if it does find a file,
but it's empty, shouldn't it also produce a warning of some kind?

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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] (patch) regression diffs on collate.linux.utf8 test

2011-10-18 Thread Tom Lane
Jeff Davis pg...@j-davis.com writes:
 On Sun, 2011-10-16 at 16:00 -0400, Tom Lane wrote:
 Hmm, yeah, I forgot to fix this regression test when I added that DETAIL
 line.  However, I don't see the need for fooling with the lc_time value?

 Here is the diff that I'm seeing on master right now with:
   make -s check EXTRA_TESTS=collate.linux.utf8

 If I qualify it as tr_TR.UTF-8 it works. Perhaps I have something
 misconfigured on my system (Ubuntu 11.10)? I just installed:
   language-pack-de
   language-pack-tr
   language-pack-sv

That's very strange.  It works as-is for me on Fedora 13, 14, and 15,
which you'd expect to have essentially the same I18N infrastructure as
Ubuntu (and I'm pretty sure I didn't install any optional language
support for Turkish).  Anybody know what the problem is?

The reason I'm resisting just changing it is that I'd prefer to minimize
the number of dependencies this regression test has on the exact
spelling of UTF-8, as that is not terribly well standardized.  On my
Fedora boxes, for instance, locale -a says that tr_TR.utf8 is the
name of that particular locale.

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) regression diffs on collate.linux.utf8 test

2011-10-18 Thread Peter Eisentraut
On tis, 2011-10-18 at 01:07 -0700, Jeff Davis wrote:
 On Sun, 2011-10-16 at 16:00 -0400, Tom Lane wrote:
  Jeff Davis pg...@j-davis.com writes:
   On master, I see a minor test error (at least on my machine) as well as
   a diff. Patch attached.
  
  Hmm, yeah, I forgot to fix this regression test when I added that DETAIL
  line.  However, I don't see the need for fooling with the lc_time value?
  
  regards, tom lane
 
 Here is the diff that I'm seeing on master right now with:
 
   make -s check EXTRA_TESTS=collate.linux.utf8
 
 If I qualify it as tr_TR.UTF-8 it works. Perhaps I have something
 misconfigured on my system (Ubuntu 11.10)? I just installed:
   language-pack-de
   language-pack-tr
   language-pack-sv
 
 in an attempt to make the test work, and it works all except for that
 lc_time settng.

I think the language-pack packages have nothing to do with it; they only
supply translations.

Possibly, things are set up so that only UTF-8 locales are installed by
default.  Since the collate.linux.utf8 requires a UTF-8 environment, it
seems reasonable to use the tr_TR.UTF-8 locale for LC_TIME, instead of
requiring an unrelated (ISO-8859-9) locale to be installed.  So I think
the change you propose is reasonable.


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


Re: [HACKERS] new compiler warnings

2011-10-18 Thread Peter Eisentraut
On tis, 2011-10-18 at 01:00 -0700, Jeff Davis wrote:
 I'm not sure if these can/should be fixed or not, but here are the
 compiler warnings I'm getting on gcc and clang on ubuntu 11.10 with -O2.
 The gcc ones are mostly new.

They are expected with gcc 4.6.  There isn't anything we can do about
them.

The clang warnings are also expected.  I understand the next clang
version will address them.

 
  GCC 
 
 $ gcc --version
 gcc (Ubuntu/Linaro 4.6.1-9ubuntu3) 4.6.1
 Copyright (C) 2011 Free Software Foundation, Inc.
 This is free software; see the source for copying conditions.  There is
 NO
 warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
 PURPOSE.
 
 warnings generated:
 
 execQual.c: In function ‘GetAttributeByNum’:
 execQual.c:1112:11: warning: the comparison will always evaluate as
 ‘true’ for the address of ‘tmptup’ will never be NULL [-Waddress]
 execQual.c: In function ‘GetAttributeByName’:
 execQual.c:1173:11: warning: the comparison will always evaluate as
 ‘true’ for the address of ‘tmptup’ will never be NULL [-Waddress]
 execQual.c: In function ‘ExecEvalFieldSelect’:
 execQual.c:3922:11: warning: the comparison will always evaluate as
 ‘true’ for the address of ‘tmptup’ will never be NULL [-Waddress]
 In file included from gram.y:12962:0:
 scan.c: In function ‘yy_try_NUL_trans’:
 scan.c:16257:23: warning: unused variable ‘yyg’ [-Wunused-variable]
 elog.c: In function ‘write_pipe_chunks’:
 elog.c:2479:8: warning: ignoring return value of ‘write’, declared with
 attribute warn_unused_result [-Wunused-result]
 elog.c:2488:7: warning: ignoring return value of ‘write’, declared with
 attribute warn_unused_result [-Wunused-result]
 elog.c: In function ‘write_console’:
 elog.c:1797:7: warning: ignoring return value of ‘write’, declared with
 attribute warn_unused_result [-Wunused-result]
 tuplesort.c: In function ‘comparetup_heap’:
 tuplesort.c:2751:12: warning: the comparison will always evaluate as
 ‘true’ for the address of ‘ltup’ will never be NULL [-Waddress]
 tuplesort.c:2752:12: warning: the comparison will always evaluate as
 ‘true’ for the address of ‘rtup’ will never be NULL [-Waddress]
 tuplesort.c: In function ‘copytup_heap’:
 tuplesort.c:2783:17: warning: the comparison will always evaluate as
 ‘true’ for the address of ‘htup’ will never be NULL [-Waddress]
 tuplesort.c: In function ‘readtup_heap’:
 tuplesort.c:2835:17: warning: the comparison will always evaluate as
 ‘true’ for the address of ‘htup’ will never be NULL [-Waddress]
 fe-connect.c: In function ‘PQconndefaults’:
 fe-connect.c:832:6: warning: the comparison will always evaluate as
 ‘false’ for the address of ‘errorBuf’ will never be NULL [-Waddress]
 fe-connect.c: In function ‘PQconninfoParse’:
 fe-connect.c:3970:6: warning: the comparison will always evaluate as
 ‘false’ for the address of ‘errorBuf’ will never be NULL [-Waddress]
 common.c: In function ‘handle_sigint’:
 common.c:247:4: warning: ignoring return value of ‘write’, declared with
 attribute warn_unused_result [-Wunused-result]
 common.c:250:4: warning: ignoring return value of ‘write’, declared with
 attribute warn_unused_result [-Wunused-result]
 common.c:251:4: warning: ignoring return value of ‘write’, declared with
 attribute warn_unused_result [-Wunused-result]
 In file included from mainloop.c:425:0:
 psqlscan.l: In function ‘evaluate_backtick’:
 psqlscan.l:1677:6: warning: the comparison will always evaluate as
 ‘false’ for the address of ‘cmd_output’ will never be NULL [-Waddress]
 
  CLANG 
 
 $ clang --version
 clang version 2.9 (tags/RELEASE_29/final)
 Target: x86_64-pc-linux-gnu
 Thread model: posix
 
 A lot of warnings of the form:
 
 ruleutils.c:698:11: warning: array index of '1' indexes past the end of
 an array (that contains 1 elements) [-Warray-bounds]
 value = fastgetattr(ht_trig, Anum_pg_trigger_tgargs,
 ^~~~
 In file included from ruleutils.c:23:
 In file included from ../../../../src/include/catalog/indexing.h:18:
 ../../../../src/include/access/htup.h:808:3: note: instantiated from:
 att_isnull((attnum)-1, (tup)-t_data-t_bits) ?
 \
 ^
 In file included from ruleutils.c:23:
 In file included from ../../../../src/include/catalog/indexing.h:18:
 In file included from ../../../../src/include/access/htup.h:18:
 ../../../../src/include/access/tupmacs.h:21:34: note: instantiated from:
 #define att_isnull(ATT, BITS) (!((BITS)[(ATT)  3]  (1  ((ATT) 
 0x07
  ^  ~~
 In file included from ruleutils.c:23:
 In file included from ../../../../src/include/catalog/indexing.h:18:
 ../../../../src/include/access/htup.h:153:9: note: array 't_bits'
 declared here
 bits8   t_bits[1];  /* bitmap of NULLs --
 VARIABLE LENGTH */
 
 

Re: [HACKERS] spinlocks on HP-UX

2011-10-18 Thread Robert Haas
On Tue, Oct 18, 2011 at 2:20 AM, Pavan Deolasee
pavan.deola...@gmail.com wrote:
 On Tue, Oct 18, 2011 at 10:04 AM, Robert Haas robertmh...@gmail.com wrote:
 Hmm, so you added the non-locked test in TAS()?  Did you try adding it
 just to TAS_SPIN()?  On Itanium, I found that it was slightly better
 to do it only in TAS_SPIN() - i.e. in the contended case.

 Would it be a good change for S_LOCK() to use TAS_SPIN()  as well ?

Well, that would be sort of missing the point of why we invented
TAS_SPIN() in the first place.  What we found on Itanium is that using
the unlocked test always was better than never doing it, but what was
even slightly better was to use the unlocked first test *only when
spinning*.  In other words, on the very first go-around, we use the
atomic instruction right away.  Only if that fails do we switch to
using the unlocked test first.

Now it's possible that on some other architecture it's better to do
the unlocked test first every time.  But it seems somewhat unlikely,
because in the hopefully-common case where the spinlock is
uncontended, it's just a waste.  If you're having enough spinlock
contention that the first TAS() is failing frequently, you need to fix
the underlying cause of the spinlock contention...

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

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


Re: [HACKERS] new compiler warnings

2011-10-18 Thread Kevin Grittner
Jeff Davis  wrote:
 
 I'm not sure if these can/should be fixed or not, but here are the
 compiler warnings I'm getting on gcc and clang on ubuntu 11.10 with
 -O2.
 
 elog.c: In function ‘write_pipe_chunks’:
 elog.c:2479:8: warning: ignoring return value of ‘write’, declared
 with attribute warn_unused_result [-Wunused-result]
 elog.c:2488:7: warning: ignoring return value of ‘write’, declared
 with attribute warn_unused_result [-Wunused-result]
 elog.c: In function ‘write_console’:
 elog.c:1797:7: warning: ignoring return value of ‘write’, declared
 with attribute warn_unused_result [-Wunused-result]
 
 common.c: In function ‘handle_sigint’:
 common.c:247:4: warning: ignoring return value of ‘write’, declared
 with attribute warn_unused_result [-Wunused-result]
 common.c:250:4: warning: ignoring return value of ‘write’, declared
 with attribute warn_unused_result [-Wunused-result]
 common.c:251:4: warning: ignoring return value of ‘write’, declared
 with attribute warn_unused_result [-Wunused-result]
 In file included from mainloop.c:425:0:
 
These we are getting only because of a stubborn insistence on coding
to the current implementation rather than the API.  It's not that
much code to code to the API instead.  I've already offered to
provide the (trivial) patch for this if there is buy-in on the idea
of coding to the API.
 
The argument against is that no implementer of the API would ever
exercise the freedom the documented API gives them to do *part* of a
write to disk and return to the caller the number of bytes written
and then allow a subsequent write request to continue the output.  I
think that the rise of virtual machine environments in big shops
provides a fairly obvious reason someone might want to do that.
 
-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] new compiler warnings

2011-10-18 Thread Robert Haas
On Tue, Oct 18, 2011 at 9:03 AM, Kevin Grittner
kevin.gritt...@wicourts.gov wrote:
 Jeff Davis  wrote:

 I'm not sure if these can/should be fixed or not, but here are the
 compiler warnings I'm getting on gcc and clang on ubuntu 11.10 with
 -O2.

 elog.c: In function ‘write_pipe_chunks’:
 elog.c:2479:8: warning: ignoring return value of ‘write’, declared
 with attribute warn_unused_result [-Wunused-result]
 elog.c:2488:7: warning: ignoring return value of ‘write’, declared
 with attribute warn_unused_result [-Wunused-result]
 elog.c: In function ‘write_console’:
 elog.c:1797:7: warning: ignoring return value of ‘write’, declared
 with attribute warn_unused_result [-Wunused-result]

 common.c: In function ‘handle_sigint’:
 common.c:247:4: warning: ignoring return value of ‘write’, declared
 with attribute warn_unused_result [-Wunused-result]
 common.c:250:4: warning: ignoring return value of ‘write’, declared
 with attribute warn_unused_result [-Wunused-result]
 common.c:251:4: warning: ignoring return value of ‘write’, declared
 with attribute warn_unused_result [-Wunused-result]
 In file included from mainloop.c:425:0:

 These we are getting only because of a stubborn insistence on coding
 to the current implementation rather than the API.  It's not that
 much code to code to the API instead.  I've already offered to
 provide the (trivial) patch for this if there is buy-in on the idea
 of coding to the API.

 The argument against is that no implementer of the API would ever
 exercise the freedom the documented API gives them to do *part* of a
 write to disk and return to the caller the number of bytes written
 and then allow a subsequent write request to continue the output.  I
 think that the rise of virtual machine environments in big shops
 provides a fairly obvious reason someone might want to do that.

Even if all we got out of it was that the compiler warnings went away,
I think that would still be a sufficient reason to do it.  And I tend
to agree with you that the warnings are legit; and defending against
them is virtually free.

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

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


Re: [HACKERS] new compiler warnings

2011-10-18 Thread Andrew Dunstan



On 10/18/2011 09:03 AM, Kevin Grittner wrote:

Jeff Davis  wrote:


I'm not sure if these can/should be fixed or not, but here are the
compiler warnings I'm getting on gcc and clang on ubuntu 11.10 with
-O2.



elog.c: In function ‘write_pipe_chunks’:
elog.c:2479:8: warning: ignoring return value of ‘write’, declared
with attribute warn_unused_result [-Wunused-result]
elog.c:2488:7: warning: ignoring return value of ‘write’, declared
with attribute warn_unused_result [-Wunused-result]
elog.c: In function ‘write_console’:
elog.c:1797:7: warning: ignoring return value of ‘write’, declared
with attribute warn_unused_result [-Wunused-result]



common.c: In function ‘handle_sigint’:
common.c:247:4: warning: ignoring return value of ‘write’, declared
with attribute warn_unused_result [-Wunused-result]
common.c:250:4: warning: ignoring return value of ‘write’, declared
with attribute warn_unused_result [-Wunused-result]
common.c:251:4: warning: ignoring return value of ‘write’, declared
with attribute warn_unused_result [-Wunused-result]
In file included from mainloop.c:425:0:


These we are getting only because of a stubborn insistence on coding
to the current implementation rather than the API.  It's not that
much code to code to the API instead.  I've already offered to
provide the (trivial) patch for this if there is buy-in on the idea
of coding to the API.

The argument against is that no implementer of the API would ever
exercise the freedom the documented API gives them to do *part* of a
write to disk and return to the caller the number of bytes written
and then allow a subsequent write request to continue the output.  I
think that the rise of virtual machine environments in big shops
provides a fairly obvious reason someone might want to do that.




There are non-disk uses of write() where partial writes are legitimate 
(e.g. pipes under some circumstances on Linux).


It is a pity we can't just tell the compiler to turn off the warning in 
a particular case.


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] new compiler warnings

2011-10-18 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes:
 It is a pity we can't just tell the compiler to turn off the warning in 
 a particular case.

I haven't tested, but won't an explicit cast to void silence the
warning?

(void) fwrite(...);

There are places, notably the calls in elog.c, where ignoring write
failures is the right thing.  I think that what Kevin was on about
was something else entirely, namely whether we need to retry writes
to disk.  I would hope that we're not simply not bothering to check
in any cases where it matters.

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] [v9.2] make_greater_string() does not return a string in some cases

2011-10-18 Thread Robert Haas
On Tue, Oct 18, 2011 at 12:11 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Mon, Oct 17, 2011 at 11:54 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 http://www.faqs.org/rfcs/rfc3629.html

 I'm still confused.  The input string is already known to be valid
 UTF-8, so the second byte (if there is one) must be between 0x80 and
 0xBF.  Therefore it will be neither 0xED nor 0xF4.

 I haven't read the patch lately, but ED and F4 are special as *first*
 bytes.  Maybe the logic isn't quite right, or you read it wrong?

I think I'll let the patch author comment on that.  It looks wrong to
me, but I just work here.

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

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


Re: [HACKERS] new compiler warnings

2011-10-18 Thread Tom Lane
Peter Eisentraut pete...@gmx.net writes:
 On tis, 2011-10-18 at 01:00 -0700, Jeff Davis wrote:
 I'm not sure if these can/should be fixed or not, but here are the
 compiler warnings I'm getting on gcc and clang on ubuntu 11.10 with -O2.
 The gcc ones are mostly new.

 They are expected with gcc 4.6.  There isn't anything we can do about
 them.

Well, we're going to have to think of something, because as more of us
move onto the newer gcc releases the annoyance level is going to become
intolerable.

I think a large fraction of the -Waddress warnings are coming from
this line in the heap_getattr macro:

AssertMacro((tup) != NULL), \

Seems to me we could just lose that test and be no worse off, since
the macro is surely gonna dump core anyway on a null pointer.

But some of the remaining -Waddress warnings are not so painless to
get rid of.  Ultimately we might have to add -Wno-address to the
default CFLAGS.

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] new compiler warnings

2011-10-18 Thread Kevin Grittner
Tom Lane t...@sss.pgh.pa.us wrote:
 
 I think that what Kevin was on about was something else entirely,
 namely whether we need to retry writes to disk.
 
I would phrase it that we need to *continue* a write to disk if the
OS chooses to write a portion of it and return to the caller with
the number actually written.  If the first write, or any later
write, actually gets an error or fails to make progress, *then* we
should consider the attempt to be done.  I don't understand the
point of not coding to the API.
 
-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] pg_ctl restart - behaviour based on wrong instance

2011-10-18 Thread Magnus Hagander
On Wednesday, October 12, 2011, Bruce Momjian wrote:

 Magnus Hagander wrote:
   I looked over this issue and I don't thinking having pg_ctl restart
 fall
   back to 'start' is a good solution. ?I am concerned about cases where
 we
   start a different server without shutting down the old server, for some
   reason. ?When they say 'restart', I think we have to assume they want a
   restart.
  
   What I did do was to document that not backing up postmaster.pid and
   postmaster.opts might help prevent pg_ctl from getting confused.
 
  Should we exclude postmaster.opts from streaming base backups? We
  already exclude postmaster.pid...

 Uh, I think so, unless my analysis was wrong.


Ok, fixed and applied.

//Magnus



-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


Re: [HACKERS] new compiler warnings

2011-10-18 Thread Peter Eisentraut
On tis, 2011-10-18 at 09:32 -0400, Tom Lane wrote:
 Andrew Dunstan and...@dunslane.net writes:
  It is a pity we can't just tell the compiler to turn off the warning in 
  a particular case.
 
 I haven't tested, but won't an explicit cast to void silence the
 warning?
 
   (void) fwrite(...);

No, tried that already.  You could try

rc = write(...);
(void) rc;

 There are places, notably the calls in elog.c, where ignoring write
 failures is the right thing.  I think that what Kevin was on about
 was something else entirely, namely whether we need to retry writes
 to disk.  I would hope that we're not simply not bothering to check
 in any cases where it matters.

No, I believe we are OK everywhere else.  We are only ignoring the
result in cases where we are trying to report errors in the first place.



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


Re: [HACKERS] new compiler warnings

2011-10-18 Thread Robert Haas
On Tue, Oct 18, 2011 at 10:06 AM, Peter Eisentraut pete...@gmx.net wrote:
 No, I believe we are OK everywhere else.  We are only ignoring the
 result in cases where we are trying to report errors in the first place.

The relevant code is:

while (len  PIPE_MAX_PAYLOAD)
{
p.proto.is_last = (dest == LOG_DESTINATION_CSVLOG ? 'F' : 'f');
p.proto.len = PIPE_MAX_PAYLOAD;
memcpy(p.proto.data, data, PIPE_MAX_PAYLOAD);
write(fd, p, PIPE_HEADER_SIZE + PIPE_MAX_PAYLOAD);
data += PIPE_MAX_PAYLOAD;
len -= PIPE_MAX_PAYLOAD;
}

Which it seems to me we could change by doing rc = write().  Then if
rc = 0, we bail out.  If not, we add and subtract rc, rather than
PIPE_MAX_PAYLOAD.  That would be barely more code, probably safer, and
would silence the warning.

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

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


Re: [HACKERS] [BUG] SSPI authentication fails on Windows when server parameter is localhost or domain name

2011-10-18 Thread Ahmed Shinwari
Hi,

My apologies for a very late reply.

I agree the fix you applied is a better one. I have verified the fix by
testing the 'postgresql-9.1.1-1-windows-x64' installer.

Thank you.


On Thu, Jul 14, 2011 at 7:23 PM, Magnus Hagander mag...@hagander.netwrote:

 On Wed, Jun 15, 2011 at 10:53, Ahmed Shinwari ahmed.shinw...@gmail.com
 wrote:
  Hi All,
 
  I faced a bug on Windows while connecting via SSPI authentication. I was
  able to find the bug and have attached the patch. Details listed below;
 
  Postgres Installer: Version 9.0.4
  OS: Windows Server 2008 R2/Windows 7

 big snip

 Thanks - great analysis!

 However, I think there is a better fix for this - simply moving a }
 one line. In particular, I'm concerned about passing the same pointer
 both as input and output to the function - I couldn't find anything in
 the documentation saying this was safe (nor did I find anything saying
 it's unsafe, but.) Especially since this code clearly behaves
 different on different versions - I've been completely unable to
 reproduce this on any of my test machines, but they are all Windows
 Server 2003.

 So - attached is a new version of the patch, how does this look to
 you? FYI, I've had Thom test this new version and it does appear to
 work fine in his scenario.


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




-- 
Ahmed Shinwari
EnterpriseDB Corporation : www.enterprisedb.com
The Enterprise Postgres Company


Re: [HACKERS] new compiler warnings

2011-10-18 Thread Kevin Grittner
Robert Haas robertmh...@gmail.com wrote:
 
 Which it seems to me we could change by doing rc = write().  Then
 if rc = 0, we bail out.  If not, we add and subtract rc, rather
 than PIPE_MAX_PAYLOAD.
 
Something along the general lines of this?:
 
http://archives.postgresql.org/pgsql-hackers/2011-02/msg01719.php
 
 That would be barely more code, probably safer, and would silence
 the warning.
 
Exactly.
 
-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] new compiler warnings

2011-10-18 Thread Kevin Grittner
Kevin Grittner kevin.gritt...@wicourts.gov wrote:
 
 http://archives.postgresql.org/pgsql-hackers/2011-02/msg01719.php
 
Although, it being a quick example of the general idea, I have an
obvious bug there -- the write location would have to be buffer +
t.
 
I think Noah might have also posted some example code a month or two
back.
 
-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] Separating bgwriter and checkpointer

2011-10-18 Thread Simon Riggs
On Wed, Oct 5, 2011 at 8:02 AM, Simon Riggs si...@2ndquadrant.com wrote:
 On Wed, Oct 5, 2011 at 5:10 AM, Dickson S. Guedes lis...@guedesoft.net 
 wrote:

 Ah ok! I started reviewing the v4 patch version, this is my comments:

 ...

 Well, all the tests was running with the default postgresql.conf in my
 laptop but I'll setup a more real world environment to test for
 performance regression. Until now I couldn't notice any significant
 difference in TPS before and after patch in a small environment. I'll
 post something soon.

 Great testing, thanks. Likely will have no effect in non-I/O swamped
 environment, but no regression expected either.


Any reason or objection to committing this patch?


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

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


Re: [HACKERS] synchronized snapshots

2011-10-18 Thread Simon Riggs
On Mon, Oct 3, 2011 at 7:09 AM, Marko Tiikkaja
marko.tiikk...@2ndquadrant.com wrote:
 On 2011-09-28 15:25, Joachim Wieland wrote:

 Yes, that's the desired behaviour, the patch add this paragraph to the
 documentation already:

 I can't believe I missed that.  My apologies.

 On 2011-09-29 05:16, Joachim Wieland wrote:

 The attached patch addresses the reported issues.

 Thanks, this one looks good to me.  Going to mark this patch as ready for
 committer.


I don't see any tests with this patch, so I personally won't be the
committer on this just yet.

Also, not sure why the snapshot id syntax has leading zeroes on first
part of the number, but not on second part. It will still sort
incorrectly if that's what we were trying to achieve. Either leave off
the leading zeroes on first part of add them to second.

We probably need some more discussion added to the README about this.

I'm also concerned that we are adding this to the BEGIN statement as
the only option. I don't have a problem with it being there, but I do
think it is a problem to make it the *only* way to use this. Altering
BEGIN gives clear problems with any API that does the begin and commit
for you, such as perl DBI, java JDBC to name just two. I can't really
see its a good implementation if we say this won't work until client
APIs follow our new non-standard syntax. I wouldn't block commit on
this point, but I think we should work on alternative ways to invoke
this feature as well.

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

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


Re: [HACKERS] [v9.2] Object access hooks with arguments support (v1)

2011-10-18 Thread Kohei KaiGai
2011/10/18 Robert Haas robertmh...@gmail.com:
 On Thu, Oct 13, 2011 at 6:48 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
    struct ObjectAccessInfoData {
        ObjectAccessType   oa_type;
        ObjectAddress         oa_address;
        union {
            struct {
                HeapTuple       newtuple;
                TupleDesc       tupdesc;  /* only if create a new relation */
                    :
            } post_create;      /* additional info for OAT_POST_CREATE event 
 */
        }
    }

 That's possibly an improvement over just passing everything opaquely,
 but it still doesn't seem very good.  I mean, this is C, not Perl or
 Python.  Anything where you pass a bunch of arguments of indeterminate
 type and hope that the person you're calling can figure it out is
 inherently pretty dangerous.  I had it in mind that the object access
 hook mechanism could serve as a simple and generic way of letting an
 external module know that an event of a certain type has occurred on a
 certain object, and to let that module gain control.  But if we have
 to pass a raft of arguments in, then it's not generic any more - it's
 just a bunch of things that are probably really need to be separate
 hooks shoved into a single function.

I got an inspiration of this idea from APIs of foreign-data-wrapper that
trusts values of each fields that contains function pointers.
Indeed, we may have a risk of backend crash, if we try to run modules
built towards v9.1 on the pgsql-v9.2devel. However, it is not avoidable,
unless we rewrite whole of the code by Java?, Python? or something? :-(

At least, we don't promise binary compatible across major versions?
If so, I don't think dangerousness is a reasonable reason why the hook
does not take any additional arguments, as long as auther of modules
can be noticed by compiler warnings.

The point to be focused on is how do we implement the target feature
(DDL permissions by sepgsql in my case) with simpleness.

 Moreover, if
 you did document it, I think it would boil down to this is what
 sepgsql happens to need, and I don't think that's an acceptable
 answer. ?We have repeatedly refused to adopt that approach in the
 past.

 Unfortunately, I still don't have a clear answer for this point.
 However, in general terms, it is impossible to design any interface without
 knowledge of its usage more or less.

 Well, that's true.  But the hook also has to be maintainable.  ISTM
 that there's no reasonable way for someone making a modification to
 the code to know whether the additional local variable that they just
 added to the function should be passed to the hook, or not.  Here, at
 least as far as I can see, there's no guiding principle that would
 enable future contributors to make an intelligent decision about that.
  And if someone wanted to write another client for the hook, it's not
 very obvious whether the particular things you've chosen to pass here
 would be relevant or not.  I think if you look over the code you'll
 find that there's nowhere else that we have a hook that looks anything
 like what you're proposing here.

In the case when the second client of the hook is proposed, a straight-
forward approach is to expand the above ObjectAccessInfoData to
satisfy the requirement of new one, if existing arguments are not enough.
Because existing modules ignore the new fields, it is harmless.

Linux adopts this approch to host multiple security modules without
confusion, although one makes decision by security label and one
other makes decision by pathname of files; they require different
information on the point to make its decision, because they ignore
unrelevant arguments; such as pathname in selinux.

I remember you worry about the number of arguments getting increased
infinity, however, it is an extreme situation. We are not an A.I to commit
proposed patches automatically, so auther of modules (including me)
will explain why the new arguments are additionally needed here, like as
pathname based security module did in Linux kernel development.

 At least, this proposition enables modules being informed using
 commonly used data type (such as HeapTuple, TupleDesc), compared
 to the past approach that tried to push all the necessary information
 into argument list individually.

 That does seem like a good direction to go in, but you're still
 passing a lot of other stuff in there.  I guess my feeling is that if
 we can't boil down the argument list to a short list of things that
 are more-or-less common to all the call sites, we probably need to
 rethink the whole design.

Honestly, the arguments of object_access_hook is a method to implement
security model of sepgsql, not a purpose of mine. So, an alternative idea is
quite welcome to implement sepgsql.
However, I think it is less invasive way than other ideas right now.

For example, I hope sepgsql to perform as follows when user create a new table.
- It computes a default security label that needs Oid of the 

Re: [HACKERS] pg_ctl restart - behaviour based on wrong instance

2011-10-18 Thread Fujii Masao
On Tue, Oct 18, 2011 at 11:02 PM, Magnus Hagander mag...@hagander.net wrote:
 Ok, fixed and applied.

You seem to have forgot to change protocol.sgml.
Patch attached.

Regards,

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


protocol_sgml_v1.patch
Description: Binary data

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


Re: [HACKERS] [v9.2] Object access hooks with arguments support (v1)

2011-10-18 Thread Robert Haas
On Tue, Oct 18, 2011 at 11:25 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 For example, I hope sepgsql to perform as follows when user create a new 
 table.
 - It computes a default security label that needs Oid of the namespace.
 - It checks db_table:{create} permission on the security label being computed.
 - In addition, it checks db_table:{insert} permission when SELECT INTO
 - Also, it checks these permissions on columns being newly created.
 - However, It does not check permissions when the tables are internally 
 created,
  such as toast tables or temporary relations created by make_new_heap().

That's superficially reasonable, but I don't feel good about passing
is_select_info to the object access hook here.  If insert permission
is required there, then it ought to be getting checked by selinux at
the same place that it's getting checked for at the DAC level.  If we
get into a situation where sepgsql is checking permissions using some
system that's totally different from what we do for DAC, I think
that's going to produce confusing and illogical results.  This is
ground we've been over before.  Similarly with fdw_srvname.  The only
excuse for passing that is to handle a permissions check for foreign
data wrappers that isn't being done for DAC: if there is a DAC
permissions check, then the MAC one should be done there also, not
someplace totally different.

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

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


Re: [HACKERS] pg_ctl restart - behaviour based on wrong instance

2011-10-18 Thread Fujii Masao
On Tue, Oct 18, 2011 at 11:02 PM, Magnus Hagander mag...@hagander.net wrote:
 Ok, fixed and applied.

You seem to have forgot to change protocol.sgml.
Patch attached.

Regards,

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


protocol_sgml_v1.patch
Description: Binary data

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


Re: [HACKERS] new compiler warnings

2011-10-18 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Tue, Oct 18, 2011 at 10:06 AM, Peter Eisentraut pete...@gmx.net wrote:
 No, I believe we are OK everywhere else.  We are only ignoring the
 result in cases where we are trying to report errors in the first place.

 The relevant code is:

 while (len  PIPE_MAX_PAYLOAD)
 {
 p.proto.is_last = (dest == LOG_DESTINATION_CSVLOG ? 'F' : 'f');
 p.proto.len = PIPE_MAX_PAYLOAD;
 memcpy(p.proto.data, data, PIPE_MAX_PAYLOAD);
 write(fd, p, PIPE_HEADER_SIZE + PIPE_MAX_PAYLOAD);
 data += PIPE_MAX_PAYLOAD;
 len -= PIPE_MAX_PAYLOAD;
 }

 Which it seems to me we could change by doing rc = write().  Then if
 rc = 0, we bail out.  If not, we add and subtract rc, rather than
 PIPE_MAX_PAYLOAD.  That would be barely more code, probably safer, and
 would silence the warning.

And it would break the code.  The whole point here is that the message
must be sent indivisibly.

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] new compiler warnings

2011-10-18 Thread Kevin Grittner
Tom Lane t...@sss.pgh.pa.us wrote:
 
 And it would break the code.  The whole point here is that the
 message must be sent indivisibly.
 
If the new code splits the message, it would previously have been
truncated.  Is that less broken?
 
-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] Separating bgwriter and checkpointer

2011-10-18 Thread Robert Haas
On Tue, Oct 18, 2011 at 9:18 AM, Simon Riggs si...@2ndquadrant.com wrote:
 Any reason or objection to committing this patch?

Not on my end, though I haven't reviewed it in detail.  One minor note
- I was mildly surprised to see that you moved this to the
checkpointer rather than leaving it in the bgwriter:

+   /* Do this once before starting the loop, then just at SIGHUP time. */
+   SyncRepUpdateSyncStandbysDefined();

My preference would probably have been to leave that in the background
writer, on the theory that the checkpointer's work is likely to be
more bursty and therefore it might be less responsive.

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

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


Re: [HACKERS] pg_ctl restart - behaviour based on wrong instance

2011-10-18 Thread Fujii Masao
On Tue, Oct 18, 2011 at 11:02 PM, Magnus Hagander mag...@hagander.net wrote:
 Ok, fixed and applied.

You seem to have forgot to change protocol.sgml.
Patch attached.

Regards,

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


protocol_sgml_v1.patch
Description: Binary data

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


Re: [HACKERS] pg_ctl restart - behaviour based on wrong instance

2011-10-18 Thread Fujii Masao
On Tue, Oct 18, 2011 at 11:02 PM, Magnus Hagander mag...@hagander.net wrote:
 Ok, fixed and applied.

You seem to have forgot to change protocol.sgml.
Patch attached.

Regards,

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


protocol_sgml_v1.patch
Description: Binary data

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


Re: [HACKERS] new compiler warnings

2011-10-18 Thread Robert Haas
On Tue, Oct 18, 2011 at 12:26 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Tue, Oct 18, 2011 at 10:06 AM, Peter Eisentraut pete...@gmx.net wrote:
 No, I believe we are OK everywhere else.  We are only ignoring the
 result in cases where we are trying to report errors in the first place.

 The relevant code is:

     while (len  PIPE_MAX_PAYLOAD)
     {
         p.proto.is_last = (dest == LOG_DESTINATION_CSVLOG ? 'F' : 'f');
         p.proto.len = PIPE_MAX_PAYLOAD;
         memcpy(p.proto.data, data, PIPE_MAX_PAYLOAD);
         write(fd, p, PIPE_HEADER_SIZE + PIPE_MAX_PAYLOAD);
         data += PIPE_MAX_PAYLOAD;
         len -= PIPE_MAX_PAYLOAD;
     }

 Which it seems to me we could change by doing rc = write().  Then if
 rc = 0, we bail out.  If not, we add and subtract rc, rather than
 PIPE_MAX_PAYLOAD.  That would be barely more code, probably safer, and
 would silence the warning.

 And it would break the code.  The whole point here is that the message
 must be sent indivisibly.

How is that different than the chunking that the while loop is already doing?

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

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


Re: [HACKERS] Separating bgwriter and checkpointer

2011-10-18 Thread Simon Riggs
On Tue, Oct 18, 2011 at 5:39 PM, Robert Haas robertmh...@gmail.com wrote:
 On Tue, Oct 18, 2011 at 9:18 AM, Simon Riggs si...@2ndquadrant.com wrote:
 Any reason or objection to committing this patch?

 Not on my end, though I haven't reviewed it in detail.  One minor note
 - I was mildly surprised to see that you moved this to the
 checkpointer rather than leaving it in the bgwriter:

 +       /* Do this once before starting the loop, then just at SIGHUP time. */
 +       SyncRepUpdateSyncStandbysDefined();

 My preference would probably have been to leave that in the background
 writer, on the theory that the checkpointer's work is likely to be
 more bursty and therefore it might be less responsive.

That needs to be in the checkpointer because that is the process that
shuts down last.

The bgwriter is now more like the walwriter. It shuts down early in
the shutdown process, while the checkpointer is last out.

So it wasn't preference, it was a requirement of the new role definitions.

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

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


Re: [HACKERS] Separating bgwriter and checkpointer

2011-10-18 Thread Robert Haas
On Tue, Oct 18, 2011 at 12:53 PM, Simon Riggs si...@2ndquadrant.com wrote:
 On Tue, Oct 18, 2011 at 5:39 PM, Robert Haas robertmh...@gmail.com wrote:
 On Tue, Oct 18, 2011 at 9:18 AM, Simon Riggs si...@2ndquadrant.com wrote:
 Any reason or objection to committing this patch?

 Not on my end, though I haven't reviewed it in detail.  One minor note
 - I was mildly surprised to see that you moved this to the
 checkpointer rather than leaving it in the bgwriter:

 +       /* Do this once before starting the loop, then just at SIGHUP time. 
 */
 +       SyncRepUpdateSyncStandbysDefined();

 My preference would probably have been to leave that in the background
 writer, on the theory that the checkpointer's work is likely to be
 more bursty and therefore it might be less responsive.

 That needs to be in the checkpointer because that is the process that
 shuts down last.

 The bgwriter is now more like the walwriter. It shuts down early in
 the shutdown process, while the checkpointer is last out.

 So it wasn't preference, it was a requirement of the new role definitions.

Oh, I see.

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

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


Re: [HACKERS] new compiler warnings

2011-10-18 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Tue, Oct 18, 2011 at 12:26 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 And it would break the code.  The whole point here is that the message
 must be sent indivisibly.

 How is that different than the chunking that the while loop is already doing?

The chunks are sent indivisibly, because they are less than the pipe
buffer size.  Read the pipe man page.  It's guaranteed that the write
will either succeed or fail as a whole, not write a partial message.
If we cared to retry a failure, there would be some point in checking
the return code.

regards, tom lane

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


Re: [HACKERS] pg_ctl restart - behaviour based on wrong instance

2011-10-18 Thread Fujii Masao
Oh, sorry for repeating the same posts. Gmail seems to have not worked
fine... :(

On Wed, Oct 19, 2011 at 1:24 AM, Fujii Masao masao.fu...@gmail.com wrote:
 On Tue, Oct 18, 2011 at 11:02 PM, Magnus Hagander mag...@hagander.net wrote:
 Ok, fixed and applied.

 You seem to have forgot to change protocol.sgml.
 Patch attached.

 Regards,

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


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

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


Re: [HACKERS] new compiler warnings

2011-10-18 Thread Robert Haas
On Tue, Oct 18, 2011 at 1:01 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Tue, Oct 18, 2011 at 12:26 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 And it would break the code.  The whole point here is that the message
 must be sent indivisibly.

 How is that different than the chunking that the while loop is already doing?

 The chunks are sent indivisibly, because they are less than the pipe
 buffer size.  Read the pipe man page.  It's guaranteed that the write
 will either succeed or fail as a whole, not write a partial message.
 If we cared to retry a failure, there would be some point in checking
 the return code.

On MacOS X v10.6.8, I see no such guarantee in the pipe(2) man page.
The man page for read(2) says:

 Upon successful completion, read(), readv(), and pread() return
the number of
 bytes actually read and placed in the buffer.  The system
guarantees to read the
 number of bytes requested if the descriptor references a normal
file that has
 that many bytes left before the end-of-file, but in no other case.

In any event, whether or not it's *possible* to have a short read is a
separate question from what we should do if it happens.  Retrying an
error doesn't seem practical, because in all likelihood the error will
recur forever and we'll go into an infinite loop.  But if we do
somehow get a short write, sending the rest of the current chunk in
the next write() does not seem materially worse than sending the next
chunk.

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

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


Re: [HACKERS] new compiler warnings

2011-10-18 Thread Tom Lane
Kevin Grittner kevin.gritt...@wicourts.gov writes:
 Tom Lane t...@sss.pgh.pa.us wrote:
 I think that what Kevin was on about was something else entirely,
 namely whether we need to retry writes to disk.
 
 I would phrase it that we need to *continue* a write to disk if the
 OS chooses to write a portion of it and return to the caller with
 the number actually written.  If the first write, or any later
 write, actually gets an error or fails to make progress, *then* we
 should consider the attempt to be done.  I don't understand the
 point of not coding to the API.

My point here is just that that's a different discussion.  You are not
talking about places where this new compiler warning is getting raised.

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] new compiler warnings

2011-10-18 Thread Peter Eisentraut
On tis, 2011-10-18 at 09:36 -0400, Tom Lane wrote:
 But some of the remaining -Waddress warnings are not so painless to
 get rid of.  Ultimately we might have to add -Wno-address to the
 default CFLAGS.

Here is the bug report to gcc on this issue:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48778

FWIW, I've been building with -Wno-error=address for months, ever since
gcc 4.6 because the default on my machine.  I don't know what other
issues one might be missing that way.


-- 
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] synchronized snapshots

2011-10-18 Thread Tom Lane
Simon Riggs si...@2ndquadrant.com writes:
 On Mon, Oct 3, 2011 at 7:09 AM, Marko Tiikkaja
 Thanks, this one looks good to me.  Going to mark this patch as ready for
 committer.

 I don't see any tests with this patch, so I personally won't be the
 committer on this just yet.

I've already taken it according to the commitfest app.  There's a lot of
things I don't like stylistically, but they all seem fixable, and I'm
working through them now.  The only actual bug I've found so far is a
race condition while setting MyProc-xmin (you can't do that separately
from verifying that the source transaction is still running, else
somebody else could see a global xmin that's gone backwards).

 Also, not sure why the snapshot id syntax has leading zeroes on first
 part of the number, but not on second part. It will still sort
 incorrectly if that's what we were trying to achieve. Either leave off
 the leading zeroes on first part of add them to second.

The first part is of fixed length, the second not so much.  I'm not
wedded to the syntax but I don't see anything wrong with it either.

 I'm also concerned that we are adding this to the BEGIN statement as
 the only option.

Huh?  The last version of the patch has it only as SET TRANSACTION
SNAPSHOT, which I think is the right 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] [v9.2] Object access hooks with arguments support (v1)

2011-10-18 Thread Kohei KaiGai
2011/10/18 Robert Haas robertmh...@gmail.com:
 On Tue, Oct 18, 2011 at 11:25 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 For example, I hope sepgsql to perform as follows when user create a new 
 table.
 - It computes a default security label that needs Oid of the namespace.
 - It checks db_table:{create} permission on the security label being 
 computed.
 - In addition, it checks db_table:{insert} permission when SELECT INTO
 - Also, it checks these permissions on columns being newly created.
 - However, It does not check permissions when the tables are internally 
 created,
  such as toast tables or temporary relations created by make_new_heap().

 That's superficially reasonable, but I don't feel good about passing
 is_select_info to the object access hook here.  If insert permission
 is required there, then it ought to be getting checked by selinux at
 the same place that it's getting checked for at the DAC level.  If we
 get into a situation where sepgsql is checking permissions using some
 system that's totally different from what we do for DAC, I think
 that's going to produce confusing and illogical results.  This is
 ground we've been over before.  Similarly with fdw_srvname.  The only
 excuse for passing that is to handle a permissions check for foreign
 data wrappers that isn't being done for DAC: if there is a DAC
 permissions check, then the MAC one should be done there also, not
 someplace totally different.

If you are suggesting DAC and MAC permissions should be checked
on the same place like as we already doing at ExecCheckRTPerms(),
I'd like to agree with the suggestion, rather than all the checks within
object_access_hook, although it will take code reworking around
access controls.

In the example table creation, heap_create_with_catalog() is invoked
by 5 routines, however, 3 of them are just internal usages, so it is not
preferable to apply permission checks on table creation

If we may have a hook on table creation next to the place currently
we checks permission on the namespace being created, it is quite
helpful to implement sepgsql.


BTW, it seems to me SELECT INTO should also check insert permission
on DAC level, because it become an incorrect assumption that owner of
table has insert permission on its creation time.

postgres=# ALTER DEFAULT PRIVILEGES FOR USER tak GRANT select ON TABLES TO tak;
ALTER DEFAULT PRIVILEGES
postgres=# \ddp
 Default access privileges
 Owner | Schema | Type  | Access privileges
---++---+---
 tak   || table | tak=r/tak
(1 row)

postgres=# SET SESSION AUTHORIZATION tak;
SET
postgres= SELECT * INTO t1 FROM (VALUES(1,'aaa'), (2,'bbb'), (3,'ccc')) AS v;
SELECT 3
postgres= SELECT * FROM t1;
 column1 | column2
-+-
   1 | aaa
   2 | bbb
   3 | ccc
(3 rows)

postgres= INSERT INTO t1 VALUES (4,'ddd');
ERROR:  permission denied for relation t1

Oops!
-- 
KaiGai Kohei kai...@kaigai.gr.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] synchronized snapshots

2011-10-18 Thread Simon Riggs
On Tue, Oct 18, 2011 at 6:22 PM, Tom Lane t...@sss.pgh.pa.us wrote:

 I'm also concerned that we are adding this to the BEGIN statement as
 the only option.

 Huh?  The last version of the patch has it only as SET TRANSACTION
 SNAPSHOT, which I think is the right way.

Sorry Tom, didn't see your name on it earlier, thats not shown on the
main CF display and I didn't think to check on the detail. Please
continue.

I misread the SET TRANSACTION docs changes. Happy with that.

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

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


Re: [HACKERS] new compiler warnings

2011-10-18 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Tue, Oct 18, 2011 at 1:01 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 The chunks are sent indivisibly, because they are less than the pipe
 buffer size.  Read the pipe man page.  It's guaranteed that the write
 will either succeed or fail as a whole, not write a partial message.
 If we cared to retry a failure, there would be some point in checking
 the return code.

 On MacOS X v10.6.8, I see no such guarantee in the pipe(2) man page.

Sorry, maybe write(2) is the place to look.  The Single Unix Spec quoth
(at http://pubs.opengroup.org/onlinepubs/9699919799/):

Write requests to a pipe or FIFO shall be handled in the same
way as a regular file with the following exceptions:

There is no file offset associated with a pipe, hence each write
request shall append to the end of the pipe.

Write requests of {PIPE_BUF} bytes or less shall not be
interleaved with data from other processes doing writes on the
same pipe. Writes of greater than {PIPE_BUF} bytes may have data
interleaved, on arbitrary boundaries, with writes by other
processes, whether or not the O_NONBLOCK flag of the file status
flags is set.

If the O_NONBLOCK flag is clear, a write request may cause the
thread to block, but on normal completion it shall return nbyte.

Note the last in particular.  Short writes are specifically disallowed
on pipes.

If this were not the case, the logging collector protocol would be
useless.

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] new compiler warnings

2011-10-18 Thread Andrew Dunstan



On 10/18/2011 01:35 PM, Tom Lane wrote:

Robert Haasrobertmh...@gmail.com  writes:

On Tue, Oct 18, 2011 at 1:01 PM, Tom Lanet...@sss.pgh.pa.us  wrote:

The chunks are sent indivisibly, because they are less than the pipe
buffer size.  Read the pipe man page.  It's guaranteed that the write
will either succeed or fail as a whole, not write a partial message.
If we cared to retry a failure, there would be some point in checking
the return code.

On MacOS X v10.6.8, I see no such guarantee in the pipe(2) man page.

Sorry, maybe write(2) is the place to look.  The Single Unix Spec quoth
(at http://pubs.opengroup.org/onlinepubs/9699919799/):

Write requests to a pipe or FIFO shall be handled in the same
way as a regular file with the following exceptions:

There is no file offset associated with a pipe, hence each write
request shall append to the end of the pipe.

Write requests of {PIPE_BUF} bytes or less shall not be
interleaved with data from other processes doing writes on the
same pipe. Writes of greater than {PIPE_BUF} bytes may have data
interleaved, on arbitrary boundaries, with writes by other
processes, whether or not the O_NONBLOCK flag of the file status
flags is set.

If the O_NONBLOCK flag is clear, a write request may cause the
thread to block, but on normal completion it shall return nbyte.

Note the last in particular.  Short writes are specifically disallowed
on pipes.

If this were not the case, the logging collector protocol would be
useless.




My dim recollection is that Tom and I and maybe some others did tests on 
a bunch of platforms at the time we introduced the protocol to make sure 
it did work this way, since it's crucial to making sure we don't get 
interleaved log lines.


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] [v9.2] Object access hooks with arguments support (v1)

2011-10-18 Thread Robert Haas
On Tue, Oct 18, 2011 at 1:23 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 If you are suggesting DAC and MAC permissions should be checked
 on the same place like as we already doing at ExecCheckRTPerms(),
 I'd like to agree with the suggestion, rather than all the checks within
 object_access_hook, although it will take code reworking around
 access controls.

Agreed.

 In the example table creation, heap_create_with_catalog() is invoked
 by 5 routines, however, 3 of them are just internal usages, so it is not
 preferable to apply permission checks on table creation

Some wit once made the remark that if a function has 10 arguments, it
has 11 arguments, meaning that functions with very large numbers of
arguments typically indicate a poor choice of abstraction boundary.
That's pretty much what I think is going on with
heap_create_with_catalog().  I think maybe some refactoring is in
order there, though I am not sure quite what.

 If we may have a hook on table creation next to the place currently
 we checks permission on the namespace being created, it is quite
 helpful to implement sepgsql.

Makes sense.

 BTW, it seems to me SELECT INTO should also check insert permission
 on DAC level, because it become an incorrect assumption that owner of
 table has insert permission on its creation time.

 postgres=# ALTER DEFAULT PRIVILEGES FOR USER tak GRANT select ON TABLES TO 
 tak;
 ALTER DEFAULT PRIVILEGES
 postgres=# \ddp
         Default access privileges
  Owner | Schema | Type  | Access privileges
 ---++---+---
  tak   |        | table | tak=r/tak
 (1 row)

 postgres=# SET SESSION AUTHORIZATION tak;
 SET
 postgres= SELECT * INTO t1 FROM (VALUES(1,'aaa'), (2,'bbb'), (3,'ccc')) AS v;
 SELECT 3
 postgres= SELECT * FROM t1;
  column1 | column2
 -+-
       1 | aaa
       2 | bbb
       3 | ccc
 (3 rows)

 postgres= INSERT INTO t1 VALUES (4,'ddd');
 ERROR:  permission denied for relation t1

 Oops!

You could make the argument that there's no real security hole there,
because the user could give himself those same privileges right back.
You could also make the argument that a SELECT .. INTO is not the same
as an INSERT and therefore INSERT privileges shouldn't be checked.  I
think there are valid arguments on both sides, but my main point is
that we shouldn't have core do it one way and sepgsql do it the other
way.  That's going to be impossible to maintain and doesn't really
make any logical sense anyway.

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

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


Re: [HACKERS] new compiler warnings

2011-10-18 Thread Kevin Grittner
Tom Lane t...@sss.pgh.pa.us wrote:
 
   If the O_NONBLOCK flag is clear, a write request may cause the
   thread to block, but on normal completion it shall return
   nbyte.
 
 Note the last in particular.  Short writes are specifically
 disallowed on pipes.
 
OK, that's pretty definitive.  I yield the point.
 
-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] new compiler warnings

2011-10-18 Thread Kevin Grittner
Andrew Dunstan and...@dunslane.net wrote:
 
 My dim recollection is that Tom and I and maybe some others did
 tests on a bunch of platforms at the time we introduced the
 protocol to make sure it did work this way, since it's crucial to
 making sure we don't get interleaved log lines.
 
Testing is good; I like testing.  But I've seen people code to
implementation details in such a way that things worked fine until
the next release of a product, when the implementation changed.  I
was surprised to see Tom, who is normally such a stickler for doing
such things correctly, apparently going the other way this time; but
it turns out that he had noted a guarantee in the API that I'd
missed.  Mystery solved.
 
Perhaps something in the comments would help people avoid making the
same mistake I did.
 
-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) regression diffs on collate.linux.utf8 test

2011-10-18 Thread Tom Lane
Peter Eisentraut pete...@gmx.net writes:
 On tis, 2011-10-18 at 01:07 -0700, Jeff Davis wrote:
 If I qualify it as tr_TR.UTF-8 it works. Perhaps I have something
 misconfigured on my system (Ubuntu 11.10)? I just installed:
 language-pack-de
 language-pack-tr
 language-pack-sv
 in an attempt to make the test work, and it works all except for that
 lc_time settng.

 I think the language-pack packages have nothing to do with it; they only
 supply translations.

 Possibly, things are set up so that only UTF-8 locales are installed by
 default.  Since the collate.linux.utf8 requires a UTF-8 environment, it
 seems reasonable to use the tr_TR.UTF-8 locale for LC_TIME, instead of
 requiring an unrelated (ISO-8859-9) locale to be installed.  So I think
 the change you propose is reasonable.

As I said to Jeff earlier, I'd rather not embed assumptions about the
spelling of encoding names into this test.  So I don't want to do this
just to get rid of an unexplained failure.  I don't entirely believe
the above theory, because it's not clear why Jeff's machine is behaving
differently from mine.

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) regression diffs on collate.linux.utf8 test

2011-10-18 Thread Peter Eisentraut
On tis, 2011-10-18 at 15:21 -0400, Tom Lane wrote:
 Peter Eisentraut pete...@gmx.net writes:
  On tis, 2011-10-18 at 01:07 -0700, Jeff Davis wrote:
  If I qualify it as tr_TR.UTF-8 it works. Perhaps I have something
  misconfigured on my system (Ubuntu 11.10)? I just installed:
  language-pack-de
  language-pack-tr
  language-pack-sv
  in an attempt to make the test work, and it works all except for that
  lc_time settng.
 
  I think the language-pack packages have nothing to do with it; they only
  supply translations.
 
  Possibly, things are set up so that only UTF-8 locales are installed by
  default.  Since the collate.linux.utf8 requires a UTF-8 environment, it
  seems reasonable to use the tr_TR.UTF-8 locale for LC_TIME, instead of
  requiring an unrelated (ISO-8859-9) locale to be installed.  So I think
  the change you propose is reasonable.
 
 As I said to Jeff earlier, I'd rather not embed assumptions about the
 spelling of encoding names into this test.  So I don't want to do this
 just to get rid of an unexplained failure.  I don't entirely believe
 the above theory, because it's not clear why Jeff's machine is behaving
 differently from mine.

Presumably because Jeff doesn't have that particular locale installed.
locale -a would clarify that.

glibc has always accepted variant locale name spellings such as UTF-8
vs utf8, so it's not a problem.


-- 
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] termination of backend waiting for sync rep generates a junk log message

2011-10-18 Thread Robert Haas
On Mon, Oct 17, 2011 at 6:53 AM, Fujii Masao masao.fu...@gmail.com wrote:
 WARNING:  canceling the wait for synchronous replication and
 terminating connection due to administrator command
 DETAIL:  The transaction has already committed locally, but might not
 have been replicated to the standby.
 backend FATAL:  terminating connection due to administrator command

 The above is the server log messages that I got when I did the procedure.
 backend  is a junk. If a backend is terminated while it's waiting for
 synchronous replication, whereToSendOutput is set to DestNone. Then,
 whereToSendOutput=DestNone makes ReadCommand() call
 InteractiveBackend() which outputs backend .

 This junk message might mess up the server log monitoring tools. I think
 it should be removed.

 The simple fix is to change InteractiveBackend() so that it calls
 CHECK_FOR_INTERRUPTS() before it outputs backend . Thought?

I'm tempted to say we should do that in PostgresMain() instead, maybe
something like this:

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 976a832..9e5557c 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3769,6 +3769,8 @@ PostgresMain(int argc, char *argv[], const char *username)
MemoryContextSwitchTo(MessageContext);
MemoryContextResetAndDeleteChildren(MessageContext);

+   CHECK_FOR_INTERRUPTS();
+
initStringInfo(input_message);

/*

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

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


Re: [HACKERS] new compiler warnings

2011-10-18 Thread Robert Haas
On Tue, Oct 18, 2011 at 3:03 PM, Kevin Grittner
kevin.gritt...@wicourts.gov wrote:
 Andrew Dunstan and...@dunslane.net wrote:

 My dim recollection is that Tom and I and maybe some others did
 tests on a bunch of platforms at the time we introduced the
 protocol to make sure it did work this way, since it's crucial to
 making sure we don't get interleaved log lines.

 Testing is good; I like testing.  But I've seen people code to
 implementation details in such a way that things worked fine until
 the next release of a product, when the implementation changed.  I
 was surprised to see Tom, who is normally such a stickler for doing
 such things correctly, apparently going the other way this time; but
 it turns out that he had noted a guarantee in the API that I'd
 missed.  Mystery solved.

 Perhaps something in the comments would help people avoid making the
 same mistake I did.

Unfortunately, whether Tom's right or not, we still don't have a
solution to the compiler warning.

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

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


Re: [HACKERS] new compiler warnings

2011-10-18 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 Unfortunately, whether Tom's right or not, we still don't have a
 solution to the compiler warning.

I don't actually see that warning on my Fedora 15 machine, with
gcc version 4.6.1 20110908 (Red Hat 4.6.1-9) (GCC)

What are the people who do see it using?

(I do see the -Waddress ones.)

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] new compiler warnings

2011-10-18 Thread Peter Eisentraut
On tis, 2011-10-18 at 15:43 -0400, Tom Lane wrote:
 Robert Haas robertmh...@gmail.com writes:
  Unfortunately, whether Tom's right or not, we still don't have a
  solution to the compiler warning.
 
 I don't actually see that warning on my Fedora 15 machine, with
   gcc version 4.6.1 20110908 (Red Hat 4.6.1-9) (GCC)
 
 What are the people who do see it using?
 
 (I do see the -Waddress ones.)
 
   regards, tom lane
 

You get the unused return value warnings with -D_FORTIFY_SOURCE=2,
which has been the default on Ubuntu for years, and has been the default
on Debian for a few weeks (if you have the hardening-wrapper package
installed or running under dpkg-buildpackage).


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


Re: [HACKERS] new compiler warnings

2011-10-18 Thread Kevin Grittner
Tom Lane t...@sss.pgh.pa.us wrote:
 
 What are the people who do see it using?
 
Currently:
 
gcc (Ubuntu/Linaro 4.5.2-8ubuntu4) 4.5.2
 
on
 
Linux version 2.6.38-11-generic (buildd@allspice) (gcc version 4.5.2
(Ubuntu/Linaro 4.5.2-8ubuntu4) ) #50-Ubuntu SMP Mon Sep 12 21:17:25
UTC 2011
 
I've seen it on earlier versions of Ubuntu and Kubuntu, but not sure
which exactly.  As Peter says, it goes back for years.
 
-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] new compiler warnings

2011-10-18 Thread Kevin Grittner
Robert Haas robertmh...@gmail.com wrote:
 
 Unfortunately, whether Tom's right or not, we still don't have a
 solution to the compiler warning.
 
Would it be too weird to do something like this for each?:
 
diff --git a/src/backend/utils/error/elog.c
b/src/backend/utils/error/elog.c
index f0b3b1f..bea5489 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -1747,6 +1747,7 @@ write_eventlog(int level, const char *line, int
len)
 static void
 write_console(const char *line, int len)
 {
+ssize_t rc;
 #ifdef WIN32
 
/*
@@ -1794,7 +1795,12 @@ write_console(const char *line, int len)
 */
 #endif
 
-   write(fileno(stderr), line, len);
+   rc = write(fileno(stderr), line, len);
+   if (rc = 0  rc != len)
+   {
+   Assert(false);
+   return;
+   }
 }
 
 /*
 
-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] new compiler warnings

2011-10-18 Thread Tom Lane
Peter Eisentraut pete...@gmx.net writes:
 On tis, 2011-10-18 at 15:43 -0400, Tom Lane wrote:
 I don't actually see that warning on my Fedora 15 machine, with
 gcc version 4.6.1 20110908 (Red Hat 4.6.1-9) (GCC)

 You get the unused return value warnings with -D_FORTIFY_SOURCE=2,
 which has been the default on Ubuntu for years, and has been the default
 on Debian for a few weeks (if you have the hardening-wrapper package
 installed or running under dpkg-buildpackage).

Ah-hah.  That's also the default on Red Hat platforms, *if* you are
building RPMs, and now that I think of it, I do see this warning when
building RPMs.  Seems weird that they'd have set it up that way though
rather than with a -W switch.

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] new compiler warnings

2011-10-18 Thread Robert Haas
On Tue, Oct 18, 2011 at 4:13 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Peter Eisentraut pete...@gmx.net writes:
 On tis, 2011-10-18 at 15:43 -0400, Tom Lane wrote:
 I don't actually see that warning on my Fedora 15 machine, with
 gcc version 4.6.1 20110908 (Red Hat 4.6.1-9) (GCC)

 You get the unused return value warnings with -D_FORTIFY_SOURCE=2,
 which has been the default on Ubuntu for years, and has been the default
 on Debian for a few weeks (if you have the hardening-wrapper package
 installed or running under dpkg-buildpackage).

 Ah-hah.  That's also the default on Red Hat platforms, *if* you are
 building RPMs, and now that I think of it, I do see this warning when
 building RPMs.  Seems weird that they'd have set it up that way though
 rather than with a -W switch.

Yeah, that's *quite* odd.

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

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


Re: [HACKERS] new compiler warnings

2011-10-18 Thread Tom Lane
Kevin Grittner kevin.gritt...@wicourts.gov writes:
 Would it be too weird to do something like this for each?:
 
 -   write(fileno(stderr), line, len);
 +   rc = write(fileno(stderr), line, len);
 +   if (rc = 0  rc != len)
 +   {
 +   Assert(false);
 +   return;
 +   }

I don't think the assert is a good idea.  If it ever did happen, that
would promote the problem from corrupted data in the log to database
crash.

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] new compiler warnings

2011-10-18 Thread Heikki Linnakangas

On 18.10.2011 23:28, Tom Lane wrote:

Kevin Grittnerkevin.gritt...@wicourts.gov  writes:

Would it be too weird to do something like this for each?:



-   write(fileno(stderr), line, len);
+   rc = write(fileno(stderr), line, len);
+   if (rc= 0  rc != len)
+   {
+   Assert(false);
+   return;
+   }


I don't think the assert is a good idea.  If it ever did happen, that
would promote the problem from corrupted data in the log to database
crash.


I believe the idea is that if there's a platform that does that, we want 
to know. In production, you don't run with assertions enabled. It makes 
sense to me, or can we fall back to logging a warning to stderr or 
something?


--
  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] new compiler warnings

2011-10-18 Thread Kevin Grittner
Tom Lane t...@sss.pgh.pa.us wrote:
 
 I don't think the assert is a good idea.  If it ever did happen,
 that would promote the problem from corrupted data in the log to
 database crash.
 
... on a --enable-cassert build.
 
If we think it's even remotely possible that it could happen, maybe
we should do the loop.  That would change the current missing log
information situation to interleaved log information.
 
But if we think it would be better for data to be missing from the
log than interleaved, the Assert could be removed and it still
suppresses the error (at least on my machine).
 
-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] termination of backend waiting for sync rep generates a junk log message

2011-10-18 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Mon, Oct 17, 2011 at 6:53 AM, Fujii Masao masao.fu...@gmail.com wrote:
 The simple fix is to change InteractiveBackend() so that it calls
 CHECK_FOR_INTERRUPTS() before it outputs backend . Thought?

 I'm tempted to say we should do that in PostgresMain() instead, maybe
 something like this:

 diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
 index 976a832..9e5557c 100644
 --- a/src/backend/tcop/postgres.c
 +++ b/src/backend/tcop/postgres.c
 @@ -3769,6 +3769,8 @@ PostgresMain(int argc, char *argv[], const char 
 *username)
 MemoryContextSwitchTo(MessageContext);
 MemoryContextResetAndDeleteChildren(MessageContext);

 +   CHECK_FOR_INTERRUPTS();
 +
 initStringInfo(input_message);

 /*

I don't like putting a CHECK_FOR_INTERRUPTS there, because it's way too
late to throw an error for the previous query.

The real problem here is probably that we're overloading the meaning of
whereToSendOutput.  The reset of that variable during shutdown was only
ever meant to prevent output from being sent to a no-longer-present
client.  It should *not* result in trying to read a query from stdin.

Another question worth asking is how is it that we're getting to
ReadCommand at all, if we have already determined that the client is
gone.  Fixing that with an additional CHECK_FOR_INTERRUPTS seems like
a crock.

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] new compiler warnings

2011-10-18 Thread Tom Lane
Kevin Grittner kevin.gritt...@wicourts.gov writes:
 Tom Lane t...@sss.pgh.pa.us wrote:
 I don't think the assert is a good idea.  If it ever did happen,
 that would promote the problem from corrupted data in the log to
 database crash.
 
 ... on a --enable-cassert build.
 
 If we think it's even remotely possible that it could happen, maybe
 we should do the loop.  That would change the current missing log
 information situation to interleaved log information.

The logging protocol is hosed either way.

 But if we think it would be better for data to be missing from the
 log than interleaved, the Assert could be removed and it still
 suppresses the error (at least on my machine).

As far as getting rid of the compiler warning is concerned, I find that
the

rc = write(...);
(void) rc;

suggestion works for me (gcc 4.6.1).  I'm inclined to do that (and
document why) rather than put in looping code that will not make
anything better.

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] new compiler warnings

2011-10-18 Thread Kevin Grittner
Tom Lane t...@sss.pgh.pa.us wrote:
 
 As far as getting rid of the compiler warning is concerned, I find
 that the
 
   rc = write(...);
   (void) rc;
 
 suggestion works for me (gcc 4.6.1).
 
That silences the warning on my machine, too.
 
-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] new compiler warnings

2011-10-18 Thread Tom Lane
I wrote:
 I think a large fraction of the -Waddress warnings are coming from
 this line in the heap_getattr macro:
   AssertMacro((tup) != NULL), \
 Seems to me we could just lose that test and be no worse off, since
 the macro is surely gonna dump core anyway on a null pointer.

Actually, all the ones in the backend are coming from that, so I went
ahead and removed that.

 But some of the remaining -Waddress warnings are not so painless to
 get rid of.  Ultimately we might have to add -Wno-address to the
 default CFLAGS.

The remaining -Waddress warnings are all from applying PQExpBufferBroken
to the address of a local variable.  We could silence them along these
lines:


*** src/interfaces/libpq/pqexpbuffer.h.orig Thu Apr 28 16:07:00 2011
--- src/interfaces/libpq/pqexpbuffer.h  Tue Oct 18 17:46:18 2011
***
*** 60,65 
--- 60,73 
((str) == NULL || (str)-maxlen == 0)
  
  /*
+  * Same, but for use when using a static or local PQExpBufferData struct.
+  * For that, a null-pointer test is useless and may draw compiler warnings.
+  *
+  */
+ #define PQExpBufferDataBroken(buf)\
+   ((buf).maxlen == 0)
+ 
+ /*
   * Initial size of the data buffer in a PQExpBuffer.
   * NB: this must be large enough to hold error messages that might
   * be returned by PQrequestCancel().
*** src/interfaces/libpq/fe-connect.c.orig  Sun Sep 25 18:43:15 2011
--- src/interfaces/libpq/fe-connect.c   Tue Oct 18 17:46:58 2011
***
*** 829,835 
PQconninfoOption *connOptions;
  
initPQExpBuffer(errorBuf);
!   if (PQExpBufferBroken(errorBuf))
return NULL;/* out of memory already :-( */
connOptions = conninfo_parse(, errorBuf, true);
termPQExpBuffer(errorBuf);
--- 829,835 
PQconninfoOption *connOptions;
  
initPQExpBuffer(errorBuf);
!   if (PQExpBufferDataBroken(errorBuf))
return NULL;/* out of memory already :-( */
connOptions = conninfo_parse(, errorBuf, true);
termPQExpBuffer(errorBuf);
***
*** 3967,3973 
if (errmsg)
*errmsg = NULL; /* default */
initPQExpBuffer(errorBuf);
!   if (PQExpBufferBroken(errorBuf))
return NULL;/* out of memory already :-( */
connOptions = conninfo_parse(conninfo, errorBuf, false);
if (connOptions == NULL  errmsg)
--- 3967,3973 
if (errmsg)
*errmsg = NULL; /* default */
initPQExpBuffer(errorBuf);
!   if (PQExpBufferDataBroken(errorBuf))
return NULL;/* out of memory already :-( */
connOptions = conninfo_parse(conninfo, errorBuf, false);
if (connOptions == NULL  errmsg)


(and one similar place in psql, which I did not bother to test).

Any objections or better ideas?

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] new compiler warnings

2011-10-18 Thread Tom Lane
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
 On 18.10.2011 23:28, Tom Lane wrote:
 I don't think the assert is a good idea.  If it ever did happen, that
 would promote the problem from corrupted data in the log to database
 crash.

 I believe the idea is that if there's a platform that does that, we want 
 to know. In production, you don't run with assertions enabled. It makes 
 sense to me, or can we fall back to logging a warning to stderr or 
 something?

Unfortunately, the problem we're dealing with here is exactly that we
can't write to stderr.  So it's a bit hard to see what we can usefully
do to report that we have a problem (short of crashing, which isn't a
net improvement).

In practice, the lack of field reports of corrupted postmaster logs
seems to me to be adequate evidence that the code does work as intended.
All we really need to do is shut gcc up about 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] Silent failure with invalid hba_file setting

2011-10-18 Thread Tom Lane
Thom Brown t...@linux.com writes:
 I noticed that if the hba_file setting in the config is uncommented
 and set to a directory instead of the full path to the file, no error
 occurs when the service starts.

When I try that, I get a boatload of errors ending with
FATAL:  could not load pg_hba.conf

I suspect what happened to you is that the directory read like an empty
file, so Postgres didn't see any error condition.  I suppose we could
add an fstat test to see if we'd opened something other than a regular
file, but I'm not terribly excited about it.

 The problem with this is you cannot get into the database as it acts
 as if it did find the hba file but found it empty.

Well, an actually empty pg_hba.conf file would have the same problem,
and it's pretty hard to see any situation where it would be useful to
start the postmaster and not let it accept any connections.  Should we
add a check to consider it an error if the file doesn't contain at least
one HBA record?

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] Silent failure with invalid hba_file setting

2011-10-18 Thread Thom Brown
On 19 October 2011 00:38, Tom Lane t...@sss.pgh.pa.us wrote:
 Thom Brown t...@linux.com writes:
 I noticed that if the hba_file setting in the config is uncommented
 and set to a directory instead of the full path to the file, no error
 occurs when the service starts.

 When I try that, I get a boatload of errors ending with
 FATAL:  could not load pg_hba.conf

 I suspect what happened to you is that the directory read like an empty
 file, so Postgres didn't see any error condition.  I suppose we could
 add an fstat test to see if we'd opened something other than a regular
 file, but I'm not terribly excited about it.

 The problem with this is you cannot get into the database as it acts
 as if it did find the hba file but found it empty.

 Well, an actually empty pg_hba.conf file would have the same problem,
 and it's pretty hard to see any situation where it would be useful to
 start the postmaster and not let it accept any connections.  Should we
 add a check to consider it an error if the file doesn't contain at least
 one HBA record?

That would solve both problems, so +1 for that.

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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] termination of backend waiting for sync rep generates a junk log message

2011-10-18 Thread Robert Haas
On Tue, Oct 18, 2011 at 5:18 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Mon, Oct 17, 2011 at 6:53 AM, Fujii Masao masao.fu...@gmail.com wrote:
 The simple fix is to change InteractiveBackend() so that it calls
 CHECK_FOR_INTERRUPTS() before it outputs backend . Thought?

 I'm tempted to say we should do that in PostgresMain() instead, maybe
 something like this:

 diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
 index 976a832..9e5557c 100644
 --- a/src/backend/tcop/postgres.c
 +++ b/src/backend/tcop/postgres.c
 @@ -3769,6 +3769,8 @@ PostgresMain(int argc, char *argv[], const char 
 *username)
                 MemoryContextSwitchTo(MessageContext);
                 MemoryContextResetAndDeleteChildren(MessageContext);

 +               CHECK_FOR_INTERRUPTS();
 +
                 initStringInfo(input_message);

                 /*

 I don't like putting a CHECK_FOR_INTERRUPTS there, because it's way too
 late to throw an error for the previous query.

 The real problem here is probably that we're overloading the meaning of
 whereToSendOutput.  The reset of that variable during shutdown was only
 ever meant to prevent output from being sent to a no-longer-present
 client.  It should *not* result in trying to read a query from stdin.

I think you're right, but am not sure how to fix it.

 Another question worth asking is how is it that we're getting to
 ReadCommand at all, if we have already determined that the client is
 gone.  Fixing that with an additional CHECK_FOR_INTERRUPTS seems like
 a crock.

We haven't determined the client is gone; we're trying to close the
connection unexpectedly.  As the comment in SyncRepWaitForLSN
explains:

/*
 * If a wait for synchronous replication is pending,
we can neither
 * acknowledge the commit nor raise ERROR or FATAL.
The latter would
 * lead the client to believe that that the
transaction aborted, which
 * is not true: it's already committed locally. The
former is no good
 * either: the client has requested synchronous
replication, and is
 * entitled to assume that an acknowledged commit is
also replicated,
 * which might not be true. So in this case we issue a
WARNING (which
 * some clients may be able to interpret) and shut off
further output.
 * We do NOT reset ProcDiePending, so that the process
will die after
 * the commit is cleaned up.
 */

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

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


Re: [HACKERS] [v9.2] Fix Leaky View Problem

2011-10-18 Thread Robert Haas
On Sun, Oct 16, 2011 at 4:46 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 Hi Robert,

 I'm a bit confusing about this sentence.

 If you can make this work, I think it could be a pretty sweet plannner
 optimization even apart from the implications for security views.
 Consider a query of this form:

 A LEFT JOIN B LEFT JOIN C

 where B is a view defined as:

 B1 JOIN B2 JOIN B3 LEFT JOIN B4 LEFT JOIN B5

 Now let's suppose that from_collapse_limit/join_collapse_limit are set
 low enough that we decline to fold these subproblems together.  If
 there happens to be a qual B.x = 1, where B.x is really B1.x, then the
 generated plan sucks, because it will basically lose the ability to
 filter B1 early, very possibly on, say, a unique index.  Or at least a
 highly selective index.


 I tried to reproduce the scenario with enough small from/join_collapse_limit
 (typically 1), but it allows to push down qualifiers into the least scan plan.

Hmm, you're right.  LIMIT 10 prevents qual pushdown, but
hitting from_collapse_limit/join_collapse_limit apparently doesn't.  I
could have sworn I've seen this work the other way, but I guess not.

 E.g)
 mytest=# SET from_collapse_limit = 1;
 mytest=# SET join_collapse_limit = 1;
 mytest=# CREATE VIEW B AS SELECT B1.* FROM B1,B2,B3 WHERE B1.x = B2.x
 AND B2.x = B3.x;
 mytest=# EXPLAIN SELECT * FROM A,B,C WHERE A.x=B.x AND B.x=C.x AND 
 f_leak(B.y);

This I wouldn't expect to have any effect anyway, because you're using
the ad-hoc join syntax rather than explicit join syntax.  But I tried
it with explicit join syntax and it seems to only constrain the join
order, not prevent qual pushdown.

 I agree with the following approach to tackle this problem in 100%.
 However, I'm unclear how from/join_collapse_limit affects to keep
 sub-queries unflatten. It seems to me it is determined based on
 the result of is_simple_subquery().

I think you are right, but I'm not sure it's right to hack
is_simple_subquery() directly.  Perhaps what we want to do is modify
pull_up_subquery()?

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

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


Re: [HACKERS] loss of transactions in streaming replication

2011-10-18 Thread Robert Haas
On Fri, Oct 14, 2011 at 7:51 AM, Fujii Masao masao.fu...@gmail.com wrote:
 On Thu, Oct 13, 2011 at 10:08 AM, Fujii Masao masao.fu...@gmail.com wrote:
 On Wed, Oct 12, 2011 at 10:29 PM, Robert Haas robertmh...@gmail.com wrote:
 On Wed, Oct 12, 2011 at 5:45 AM, Fujii Masao masao.fu...@gmail.com wrote:
 In 9.2dev and 9.1, when walreceiver detects an error while sending data to
 WAL stream, it always emits ERROR even if there are data available in the
 receive buffer. This might lead to loss of transactions because such
 remaining data are not received by walreceiver :(

 Won't it just reconnect?

 Yes if the master is running normally. OTOH, if the master is not running 
 (i.e.,
 failover case), the standby cannot receive again the data which it failed to
 receive.

 I found this issue when I shut down the master. When the master shuts down,
 it sends the shutdown checkpoint record, but I found that the standby failed
 to receive it.

 Patch attached.

 The patch changes walreceiver so that it doesn't emit ERROR just yet even
 if it fails to send data to WAL stream. Then, after all available data have 
 been
 received and flushed to the disk, it emits ERROR.

 If the patch is OK, it should be backported to v9.1.

Convince me.  :-)

My reading of the situation is that you're talking about a problem
that will only occur if, while the master is in the process of
shutting down, a network error occurs.  I am not sure it's a good idea
to convolute the code to handle that case, because (1) there are going
to be many similar situations where nothing within our power is
sufficient to prevent WAL from failing to make it to the standby and
(2) for this marginal improvement, you're giving up including
PQerrorMessage(streamConn) in the error message that ultimately gets
omitted, which seems like a substantial regression as far as
debuggability is concerned.  Even if we do decide that we want the
change in behavior, I see no compelling reason to back-patch it.
Stable releases are supposed to be stable, not change behavior because
we thought of something we like better than what we originally
released.

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

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


Re: [HACKERS] Online base backup from the hot-standby

2011-10-18 Thread Jun Ishiduka

  +   /*
  +* The backend writes WAL of FPW at checkpoint. However, The 
  backend do
  +* not need to write WAL of FPW at checkpoint shutdown because 
  it
  +* performs when startup finishes.
  +*/
  +   XLogReportParameters(REPORT_ON_BACKEND);
  
  I'm still unclear why that WAL doesn't need to be written at shutdown
  checkpoint.
  Anyway, the first sentence in the above comments is not right. Not a 
  backend but
  a bgwriter writes that WAL at checkpoint.
  
  The second also seems not to be right. It implies that a shutdown 
  checkpoint is
  performed only at end of startup. But it may be done when smart or fast 
  shutdown
  is requested.
 
 
 Okay. 
 I change to the following messages.
 
 /* 
  * The bgwriter writes WAL of FPW at checkpoint. But does not at shutdown.
  * Because XLogReportParameters() is always called at the end of startup
  * process, it does not need to be called at shutdown.
  */
 
 
 In addition, I change macro name.
 
 REPORT_ON_BACKEND - REPORT_ON_BGWRITER

I have updated as above-comment.
Please check this.

Regards.


Jun Ishizuka
NTT Software Corporation
TEL:045-317-7018
E-Mail: ishizuka@po.ntts.co.jp



standby_online_backup_09base-06fpw.patch
Description: Binary data

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


Re: [HACKERS] [v9.2] Fix Leaky View Problem

2011-10-18 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Sun, Oct 16, 2011 at 4:46 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 I tried to reproduce the scenario with enough small from/join_collapse_limit
 (typically 1), but it allows to push down qualifiers into the least scan 
 plan.

 Hmm, you're right.  LIMIT 10 prevents qual pushdown, but
 hitting from_collapse_limit/join_collapse_limit apparently doesn't.  I
 could have sworn I've seen this work the other way, but I guess not.

No, the collapse_limit variables are entirely unrelated to subquery
flattening, or to qual pushdown for that matter.  They only restrict the
number of join paths we consider.  And we will attempt to push down
quals into an unflattened subquery, too, if it looks safe.  See
subquery_is_pushdown_safe, qual_is_pushdown_safe, etc in allpaths.c.

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] termination of backend waiting for sync rep generates a junk log message

2011-10-18 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Tue, Oct 18, 2011 at 5:18 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Another question worth asking is how is it that we're getting to
 ReadCommand at all, if we have already determined that the client is
 gone.  Fixing that with an additional CHECK_FOR_INTERRUPTS seems like
 a crock.

 We haven't determined the client is gone; we're trying to close the
 connection unexpectedly.  As the comment in SyncRepWaitForLSN
 explains:

 /*
  * If a wait for synchronous replication is pending,
 we can neither
  * acknowledge the commit nor raise ERROR or FATAL.
 The latter would
  * lead the client to believe that that the
 transaction aborted, which
  * is not true: it's already committed locally. The
 former is no good
  * either: the client has requested synchronous
 replication, and is
  * entitled to assume that an acknowledged commit is
 also replicated,
  * which might not be true. So in this case we issue a
 WARNING (which
  * some clients may be able to interpret) and shut off
 further output.
  * We do NOT reset ProcDiePending, so that the process
 will die after
  * the commit is cleaned up.
  */

Hmm.  Maybe the real answer is this code is abusing whereToSendOutput
(and about six other things besides).  I'll try to think of a better
solution, but not tonight.

One thing worth asking is why we're willing to violate half a dozen
different coding rules if we see ProcDiePending, yet we're perfectly
happy to rely on the client understanding a WARNING for the
QueryCancelPending case.  Another is whether this whole function isn't
complete BS in the first place, since it appears to be coded on the
obviously-false assumption that nothing it calls can throw elog(ERROR)
--- and of course, if any of those functions do throw ERROR, all the
argumentation here goes out the window.

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] new compiler warnings

2011-10-18 Thread Peter Eisentraut
On tis, 2011-10-18 at 16:13 -0400, Tom Lane wrote:
 Peter Eisentraut pete...@gmx.net writes:
  On tis, 2011-10-18 at 15:43 -0400, Tom Lane wrote:
  I don't actually see that warning on my Fedora 15 machine, with
  gcc version 4.6.1 20110908 (Red Hat 4.6.1-9) (GCC)
 
  You get the unused return value warnings with -D_FORTIFY_SOURCE=2,
  which has been the default on Ubuntu for years, and has been the default
  on Debian for a few weeks (if you have the hardening-wrapper package
  installed or running under dpkg-buildpackage).
 
 Ah-hah.  That's also the default on Red Hat platforms, *if* you are
 building RPMs, and now that I think of it, I do see this warning when
 building RPMs.  Seems weird that they'd have set it up that way though
 rather than with a -W switch.

There is a switch -Wunused-result, but it's on by default.



-- 
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] Silent failure with invalid hba_file setting

2011-10-18 Thread Peter Eisentraut
On tis, 2011-10-18 at 18:38 -0400, Tom Lane wrote:
  The problem with this is you cannot get into the database as it acts
  as if it did find the hba file but found it empty.
 
 Well, an actually empty pg_hba.conf file would have the same problem,
 and it's pretty hard to see any situation where it would be useful to
 start the postmaster and not let it accept any connections.  Should we
 add a check to consider it an error if the file doesn't contain at least
 one HBA record?

If you try to connect and it doesn't find a record, it will tell you.

I wouldn't add extra special checks for that.  It might not be
completely unreasonable to have a standby that no one can connect to,
for example.


-- 
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] termination of backend waiting for sync rep generates a junk log message

2011-10-18 Thread Robert Haas
On Tue, Oct 18, 2011 at 11:27 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 One thing worth asking is why we're willing to violate half a dozen
 different coding rules if we see ProcDiePending, yet we're perfectly
 happy to rely on the client understanding a WARNING for the
 QueryCancelPending case.  Another is whether this whole function isn't
 complete BS in the first place, since it appears to be coded on the
 obviously-false assumption that nothing it calls can throw elog(ERROR)
 --- and of course, if any of those functions do throw ERROR, all the
 argumentation here goes out the window.

Well, there is a general problem that anything which throws an ERROR
too late in the commit path is Evil; and sync rep makes that worse to
the extent that it adds more stuff late in the commit path, but it
didn't invent the problem.  What it did do is add stuff late in the
commit path that can block for a potentially unbounded period of time,
and I don't see that there are any solutions to that problem that
aren't somewhat grotty.

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

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


Re: [HACKERS] Silent failure with invalid hba_file setting

2011-10-18 Thread Tom Lane
Peter Eisentraut pete...@gmx.net writes:
 On tis, 2011-10-18 at 18:38 -0400, Tom Lane wrote:
 Well, an actually empty pg_hba.conf file would have the same problem,
 and it's pretty hard to see any situation where it would be useful to
 start the postmaster and not let it accept any connections.  Should we
 add a check to consider it an error if the file doesn't contain at least
 one HBA record?

 If you try to connect and it doesn't find a record, it will tell you.

Yeah, but the damage is already done.  I see the main practical benefit
of this being to prevent accidental loading of a trashed pg_hba file.

 I wouldn't add extra special checks for that.  It might not be
 completely unreasonable to have a standby that no one can connect to,
 for example.

Well, you couldn't monitor its state then, so I don't find that example
very convincing.  But if you were intent on having that, you could
easily set up a pg_hba file containing only reject entries.

regards, tom lane

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


Re: [HACKERS] pg_ctl restart - behaviour based on wrong instance

2011-10-18 Thread Robert Haas
On Tue, Oct 18, 2011 at 12:18 PM, Fujii Masao masao.fu...@gmail.com wrote:
 On Tue, Oct 18, 2011 at 11:02 PM, Magnus Hagander mag...@hagander.net wrote:
 Ok, fixed and applied.

 You seem to have forgot to change protocol.sgml.
 Patch attached.

Committed.

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

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


Re: [HACKERS] Adding CORRESPONDING to Set Operations

2011-10-18 Thread Robert Haas
On Sun, Oct 16, 2011 at 7:46 PM, Kerem Kat kerem...@gmail.com wrote:
 CORRESPONDING clause take 2

You should probably read this:

http://wiki.postgresql.org/wiki/Submitting_a_Patch

And add your patch here:

https://commitfest.postgresql.org/action/commitfest_view/open

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

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


Re: [HACKERS] termination of backend waiting for sync rep generates a junk log message

2011-10-18 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Tue, Oct 18, 2011 at 11:27 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 One thing worth asking is why we're willing to violate half a dozen
 different coding rules if we see ProcDiePending, yet we're perfectly
 happy to rely on the client understanding a WARNING for the
 QueryCancelPending case.  Another is whether this whole function isn't
 complete BS in the first place, since it appears to be coded on the
 obviously-false assumption that nothing it calls can throw elog(ERROR)
 --- and of course, if any of those functions do throw ERROR, all the
 argumentation here goes out the window.

 Well, there is a general problem that anything which throws an ERROR
 too late in the commit path is Evil; and sync rep makes that worse to
 the extent that it adds more stuff late in the commit path, but it
 didn't invent the problem.  What it did do is add stuff late in the
 commit path that can block for a potentially unbounded period of time,
 and I don't see that there are any solutions to that problem that
 aren't somewhat grotty.

No doubt, but fantasizing about what you are or are not controlling
doesn't help ... and AFAICT this code is mostly fantasy.  Anyway,
I don't have a better proposal right offhand; will think about 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] (patch) regression diffs on collate.linux.utf8 test

2011-10-18 Thread Jeff Davis
On Tue, 2011-10-18 at 22:25 +0300, Peter Eisentraut wrote:
 Presumably because Jeff doesn't have that particular locale installed.
 locale -a would clarify that.

$ locale -a |grep -i tr
tr_CY.utf8
tr_TR.utf8

So, yes, I only have the UTF8 version. I didn't realize they were
different -- do you happen to know what package I need for just plain
tr_TR?

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] (patch) regression diffs on collate.linux.utf8 test

2011-10-18 Thread Tom Lane
Jeff Davis pg...@j-davis.com writes:
 On Tue, 2011-10-18 at 22:25 +0300, Peter Eisentraut wrote:
 Presumably because Jeff doesn't have that particular locale installed.
 locale -a would clarify that.

 $ locale -a |grep -i tr
 tr_CY.utf8
 tr_TR.utf8

 So, yes, I only have the UTF8 version.

Wow, that's interesting.  Digging around on my Fedora box, I can't find
any suggestion that it's even possible to subdivide the locale settings
like that.  I only see one source file for tr_TR --- that's
/usr/share/i18n/locales/tr_TR --- and it looks like all the stuff under 
/usr/share/i18n/locales/ is compiled into one big run-time file
/usr/lib/locale/locale-archive.

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] Silent failure with invalid hba_file setting

2011-10-18 Thread Magnus Hagander
On Oct 19, 2011 6:21 AM, Tom Lane t...@sss.pgh.pa.us wrote:

 Peter Eisentraut pete...@gmx.net writes:
  On tis, 2011-10-18 at 18:38 -0400, Tom Lane wrote:
  Well, an actually empty pg_hba.conf file would have the same problem,
  and it's pretty hard to see any situation where it would be useful to
  start the postmaster and not let it accept any connections.  Should we
  add a check to consider it an error if the file doesn't contain at
least
  one HBA record?

  If you try to connect and it doesn't find a record, it will tell you.

 Yeah, but the damage is already done.  I see the main practical benefit
 of this being to prevent accidental loading of a trashed pg_hba file.

Yeah, definitely. It's very much a pita when you accidentally do that with a
syntax error on 8.4, %. So while I haven't actually managed to hit his
specific problem myself, +1 for this approach.

  I wouldn't add extra special checks for that.  It might not be
  completely unreasonable to have a standby that no one can connect to,
  for example.

 Well, you couldn't monitor its state then, so I don't find that example
 very convincing.  But if you were intent on having that, you could
 easily set up a pg_hba file containing only reject entries.


Yeah, seems reasonable to put a (very) small amount of extra work in the
path of a very uncommon scenario in order to protect users in the common
one...

/Magnus