Re: [HACKERS] PostgreSQL Service on Windows does not start. ~ is not a valid Win32 application

2013-10-28 Thread Asif Naeem
Hi Naoya,

I am not able to reproduce the problem. Do you mean pg windows service
installed by installer is not working or bin\pg_ctl binary is not accepting
spaces in the patch ?. Following worked for me i.e.

C:\Users\asif\Desktop\Program files\9.3bin\pg_ctl -D
 C:\Users\asif\Desktop\Program files\9.3\data1 -l logfile start
 server starting


Can you please share the exact steps ?. Thanks.

Regards,
Muhammad Asif Naeem



On Mon, Oct 28, 2013 at 10:26 AM, Naoya Anzai anzai-na...@mxu.nes.nec.co.jp
 wrote:

 Hi All,

 I have found a case that PostgreSQL Service does not start.
 When it happens, the following error appears.

  is not a valid Win32 application

 This failure occurs when the following conditions are true.

 1. There is postgres.exe in any directory that contains a space,
such as Program Files.

e.g.)
C:\Program Files\PostgreSQL\bin\postgres.exe

 2. A file using the first white space-delimited
tokens of that directory as the file name exists,
and there is it in the same hierarchy.

e.g.)
C:\Program //file

 pg_ctl.exe as PostgreSQL Service creates a postgres
 process using an absolute path which indicates the
 location of postgres.exe,but the path is not enclosed
 in quotation.

 Therefore,if the above-mentioned conditions are true,
 CreateProcessAsUser(a Windows Function called by pg_ctl.exe)
 tries to create a process using the other file such
 as Program, so the service fails to start.

 Accordingly, I think that the command path should be
 enclosed in quotation.

 I created a patch to fix this failure,
 So could anyone confirm?

 Regards,

 Naoya

 ---
 Naoya Anzai
 Engineering Department
 NEC Soft, Ltd.
 E-Mail: anzai-na...@mxu.nes.nec.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] Example query causing param_info to be set in plain rel path

2013-10-28 Thread Ashutosh Bapat
No adding OFFSET there too didn't give the expected result. The lateral was
handled in subquery and passed as param to the underlying table scan.

I am particularly interested in tables (unlike functions or subqueries)
since, the table scans are shipped to the datanodes and I wanted to test
the effect of lateral in such cases. OTH, functions involving access to the
tables or subqueries are initiated on the coordinators, where lateral gets
executed in the same way as PostgreSQL.

If it's so hard to come up with an example query which would cause
lateral_relids to be set in RelOptInfo of a table, then it's very likely
that relevant code is untested in PostgreSQL.


On Fri, Oct 25, 2013 at 7:11 PM, Tom Lane t...@sss.pgh.pa.us wrote:

 Ashutosh Bapat ashutosh.ba...@enterprisedb.com writes:
  In order to test various cases of LATERAL join in Postgres-XC, I am
 trying
  to find a query where RelOptInof-lateral_relids would get set for plain
  base relations.

 I think you need a lateral reference in a function or VALUES FROM-item.
 As you say, plain sub-selects are likely to get flattened.  (Possibly
 if you stuck in a flattening fence such as OFFSET 0, you could get the
 case to happen with a sub-select FROM item, but I'm too lazy to check.)

 regards, tom lane




-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company


Re: [HACKERS] PostgreSQL Service on Windows does not start. ~ is not a valid Win32 application

2013-10-28 Thread Naoya Anzai
Hi, Asif.

Thank you for response.


   C:\Users\asif\Desktop\Program files\9.3bin\pg_ctl -D 
 C:\Users\asif\Desktop\Program files\9.3\data1 -l logfile start
   server starting

This failure does not occur by the command line.
PostgreSQL needs to start by Windows Service.

Additionally,In this case, 
A file Program needs to be exist at C:\Users\asif\Desktop\, and 
postgres.exe needs to be exist at C:\Users\asif\Desktop\Program 
files\9.3\bin.

C:\Users\asif\Desktop\Program files\9.3\bindir
...
4,435,456   postgres.exe
   80,896   pg_ctl.exe
...

C:\Users\asif\Desktoppdir
...
0  Program
DIR  Program files
...


Regards,
Naoya

 Hi Naoya,
 
 I am not able to reproduce the problem. Do you mean pg windows service 
 installed by installer is not working or bin\pg_ctl binary is not accepting 
 spaces in the patch ?. Following worked for me i.e.
 
 
   C:\Users\asif\Desktop\Program files\9.3bin\pg_ctl -D 
 C:\Users\asif\Desktop\Program files\9.3\data1 -l logfile start
   server starting
 
 
 Can you please share the exact steps ?. Thanks.
 
 
 Regards,
 Muhammad Asif Naeem
 
 
 
 On Mon, Oct 28, 2013 at 10:26 AM, Naoya Anzai anzai-na...@mxu.nes.nec.co.jp 
 wrote:
 
 
   Hi All,
   
   I have found a case that PostgreSQL Service does not start.
   When it happens, the following error appears.
   
is not a valid Win32 application
   
   This failure occurs when the following conditions are true.
   
   1. There is postgres.exe in any directory that contains a space,
  such as Program Files.
   
  e.g.)
  C:\Program Files\PostgreSQL\bin\postgres.exe
   
   2. A file using the first white space-delimited
  tokens of that directory as the file name exists,
  and there is it in the same hierarchy.
   
  e.g.)
  C:\Program //file
   
   pg_ctl.exe as PostgreSQL Service creates a postgres
   process using an absolute path which indicates the
   location of postgres.exe,but the path is not enclosed
   in quotation.
   
   Therefore,if the above-mentioned conditions are true,
   CreateProcessAsUser(a Windows Function called by pg_ctl.exe)
   tries to create a process using the other file such
   as Program, so the service fails to start.
   
   Accordingly, I think that the command path should be
   enclosed in quotation.
   
   I created a patch to fix this failure,
   So could anyone confirm?
   
   Regards,
   
   Naoya
   
   ---
   Naoya Anzai
   Engineering Department
   NEC Soft, Ltd.
   E-Mail: anzai-na...@mxu.nes.nec.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
   
   
 
 
 

以上、よろしくお願い致します。


NECソフト株式会社
PFシステム事業部 テーマソフト開発G
安西 直也

外線(03)5534-2353
内線(8)57-40364
Mail:NES-N2363
E-mail:anzai-na...@mxu.nes.nec.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] Detection of nested function calls

2013-10-28 Thread Hugo Mercier
Le 25/10/2013 18:44, Tom Lane a écrit :
 Hugo Mercier hugo.merc...@oslandia.com writes:
 Le 25/10/2013 17:20, Tom Lane a écrit :
 How do you tell the difference between

 foo(col1, bar(col2))
 foo(bar(col1), col2)
 
 Still not sure to understand ...
 I assume foo() takes two argument of type A.
 bar() can take one argument of A or another type B.
 
 I was assuming everything was the same datatype in this example, ie
 col1, col2, and the result of bar() are all type A.
 
 The point I'm trying to make is that in the first case, foo would be
 receiving a first argument that was flat and a second that was not flat;
 while in the second case, it would be receiving a first argument that was
 not flat and a second that was flat.  The expression labeling you're
 proposing does not help it tell the difference.

No it does not. It's then up to the data type to store whether it is
flat or not. And every functions manipulating this type is assumed to be
aware of this flat/non-flat flagging.


 
 Another point here is that there's no good reason to suppose that a
 function should return a flattened value just because it's at the outer
 level of its syntactic expression.  For example, if we're doing a plain
 SELECT foo(...) FROM ..., the next thing that will happen with that value
 is it'll be fed to the output function for the datatype.  Maybe that
 output function would like to have a non-flat input value, too, to save
 the time of transforming back to that representation.  On the other hand,
 if it's a SELECT ... ORDER BY ... and the planner chooses to do the ORDER
 BY with a final sort step, we'll probably have to flatten the value to
 pass it through sorting.  (Or possibly not --- perhaps we could just pass
 the toast token through sorting?)  There are a lot of considerations here
 and it's really unreasonable to expect that static expression labeling
 will be able to do the right thing every time.

Again, my proposal is very conservative here. It does not expect to
optimize all spots where copies are not necessary. Only at a some level
of function evaluation with ... some assumptions.

 
 Basically the only way to make this work reliably is for Datums to be
 self-identifying as to whether they're flat or structured values; then
 make code do the right thing on-the-fly at runtime depending on what kind
 of Datum it gets.  Once you've done that, I don't see that parse-time
 labeling of expression nesting adds anything useful.  As Andres said,
 the provisions for toasted datums are a good precedent, and none of that
 depends on parse-time decisions.
 

This is something I have to investigate, thanks for pointing it out.
What I've understood so far is that there is room for new flags in the
TOAST mechanism, so the idea would be to add a new strategy where opaque
pointers could be stored. And it would then require a way for extensions
to register their own (de)toasting functions, right ?


-- 
Hugo Mercier
Oslandia


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


Re: [HACKERS] PostgreSQL Service on Windows does not start. ~ is not a valid Win32 application

2013-10-28 Thread Asif Naeem
It is related to windows unquoted service path vulnerability in the the
installer that creates service path without quotes that make service.exe to
look for undesirable path for executable.

postgresql-9.3 service path : C:/Users/asif/Desktop/Program
files/9.3/bin/pg_ctl.exe runservice -N postgresql-9.3 -D
C:/Users/asif/Desktop/Program files/9.3/data -w

service.exe

 C:\Users\asif\Desktop\Program NAME NOT FOUND
 C:\Users\asif\Desktop\Program.exe NAME NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe ACCESS DENIED
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe ACCESS DENIED

 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice NAME
 NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice.exe
 NAME NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 NAME NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N.exe
   NAME NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3.exe NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D.exe NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program.exe NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data NAME
 INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data.exe
 NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data -w
 NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data -w.exe
   NAME INVALID


Fix :

postgresql-9.3 service path : C:/Users/asif/Desktop/Program
files/9.3/bin/pg_ctl.exe runservice -N postgresql-9.3 -D
C:/Users/asif/Desktop/Program files/9.3/data -w

It would be good if this is reported on pg installer forum or security
forum. Thanks.

Regards,
Asif Naeem

On Mon, Oct 28, 2013 at 12:06 PM, Naoya Anzai anzai-na...@mxu.nes.nec.co.jp
wrote:

 Hi, Asif.

 Thank you for response.


C:\Users\asif\Desktop\Program files\9.3bin\pg_ctl -D
C:\Users\asif\Desktop\Program files\9.3\data1 -l logfile start
server starting

 This failure does not occur by the command line.
 PostgreSQL needs to start by Windows Service.

 Additionally,In this case,
 A file Program needs to be exist at C:\Users\asif\Desktop\, and
 postgres.exe needs to be exist at C:\Users\asif\Desktop\Program
files\9.3\bin.
 
 C:\Users\asif\Desktop\Program files\9.3\bindir
 ...
 4,435,456   postgres.exe
80,896   pg_ctl.exe
 ...

 C:\Users\asif\Desktoppdir
 ...
 0  Program
 DIR  Program files
 ...
 

 Regards,
 Naoya

  Hi Naoya,
 
  I am not able to reproduce the problem. Do you mean pg windows service
installed by installer is not working or bin\pg_ctl binary is not accepting
spaces in the patch ?. Following worked for me i.e.
 
 
C:\Users\asif\Desktop\Program files\9.3bin\pg_ctl -D
C:\Users\asif\Desktop\Program files\9.3\data1 -l logfile start
server starting
 
 
  Can you please share the exact steps ?. Thanks.
 
 
  Regards,
  Muhammad Asif Naeem
 
 
 
  On Mon, Oct 28, 2013 at 10:26 AM, Naoya Anzai 
anzai-na...@mxu.nes.nec.co.jp wrote:
 
 
Hi All,
 
I have found a case that PostgreSQL Service does not start.
When it happens, the following error appears.
 
 is not a valid Win32 application
 
This failure occurs when the following conditions are true.
 
1. There is postgres.exe in any directory that contains a space,
   such as Program Files.
 
   e.g.)
   C:\Program Files\PostgreSQL\bin\postgres.exe
 
2. A file using the first white space-delimited
   tokens of that directory as the file name exists,
   and there is it in the same hierarchy.
 
   e.g.)
   C:\Program //file
 
pg_ctl.exe as PostgreSQL Service creates a postgres
process using an absolute path which indicates the
location of postgres.exe,but the path is not enclosed
in quotation.
 
Therefore,if the above-mentioned conditions are true,
CreateProcessAsUser(a Windows Function called by pg_ctl.exe)
tries to create a process using the other file such
as Program, so the 

Re: [HACKERS] Detection of nested function calls

2013-10-28 Thread Andres Freund
On 2013-10-28 09:13:06 +0100, Hugo Mercier wrote:
 Le 25/10/2013 18:44, Tom Lane a écrit :
  Hugo Mercier hugo.merc...@oslandia.com writes:
  Le 25/10/2013 17:20, Tom Lane a écrit :
  How do you tell the difference between
 
  foo(col1, bar(col2))
  foo(bar(col1), col2)
  
  Still not sure to understand ...
  I assume foo() takes two argument of type A.
  bar() can take one argument of A or another type B.
  
  I was assuming everything was the same datatype in this example, ie
  col1, col2, and the result of bar() are all type A.
  
  The point I'm trying to make is that in the first case, foo would be
  receiving a first argument that was flat and a second that was not flat;
  while in the second case, it would be receiving a first argument that was
  not flat and a second that was flat.  The expression labeling you're
  proposing does not help it tell the difference.
 
 No it does not. It's then up to the data type to store whether it is
 flat or not. And every functions manipulating this type is assumed to be
 aware of this flat/non-flat flagging.

But what if the in-memory type contains pointers and is copied or
spilled to disk? There needs to be a mechanism handling that case.

  Basically the only way to make this work reliably is for Datums to be
  self-identifying as to whether they're flat or structured values; then
  make code do the right thing on-the-fly at runtime depending on what kind
  of Datum it gets.  Once you've done that, I don't see that parse-time
  labeling of expression nesting adds anything useful.  As Andres said,
  the provisions for toasted datums are a good precedent, and none of that
  depends on parse-time decisions.
  
 
 This is something I have to investigate, thanks for pointing it out.
 What I've understood so far is that there is room for new flags in the
 TOAST mechanism, so the idea would be to add a new strategy where opaque
 pointers could be stored. And it would then require a way for extensions
 to register their own (de)toasting functions, right ?

I think we'd need another argument to CREATE FUNCTION like SERIALIZE
pointing to a function that that has to return data that can be stored
on disk. Deserialization would be up to individual functions.

Depending on the specification this might turn out to be slightly
invasive, tuplestore/sort et al probably have to care...

Greetings,

Andres Freund

-- 
 Andres Freund 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] Document update in alter_foreign_data_wrapper.sgml

2013-10-28 Thread Etsuro Fujita
I wrote:
 ISTM the document in alter_foreign_data_wrapper.sgml and the comment in
 foreigncmds.c should be updated.  Please find attached a patch.

I've noticed that the document in create_foreign_data_wrapper.sgml should also
be updated.  Attached is an updated version of the patch.

Thanks,

Best regards,
Etsuro Fujita


fdw-doc-update-2.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] Detection of nested function calls

2013-10-28 Thread Pavel Stehule
2013/10/28 Andres Freund and...@2ndquadrant.com

 On 2013-10-28 09:13:06 +0100, Hugo Mercier wrote:
  Le 25/10/2013 18:44, Tom Lane a écrit :
   Hugo Mercier hugo.merc...@oslandia.com writes:
   Le 25/10/2013 17:20, Tom Lane a écrit :
   How do you tell the difference between
  
   foo(col1, bar(col2))
   foo(bar(col1), col2)
  
   Still not sure to understand ...
   I assume foo() takes two argument of type A.
   bar() can take one argument of A or another type B.
  
   I was assuming everything was the same datatype in this example, ie
   col1, col2, and the result of bar() are all type A.
  
   The point I'm trying to make is that in the first case, foo would be
   receiving a first argument that was flat and a second that was not
 flat;
   while in the second case, it would be receiving a first argument that
 was
   not flat and a second that was flat.  The expression labeling you're
   proposing does not help it tell the difference.
 
  No it does not. It's then up to the data type to store whether it is
  flat or not. And every functions manipulating this type is assumed to be
  aware of this flat/non-flat flagging.

 But what if the in-memory type contains pointers and is copied or
 spilled to disk? There needs to be a mechanism handling that case.

   Basically the only way to make this work reliably is for Datums to be
   self-identifying as to whether they're flat or structured values; then
   make code do the right thing on-the-fly at runtime depending on what
 kind
   of Datum it gets.  Once you've done that, I don't see that parse-time
   labeling of expression nesting adds anything useful.  As Andres said,
   the provisions for toasted datums are a good precedent, and none of
 that
   depends on parse-time decisions.
  
 
  This is something I have to investigate, thanks for pointing it out.
  What I've understood so far is that there is room for new flags in the
  TOAST mechanism, so the idea would be to add a new strategy where opaque
  pointers could be stored. And it would then require a way for extensions
  to register their own (de)toasting functions, right ?

 I think we'd need another argument to CREATE FUNCTION like SERIALIZE
 pointing to a function that that has to return data that can be stored
 on disk. Deserialization would be up to individual functions.

 Depending on the specification this might turn out to be slightly
 invasive, tuplestore/sort et al probably have to care...


Then you need a functions than prepare a clone of unpacked data too.

Regards

Pavel



 Greetings,

 Andres Freund

 --
  Andres Freund 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] PostgreSQL Service on Windows does not start. ~ is not a valid Win32 application

2013-10-28 Thread Dave Page
Sandeep, can you look at this please? Thanks.

On Mon, Oct 28, 2013 at 8:18 AM, Asif Naeem anaeem...@gmail.com wrote:
 It is related to windows unquoted service path vulnerability in the the
 installer that creates service path without quotes that make service.exe to
 look for undesirable path for executable.

 postgresql-9.3 service path : C:/Users/asif/Desktop/Program
 files/9.3/bin/pg_ctl.exe runservice -N postgresql-9.3 -D
 C:/Users/asif/Desktop/Program files/9.3/data -w

 service.exe

 C:\Users\asif\Desktop\Program NAME NOT FOUND
 C:\Users\asif\Desktop\Program.exe NAME NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe ACCESS DENIED
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe ACCESS DENIED
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice NAME
 NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice.exe
 NAME NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 NAME NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N.exe
 NAME NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3.exe NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D.exe NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program.exe NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data NAME
 INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data.exe
 NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data -w
 NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data -w.exe
 NAME INVALID


 Fix :

 postgresql-9.3 service path : C:/Users/asif/Desktop/Program
 files/9.3/bin/pg_ctl.exe runservice -N postgresql-9.3 -D
 C:/Users/asif/Desktop/Program files/9.3/data -w

 It would be good if this is reported on pg installer forum or security
 forum. Thanks.

 Regards,
 Asif Naeem

 On Mon, Oct 28, 2013 at 12:06 PM, Naoya Anzai
 anzai-na...@mxu.nes.nec.co.jp wrote:

 Hi, Asif.

 Thank you for response.


C:\Users\asif\Desktop\Program files\9.3bin\pg_ctl -D
  C:\Users\asif\Desktop\Program files\9.3\data1 -l logfile start
server starting

 This failure does not occur by the command line.
 PostgreSQL needs to start by Windows Service.

 Additionally,In this case,
 A file Program needs to be exist at C:\Users\asif\Desktop\, and
 postgres.exe needs to be exist at C:\Users\asif\Desktop\Program
 files\9.3\bin.
 
 C:\Users\asif\Desktop\Program files\9.3\bindir
 ...
 4,435,456   postgres.exe
80,896   pg_ctl.exe
 ...

 C:\Users\asif\Desktoppdir
 ...
 0  Program
 DIR  Program files
 ...
 

 Regards,
 Naoya

  Hi Naoya,
 
  I am not able to reproduce the problem. Do you mean pg windows service
  installed by installer is not working or bin\pg_ctl binary is not accepting
  spaces in the patch ?. Following worked for me i.e.
 
 
C:\Users\asif\Desktop\Program files\9.3bin\pg_ctl -D
  C:\Users\asif\Desktop\Program files\9.3\data1 -l logfile start
server starting
 
 
  Can you please share the exact steps ?. Thanks.
 
 
  Regards,
  Muhammad Asif Naeem
 
 
 
  On Mon, Oct 28, 2013 at 10:26 AM, Naoya Anzai
  anzai-na...@mxu.nes.nec.co.jp wrote:
 
 
Hi All,
 
I have found a case that PostgreSQL Service does not start.
When it happens, the following error appears.
 
 is not a valid Win32 application
 
This failure occurs when the following conditions are true.
 
1. There is postgres.exe in any directory that contains a space,
   such as Program Files.
 
   e.g.)
   C:\Program Files\PostgreSQL\bin\postgres.exe
 
2. A file using the first white space-delimited
   tokens of that directory as the file name exists,
   and there is it in the same hierarchy.
 
   e.g.)
   C:\Program //file
 
pg_ctl.exe as PostgreSQL Service creates a postgres
process using an absolute path which indicates the
location of postgres.exe,but the path is not enclosed
in quotation.
 
Therefore,if the above-mentioned conditions are true,

Re: [HACKERS] Detection of nested function calls

2013-10-28 Thread Andres Freund
On 2013-10-28 10:12:41 +0100, Pavel Stehule wrote:
  I think we'd need another argument to CREATE FUNCTION like SERIALIZE
  pointing to a function that that has to return data that can be stored
  on disk. Deserialization would be up to individual functions.
 
  Depending on the specification this might turn out to be slightly
  invasive, tuplestore/sort et al probably have to care...

 Then you need a functions than prepare a clone of unpacked data too.

Why? In those case we can (and should) just store the ondisk
representation.

Greetings,

Andres Freund

PS: Could you please try to trim the quoted emails a bit?

-- 
 Andres Freund 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] Detection of nested function calls

2013-10-28 Thread Hugo Mercier
Le 28/10/2013 09:39, Andres Freund a écrit :
 On 2013-10-28 09:13:06 +0100, Hugo Mercier wrote:
 Le 25/10/2013 18:44, Tom Lane a écrit :
 Hugo Mercier hugo.merc...@oslandia.com writes:
 Le 25/10/2013 17:20, Tom Lane a écrit :
 How do you tell the difference between

 The point I'm trying to make is that in the first case, foo would be
 receiving a first argument that was flat and a second that was not flat;
 while in the second case, it would be receiving a first argument that was
 not flat and a second that was flat.  The expression labeling you're
 proposing does not help it tell the difference.

 No it does not. It's then up to the data type to store whether it is
 flat or not. And every functions manipulating this type is assumed to be
 aware of this flat/non-flat flagging.
 
 But what if the in-memory type contains pointers and is copied or
 spilled to disk? There needs to be a mechanism handling that case.

It must not happen. The 'nested' boolean may be seen as everything
returning from this function may be stored on disk at any time, so
serialize it for nested==0.

If there is another mechanism to tell, inside a function, if the result
will be stored (stored on disk, copied to another context, ...) or
not, then I'll be happy with that.



 I think we'd need another argument to CREATE FUNCTION like SERIALIZE
 pointing to a function that that has to return data that can be stored
 on disk. Deserialization would be up to individual functions.

Either as argument to CREATE FUNCTION or to CREATE TYPE, right ?

Ok, so a user function calls PG_DETOAST to get its input. The most
nested will get it straight from where it is stored.
Then the function can decide to deserialize it in its own format,
process it, and return it as is, with probably a call to
PG_RETURN(pointer). Nested function will get their inputs still from
PG_DETOAST and can use them directly.
But for the last function in the nesting chain, how the pointer will be
serialized back to something storeable ? i.e. who will call the
serialize function declared in CREATE (FUNCTION|TYPE) ?

-- 
Hugo Mercier
Oslandia


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


Re: [HACKERS] Detection of nested function calls

2013-10-28 Thread Andres Freund
On 2013-10-28 10:29:59 +0100, Hugo Mercier wrote:
 Le 28/10/2013 09:39, Andres Freund a écrit :
  On 2013-10-28 09:13:06 +0100, Hugo Mercier wrote:
  Le 25/10/2013 18:44, Tom Lane a écrit :
  Hugo Mercier hugo.merc...@oslandia.com writes:
  Le 25/10/2013 17:20, Tom Lane a écrit :
  How do you tell the difference between
 
  The point I'm trying to make is that in the first case, foo would be
  receiving a first argument that was flat and a second that was not flat;
  while in the second case, it would be receiving a first argument that was
  not flat and a second that was flat.  The expression labeling you're
  proposing does not help it tell the difference.
 
  No it does not. It's then up to the data type to store whether it is
  flat or not. And every functions manipulating this type is assumed to be
  aware of this flat/non-flat flagging.
  
  But what if the in-memory type contains pointers and is copied or
  spilled to disk? There needs to be a mechanism handling that case.
 
 It must not happen. The 'nested' boolean may be seen as everything
 returning from this function may be stored on disk at any time, so
 serialize it for nested==0.

I don't think that's sufficient. There'll be lots of places where you'd
need to special-case hack this logic.
Think of SELECT aggregate(somefunc(foo)) FROM ... GROUP BY something_else;


 If there is another mechanism to tell, inside a function, if the result
 will be stored (stored on disk, copied to another context, ...) or
 not, then I'll be happy with that.

I don't think telling the function that is the right approach at all.

  I think we'd need another argument to CREATE FUNCTION like SERIALIZE
  pointing to a function that that has to return data that can be stored
  on disk. Deserialization would be up to individual functions.
 
 Either as argument to CREATE FUNCTION or to CREATE TYPE, right ?

Err, CREATE TYPE, yes.

 Ok, so a user function calls PG_DETOAST to get its input. The most
 nested will get it straight from where it is stored.
 Then the function can decide to deserialize it in its own format,
 process it, and return it as is, with probably a call to
 PG_RETURN(pointer). Nested function will get their inputs still from
 PG_DETOAST and can use them directly.
 But for the last function in the nesting chain, how the pointer will be
 serialized back to something storeable ? i.e. who will call the
 serialize function declared in CREATE (FUNCTION|TYPE) ?

Something around toast_insert_or_update(). We'd need to set
HeapTupleHasExternal() for those kind of tuples or similar so it gets
called, but that shouldn't be the biggest problem.

Greetings,

Andres Freund

-- 
 Andres Freund 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] ERROR : 'tuple concurrently updated'

2013-10-28 Thread Stéphan BEUZE

Le 19/10/2013 05:21, Amit Kapila a écrit :

On Fri, Oct 18, 2013 at 3:43 PM, Stéphan BEUZE
stephan.be...@douane.finances.gouv.fr  wrote:

Here I provide more details about the environment where the error occurs:

* ENVIRONMENT
Client:
  Java Web Application running on JBoss 5.0.0.GA - JDK 1.6.0_24 64bit

Server:
 Postgresql 9.2.4, compiled by Visual C++ build 1600, 64bit

Client and Server run on the same platform:
 Windows 7 Professional SP1 (2009)


* STRUCTURES
CREATE ROLE rec LOGIN  NOSUPERUSER NOINHERIT NOCREATEDB NOCREATEROLE
NOREPLICATION;
CREATE ROLE rec_lct LOGIN  NOSUPERUSER NOINHERIT NOCREATEDB NOCREATEROLE
NOREPLICATION;

CREATE SCHEMA rec  AUTHORIZATION rec;

GRANT ALL ON SCHEMA rec TO rec;
GRANT USAGE ON SCHEMA rec TO rec_lct;

ALTER ROLE rec SET search_path = rec;
ALTER ROLE rec_lct SET search_path = rec;

SET SCHEMA 'rec'

CREATE SEQUENCE stats_sequence
   INCREMENT 1
   MINVALUE 1
   MAXVALUE 9223372036854775807
   START 1
   CACHE 120
   CYCLE;
ALTER TABLE stats_sequence OWNER TO rec;
GRANT ALL ON TABLE stats_sequence TO rec;
GRANT UPDATE ON TABLE stats_sequence TO rec_lct;

   CREATE TABLE my_stat

 (
   id bigint NOT NULL,
   creation date NOT NULL DEFAULT current_date,

   client_addr text NOT NULL,
   pid integer NOT NULL,
   usename name NOT NULL,
   CONSTRAINT my_stat _pkey PRIMARY KEY (id)

 )
 WITH (
   OIDS=FALSE
 );

ALTER TABLE statistiques_connexions OWNER TO rec;
GRANT ALL ON TABLE statistiques_connexions TO rec;
GRANT SELECT, INSERT ON TABLE statistiques_connexions TO rec_lct;

Is this table statistiques_connexions used for something different
from my_stat or this is actual name of my_stat used in your
application?

Sorry, I forgot to translate this part of my code to plain english.
Instead of *statistiques_connexions* please read *my_stat* anywhere it 
appears.



CREATE INDEX statistiques_connexions_idx_creation
   ON statistiques_connexions
   USING btree
   (creation);

CREATE INDEX statistiques_connexions_idx_ukey
   ON statistiques_connexions
   USING btree
   (creation, pid, client_addr COLLATE pg_catalog.default, usename);


* CONTEXT
Two Java threads are created. One is connected with 'rec' user, while the
other one
is connected with 'rec_lct' user.

The threads don't create themselves their JDBC connections.
Instead, they each have their own pooled datasource preconfigured.
The pooled datasources are managed by the same connection pool
library: c3p0 0.9.1. The pooled datasources each open 3 connections
on startup. They can make this number of connections variate from 1 to 5
connections.

In our development context, this number of connections stay at 3.

The threads run the following query every 500 ms.

With the above information, it is difficult to imagine the cause of
problem, is it possible for you to write a separate test which you can
post here, if you can write using some scripts or libpq, that would
also be sufficient.
Is it OK if I send a test case written in Java ? Or is there a well 
defined way to post test case ?






 WITH raw_stat AS (
 SELECT
host(client_addr) as client_addr,
pid ,
usename
 FROM
pg_stat_activity
 WHERE
usename = current_user
 )
 INSERT INTO my_stat(id, client_addr, pid, usename)
 SELECT
  nextval('mystat_sequence'), t.client_addr, t.pid, t.usename
 FROM (
 SELECT
 client_addr, pid, usename
 FROM
 raw_stat s
 WHERE
 NOT EXISTS (
SELECT
   NULL
FROM
   my_stat u
WHERE
   current_date = u.creation
AND
   s.pid = u.pid
AND
   s.client_addr = u.client_addr
AND
   s.usename = u.usename
 )
 ) t;


What can be observed first is that, at the beginning, everything run
smoothly.
Then unpredictably, the error 'tuple concurrently updated' appears...
Needless to say, that it disappears too... unpredictably.
Sometimes, it can shows up contisnously.

Do you see any other problem due to this error in your database?
No I don't see anything else. The problem appears only when two 
concurrent sessions , with different users in my case,

performs the above query.

Stephan




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


Re: [HACKERS] Detection of nested function calls

2013-10-28 Thread Pavel Stehule
2013/10/28 Andres Freund and...@2ndquadrant.com

 On 2013-10-28 10:12:41 +0100, Pavel Stehule wrote:
   I think we'd need another argument to CREATE FUNCTION like SERIALIZE
   pointing to a function that that has to return data that can be stored
   on disk. Deserialization would be up to individual functions.
  
   Depending on the specification this might turn out to be slightly
   invasive, tuplestore/sort et al probably have to care...

  Then you need a functions than prepare a clone of unpacked data too.

 Why? In those case we can (and should) just store the ondisk
 representation.



ok,

Pavel



 Greetings,

 Andres Freund

 PS: Could you please try to trim the quoted emails a bit?

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



Re: [HACKERS] PostgreSQL Service on Windows does not start. ~ is not a valid Win32 application

2013-10-28 Thread Sandeep Thakkar
Hi Dave

We register the service using pg_ctl. When I manually executed the
following on the command prompt, I saw that the service path of the
registered service did not have the pg_ctl.exe path in quotes. May be it
should be handled in the pg_ctl code.

*c:\Users\Sandeep Thakkar\Documents*c:\Program
Files\PostgreSQL\9.3\bin\pg_ctl.e
xe register -N pg-9.3 -U NT AUTHORITY\NetworkService -D c:\Program
Files\P
ostgreSQL\9.3\data -w

Naoya,  I could not find your patch here. Can you please share it again?



On Mon, Oct 28, 2013 at 2:53 PM, Dave Page dp...@pgadmin.org wrote:

 Sandeep, can you look at this please? Thanks.

 On Mon, Oct 28, 2013 at 8:18 AM, Asif Naeem anaeem...@gmail.com wrote:
  It is related to windows unquoted service path vulnerability in the the
  installer that creates service path without quotes that make service.exe
 to
  look for undesirable path for executable.
 
  postgresql-9.3 service path : C:/Users/asif/Desktop/Program
  files/9.3/bin/pg_ctl.exe runservice -N postgresql-9.3 -D
  C:/Users/asif/Desktop/Program files/9.3/data -w
 
  service.exe
 
  C:\Users\asif\Desktop\Program NAME NOT FOUND
  C:\Users\asif\Desktop\Program.exe NAME NOT FOUND
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe ACCESS DENIED
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe ACCESS DENIED
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice
 NAME
  NOT FOUND
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice.exe
  NAME NOT FOUND
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  NAME NOT FOUND
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N.exe
  NAME NOT FOUND
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3.exe NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D.exe NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D C:\Users\asif\Desktop\Program NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D C:\Users\asif\Desktop\Program.exe NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data
 NAME
  INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data.exe
  NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data -w
  NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data
 -w.exe
  NAME INVALID
 
 
  Fix :
 
  postgresql-9.3 service path : C:/Users/asif/Desktop/Program
  files/9.3/bin/pg_ctl.exe runservice -N postgresql-9.3 -D
  C:/Users/asif/Desktop/Program files/9.3/data -w
 
  It would be good if this is reported on pg installer forum or security
  forum. Thanks.
 
  Regards,
  Asif Naeem
 
  On Mon, Oct 28, 2013 at 12:06 PM, Naoya Anzai
  anzai-na...@mxu.nes.nec.co.jp wrote:
 
  Hi, Asif.
 
  Thank you for response.
 
 
 C:\Users\asif\Desktop\Program files\9.3bin\pg_ctl -D
   C:\Users\asif\Desktop\Program files\9.3\data1 -l logfile start
 server starting
 
  This failure does not occur by the command line.
  PostgreSQL needs to start by Windows Service.
 
  Additionally,In this case,
  A file Program needs to be exist at C:\Users\asif\Desktop\, and
  postgres.exe needs to be exist at C:\Users\asif\Desktop\Program
  files\9.3\bin.
  
  C:\Users\asif\Desktop\Program files\9.3\bindir
  ...
  4,435,456   postgres.exe
 80,896   pg_ctl.exe
  ...
 
  C:\Users\asif\Desktoppdir
  ...
  0  Program
  DIR  Program files
  ...
  
 
  Regards,
  Naoya
 
   Hi Naoya,
  
   I am not able to reproduce the problem. Do you mean pg windows service
   installed by installer is not working or bin\pg_ctl binary is not
 accepting
   spaces in the patch ?. Following worked for me i.e.
  
  
 C:\Users\asif\Desktop\Program files\9.3bin\pg_ctl -D
   C:\Users\asif\Desktop\Program files\9.3\data1 -l logfile start
 server starting
  
  
   Can you please share the exact steps ?. Thanks.
  
  
   Regards,
   Muhammad Asif Naeem
  
  
  
   On Mon, Oct 28, 2013 at 10:26 AM, Naoya Anzai
   anzai-na...@mxu.nes.nec.co.jp wrote:
  
  
 Hi All,
  
 I have found a case that PostgreSQL Service does not start.
 When it happens, the following error appears.
  
  is not a valid Win32 application
  
 This failure occurs when the following 

Re: [HACKERS] ERROR : 'tuple concurrently updated'

2013-10-28 Thread Amit Kapila
On Mon, Oct 28, 2013 at 3:22 PM, Stéphan BEUZE
stephan.be...@douane.finances.gouv.fr wrote:
 Le 19/10/2013 05:21, Amit Kapila a écrit :
 On Fri, Oct 18, 2013 at 3:43 PM, Stéphan BEUZE
 stephan.be...@douane.finances.gouv.fr  wrote:
 * CONTEXT
 Two Java threads are created. One is connected with 'rec' user, while the
 other one
 is connected with 'rec_lct' user.

 The threads don't create themselves their JDBC connections.
 Instead, they each have their own pooled datasource preconfigured.
 The pooled datasources are managed by the same connection pool
 library: c3p0 0.9.1. The pooled datasources each open 3 connections
 on startup. They can make this number of connections variate from 1 to 5
 connections.

 In our development context, this number of connections stay at 3.

 The threads run the following query every 500 ms.

 With the above information, it is difficult to imagine the cause of
 problem, is it possible for you to write a separate test which you can
 post here, if you can write using some scripts or libpq, that would
 also be sufficient.

 Is it OK if I send a test case written in Java ? Or is there a well defined
 way to post test case ?

It is better if you can give simplified 'C' test, but I don't think
there is any problem with Java test case, might be someone knows java
can try with that test. You can post the Java test and see if someone
could reproduce and tell you the exact problem, else you can write a
'C' test and post that as well.

With Regards,
Amit Kapila.
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] Document update in alter_foreign_data_wrapper.sgml

2013-10-28 Thread Tom Lane
Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes:
 I wrote:
 ISTM the document in alter_foreign_data_wrapper.sgml and the comment in
 foreigncmds.c should be updated.  Please find attached a patch.

 I've noticed that the document in create_foreign_data_wrapper.sgml should also
 be updated.  Attached is an updated version of the patch.

Applied with a bit of additional wordsmithing.

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] Detection of nested function calls

2013-10-28 Thread Tom Lane
Hugo Mercier hugo.merc...@oslandia.com writes:
 Le 25/10/2013 18:44, Tom Lane a écrit :
 The point I'm trying to make is that in the first case, foo would be
 receiving a first argument that was flat and a second that was not flat;
 while in the second case, it would be receiving a first argument that was
 not flat and a second that was flat.  The expression labeling you're
 proposing does not help it tell the difference.

 No it does not. It's then up to the data type to store whether it is
 flat or not. And every functions manipulating this type is assumed to be
 aware of this flat/non-flat flagging.

If the data must contain such markers, then what do you need the proposed
expression labeling for?  It's awkward and ultimately insufficient.

What you do need, as Andres is saying, is a datatype-specific function
that will re-flatten a non-flat Datum; and the core code has to be aware
to call this when preparing data to be stored on disk.  Once you have
that, every C function returning this datatype can make its own choice
of whether to return a flat or non-flat value.  Probably the bias would be
towards the latter choice, but there might be cases where it's easier to
return a flattened value.  The important point here is that it's a whole
lot easier all around if the choice is made on-the-fly at runtime, rather
than trying to determine what will happen at parse time.

 What I've understood so far is that there is room for new flags in the
 TOAST mechanism, so the idea would be to add a new strategy where opaque
 pointers could be stored. And it would then require a way for extensions
 to register their own (de)toasting functions, right ?

The idea I'm thinking about at the moment is that toast tokens of this
sort might each contain a function pointer to the required flattening
function.  This avoids an expensive catalog lookup when flattening is
needed.  We'd never accept such a thing for data destined for disk;
but since the whole point here is that such data lives only in memory,
I can't see anything wrong with including a function pointer in 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] logical changeset generation v6.2

2013-10-28 Thread Robert Haas
On Fri, Oct 25, 2013 at 7:57 AM, Andres Freund and...@2ndquadrant.com wrote:
 However, I'm leery about the idea of using a relation fork for this.
 I'm not sure whether that's what you had it mind, but it gives me the
 willies.  First, it adds distributed overhead to the system, as
 previously discussed; and second, I think the accounting may be kind
 of tricky, especially in the face of multiple rewrites.  I'd be more
 inclined to find a separate place to store the mappings.  Note that,
 AFAICS, there's no real need for the mapping file to be
 block-structured, and I believe they'll be written first (with no
 readers) and subsequently only read (with no further writes) and
 eventually deleted.

 I was thinking of storing it along other data used during logical
 decoding and let decoding's cleanup clean up that data as well. All the
 information for that should be there.

That seems OK.

 There's one snag I currently can see, namely that we actually need to
 prevent that a formerly dropped relfilenode is getting reused. Not
 entirely sure what the best way for that is.

I'm not sure in detail, but it seems to me that this all part of the
same picture.  If you're tracking changed relfilenodes, you'd better
track dropped ones as well.  Completely aside from this issue, what
keeps a relation from being dropped before we've decoded all of the
changes made to its data before the point at which it was dropped?  (I
hope the answer isn't nothing.)

 One possible objection to this is that it would preclude decoding on a
 standby, which seems like a likely enough thing to want to do.  So
 maybe it's best to WAL-log the changes to the mapping file so that the
 standby can reconstruct it if needed.

 The mapping file probably can be one big wal record, so it should be
 easy enough to do.

It might be better to batch it, because if you rewrite a big relation,
and the record is really big, everyone else will be frozen out of
inserting WAL for as long as that colossal record is being written and
synced.  If it's inserted in reasonably-sized chunks, the rest of the
system won't be starved as badly.

-- 
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] logical changeset generation v6.2

2013-10-28 Thread Robert Haas
On Fri, Oct 25, 2013 at 8:14 AM, Andres Freund and...@2ndquadrant.com wrote:
 So, I thought about this for some more and I think I've a partial
 solution to the problem.

 The worst thing about deadlocks that occur in the above is that they
 could be the VACUUM FULL waiting for the restart LSN[1] of a decoding
 slot to progress, but the restart LSN cannot progress because the slot
 is waiting for a xid/transaction to end which is being blocked by the
 lock upgrade from VACUUM FULL. Such conflicts are not visible to the
 deadlock detector, which obviously is bad.
 I've prototyped this (~25 lines) and this happens pretty frequently. But
 it turns out that we can actually fix this by exporting (to shared
 memory) the oldest in-progress xid of a decoding slot. Then the waiting
 code can do a XactLockTableWait() for that xid...

 I wonder if this is isn't maybe sufficient. Yes, it can deadlock, but
 that's already the case for VACUUM FULLs of system tables, although less
 likely. And it will be detected/handled.
 There's one more snag though, we currently allow CLUSTER system_table;
 in an existing transaction. I think that'd have to be disallowed.

It wouldn't bother me too much to restrict CLUSTER system_table by
PreventTransactionChain() at wal_level = logical, but obviously it
would be nicer if we *didn't* have to do that.

In general, I don't think waiting on an XID is sufficient because a
process can acquire a heavyweight lock without having an XID.  Perhaps
use the VXID instead?

One thought I had about waiting for decoding to catch up is that you
might do it before acquiring the lock.  Of course, you then have a
problem if you get behind again before acquiring the lock.  It's
tempting to adopt the solution we used for RangeVarGetRelidExtended,
namely: wait for catchup without the lock, acquire the lock, see
whether we're still caught up if so great else release lock and loop.
But there's probably too much starvation risk to get away with that.

On the whole, I'm leaning toward thinking that the other solution
(recording the old-to-new CTID mappings generated by CLUSTER to the
extent that they are needed) is probably more elegant.

-- 
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] Re: Using indexes for ORDER BY and PARTITION BY clause in windowing functions

2013-10-28 Thread Sameer Kumar



  Agree that windowing function will return all the rows compared to max
 and
  group by returing only max rows per group. But even while arriving at the
  aggregate/sorting windowing function seems to spend more effort than
 group
  by/order by.

 (I'll apologise in advance for possible misreading..)

 The most cause of the difference in time comes from sorting. Over
 90% of total execution time has elapsed while sorting
 (49ms-2733ms) for the one using windowing function. If this sort
 were useless, the execution time would be less than 300 ms -
 seems comparable enough to group-by query.

 I will agree with you on this point.



 | Subquery Scan on __unnamed_subquery_0
 |  (actual time=2606.075..2953.937 rows=558 loops=1)
 |   Filter: (__unnamed_subquery_0.rn = 1)
 |   -  WindowAgg  (actual time=2606.063..2928.061 rows=122880 loops=1)
 | -  Sort (actual time=2606.020..2733.677 rows=122880 loops=1)
 |   Sort Key: student_score.course, student_score.score
 |   -  Seq Scan on student_score
 |   (actual time=0.009..49.026 rows=122880 loops=1)

 As you see in above plan, sorting key is (course, score). If your
 point is the overall performance but not reusing a kind of
 'hash', there's a big chance to eliminate this sorting if you are
 able to have an additional index, say,

 =# create index idx_co_sc on student_score using btree (course, score);

 With this index, you will get a different plan like this,

 Exactly my point, can we look at making windowing functions smart and make
use of available indexes?



  uniontest=# explain analyze select student_name from (select
 student_name, dense_rank() over(partition by course order by score) rn,
 score from student_score) rnn where rn=2;
QUERY PLAN
 
 ---
   Subquery Scan on rnn  (actual time=0.088..319.403 rows=135 loops=1)
 Filter: (rnn.rn = 2)
 Rows Removed by Filter: 122746
 -  WindowAgg  (actual time=0.037..296.851 rows=122881 loops=1)
   -  Index Scan using idx_co_sc on student_score
 (actual time=0.027..111.333 rows=122881 loops=1)
   Total runtime: 319.483 ms

 Does this satisfies your needs?

 Not exactly. If I have missed to mention, this is not a production issue
for me. I am trying to see if PostgreSQL planner produces best plans for
Data Warehouse and mining oriented queries.

 ===
  Another thing, (I may be stupid and naive here) does PostgreSQL
  re-uses the hash which has been already created for sort. In
  this case the inner query must have created a hash for windoing
  aggregate. Can't we use that same one while applying the the
  filter rn=1 ?

 Generally saying, hashes cannot yield ordered output by its
 nature, I believe.

 I think Hashes can be efficiently used for sorting (and I believe they are
used for joins too when a pre-sorted data set is not available via
indexes). This again could my misinterpretation.

Windowing function (execnode) always receives tuples sequentially
 in the window-defined order (as you see in the explained plan
 above) then processes the tuples in semi tuple-by-tuple manner to
 perform per-frame aggregaion, and finally outputs tuples of the
 same number to input. And furthermore, dense_rank() doesn't even
 need per-frame aggregations. So none of the planners so far seems
 to have chance to use a kind of hash tables to culculate/execute
 windowing fucntions. On the another point, automatically
 preserving some internal data within a query beyond the end of
 the query brings in 'when to discard it?' problem.


 I lost you somewhere here. My be this is above my pay-grade :-)
Well, at least with Oracle and DB2 planners I have seen that the plan
produced with dense_rank performs better than a series of nested SELECT
MAX().

Regards
Sameer


Re: [HACKERS] RULE regression test fragility?

2013-10-28 Thread Robert Haas
On Sat, Oct 26, 2013 at 12:02 PM, Andres Freund and...@2ndquadrant.com wrote:
 Imo what it does looks sane - it adds parentheses whenever a child of a
 set operation is a set operation again to make sure the order in which
 the generated set operations are parsed/interpreted stays the same.

But UNION ALL is associative.

-- 
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] PL/Python: domain over array support

2013-10-28 Thread Robert Haas
On Sat, Oct 26, 2013 at 9:17 AM, Rodolfo Campero
rodolfo.camp...@anachronics.com wrote:
 The attached patch add support of domains over arrays to PL/Python (eg:
 CREATE DOMAIN my_domain AS integer[]).

 Basically it just uses get_base_element_type instead of get_element_type in
 plpy_typeio.c, and uses domain_check before returning a sequence as array in
 PLySequence_ToArray whenever appropriate.

 There's one line I'm not sure about; I modified a switch statement (line
 427):
 switch (element_type ? element_type : getBaseType(arg-typoid))
 The rationale is that when element_type is set, it is already a base type,
 because there is no support of arrays of domains in PostgreSQL, but this may
 not held true in the future.

Please add your patch here so that it doesn't get forgotten about:

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] RULE regression test fragility?

2013-10-28 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Sat, Oct 26, 2013 at 12:02 PM, Andres Freund and...@2ndquadrant.com 
 wrote:
 Imo what it does looks sane - it adds parentheses whenever a child of a
 set operation is a set operation again to make sure the order in which
 the generated set operations are parsed/interpreted stays the same.

 But UNION ALL is associative.

In theory, yeah.

In practice, this could for example affect the parser's choices of
column datatypes for the UNION result.  We could perhaps side-step
that by forcing datatype labeling in the UNION arms, but I'm not
prepared to bet that ruleutils' output would be right if we just
summarily removed the parentheses.

regards, tom lane


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


[HACKERS] OSX doesn't accept identical source/target for strcpy() anymore

2013-10-28 Thread Andres Freund
Hi,

On -bugs it was reported that initdb of 9.3 failed with a
assertion.

On 2013-10-28 16:52:13 +0100, Matthias Schmitt wrote:
  In that case, could you enable coredumps and get a backtrace from that
  coredump? I unfortunately have zero clue about OSX, so I can't really
  help you with that.
 
 Yes, you are right. I totally forgot about the crash logs in OS X. Here it is:

Ah, ok. I see the problem...

 Process: postgres [68949]
 Path:/Users/*/postgres
 Identifier:  postgres
 Version: 0
 Code Type:   X86-64 (Native)
 Parent Process:  sh [68948]
 Responsible: Terminal [411]
 User ID: 502
 
 Date/Time:   2013-10-28 16:46:28.188 +0100
 OS Version:  Mac OS X 10.9 (13A603)

 Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
 0   libsystem_kernel.dylib0x7fff93e00866 __pthread_kill + 10
 1   libsystem_pthread.dylib   0x7fff92c7335c pthread_kill + 92
 2   libsystem_c.dylib 0x7fff8c5a5bba abort + 125
 3   libsystem_c.dylib 0x7fff8c5a5d31 abort_report_np + 181
 4   libsystem_c.dylib 0x7fff8c5c98c5 __chk_fail + 48
 5   libsystem_c.dylib 0x7fff8c5c98d5 __chk_fail_overlap + 
 16
 6   libsystem_c.dylib 0x7fff8c5c9906 __chk_overlap + 49
 7   libsystem_c.dylib 0x7fff8c5c9a59 __strncpy_chk + 78
 8   postgres  0x00010b4c9045 namestrcpy + 86
 9   postgres  0x00010b1901f2 TupleDescInitEntry + 
 99

It seems the newest version of OSX is more strict about strcpy than
previous ones. So the issue is likely the upgraded OS version. Which
means we're going to see this more frequently now.

There have been previous discussions about fixing strcpy calls with
identical source/destination (same for memcpy) but it was deemed not
worth the effort. I don't really see an alternative to fixing it now.

Greetings,

Andres Freund

-- 
 Andres Freund 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] dsm use of uint64

2013-10-28 Thread Robert Haas
On Sun, Oct 27, 2013 at 11:34 PM, Noah Misch n...@leadboat.com wrote:
 On Fri, Oct 25, 2013 at 10:11:41PM -0400, Robert Haas wrote:
 When I wrote the dynamic shared memory patch, I used uint64 everywhere
 to measure sizes - rather than, as we do for the main shared memory
 segment, Size.  This now seems to me to have been the wrong decision;
 I'm finding that it's advantageous to make dynamic shared memory
 behave as much like the main shared memory segment as is reasonably
 possible, and using Size facilitates the use of MAXALIGN(),
 TYPEALIGN(), etc. as well as things like add_size() and mul_size()
 which are just as relevant in the dynamic shared memory case as they
 are for the main shared memory segment.

 Therefore, I propose to apply the attached patch.

 +1.

OK, committed.

 The simplicity of platform-independent type sizing had some attraction,
 but not so much to justify this sort of friction with the rest of the system.

That's a good way of putting it.  I'm repeatedly learning - invariably
the hard way - that everything the main shared memory segment is or
does needs a parallel for dynamic shared memory, and the closer the
parallel, the better.

-- 
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] logical changeset generation v6.2

2013-10-28 Thread Andres Freund
On 2013-10-28 12:04:01 -0400, Robert Haas wrote:
 On Fri, Oct 25, 2013 at 8:14 AM, Andres Freund and...@2ndquadrant.com wrote:

  I wonder if this is isn't maybe sufficient. Yes, it can deadlock, but
  that's already the case for VACUUM FULLs of system tables, although less
  likely. And it will be detected/handled.
  There's one more snag though, we currently allow CLUSTER system_table;
  in an existing transaction. I think that'd have to be disallowed.
 
 It wouldn't bother me too much to restrict CLUSTER system_table by
 PreventTransactionChain() at wal_level = logical, but obviously it
 would be nicer if we *didn't* have to do that.
 
 In general, I don't think waiting on an XID is sufficient because a
 process can acquire a heavyweight lock without having an XID.  Perhaps
 use the VXID instead?

But decoding doesn't care about transactions that haven't used an XID
yet (since that means they haven't modified the catalog), so that
shouldn't be problematic.

 One thought I had about waiting for decoding to catch up is that you
 might do it before acquiring the lock.  Of course, you then have a
 problem if you get behind again before acquiring the lock.  It's
 tempting to adopt the solution we used for RangeVarGetRelidExtended,
 namely: wait for catchup without the lock, acquire the lock, see
 whether we're still caught up if so great else release lock and loop.
 But there's probably too much starvation risk to get away with that.

I think we'd pretty much always starve in that case. It'd be different
if we could detect that there weren't any writes to the table
inbetween. I can see doing that using a locking hack like autovac uses,
but brr, that'd be ugly.

 On the whole, I'm leaning toward thinking that the other solution
 (recording the old-to-new CTID mappings generated by CLUSTER to the
 extent that they are needed) is probably more elegant.

I personally still think that the wide cmin/cmax solution is *much*
more elegant, simpler and actually can be used for other things than
logical decoding.
Since you don't seem to agree I am going to write a prototype using such
a mapping to see how it will look though.

Greetings,

Andres Freund

-- 
 Andres Freund 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] logical changeset generation v6.2

2013-10-28 Thread Robert Haas
On Mon, Oct 28, 2013 at 12:17 PM, Andres Freund and...@2ndquadrant.com wrote:
 In general, I don't think waiting on an XID is sufficient because a
 process can acquire a heavyweight lock without having an XID.  Perhaps
 use the VXID instead?

 But decoding doesn't care about transactions that haven't used an XID
 yet (since that means they haven't modified the catalog), so that
 shouldn't be problematic.

Hmm, maybe.  But what if the deadlock has more members?  e.g. A is
blocking decoding by holding AEL w/no XID, and B is blocking A by
doing VF on a rel A needs, and decoding is blocking B.

-- 
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] RULE regression test fragility?

2013-10-28 Thread Robert Haas
On Mon, Oct 28, 2013 at 12:10 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Sat, Oct 26, 2013 at 12:02 PM, Andres Freund and...@2ndquadrant.com 
 wrote:
 Imo what it does looks sane - it adds parentheses whenever a child of a
 set operation is a set operation again to make sure the order in which
 the generated set operations are parsed/interpreted stays the same.

 But UNION ALL is associative.

 In theory, yeah.

 In practice, this could for example affect the parser's choices of
 column datatypes for the UNION result.  We could perhaps side-step
 that by forcing datatype labeling in the UNION arms, but I'm not
 prepared to bet that ruleutils' output would be right if we just
 summarily removed the parentheses.

Well, if it were actually associative, then A UNION ALL B UNION ALL C
would be equivalent to either A UNION ALL (B UNION ALL C) or (A UNION
ALL B) UNION ALL C.  But even if it's *NOT* associative, it must be
equivalent to one of those.  (If not, then including the parentheses
in the output is wrong.)  So we could leave the parentheses out in
whichever case it's equivalent to.

I don't feel strongly that this has to be done; it's obviously not a
project priority.  But if we're uncomfortable about the way that these
constructs are being formatted during deparsing, eliminating
unnecessary nesting levels could potentially help.  I fairly commonly
write queries that involve multiple UNION ALL branches and, no matter
how clever we are, having that lead to progressively deeper nesting at
each level is not going to look nice.

-- 
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] OSX doesn't accept identical source/target for strcpy() anymore

2013-10-28 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes:
 There have been previous discussions about fixing strcpy calls with
 identical source/destination (same for memcpy) but it was deemed not
 worth the effort. I don't really see an alternative to fixing it now.

I'm not seeing this with bare-bones 

./configure --enable-debug --enable-cassert

on an OS X Mavericks machine with Xcode downloaded today.  So there
is something unidentified as yet about Matthias's configuration.

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] OSX doesn't accept identical source/target for strcpy() anymore

2013-10-28 Thread Robert Haas
On Mon, Oct 28, 2013 at 12:11 PM, Andres Freund and...@2ndquadrant.com wrote:
 Hi,

 On -bugs it was reported that initdb of 9.3 failed with a
 assertion.

 On 2013-10-28 16:52:13 +0100, Matthias Schmitt wrote:
  In that case, could you enable coredumps and get a backtrace from that
  coredump? I unfortunately have zero clue about OSX, so I can't really
  help you with that.

 Yes, you are right. I totally forgot about the crash logs in OS X. Here it 
 is:

 Ah, ok. I see the problem...

 Process: postgres [68949]
 Path:/Users/*/postgres
 Identifier:  postgres
 Version: 0
 Code Type:   X86-64 (Native)
 Parent Process:  sh [68948]
 Responsible: Terminal [411]
 User ID: 502

 Date/Time:   2013-10-28 16:46:28.188 +0100
 OS Version:  Mac OS X 10.9 (13A603)

 Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
 0   libsystem_kernel.dylib0x7fff93e00866 __pthread_kill + 10
 1   libsystem_pthread.dylib   0x7fff92c7335c pthread_kill + 92
 2   libsystem_c.dylib 0x7fff8c5a5bba abort + 125
 3   libsystem_c.dylib 0x7fff8c5a5d31 abort_report_np + 
 181
 4   libsystem_c.dylib 0x7fff8c5c98c5 __chk_fail + 48
 5   libsystem_c.dylib 0x7fff8c5c98d5 __chk_fail_overlap 
 + 16
 6   libsystem_c.dylib 0x7fff8c5c9906 __chk_overlap + 49
 7   libsystem_c.dylib 0x7fff8c5c9a59 __strncpy_chk + 78
 8   postgres  0x00010b4c9045 namestrcpy + 86
 9   postgres  0x00010b1901f2 TupleDescInitEntry 
 + 99

 It seems the newest version of OSX is more strict about strcpy than
 previous ones. So the issue is likely the upgraded OS version. Which
 means we're going to see this more frequently now.

 There have been previous discussions about fixing strcpy calls with
 identical source/destination (same for memcpy) but it was deemed not
 worth the effort. I don't really see an alternative to fixing it now.

Ugh.  Why in the world would Apple break this?  We could try to go
through our code and fix this, but catching all of the instances is
likely to be hard, and everyone else who writes software of any
complexity is going to have the same darn problme.

-- 
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] Detection of nested function calls

2013-10-28 Thread Robert Haas
On Mon, Oct 28, 2013 at 11:12 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 The idea I'm thinking about at the moment is that toast tokens of this
 sort might each contain a function pointer to the required flattening
 function.  This avoids an expensive catalog lookup when flattening is
 needed.  We'd never accept such a thing for data destined for disk;
 but since the whole point here is that such data lives only in memory,
 I can't see anything wrong with including a function pointer in it.

This might be OK, but it bloats the in-memory representation.  For
small data types like numeric that might well be significant.

-- 
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] RULE regression test fragility?

2013-10-28 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 ... we could leave the parentheses out in
 whichever case it's equivalent to.

Ah, I see what you're getting at now.  Yeah, that might be a useful
readability improvement.

 ... I fairly commonly
 write queries that involve multiple UNION ALL branches and, no matter
 how clever we are, having that lead to progressively deeper nesting at
 each level is not going to look nice.

Agreed.  I was wondering myself whether we couldn't fix things so that
all the branches are indented the same, even with parens.

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] Detection of nested function calls

2013-10-28 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Mon, Oct 28, 2013 at 11:12 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 The idea I'm thinking about at the moment is that toast tokens of this
 sort might each contain a function pointer to the required flattening
 function.

 This might be OK, but it bloats the in-memory representation.  For
 small data types like numeric that might well be significant.

Meh.  If you don't include a function pointer you will still need the OID
of the datatype or the decompression function, so it's not like omitting
it is free.  In any case, the design target here is for data values that
are going to be quite large, so an extra 4 bytes or whatever in the
reference object doesn't really seem to me to be something to stress over.

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] Detection of nested function calls

2013-10-28 Thread Andres Freund
On 2013-10-28 12:42:28 -0400, Tom Lane wrote:
 Robert Haas robertmh...@gmail.com writes:
  On Mon, Oct 28, 2013 at 11:12 AM, Tom Lane t...@sss.pgh.pa.us wrote:
  The idea I'm thinking about at the moment is that toast tokens of this
  sort might each contain a function pointer to the required flattening
  function.
 
  This might be OK, but it bloats the in-memory representation.  For
  small data types like numeric that might well be significant.
 
 Meh.  If you don't include a function pointer you will still need the OID
 of the datatype or the decompression function, so it's not like omitting
 it is free.

That's what I thought at first too - but I am not sure it's actually
true. The reason we need to include the toastrelid in varatt_externals
(which I guess you are thinking of, like me) is that we need to be able
to resolve naked Datums to their original value without any context.
But at the locations where we'd need to call the memory
representation-disk conversion function we should have a TupleDesc with
type information, so we could lookup the needed information there.

 In any case, the design target here is for data values that
 are going to be quite large, so an extra 4 bytes or whatever in the
 reference object doesn't really seem to me to be something to stress
 over.

I'd actually be happy if we can get this to work for numeric as well - I
have seen several workloads where that's a bottleneck. Not that I am
sure that the 8bytes for a pointer would be the problem there (in
contrast to additional typecache lookups).

Greetings,

Andres Freund

-- 
 Andres Freund 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] Detection of nested function calls

2013-10-28 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes:
 On 2013-10-28 12:42:28 -0400, Tom Lane wrote:
 Meh.  If you don't include a function pointer you will still need the OID
 of the datatype or the decompression function, so it's not like omitting
 it is free.

 That's what I thought at first too - but I am not sure it's actually
 true. The reason we need to include the toastrelid in varatt_externals
 (which I guess you are thinking of, like me) is that we need to be able
 to resolve naked Datums to their original value without any context.
 But at the locations where we'd need to call the memory
 representation-disk conversion function we should have a TupleDesc with
 type information, so we could lookup the needed information there.

I don't think that's a safe assumption at all.  We need to be able to do
flattening anywhere PG_DETOAST_DATUM() can be called.

In any case, my point here is largely that I don't want to add a catalog
lookup to the operation.  This whole proposal is basically about trading
greater short-term memory usage to gain speed, so griping about an extra 4
or so bytes per value seems to me to be missing the point completely.
Or to put it even more bluntly: if you've not realized that the extra
palloc overhead of an out-of-line instance of the datum will swamp what
we're talking about here, you need to realize that.

 In any case, the design target here is for data values that
 are going to be quite large, so an extra 4 bytes or whatever in the
 reference object doesn't really seem to me to be something to stress
 over.

 I'd actually be happy if we can get this to work for numeric as well - I
 have seen several workloads where that's a bottleneck. Not that I am
 sure that the 8bytes for a pointer would be the problem there (in
 contrast to additional typecache lookups).

Yeah.  The other point worth considering is that there will not likely be
all that many such values floating around at once, so even if there does
end up being a significant percentage bloat in the size of a non-flat
numeric, I doubt anyone will notice.

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] Detection of nested function calls

2013-10-28 Thread k...@rice.edu
On Mon, Oct 28, 2013 at 05:48:55PM +0100, Andres Freund wrote:
 On 2013-10-28 12:42:28 -0400, Tom Lane wrote:
  Robert Haas robertmh...@gmail.com writes:
   On Mon, Oct 28, 2013 at 11:12 AM, Tom Lane t...@sss.pgh.pa.us wrote:
   The idea I'm thinking about at the moment is that toast tokens of this
   sort might each contain a function pointer to the required flattening
   function.
  
   This might be OK, but it bloats the in-memory representation.  For
   small data types like numeric that might well be significant.
  
  Meh.  If you don't include a function pointer you will still need the OID
  of the datatype or the decompression function, so it's not like omitting
  it is free.
 
 That's what I thought at first too - but I am not sure it's actually
 true. The reason we need to include the toastrelid in varatt_externals
 (which I guess you are thinking of, like me) is that we need to be able
 to resolve naked Datums to their original value without any context.
 But at the locations where we'd need to call the memory
 representation-disk conversion function we should have a TupleDesc with
 type information, so we could lookup the needed information there.
 
  In any case, the design target here is for data values that
  are going to be quite large, so an extra 4 bytes or whatever in the
  reference object doesn't really seem to me to be something to stress
  over.
 
 I'd actually be happy if we can get this to work for numeric as well - I
 have seen several workloads where that's a bottleneck. Not that I am
 sure that the 8bytes for a pointer would be the problem there (in
 contrast to additional typecache lookups).
 
 Greetings,
 
 Andres Freund
 
With the type information available, you could have a single lookup table
per backend with the function pointer so the space would be negligible
amortized over all of the datums of each type.

Regards,
Ken


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


Re: [HACKERS] RULE regression test fragility?

2013-10-28 Thread Robert Haas
On Mon, Oct 28, 2013 at 12:34 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 ... we could leave the parentheses out in
 whichever case it's equivalent to.

 Ah, I see what you're getting at now.  Yeah, that might be a useful
 readability improvement.

 ... I fairly commonly
 write queries that involve multiple UNION ALL branches and, no matter
 how clever we are, having that lead to progressively deeper nesting at
 each level is not going to look nice.

 Agreed.  I was wondering myself whether we couldn't fix things so that
 all the branches are indented the same, even with parens.

Hmm, yeah, maybe.  Not sure how ugly it'd be.

-- 
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] Detection of nested function calls

2013-10-28 Thread Andres Freund
On 2013-10-28 13:41:46 -0400, Tom Lane wrote:
 Andres Freund and...@2ndquadrant.com writes:
  On 2013-10-28 12:42:28 -0400, Tom Lane wrote:
  Meh.  If you don't include a function pointer you will still need the OID
  of the datatype or the decompression function, so it's not like omitting
  it is free.
 
  That's what I thought at first too - but I am not sure it's actually
  true. The reason we need to include the toastrelid in varatt_externals
  (which I guess you are thinking of, like me) is that we need to be able
  to resolve naked Datums to their original value without any context.
  But at the locations where we'd need to call the memory
  representation-disk conversion function we should have a TupleDesc with
  type information, so we could lookup the needed information there.
 
 I don't think that's a safe assumption at all.  We need to be able to do
 flattening anywhere PG_DETOAST_DATUM() can be called.

I am not sure we want things to work along those lines. I'd rather make
PG_DETOAST_DATUM pass along such in-memory Datums unchanged and require
any funtion that wants to poke into into the Datum in detail to know
about the different representations. That will require a bit more
widespread changes in functions using those types natively, but it will
make it more realistic to use the optimization across much of the code
that detoasts Datums generally.

 In any case, my point here is largely that I don't want to add a catalog
 lookup to the operation.  This whole proposal is basically about trading
 greater short-term memory usage to gain speed, so griping about an extra 4
 or so bytes per value seems to me to be missing the point completely.
 Or to put it even more bluntly: if you've not realized that the extra
 palloc overhead of an out-of-line instance of the datum will swamp what
 we're talking about here, you need to realize that.

I am not arguing against this at all though.

Greetings,

Andres Freund

-- 
 Andres Freund 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] better atomics

2013-10-28 Thread Robert Haas
On Wed, Oct 16, 2013 at 12:52 PM, Andres Freund and...@2ndquadrant.com wrote:
 I have a related problem, which is that some code I'm currently
 working on vis-a-vis parallelism can run lock-free on platforms with
 atomic 8 bit assignment but needs a spinlock or two elsewhere.  So I'd
 want to use pg_atomic_store_u64(), but I'd also need a clean way to
 test, at compile time, whether it's available.

 Yes, definitely. There should be a couple of #defines that declare
 whether non-prerequisite options are supported. I'd guess we want at least:
 * 8byte math
 * 16byte compare_and_swap

I'm not terribly excited about relying on 16-byte CAS, but I agree
that 8-byte math, at least, is important.  I've not been successful in
finding any evidence that gcc has preprocessor symbols to tell us
about the properties of 8-byte loads and stores.  The closest thing
that I found is:

http://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html

That page provides intrinsics for 8-byte atomic loads and stores,
among other things.  But it seems that the only method for determining
whether they work on a particular target is to compile a test program
and see whether it complains about __atomic_load_n and/or
__atomic_store_n being unresolved symbols.  I suppose we could add a
configure test for that.  Yuck.

Anyone have a better idea?

-- 
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] OSX doesn't accept identical source/target for strcpy() anymore

2013-10-28 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Mon, Oct 28, 2013 at 12:11 PM, Andres Freund and...@2ndquadrant.com 
 wrote:
 There have been previous discussions about fixing strcpy calls with
 identical source/destination (same for memcpy) but it was deemed not
 worth the effort. I don't really see an alternative to fixing it now.

 Ugh.  Why in the world would Apple break this?

It's broken already; the C and POSIX standards say of strncpy:

If copying takes place between objects that overlap, the behavior is undefined.

Both gcc and glibc have been moving steadily in the direction of
aggressively exploiting undefined behavior cases for optimization
purposes.  I don't know if there is yet a platform where strncpy with
src == dest behaves oddly, but we'd be foolish to imagine that it's
not going to happen eventually.  If anything, Apple is probably doing
us a service by making it obvious where we're failing to adhere to spec.

However ... I still can't replicate this here, and as you say, there's
about zero chance of keeping our code clean of this problem unless we
can set up a buildfarm member that will catch 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] better atomics

2013-10-28 Thread Andres Freund
On 2013-10-28 14:10:48 -0400, Robert Haas wrote:
 On Wed, Oct 16, 2013 at 12:52 PM, Andres Freund and...@2ndquadrant.com 
 wrote:
  I have a related problem, which is that some code I'm currently
  working on vis-a-vis parallelism can run lock-free on platforms with
  atomic 8 bit assignment but needs a spinlock or two elsewhere.  So I'd
  want to use pg_atomic_store_u64(), but I'd also need a clean way to
  test, at compile time, whether it's available.
 
  Yes, definitely. There should be a couple of #defines that declare
  whether non-prerequisite options are supported. I'd guess we want at least:
  * 8byte math
  * 16byte compare_and_swap
 
 I'm not terribly excited about relying on 16-byte CAS, but I agree
 that 8-byte math, at least, is important.  I've not been successful in
 finding any evidence that gcc has preprocessor symbols to tell us
 about the properties of 8-byte loads and stores.  The closest thing
 that I found is:

I am talking about making 16byte CAS explicitly optional though? I think
if code wants to optionally make use of it (e.g. on x86 where it's been
available basically forever) that's fine. It just has to be optional.
Or are you saying you're simply not interested in 16byte CAS generally?

Same thing for 8byte math - there's no chance that that is going to be
supported over all platforms anytime soon, so it has to be optional.

 http://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html
 
 That page provides intrinsics for 8-byte atomic loads and stores,
 among other things.  But it seems that the only method for determining
 whether they work on a particular target is to compile a test program
 and see whether it complains about __atomic_load_n and/or
 __atomic_store_n being unresolved symbols.  I suppose we could add a
 configure test for that.  Yuck.

Well, that's pretty easy to integrate into configure - and works on
crossbuilds. So I think that'd be ok.

But I actually think this is going to be a manual thing, atomic 8byte
math will fall back to kernel emulation on several targets, so we
probably want some defines to explicitly declare it supported on targets
where that's not the case.

Greetings,

Andres Freund

-- 
 Andres Freund 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] OSX doesn't accept identical source/target for strcpy() anymore

2013-10-28 Thread Andres Freund
On 2013-10-28 14:11:12 -0400, Tom Lane wrote:
 Robert Haas robertmh...@gmail.com writes:
  On Mon, Oct 28, 2013 at 12:11 PM, Andres Freund and...@2ndquadrant.com 
  wrote:
  There have been previous discussions about fixing strcpy calls with
  identical source/destination (same for memcpy) but it was deemed not
  worth the effort. I don't really see an alternative to fixing it now.
 
  Ugh.  Why in the world would Apple break this?
 
 It's broken already; the C and POSIX standards say of strncpy:
 
 If copying takes place between objects that overlap, the behavior is 
 undefined.
 
 Both gcc and glibc have been moving steadily in the direction of
 aggressively exploiting undefined behavior cases for optimization
 purposes.  I don't know if there is yet a platform where strncpy with
 src == dest behaves oddly, but we'd be foolish to imagine that it's
 not going to happen eventually.  If anything, Apple is probably doing
 us a service by making it obvious where we're failing to adhere to spec.
 
 However ... I still can't replicate this here, and as you say, there's
 about zero chance of keeping our code clean of this problem unless we
 can set up a buildfarm member that will catch it.

It'd be neat if we could get a buildfarm animal up that uses valgrind -
which would catch such and lots of other errors. That's where the topic
has come up in the past:
http://www.postgresql.org/message-id/20110312133224.GA7833%40tornado.gateway.2wire.net
http://www.postgresql.org/message-id/20130217142209.ga5...@awork2.anarazel.de

Greetings,

Andres Freund

-- 
 Andres Freund 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] Detection of nested function calls

2013-10-28 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes:
 On 2013-10-28 13:41:46 -0400, Tom Lane wrote:
 I don't think that's a safe assumption at all.  We need to be able to do
 flattening anywhere PG_DETOAST_DATUM() can be called.

 I am not sure we want things to work along those lines. I'd rather make
 PG_DETOAST_DATUM pass along such in-memory Datums unchanged and require
 any funtion that wants to poke into into the Datum in detail to know
 about the different representations.

No; see my upthread comments.  I think what we want to do is to have
PG_DETOAST_DATUM automatically flatten non-flat datums, and to require
functions that can cope with non-flat inputs to use a new argument
fetching macro, exactly along the lines of what we did with non-aligned
toasted values awhile ago (see PG_GETARG_TEXT_PP and suchlike).  That way,
code that hasn't yet been updated to deal with non-flat datums will still
work, if a bit inefficiently compared to what we'd like.
Non-performance-critical functions might never get updated at all.

If we do what you're suggesting here, any attempt to de-flatten a datatype
will be a flag day on which *every* *single* *function* that deals with
that datatype has to change simultaneously.  That will basically destroy
our chance of ever doing anything about widely-used types like arrays.
This feature *must* be something that we can install support for
incrementally (ie one function at a time), the same way we did with
non-aligned toasted values, or for that matter with several previous
global changes like the adoption of V1 call convention.

 That will require a bit more
 widespread changes in functions using those types natively, but it will
 make it more realistic to use the optimization across much of the code
 that detoasts Datums generally.

You've got that exactly backward; if it's a source code flag day it will
never happen at all.  We need to change code when it gets updated to
handle the case, not run around and try to find every function we're not
updating.

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] OSX doesn't accept identical source/target for strcpy() anymore

2013-10-28 Thread didier
Hi,


On Mon, Oct 28, 2013 at 7:11 PM, Tom Lane t...@sss.pgh.pa.us wrote:


 If copying takes place between objects that overlap, the behavior is
 undefined.

 Both gcc and glibc have been moving steadily in the direction of
 aggressively exploiting undefined behavior cases for optimization
 purposes.  I don't know if there is yet a platform where strncpy with
 src == dest behaves oddly, but we'd be foolish to imagine that it's
 not going to happen eventually.  If anything, Apple is probably doing
 us a service by making it obvious where we're failing to adhere to spec.

 However ... I still can't replicate this here, and as you say, there's
 about zero chance of keeping our code clean of this problem unless we
 can set up a buildfarm member that will catch it.

 regards, tom lane


I haven't a 10.9 box for double checking but there's a gcc command line
triggering the same assert for strcpy and gcc at
http://lists.gnu.org/archive/html/bug-bash/2013-07/msg00011.html.

Didier


Re: [HACKERS] better atomics

2013-10-28 Thread Robert Haas
On Mon, Oct 28, 2013 at 2:19 PM, Andres Freund and...@2ndquadrant.com wrote:
 I'm not terribly excited about relying on 16-byte CAS, but I agree
 that 8-byte math, at least, is important.  I've not been successful in
 finding any evidence that gcc has preprocessor symbols to tell us
 about the properties of 8-byte loads and stores.  The closest thing
 that I found is:

 I am talking about making 16byte CAS explicitly optional though? I think
 if code wants to optionally make use of it (e.g. on x86 where it's been
 available basically forever) that's fine. It just has to be optional.
 Or are you saying you're simply not interested in 16byte CAS generally?

I am just not interested in it generally.  Relying on too many OS or
architecture-specific primitives has several disadvantages.  It's
going to be a nuisance on more obscure platforms where they may or may
not be supported and may or may not work right even if supported.
Even once we get things working right everywhere, it'll mean that
performance may suffer on non-mainstream platforms.  And I think in
many cases it may suggest that we're using an architecture-specific
quirk to work around a fundamental problem with our algorithms.  I'm
more or less convinced of the need for 8-byte atomic reads and writes,
test-and-set, 8-byte compare-and-swap, and 8-byte fetch-and-add.  But
beyond that I'm less sold.  Most of the academic papers I've read on
implementing lock-free or highly-parallel constructs attempt to
confine themselves to 8-byte operations with 8-byte compare-and-swap,
and I'm a bit disposed to think we ought to try to hew to that as
well.  I'm not dead set against going further, but I lean against it,
for all of the reasons mentioned above.

 Same thing for 8byte math - there's no chance that that is going to be
 supported over all platforms anytime soon, so it has to be optional.

Agreed.

 http://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html

 That page provides intrinsics for 8-byte atomic loads and stores,
 among other things.  But it seems that the only method for determining
 whether they work on a particular target is to compile a test program
 and see whether it complains about __atomic_load_n and/or
 __atomic_store_n being unresolved symbols.  I suppose we could add a
 configure test for that.  Yuck.

 Well, that's pretty easy to integrate into configure - and works on
 crossbuilds. So I think that'd be ok.

 But I actually think this is going to be a manual thing, atomic 8byte
 math will fall back to kernel emulation on several targets, so we
 probably want some defines to explicitly declare it supported on targets
 where that's not the case.

Hmm, OK.  I had not come across such cases.  There are architectures
where it Just Works (like x64_64), architectures where you can make it
work by using special instructions (like x86), and (presumably)
architectures where it doesn't work at all.  Places where it works
using really slow kernel emulation are yet another category to worry
about.  Unfortunately, I have not found any good source that describes
which architectures fall into which category.  Without that, pulling
this together seems intimidating, unless we just declare it working
for x86_64, disable it everywhere else, and wait for people running on
other architectures to complain.

I wonder whether it'd be safe to assume that any machine where
pointers are 8 bytes has 8-byte atomic loads and stores.  I bet there
is a counterexample somewhere.  :-(

-- 
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] Detection of nested function calls

2013-10-28 Thread Andres Freund
On 2013-10-28 14:26:20 -0400, Tom Lane wrote:
 Andres Freund and...@2ndquadrant.com writes:
  On 2013-10-28 13:41:46 -0400, Tom Lane wrote:
  I don't think that's a safe assumption at all.  We need to be able to do
  flattening anywhere PG_DETOAST_DATUM() can be called.
 
  I am not sure we want things to work along those lines. I'd rather make
  PG_DETOAST_DATUM pass along such in-memory Datums unchanged and require
  any funtion that wants to poke into into the Datum in detail to know
  about the different representations.
 
 No; see my upthread comments.  I think what we want to do is to have
 PG_DETOAST_DATUM automatically flatten non-flat datums, and to require
 functions that can cope with non-flat inputs to use a new argument
 fetching macro, exactly along the lines of what we did with non-aligned
 toasted values awhile ago (see PG_GETARG_TEXT_PP and suchlike).  That way,
 code that hasn't yet been updated to deal with non-flat datums will still
 work, if a bit inefficiently compared to what we'd like.
 Non-performance-critical functions might never get updated at all.

My problem isn't datatype specific functions doing superflous
detoasting. If it were just them, I'd be perfectly happy going your way.
My concern is type-independent code detoasting everything without giving
the type specific code any say over it. Like, printtup.c, spi.c,
rowtype.c...
I guess we'll have to spread knowledge over the the new toast type to
those places then.

 If we do what you're suggesting here, any attempt to de-flatten a datatype
 will be a flag day on which *every* *single* *function* that deals with
 that datatype has to change simultaneously.  That will basically destroy
 our chance of ever doing anything about widely-used types like arrays.
 This feature *must* be something that we can install support for
 incrementally (ie one function at a time), the same way we did with
 non-aligned toasted values, or for that matter with several previous
 global changes like the adoption of V1 call convention.

I don't think this is a change on the same scale as V1 call conventions
or short varlenas which are type independent because a type explicitly
has to sign up for it.
E.g. the numeric storage is private to numeric.c, so it'd be perfectly
fine to change the numeric representation in a flag day manner as long
as we still can read the old representation.

I grant you that arrays are *the* counter example to this.

Greetings,

Andres Freund

-- 
 Andres Freund 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] PL/Python: domain over array support

2013-10-28 Thread Rodolfo Campero
Done, thanks.


2013/10/28 Robert Haas robertmh...@gmail.com

 On Sat, Oct 26, 2013 at 9:17 AM, Rodolfo Campero
 rodolfo.camp...@anachronics.com wrote:
  The attached patch add support of domains over arrays to PL/Python (eg:
  CREATE DOMAIN my_domain AS integer[]).
 
  Basically it just uses get_base_element_type instead of get_element_type
 in
  plpy_typeio.c, and uses domain_check before returning a sequence as
 array in
  PLySequence_ToArray whenever appropriate.
 
  There's one line I'm not sure about; I modified a switch statement (line
  427):
  switch (element_type ? element_type : getBaseType(arg-typoid))
  The rationale is that when element_type is set, it is already a base
 type,
  because there is no support of arrays of domains in PostgreSQL, but this
 may
  not held true in the future.

 Please add your patch here so that it doesn't get forgotten about:

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

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



Re: [HACKERS] OSX doesn't accept identical source/target for strcpy() anymore

2013-10-28 Thread Andrew Dunstan


On 10/28/2013 02:26 PM, Andres Freund wrote:


It'd be neat if we could get a buildfarm animal up that uses valgrind -
which would catch such and lots of other errors. That's where the topic
has come up in the past:
http://www.postgresql.org/message-id/20110312133224.GA7833%40tornado.gateway.2wire.net
http://www.postgresql.org/message-id/20130217142209.ga5...@awork2.anarazel.de




How exactly is it going to do that?

Fundamentally, the buildfarm client is simply glue to run existing build 
and test code, collect the results, and send them to the server. AFAICT 
there are no configure or make targets for running under valgrind.


If someone provides the requisite support in the build system for this 
I'll be happy to add buildfarm support for it.


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] better atomics

2013-10-28 Thread Andres Freund
On 2013-10-28 15:02:41 -0400, Robert Haas wrote:
 On Mon, Oct 28, 2013 at 2:19 PM, Andres Freund and...@2ndquadrant.com wrote:
  I'm not terribly excited about relying on 16-byte CAS, but I agree
  that 8-byte math, at least, is important.  I've not been successful in
  finding any evidence that gcc has preprocessor symbols to tell us
  about the properties of 8-byte loads and stores.  The closest thing
  that I found is:
 
  I am talking about making 16byte CAS explicitly optional though? I think
  if code wants to optionally make use of it (e.g. on x86 where it's been
  available basically forever) that's fine. It just has to be optional.
  Or are you saying you're simply not interested in 16byte CAS generally?
 
 I am just not interested in it generally.  Relying on too many OS or
 architecture-specific primitives has several disadvantages.  It's
 going to be a nuisance on more obscure platforms where they may or may
 not be supported and may or may not work right even if supported.
 Even once we get things working right everywhere, it'll mean that
 performance may suffer on non-mainstream platforms.

But it'll suffer against the *increased* performance on modern
platforms, it shouldn't suffer noticeably against the previous
performance on the older platform if we're doing our homework...

 Most of the academic papers I've read on
 implementing lock-free or highly-parallel constructs attempt to
 confine themselves to 8-byte operations with 8-byte compare-and-swap,
 and I'm a bit disposed to think we ought to try to hew to that as
 well.  I'm not dead set against going further, but I lean against it,
 for all of the reasons mentioned above.

I think there are quite some algorithms relying on 16byte CAS, that's
why I was thinking about it at all. I think it's easier to add support
for it in the easier trawl through the compilers, but I won't argue much
for it otherwise for now.

  But I actually think this is going to be a manual thing, atomic 8byte
  math will fall back to kernel emulation on several targets, so we
  probably want some defines to explicitly declare it supported on targets
  where that's not the case.
 
 Hmm, OK.  I had not come across such cases.

E.g. ARM/linux which we probably cannot ignore.

 Places where it works
 using really slow kernel emulation are yet another category to worry
 about.  Unfortunately, I have not found any good source that describes
 which architectures fall into which category.  Without that, pulling
 this together seems intimidating, unless we just declare it working
 for x86_64, disable it everywhere else, and wait for people running on
 other architectures to complain.

Yes, I think whitelisting targs is a sensible approach here. I am pretty
sure we can do better than just x86-64, but that's easily doable in an
incremental fashion.

 I wonder whether it'd be safe to assume that any machine where
 pointers are 8 bytes has 8-byte atomic loads and stores.  I bet there
 is a counterexample somewhere.  :-(

Sparc64 :(.

Btw, could you quickly give some keywords what you're thinking about
making lockless?
I mainly am thinking about lwlocks and buffer pins so far. Nothing
really ambitious.

Greetings,

Andres Freund

-- 
 Andres Freund 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] OSX doesn't accept identical source/target for strcpy() anymore

2013-10-28 Thread Andres Freund
On 2013-10-28 15:20:20 -0400, Andrew Dunstan wrote:
 
 On 10/28/2013 02:26 PM, Andres Freund wrote:
 
 It'd be neat if we could get a buildfarm animal up that uses valgrind -
 which would catch such and lots of other errors. That's where the topic
 has come up in the past:
 http://www.postgresql.org/message-id/20110312133224.GA7833%40tornado.gateway.2wire.net
 http://www.postgresql.org/message-id/20130217142209.ga5...@awork2.anarazel.de
 
 
 
 How exactly is it going to do that?

The easiest method - somewhat slower than necessary - is to just run
valgrind --suppressions=$srcdir/src/tools/valgrind.supp make check.

But the buildfarm supports running a postgres install before
installcheck, right? If we could run just that step using valgrind we'd
be very well of I think because we'd not run valgrind (slow!) if there
are plain regression failures around.

[.. looking for sources ...]
start_db in
https://github.com/PGBuildFarm/client-code/blob/master/run_build.pl is
where the server's run, right? Hm. That uses pg_ctl and not the server
itself and relies on pg_ctl -w returning when the server is
started... So it's not easy to make it use valgrind properly.

We could hack it by replacing bin/postgres with a wrapper around
valgrind that invokes postgres, but ick. Maybe we can make pg_ctl start
$PG_POSTGRES_BINARY instead of postgres if defined?

Better ideas?

 Fundamentally, the buildfarm client is simply glue to run existing build and
 test code, collect the results, and send them to the server. AFAICT there
 are no configure or make targets for running under valgrind.

 If someone provides the requisite support in the build system for this I'll
 be happy to add buildfarm support for it.

It'd be relatively easy to add support for make check (not installcheck)
wrapping postgres in valgrind via pg_regress, but I am not sure that's
the best way to go.

I think defining an additional CFLAG (USE_VALGRIND) shouldn't be a
problem?

Greetings,

Andres Freund

-- 
 Andres Freund 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] better atomics

2013-10-28 Thread Robert Haas
On Mon, Oct 28, 2013 at 3:32 PM, Andres Freund and...@2ndquadrant.com wrote:
 I wonder whether it'd be safe to assume that any machine where
 pointers are 8 bytes has 8-byte atomic loads and stores.  I bet there
 is a counterexample somewhere.  :-(

 Sparc64 :(.

 Btw, could you quickly give some keywords what you're thinking about
 making lockless?
 I mainly am thinking about lwlocks and buffer pins so far. Nothing
 really ambitious.

Well, I was going to use it for some code I'm working on for
parallelism, but I just tested the overhead of a spinlock, and it was
zero, possibly negative.  So I may not have an immediate application.

-- 
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] OSX doesn't accept identical source/target for strcpy() anymore

2013-10-28 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes:
 It'd be relatively easy to add support for make check (not installcheck)
 wrapping postgres in valgrind via pg_regress, but I am not sure that's
 the best way to go.

 I think defining an additional CFLAG (USE_VALGRIND) shouldn't be a
 problem?

CFLAGS doesn't seem to have anything to do with this.  I'd be more
inclined to add a --wrapper=prog argument to pg_regress and invoke
it with something like --wrapper=valgrind --trace-children=yes.

The larger problem though is what you'd do with the output.  There's
enough false-positive noise from valgrind that I can't see having
the buildfarm run just fail if there are any messages.  What to do
instead isn't very clear.

Anyway, to get back to the original problem, I've confirmed that
valgrind complains about the particular case at hand:

==9497== Source and destination overlap in strncpy(0xe1d5a3d, 0xe1d5a3d, 64)
==9497==at 0x4A081EF: strncpy (mc_replace_strmem.c:476)
==9497==by 0x6D2398: namestrcpy (name.c:221)
==9497==by 0x45F478: TupleDescInitEntry (tupdesc.c:507)
==9497==by 0x756FE1: internal_get_result_type (funcapi.c:557)
==9497==by 0x75727C: get_expr_result_type (funcapi.c:235)
==9497==by 0x534146: expandRecordVariable (parse_target.c:1524)
==9497==by 0x52C267: ParseFuncOrColumn (parse_func.c:1494)
==9497==by 0x5285CF: transformExprRecurse (parse_expr.c:463)
==9497==by 0x528DB1: transformExpr (parse_expr.c:117)
==9497==by 0x5350C5: transformTargetEntry (parse_target.c:94)
==9497==by 0x535AD4: transformTargetList (parse_target.c:167)
==9497==by 0x505FFF: transformStmt (analyze.c:929)

It seems to me the most reasonable fix for this is to make
TupleDescInitEntry notice that the passed attributeName points
at the tupdesc's name field and not call namestrcpy if so.
This would go with an API clarification stating that callers can
pass that if they want the name field to be unchanged.  (Or we
could invent some other way to signal that, but note that NULL
is already in use for a different purpose.)

Another possibly-useful approach would be to hack namestrcpy itself
to handle name == str specially.  However, that's legitimizing a
usage that's really a type cheat, so I don't like it as much, even
though it might fix more cases besides this one.

Any other thoughts 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] better atomics

2013-10-28 Thread Heikki Linnakangas

On 28.10.2013 21:32, Andres Freund wrote:

On 2013-10-28 15:02:41 -0400, Robert Haas wrote:

Most of the academic papers I've read on
implementing lock-free or highly-parallel constructs attempt to
confine themselves to 8-byte operations with 8-byte compare-and-swap,
and I'm a bit disposed to think we ought to try to hew to that as
well.  I'm not dead set against going further, but I lean against it,
for all of the reasons mentioned above.


I think there are quite some algorithms relying on 16byte CAS, that's
why I was thinking about it at all. I think it's easier to add support
for it in the easier trawl through the compilers, but I won't argue much
for it otherwise for now.


Many algorithms require a 2*(pointer width) CAS instruction. On 64-bit 
platforms that's 16 bytes, but on 32-bit platforms an 8 byte version 
will suffice.


- Heikki


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


Re: [HACKERS] better atomics

2013-10-28 Thread Tom Lane
Heikki Linnakangas hlinnakan...@vmware.com writes:
 On 28.10.2013 21:32, Andres Freund wrote:
 I think there are quite some algorithms relying on 16byte CAS, that's
 why I was thinking about it at all. I think it's easier to add support
 for it in the easier trawl through the compilers, but I won't argue much
 for it otherwise for now.

 Many algorithms require a 2*(pointer width) CAS instruction. On 64-bit 
 platforms that's 16 bytes, but on 32-bit platforms an 8 byte version 
 will suffice.

You're both just handwaving.  How many is many, and which ones might
we actually have enough use for to justify dealing with such a dependency?
I don't think we should buy into this without some pretty concrete
justification.

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] OSX doesn't accept identical source/target for strcpy() anymore

2013-10-28 Thread Peter Geoghegan
On Mon, Oct 28, 2013 at 6:11 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Both gcc and glibc have been moving steadily in the direction of
 aggressively exploiting undefined behavior cases for optimization
 purposes.  I don't know if there is yet a platform where strncpy with
 src == dest behaves oddly, but we'd be foolish to imagine that it's
 not going to happen eventually.  If anything, Apple is probably doing
 us a service by making it obvious where we're failing to adhere to spec.

It's worth being aware of the fact that the upcoming GCC 4.9 release
is expected to ship with an Undefined Behavior Sanitizer, as
described here:

http://gcc.gnu.org/gcc-4.9/changes.html

-- 
Peter Geoghegan


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


Re: [HACKERS] Detection of nested function calls

2013-10-28 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes:
 On 2013-10-28 14:26:20 -0400, Tom Lane wrote:
 No; see my upthread comments.  I think what we want to do is to have
 PG_DETOAST_DATUM automatically flatten non-flat datums, and to require
 functions that can cope with non-flat inputs to use a new argument
 fetching macro, exactly along the lines of what we did with non-aligned
 toasted values awhile ago (see PG_GETARG_TEXT_PP and suchlike).

 My problem isn't datatype specific functions doing superflous
 detoasting. If it were just them, I'd be perfectly happy going your way.
 My concern is type-independent code detoasting everything without giving
 the type specific code any say over it. Like, printtup.c, spi.c,
 rowtype.c...

In all those cases, if we're detoasting at all then there is probably good
reason to flatten as well.  Or if not, we'll teach the code the
difference.  Just as with function arguments, it can never be *wrong* to
flatten, it might only be inefficient --- so we'll improve the
inefficiencies.  One at a time.

 If we do what you're suggesting here, any attempt to de-flatten a datatype
 will be a flag day on which *every* *single* *function* that deals with
 that datatype has to change simultaneously.

 I don't think this is a change on the same scale as V1 call conventions
 or short varlenas which are type independent because a type explicitly
 has to sign up for it.

I think it's going to be more widely adopted than you think, unless we
make it so impractical to adopt that only one or two types ever do it.

 E.g. the numeric storage is private to numeric.c, so it'd be perfectly
 fine to change the numeric representation in a flag day manner as long
 as we still can read the old representation.

So in other words, you believe there is no extension anywhere that deals
with numeric values?  Sorry, I don't believe that.  And even if I believed
it for numeric, the assumption certainly falls to the ground once you
extend it to every datatype that might have use for this facility.

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] better atomics

2013-10-28 Thread Andres Freund
On 2013-10-28 16:06:47 -0400, Tom Lane wrote:
 Heikki Linnakangas hlinnakan...@vmware.com writes:
  On 28.10.2013 21:32, Andres Freund wrote:
  I think there are quite some algorithms relying on 16byte CAS, that's
  why I was thinking about it at all. I think it's easier to add support
  for it in the easier trawl through the compilers, but I won't argue much
  for it otherwise for now.
 
  Many algorithms require a 2*(pointer width) CAS instruction. On 64-bit 
  platforms that's 16 bytes, but on 32-bit platforms an 8 byte version 
  will suffice.
 
 You're both just handwaving.  How many is many, and which ones might
 we actually have enough use for to justify dealing with such a dependency?
 I don't think we should buy into this without some pretty concrete
 justification.

Well, I don't think either of us is suggesting to make it required. But
it's going to be painful to go through the list of compilers repeatedly
instead of just adding all operations at one. I am willing to do that
for several compilers once, but I don't want to do it in each and every
feature submission using another atomic op.

Greetings,

Andres Freund

-- 
 Andres Freund 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] better atomics

2013-10-28 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes:
 On 2013-10-28 16:06:47 -0400, Tom Lane wrote:
 You're both just handwaving.  How many is many, and which ones might
 we actually have enough use for to justify dealing with such a dependency?
 I don't think we should buy into this without some pretty concrete
 justification.

 Well, I don't think either of us is suggesting to make it required. But
 it's going to be painful to go through the list of compilers repeatedly
 instead of just adding all operations at one. I am willing to do that
 for several compilers once, but I don't want to do it in each and every
 feature submission using another atomic op.

Basically I'm arguing that that choice is backwards.  We should decide
first what the list of supported atomics is going to be, and each and
every element of that list has to have a convincing concrete argument
why we need to support it.  Not we might want this someday.  Because
when someday comes, that's when we'd be paying the price of finding out
which platforms actually support the primitive correctly and with what
performance.  Or if someday never comes, we're not ahead either.

Or if you like: no matter what you do, the first feature submission
that makes use of a given atomic op is going to suffer pain.  I don't
want to still be suffering that pain two or three years out.  The shorter
the list of allowed atomic ops, the sooner we'll be done with debugging
them.

regards, tom lane


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


Re: [HACKERS] OSX doesn't accept identical source/target for strcpy() anymore

2013-10-28 Thread Andres Freund
On 2013-10-28 16:02:36 -0400, Tom Lane wrote:
 Andres Freund and...@2ndquadrant.com writes:
  It'd be relatively easy to add support for make check (not installcheck)
  wrapping postgres in valgrind via pg_regress, but I am not sure that's
  the best way to go.
 
  I think defining an additional CFLAG (USE_VALGRIND) shouldn't be a
  problem?
 
 CFLAGS doesn't seem to have anything to do with this.  I'd be more
 inclined to add a --wrapper=prog argument to pg_regress and invoke
 it with something like --wrapper=valgrind --trace-children=yes.

Err. I am *obviously* not saying that it makes the backend run under
valgrind. But if we're going to have a buildfarm animal running
valgrind, it'd be useful to run to let it catch all errors it can
instead of hiding many of them via mcxt/aset.c? Right?

 The larger problem though is what you'd do with the output.  There's
 enough false-positive noise from valgrind that I can't see having
 the buildfarm run just fail if there are any messages.  What to do
 instead isn't very clear.

The false positives should be gone using the suppressions file we ship
these days (--suppressions=/path/to/pg/src/tools/valgrind.supp). We
might miss some more cases there, but it should be fairly easy to extend
it.

 It seems to me the most reasonable fix for this is to make
 TupleDescInitEntry notice that the passed attributeName points
 at the tupdesc's name field and not call namestrcpy if so.
 This would go with an API clarification stating that callers can
 pass that if they want the name field to be unchanged.

+1

 Another possibly-useful approach would be to hack namestrcpy itself
 to handle name == str specially.  However, that's legitimizing a
 usage that's really a type cheat, so I don't like it as much, even
 though it might fix more cases besides this one.

-1

Greetings,

Andres Freund

-- 
 Andres Freund 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] better atomics

2013-10-28 Thread Andres Freund
On 2013-10-28 16:29:35 -0400, Tom Lane wrote:
 Andres Freund and...@2ndquadrant.com writes:
  On 2013-10-28 16:06:47 -0400, Tom Lane wrote:
  You're both just handwaving.  How many is many, and which ones might
  we actually have enough use for to justify dealing with such a dependency?
  I don't think we should buy into this without some pretty concrete
  justification.
 
  Well, I don't think either of us is suggesting to make it required. But
  it's going to be painful to go through the list of compilers repeatedly
  instead of just adding all operations at one. I am willing to do that
  for several compilers once, but I don't want to do it in each and every
  feature submission using another atomic op.
 
 Basically I'm arguing that that choice is backwards.  We should decide
 first what the list of supported atomics is going to be, and each and
 every element of that list has to have a convincing concrete argument
 why we need to support it.

The list I have previously suggested was:

* pg_atomic_load_u32(uint32 *)
* uint32 pg_atomic_store_u32(uint32 *)

To be able to write code without spreading volatiles all over while
making it very clear that that part of the code is worthy of attention.

* uint32 pg_atomic_exchange_u32(uint32 *ptr, uint32 val)
* bool pg_atomic_compare_exchange_u32(uint32 *ptr, uint32 *expected, uint32 
newval)

Used in most lockfree algorithms, can be used to provide fallbacks for
when any of the other 32bit operations is not implemented for a platform
(so it's easier to bring up a platform).
E.g. used in in the wait-free LW_SHARED patch.

* uint32 pg_atomic_fetch_add_u32(uint32 *ptr, uint32 add)

Plain math.
E.g. used in in the wait-free LW_SHARED patch.

* uint32 pg_atomic_fetch_and_u32(uint32 *ptr, uint32 add)
* uint32 pg_atomic_fetch_or_u32(uint32 *ptr, uint32 add)

Useful for (un-)setting flags atomically. Needed for (my approach to)
spinlock-less Pin/UnpinBuffer.

* u64 variants of the above

lockfree list manipulation need at least pointer width operations of at
least compare_exchange().

I *personally* don't have a case for 8byte math yet, but I am pretty
sure parallel sort might be interested in it.

* bool pg_atomic_test_set(void *ptr)
* void pg_atomic_clear(void *ptr)

Can be used as the implementation for spinlocks based on compiler
intrinsics if no native implementation is existing. Makes it easier to
bring up new platforms.

Wrappers around the above:
* uint32 pg_atomic_fetch_sub_u32(uint32 *ptr, uint32 add)
Generic wrapper that imo makes the code easier to read. No
per-platform/compiler overhead.

* pg_atomic_add_until_u32(uint32 *ptr, uint32 val, uint32 limit)
Useful for managing refcounts like pin counts.

* pg_atomic_(add|sub|and|or)_fetch_u32()
Generic wrappers for more legible code.  No
per-platform/compiler overhead.

* pg_atomic_compare_and_swap_128()

Many algorithms are faster (e.g. some lockless hashtables, which'd be
great for the buffer mapping) when a double-pointer-width CAS is
available, but also work with an pointer-width CAS in a less efficient
manner.

 Or if you like: no matter what you do, the first feature submission
 that makes use of a given atomic op is going to suffer pain.  I don't
 want to still be suffering that pain two or three years out.  The shorter
 the list of allowed atomic ops, the sooner we'll be done with debugging
 them.

Yea. As, partially, even evidenced by this discussion ;).

Which would you like to see go?

Greetings,

Andres Freund

-- 
 Andres Freund 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] PostgreSQL Service on Windows does not start. ~ is not a valid Win32 application

2013-10-28 Thread Asif Naeem
Hi Sandeep,

PFA Naoya's patch (pg_ctl.c.patch).

Hi Naoya,

Good finding. I have attached another version of patch
(pg_ctl.c_windows_vulnerability.patch) attached that has fewer lines of
code changes, can you please take a look ?. Thanks.

Best Regards,
Asif Naeem


On Mon, Oct 28, 2013 at 4:46 PM, Sandeep Thakkar 
sandeep.thak...@enterprisedb.com wrote:

 Hi Dave

 We register the service using pg_ctl. When I manually executed the
 following on the command prompt, I saw that the service path of the
 registered service did not have the pg_ctl.exe path in quotes. May be it
 should be handled in the pg_ctl code.

 *c:\Users\Sandeep Thakkar\Documents*c:\Program
 Files\PostgreSQL\9.3\bin\pg_ctl.e
 xe register -N pg-9.3 -U NT AUTHORITY\NetworkService -D c:\Program
 Files\P
 ostgreSQL\9.3\data -w

 Naoya,  I could not find your patch here. Can you please share it again?



 On Mon, Oct 28, 2013 at 2:53 PM, Dave Page dp...@pgadmin.org wrote:

 Sandeep, can you look at this please? Thanks.

 On Mon, Oct 28, 2013 at 8:18 AM, Asif Naeem anaeem...@gmail.com wrote:
  It is related to windows unquoted service path vulnerability in the the
  installer that creates service path without quotes that make
 service.exe to
  look for undesirable path for executable.
 
  postgresql-9.3 service path : C:/Users/asif/Desktop/Program
  files/9.3/bin/pg_ctl.exe runservice -N postgresql-9.3 -D
  C:/Users/asif/Desktop/Program files/9.3/data -w
 
  service.exe
 
  C:\Users\asif\Desktop\Program NAME NOT FOUND
  C:\Users\asif\Desktop\Program.exe NAME NOT FOUND
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe ACCESS
 DENIED
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe ACCESS
 DENIED
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice
 NAME
  NOT FOUND
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice.exe
  NAME NOT FOUND
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  NAME NOT FOUND
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice
 -N.exe
  NAME NOT FOUND
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3.exe NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D.exe NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D C:\Users\asif\Desktop\Program NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D C:\Users\asif\Desktop\Program.exe NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data
 NAME
  INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data.exe
  NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data -w
  NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data
 -w.exe
  NAME INVALID
 
 
  Fix :
 
  postgresql-9.3 service path : C:/Users/asif/Desktop/Program
  files/9.3/bin/pg_ctl.exe runservice -N postgresql-9.3 -D
  C:/Users/asif/Desktop/Program files/9.3/data -w
 
  It would be good if this is reported on pg installer forum or security
  forum. Thanks.
 
  Regards,
  Asif Naeem
 
  On Mon, Oct 28, 2013 at 12:06 PM, Naoya Anzai
  anzai-na...@mxu.nes.nec.co.jp wrote:
 
  Hi, Asif.
 
  Thank you for response.
 
 
 C:\Users\asif\Desktop\Program files\9.3bin\pg_ctl -D
   C:\Users\asif\Desktop\Program files\9.3\data1 -l logfile start
 server starting
 
  This failure does not occur by the command line.
  PostgreSQL needs to start by Windows Service.
 
  Additionally,In this case,
  A file Program needs to be exist at C:\Users\asif\Desktop\, and
  postgres.exe needs to be exist at C:\Users\asif\Desktop\Program
  files\9.3\bin.
  
  C:\Users\asif\Desktop\Program files\9.3\bindir
  ...
  4,435,456   postgres.exe
 80,896   pg_ctl.exe
  ...
 
  C:\Users\asif\Desktoppdir
  ...
  0  Program
  DIR  Program files
  ...
  
 
  Regards,
  Naoya
 
   Hi Naoya,
  
   I am not able to reproduce the problem. Do you mean pg windows
 service
   installed by installer is not working or bin\pg_ctl binary is not
 accepting
   spaces in the patch ?. Following worked for me i.e.
  
  
 C:\Users\asif\Desktop\Program files\9.3bin\pg_ctl -D
   C:\Users\asif\Desktop\Program files\9.3\data1 -l logfile start
 server starting
  
  
   Can you please share the exact steps ?. Thanks.
  
  
   

Re: [HACKERS] OSX doesn't accept identical source/target for strcpy() anymore

2013-10-28 Thread Peter Eisentraut
On 10/28/13, 4:11 PM, Peter Geoghegan wrote:
 On Mon, Oct 28, 2013 at 6:11 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Both gcc and glibc have been moving steadily in the direction of
 aggressively exploiting undefined behavior cases for optimization
 purposes.  I don't know if there is yet a platform where strncpy with
 src == dest behaves oddly, but we'd be foolish to imagine that it's
 not going to happen eventually.  If anything, Apple is probably doing
 us a service by making it obvious where we're failing to adhere to spec.
 
 It's worth being aware of the fact that the upcoming GCC 4.9 release
 is expected to ship with an Undefined Behavior Sanitizer, as
 described here:
 
 http://gcc.gnu.org/gcc-4.9/changes.html

Address Sanitizer, which is already in clang and gcc, does catch the
case were are talking about (as was previously discussed).  But its only
reaction is crashing, and in my testing it's crashing already before it
gets to this place, so we'd have to put in some more work before it will
be useful.



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


[HACKERS] What hook would you recommend for one time, post authentication?

2013-10-28 Thread Daniel Farina
What hook would you recommend that matches this criteria:

* Runs post-authentication

* ..Once

I was putting together a little extension module[0] intended to do
connection limits out-of-band with the catalog (so that hot standbys
and primaries can have different imposed connection limits), but am
stymied because I can't locate a hook matching this description.

My general approach has been to try to use
GetUserNameFromId(GetSessionUserId()), but this requires
InitializeSessionUserId be called first, and that has been causing me
some trouble.

ClientAuthentication_hook is too early (authentication has not yet
happened, InitializeSessionUserId has not been called).  Many of the
other hooks are run per query (like the Executor hooks).  And,
postinit.c is not giving me a lot of clues here and nothing with the
lexeme 'hook' is giving me a lot of confidence as seen in
typedefs.list/grep.

I appreciate any advice one can supply, thank you.

[0]: https://github.com/fdr/pg_connlimit


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


Re: [HACKERS] PostgreSQL Service on Windows does not start. ~ is not a valid Win32 application

2013-10-28 Thread Naoya Anzai
Hi, Asif

Thank you for providing my patch (pg_ctl.c.patch) to Sandeep on my behalf.

 Good finding. I have attached another version of patch 
 (pg_ctl.c_windows_vulnerability.patch) attached that has fewer lines of code 
 changes, can you please take a look ?. Thanks.

I think your patch is not sufficient to fix.
Not only pg_ctl.exe but postgres.exe also have the same problem.
Even if your patch is attached, 
A Path of postgres.exe passed to CreateRestrictedProcess is not enclosed in 
quotation.(See pgwin32_ServiceMain at pg_ctl.c) 

So, processing enclosed in quotation should do in both conditions.

Regards, 
Naoya

---
Naoya Anzai
Engineering Department
NEC Soft, Ltd.
E-Mail: anzai-na...@mxu.nes.nec.co.jp
---


 Hi Sandeep,
 
 PFA Naoya's patch (pg_ctl.c.patch). 
 
 Hi Naoya,
 
 Good finding. I have attached another version of patch 
 (pg_ctl.c_windows_vulnerability.patch) attached that has fewer lines of code 
 changes, can you please take a look ?. Thanks.
 
 Best Regards,
 Asif Naeem
 
 
 On Mon, Oct 28, 2013 at 4:46 PM, Sandeep Thakkar 
 sandeep.thak...@enterprisedb.com wrote:
 
 
   Hi Dave
 
   We register the service using pg_ctl. When I manually executed the 
 following on the command prompt, I saw that the service path of the 
 registered service did not have the pg_ctl.exe path in quotes. May be it 
 should be handled in the pg_ctl code. 
 
   c:\Users\Sandeep Thakkar\Documentsc:\Program 
 Files\PostgreSQL\9.3\bin\pg_ctl.e
   xe register -N pg-9.3 -U NT AUTHORITY\NetworkService -D 
 c:\Program Files\P
   ostgreSQL\9.3\data -w
 
   Naoya,  I could not find your patch here. Can you please share it 
 again? 
 
 
 
   On Mon, Oct 28, 2013 at 2:53 PM, Dave Page dp...@pgadmin.org wrote:
   
 
   Sandeep, can you look at this please? Thanks.
   
   On Mon, Oct 28, 2013 at 8:18 AM, Asif Naeem 
 anaeem...@gmail.com wrote:
It is related to windows unquoted service path vulnerability 
 in the the
installer that creates service path without quotes that make 
 service.exe to
look for undesirable path for executable.
   
postgresql-9.3 service path : C:/Users/asif/Desktop/Program
files/9.3/bin/pg_ctl.exe runservice -N postgresql-9.3 -D
C:/Users/asif/Desktop/Program files/9.3/data -w
   
service.exe
   
C:\Users\asif\Desktop\Program NAME NOT FOUND
C:\Users\asif\Desktop\Program.exe NAME NOT FOUND
C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe 
 ACCESS DENIED
C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe 
 ACCESS DENIED
C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe 
 runservice NAME
NOT FOUND
C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe 
 runservice.exe
NAME NOT FOUND
C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe 
 runservice -N
NAME NOT FOUND
C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe 
 runservice -N.exe
NAME NOT FOUND
C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe 
 runservice -N
postgresql-9.3 NAME INVALID
C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe 
 runservice -N
postgresql-9.3.exe NAME INVALID
C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe 
 runservice -N
postgresql-9.3 -D NAME INVALID
C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe 
 runservice -N
postgresql-9.3 -D.exe NAME INVALID
C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe 
 runservice -N
postgresql-9.3 -D C:\Users\asif\Desktop\Program NAME 
 INVALID
C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe 
 runservice -N
postgresql-9.3 -D C:\Users\asif\Desktop\Program.exe NAME 
 INVALID
C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe 
 runservice -N
postgresql-9.3 -D C:\Users\asif\Desktop\Program 
 files\9.3\data NAME
INVALID
C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe 
 runservice -N
postgresql-9.3 -D C:\Users\asif\Desktop\Program 
 files\9.3\data.exe
NAME INVALID
C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe 
 runservice -N
postgresql-9.3 -D C:\Users\asif\Desktop\Program 
 files\9.3\data -w
NAME INVALID
C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe 
 runservice -N
postgresql-9.3 -D C:\Users\asif\Desktop\Program 
 files\9.3\data -w.exe
NAME INVALID
   
   
Fix 

Re: [HACKERS] OSX doesn't accept identical source/target for strcpy() anymore

2013-10-28 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes:
 On 2013-10-28 16:02:36 -0400, Tom Lane wrote:
 The larger problem though is what you'd do with the output.  There's
 enough false-positive noise from valgrind that I can't see having
 the buildfarm run just fail if there are any messages.  What to do
 instead isn't very clear.

 The false positives should be gone using the suppressions file we ship
 these days (--suppressions=/path/to/pg/src/tools/valgrind.supp). We
 might miss some more cases there, but it should be fairly easy to extend
 it.

They're not all gone according to my testing; but there are far worse
problems:

1. The output goes to stderr which means it's mixed in with the backend's
normal log chatter.

2. valgrind causes autovacuum to dump core, at least on my box (RHEL6).
I'm prepared to believe that this is some relatively old bug that Red Hat
hasn't gotten round to including a patch for, but still it doesn't leave
me with any warm fuzzy feeling about the practicality of routine valgrind
testing.

regards, tom lane


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


[HACKERS] missing RelationCloseSmgr in FreeFakeRelcacheEntry?

2013-10-28 Thread Andres Freund
Hi,

I've started a valgrind run earlier when trying to run the regression
tests with valgrind --error-exitcode=122 (to cause the regression tests
to fail visibly) but it crashed frequently...

One of them was:

==2184== Invalid write of size 8
==2184==at 0x76787F: smgrclose (smgr.c:284)
==2184==by 0x4ED717: xact_redo_commit_internal (xact.c:4676)
==2184==by 0x4ED7FF: xact_redo_commit (xact.c:4712)
==2184==by 0x4EDB0D: xact_redo (xact.c:4838)
==2184==by 0x50355D: StartupXLOG (xlog.c:6809)
==2184==by 0x70AA1E: StartupProcessMain (startup.c:224)
==2184==by 0x512B38: AuxiliaryProcessMain (bootstrap.c:429)
==2184==by 0x709C43: StartChildProcess (postmaster.c:5063)
==2184==by 0x7086EA: PostmasterStateMachine (postmaster.c:3544)
==2184==by 0x7072F1: reaper (postmaster.c:2801)
==2184==by 0x57B325F: ??? (in /lib/x86_64-linux-gnu/libc-2.17.so)
==2184==by 0x585F822: __select_nocancel (syscall-template.S:81)
==2184==  Address 0x5f63410 is 5,584 bytes inside a recently re-allocated block 
of size 8,192 alloc'd
==2184==at 0x402ACEC: malloc (vg_replace_malloc.c:270)
==2184==by 0x8B3F8E: AllocSetAlloc (aset.c:854)
==2184==by 0x8B623B: MemoryContextAlloc (mcxt.c:581)
==2184==by 0x8B5F93: MemoryContextCreate (mcxt.c:522)
==2184==by 0x8B33C4: AllocSetContextCreate (aset.c:444)
==2184==by 0x8B55DD: MemoryContextInit (mcxt.c:110)
==2184==by 0x703B17: PostmasterMain (po

Which seems to indicate a dangling -rd_smgr pointer, confusing the heck
out of me because I couldn't see how that'd happen.

Looking a bit closer it seems to me that the fake relcache
infrastructure seems to neglect the chance that something used the fake
entry to read something which will have done a RelationOpenSmgr(). Which
in turn will have set rel-rd_smgr-smgr_owner to rel.
When we then pfree() the fake relation in FreeFakeRelcacheEntry we have
a dangling pointer.

It confuses me a bit that this doesn't cause issues during recovery more
frequently... The code seems to have been that way since
a213f1ee6c5a1bbe1f074ca201975e76ad2ed50c which introduced fake relcache
entries. It looks like XLogCloseRelationCache() indirectly has done a
RelationCloseSmgr().

diff --git a/src/backend/access/transam/xlogutils.c 
b/src/backend/access/transam/xlogutils.c
index 5429d5e..ee7c892 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -433,6 +433,7 @@ CreateFakeRelcacheEntry(RelFileNode rnode)
 void
 FreeFakeRelcacheEntry(Relation fakerel)
 {
+   RelationCloseSmgr(fakerel);
pfree(fakerel);
 }

Greetings,

Andres Freund

-- 
 Andres Freund 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] What hook would you recommend for one time, post authentication?

2013-10-28 Thread Tom Lane
Daniel Farina dan...@heroku.com writes:
 What hook would you recommend that matches this criteria:
 * Runs post-authentication

 * ..Once

ClientAuthentication_hook

 My general approach has been to try to use
 GetUserNameFromId(GetSessionUserId()), but this requires
 InitializeSessionUserId be called first, and that has been causing me
 some trouble.

So don't do that.  The HBA code uses 

roleid = get_role_oid(port-user_name, true);

and actually if you just want the string form you could use
port-user_name without any extra pushups (bearing in mind that
it might or might not be a valid user name).

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] ERROR : 'tuple concurrently updated'

2013-10-28 Thread Craig Ringer
On 10/28/2013 05:52 PM, Stéphan BEUZE wrote:
 Is it OK if I send a test case written in Java ? Or is there a well
 defined way to post test case ?

A standalone test case written in Java is pretty easy to run. Just
provide build and run instructions - for example, if it's a stand-alone
file, install the JDK for your OS (install OpenJDK from package
management if on Linux) then:

javac TheClass.java
java -cp postgresql-9.2-1003.jdbc3.jar: TheClass

Most PostgreSQL users on this list won't have much if any Java tooling
installed, won't know Ant, Maven, JDBC drivers, etc. So Java test cases
will need to be documented for a from-scratch start. I'm happy to run
any test case, and I _have_ used a bunch of Java tools.

-- 
 Craig Ringer   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] OSX doesn't accept identical source/target for strcpy() anymore

2013-10-28 Thread Andres Freund
On 2013-10-28 21:14:48 -0400, Tom Lane wrote:
 Andres Freund and...@2ndquadrant.com writes:
  On 2013-10-28 16:02:36 -0400, Tom Lane wrote:
  The larger problem though is what you'd do with the output.  There's
  enough false-positive noise from valgrind that I can't see having
  the buildfarm run just fail if there are any messages.  What to do
  instead isn't very clear.
 
  The false positives should be gone using the suppressions file we ship
  these days (--suppressions=/path/to/pg/src/tools/valgrind.supp). We
  might miss some more cases there, but it should be fairly easy to extend
  it.
 
 They're not all gone according to my testing; but there are far worse
 problems:

Spurious or real bugs? Inside PG or libc?



 1. The output goes to stderr which means it's mixed in with the backend's
 normal log chatter.

That's relatively easy to fix. We could just pass --log-file
redirecting the errors somewhere special and display them there.

What I've done so far is to tell valgrind to let child processes with
errors exit with a non-zero exitcode using the --error-exitcode
parameter and specify -q to reduce the chatter upon normal process
exit. That gives at least some correlation to the errors in the failed
tests.

 2. valgrind causes autovacuum to dump core, at least on my box (RHEL6).
 I'm prepared to believe that this is some relatively old bug that Red Hat
 hasn't gotten round to including a patch for, but still it doesn't leave
 me with any warm fuzzy feeling about the practicality of routine valgrind
 testing.

Yea, I know which bug that is, I've pushed the valgrind guys into fixing
it... valgrind used to get confused about stack alignment in signal
handlers causing instructions that care about that (mostly xmm* register
using ones) to fail. elog() fails because we frequently pass many
parameters.
Since we fork processes from inside signal handlers, that situation
happens pretty often.

https://bugs.kde.org/show_bug.cgi?id=280114

3. valgrind gets floating point computations for
exp(larger_negative_double) wrong and returns the wrong error message:

regression=# SELECT exp(-808.3::float8);
ERROR:  value out of range: overflow

exp sets errno=ERANGE and returns inf. That's not supposed to happen
according to my exp(3)...

Greetings,

Andres Freund

-- 
 Andres Freund 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] OSX doesn't accept identical source/target for strcpy() anymore

2013-10-28 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes:
 On 2013-10-28 21:14:48 -0400, Tom Lane wrote:
 They're not all gone according to my testing; but there are far worse
 problems:

 Spurious or real bugs? Inside PG or libc?

I saw a bunch of uninitialized-value complaints in initdb, apparently from
places in BootstrapXLog that write out uninitialized pad bytes.  I didn't
get far in testing the main regression tests because of the autovacuum
crash problem.

 3. valgrind gets floating point computations for
 exp(larger_negative_double) wrong and returns the wrong error message:
 regression=# SELECT exp(-808.3::float8);
 ERROR:  value out of range: overflow

Ugh ...

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] OSX doesn't accept identical source/target for strcpy() anymore

2013-10-28 Thread Noah Misch
On Mon, Oct 28, 2013 at 09:14:48PM -0400, Tom Lane wrote:
 Andres Freund and...@2ndquadrant.com writes:
  On 2013-10-28 16:02:36 -0400, Tom Lane wrote:
  The larger problem though is what you'd do with the output.  There's
  enough false-positive noise from valgrind that I can't see having
  the buildfarm run just fail if there are any messages.  What to do
  instead isn't very clear.
 
  The false positives should be gone using the suppressions file we ship
  these days (--suppressions=/path/to/pg/src/tools/valgrind.supp). We
  might miss some more cases there, but it should be fairly easy to extend
  it.
 
 They're not all gone according to my testing

True.  Getting a clean Valgrind report is similar to getting the build
warning-free.  Variations of compiler version, optimization level, host OS,
and CPU architecture can all affect the set of errors Valgrind reports, just
as they affect compiler warnings.  Valgrind cleanliness has a lot of catching
up to do.

I never ran initdb under Valgrind, just make installcheck, so that's novel
territory.

 1. The output goes to stderr which means it's mixed in with the backend's
 normal log chatter.

As Andres explained, this is strictly a local configuration choice.

 2. valgrind causes autovacuum to dump core, at least on my box (RHEL6).

Don't bother with versions older than Valgrind 3.8.1.  Besides having a fix
for that bug, it runs PostgreSQL an order of magnitude faster, per the comment
in pg_config_manual.h.

nm

-- 
Noah Misch
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] OSX doesn't accept identical source/target for strcpy() anymore

2013-10-28 Thread Noah Misch
On Mon, Oct 28, 2013 at 04:02:36PM -0400, Tom Lane wrote:
 It seems to me the most reasonable fix for this is to make
 TupleDescInitEntry notice that the passed attributeName points
 at the tupdesc's name field and not call namestrcpy if so.

+1

-- 
Noah Misch
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] OSX doesn't accept identical source/target for strcpy() anymore

2013-10-28 Thread Tom Lane
Noah Misch n...@leadboat.com writes:
 On Mon, Oct 28, 2013 at 09:14:48PM -0400, Tom Lane wrote:
 2. valgrind causes autovacuum to dump core, at least on my box (RHEL6).

 Don't bother with versions older than Valgrind 3.8.1.

$ rpm -qa | grep valgrind
valgrind-3.8.1-3.2.el6.x86_64

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] OSX doesn't accept identical source/target for strcpy() anymore

2013-10-28 Thread Andres Freund
On 2013-10-28 22:20:02 -0400, Noah Misch wrote:
  2. valgrind causes autovacuum to dump core, at least on my box (RHEL6).
 
 Don't bother with versions older than Valgrind 3.8.1.  Besides having a fix
 for that bug, it runs PostgreSQL an order of magnitude faster, per the comment
 in pg_config_manual.h.

I don't think that bugfix is in upstream's 3.8.1 given that was released
months earlier than the bugfix was committed... Might be backported in
your package though...

Greetings,

Andres Freund

-- 
 Andres Freund 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] OSX doesn't accept identical source/target for strcpy() anymore

2013-10-28 Thread Noah Misch
On Mon, Oct 28, 2013 at 10:30:10PM -0400, Tom Lane wrote:
 Noah Misch n...@leadboat.com writes:
  On Mon, Oct 28, 2013 at 09:14:48PM -0400, Tom Lane wrote:
  2. valgrind causes autovacuum to dump core, at least on my box (RHEL6).
 
  Don't bother with versions older than Valgrind 3.8.1.
 
 $ rpm -qa | grep valgrind
 valgrind-3.8.1-3.2.el6.x86_64

Apparently I only dreamt the bug was gone in Valgrind 3.8.1; my usual testing
configuration has autovacuum=off.  Sorry for the noise.

-- 
Noah Misch
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] OSX doesn't accept identical source/target for strcpy() anymore

2013-10-28 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes:
 On 2013-10-28 21:14:48 -0400, Tom Lane wrote:
 2. valgrind causes autovacuum to dump core, at least on my box (RHEL6).

 Yea, I know which bug that is, I've pushed the valgrind guys into fixing
 it...
 https://bugs.kde.org/show_bug.cgi?id=280114

Thanks, I whined to Red Hat at
https://bugzilla.redhat.com/show_bug.cgi?id=1024162
and also updated our wiki page about Valgrind (where somebody overhastily
removed all mention of the issue ...)

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] What hook would you recommend for one time, post authentication?

2013-10-28 Thread Daniel Farina
On Mon, Oct 28, 2013 at 6:27 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 roleid = get_role_oid(port-user_name, true);

Thank you for that, that appears to work very well to my purpose, as
does ClientAuthentication_hook, now.


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


[HACKERS] Prototype row-security write trigger

2013-10-28 Thread Craig Ringer
The following trigger is a PL/PgSQL prototype of a row-security trigger
to enforce row-security policy on writes.

I'm not proposing it for use as-is obviously, I'm just looking into how
things work and things to fix.

The biggest problem here is that the policy can by bypassed by a trigger
that runs after this one, and PostgreSQL has no permissions model to
force some system triggers to run first or last. A solution to this
would be desirable to prevent users from breaking referential integrity
constraint checks as well as to allow proper row security enforcement.

The second problem is that performance is pretty ugly because of the
need to look up the row security constraint each time. Moving this
trigger into C and using the relcache should help with that, making it
no better or worse than FK constraint checks. That'd also make for a
faster superuser test than this version offers.

Finally, while this will prevent rows that violate the table's RLS
constraint from being inserted, it does NOT prevent probing for foreign
key constraints because the FK check trigger doesn't respect RLS. Rather
than try to implement those checks again in the RLS write trigger I'd
like to teach FK triggers to respect RLS rules instead.

Thoughts/comments?

CREATE OR REPLACE FUNCTION rowsecurity_check() RETURNS TRIGGER AS $$
DECLARE
  rowsecurity text;
  rowcount integer;
BEGIN
  IF (SELECT usesuper FROM pg_user WHERE usename = current_user) THEN
RETURN NEW;
  END IF;
  rowsecurity = (
SELECT pg_catalog.pg_get_expr(rs.rsecqual, c.oid)
FROM pg_class c
INNER JOIN pg_rowsecurity rs ON (c.relhasrowsecurity AND
rs.rsecrelid = c.oid)
INNER JOIN pg_namespace n ON (c.relnamespace = n.oid)
WHERE c.relname = TG_TABLE_NAME
AND n.nspname = TG_TABLE_SCHEMA
  );
  IF rowsecurity IS NOT NULL THEN
-- for the NEW row, determine if it would be RLS-visible if written
EXECUTE 'SELECT 1 FROM (SELECT ($1).*) x WHERE ' || rowsecurity
USING new;
GET DIAGNOSTICS rowcount = ROW_COUNT;
RAISE NOTICE 'Blah %',rowcount;
IF rowcount = 0 THEN
   RAISE insufficient_privilege USING MESSAGE = 'Row-security policy
prohibits new tuple value';
END IF;
  END IF;
  RETURN NEW;
END;
$$
LANGUAGE plpgsql;

CREATE TRIGGER _rowsecurity_check
BEFORE INSERT OR UPDATE ON rls_regress_schema.document
FOR EACH ROw EXECUTE PROCEDURE rowsecurity_check();


You'd usually use this in conjunction with another BEFORE trigger that
modifies the row being written to ensure appropriate security attributes
are set; something like:

CREATE OR REPLACE FUNCTION set_userid_on_write() RETURNS trigger AS $$
BEGIN
  NEW.dauthor := current_user;
  RETURN new;
END;
$$
LANGUAGE PLPGSQL;

CREATE TRIGGER zzza_set_current_user
BEFORE INSERT OR UPDATE ON document
FOR EACH ROW EXECUTE PROCEDURE set_userid_on_write();

... or whatever is appropriate for your security model.

-- 
 Craig Ringer   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] PostgreSQL Service on Windows does not start. ~ is not a valid Win32 application

2013-10-28 Thread Sandeep Thakkar
So, this is not an installer issue. Is this bug raised to the PostgreSQL
community? If yes, you should submit the patch there.


On Tue, Oct 29, 2013 at 6:23 AM, Naoya Anzai
anzai-na...@mxu.nes.nec.co.jpwrote:

 Hi, Asif

 Thank you for providing my patch (pg_ctl.c.patch) to Sandeep on my behalf.

  Good finding. I have attached another version of patch
 (pg_ctl.c_windows_vulnerability.patch) attached that has fewer lines of
 code changes, can you please take a look ?. Thanks.

 I think your patch is not sufficient to fix.
 Not only pg_ctl.exe but postgres.exe also have the same problem.
 Even if your patch is attached,
 A Path of postgres.exe passed to CreateRestrictedProcess is not enclosed
 in quotation.(See pgwin32_ServiceMain at pg_ctl.c)

 So, processing enclosed in quotation should do in both conditions.

 Regards,
 Naoya

 ---
 Naoya Anzai
 Engineering Department
 NEC Soft, Ltd.
 E-Mail: anzai-na...@mxu.nes.nec.co.jp
 ---


  Hi Sandeep,
 
  PFA Naoya's patch (pg_ctl.c.patch).
 
  Hi Naoya,
 
  Good finding. I have attached another version of patch
 (pg_ctl.c_windows_vulnerability.patch) attached that has fewer lines of
 code changes, can you please take a look ?. Thanks.
 
  Best Regards,
  Asif Naeem
 
 
  On Mon, Oct 28, 2013 at 4:46 PM, Sandeep Thakkar 
 sandeep.thak...@enterprisedb.com wrote:
 
 
Hi Dave
 
We register the service using pg_ctl. When I manually executed the
 following on the command prompt, I saw that the service path of the
 registered service did not have the pg_ctl.exe path in quotes. May be it
 should be handled in the pg_ctl code.
 
c:\Users\Sandeep Thakkar\Documentsc:\Program
 Files\PostgreSQL\9.3\bin\pg_ctl.e
xe register -N pg-9.3 -U NT AUTHORITY\NetworkService -D
 c:\Program Files\P
ostgreSQL\9.3\data -w
 
Naoya,  I could not find your patch here. Can you please share it
 again?
 
 
 
On Mon, Oct 28, 2013 at 2:53 PM, Dave Page dp...@pgadmin.org
 wrote:
 
 
Sandeep, can you look at this please? Thanks.
 
On Mon, Oct 28, 2013 at 8:18 AM, Asif Naeem 
 anaeem...@gmail.com wrote:
 It is related to windows unquoted service path
 vulnerability in the the
 installer that creates service path without quotes that
 make service.exe to
 look for undesirable path for executable.

 postgresql-9.3 service path :
 C:/Users/asif/Desktop/Program
 files/9.3/bin/pg_ctl.exe runservice -N postgresql-9.3
 -D
 C:/Users/asif/Desktop/Program files/9.3/data -w

 service.exe

 C:\Users\asif\Desktop\Program NAME NOT FOUND
 C:\Users\asif\Desktop\Program.exe NAME NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 ACCESS DENIED
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 ACCESS DENIED
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice NAME
 NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice.exe
 NAME NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice -N
 NAME NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice -N.exe
 NAME NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice -N
 postgresql-9.3 NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice -N
 postgresql-9.3.exe NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice -N
 postgresql-9.3 -D NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice -N
 postgresql-9.3 -D.exe NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program NAME
 INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program.exe
 NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program
 files\9.3\data NAME
 INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program
 files\9.3\data.exe
 NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program
 files\9.3\data -w
 NAME INVALID
 

Re: [HACKERS] tracking commit timestamps

2013-10-28 Thread Amit Kapila
On Wed, Oct 23, 2013 at 3:46 AM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
 Hi,

 There has been some interest in keeping track of timestamp of
 transaction commits.  This patch implements that.

Some of the use cases, I could think of are
1. Is it for usecases such that if user want to read all data of table
where transaction commit_ts = '2012-04-04 09:30:00'?
2. for replication systems, may be the middleware can use it to replay
transactions in some remote system.
3. Is there any use of this feature in logical-rep/change data extraction?

 There are some seemingly curious choices here.  First, this module can
 be disabled, and in fact it's turned off by default.  At startup, we
 verify whether it's enabled, and create the necessary SLRU segments if
 so.  And if the server is started with this disabled, we set the oldest
 value we know about to avoid trying to read the commit TS of
 transactions of which we didn't keep record.  The ability to turn this
 off is there to avoid imposing the overhead on systems that don't need
 this feature.

 Another thing of note is that we allow for some extra data alongside the
 timestamp proper.  This might be useful for a replication system that
 wants to keep track of the origin node ID of a committed transaction,
 for example.  Exactly what will we do with the bit space we have is
 unclear, so I have kept it generic and called it commit extra data.

commit extra data can be LSN of commit log record, but I think it
will also depend on how someone wants to use this feature.
To suggest for storing LSN, I had referred information at below page
which describes about similar information for each transaction.
http://technet.microsoft.com/en-us/library/cc645959.aspx


 This offers the chance for outside modules to set the commit TS of a
 transaction; there is support for WAL-logging such values.  But the core
 user of the feature (RecordTransactionCommit) doesn't use it, because
 xact.c's WAL logging itself is enough.

I have one question for the case when commits is set from
RecordTransactionCommit().

*** 1118,1123  RecordTransactionCommit(void)
--- 1119,1132 
  }

  /*
+ * We don't need to log the commit timestamp separately since the commit
+ * record logged above has all the necessary action to set the timestamp
+ * again.
+ */
+ TransactionTreeSetCommitTimestamp(xid, nchildren, children,
+  xactStopTimestamp, 0, false);
+

Here for CLOG, we are doing Xlogflush before writing to Clog page, but
Committs writes timestamp before XlogFlush().
Won't that create problem for synchronous commit as Checkpoint can
takecare of flushing Xlog for relation pages before flush of page,
but for Clog/Committs RecordTransactionCommit() should take care of doing it.


With Regards,
Amit Kapila.
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