[HACKERS] Fwd: sql/med review - problems with patching

2010-07-13 Thread Pavel Stehule
Hello

please, can you refresh patch, please?

[pa...@nemesis pgsql]$ patch -p1 < backend.patch
patching file src/backend/access/common/reloptions.c
patching file src/backend/catalog/Makefile
patching file src/backend/catalog/aclchk.c
patching file src/backend/catalog/dependency.c
patching file src/backend/catalog/heap.c
patching file src/backend/catalog/system_views.sql
patching file src/backend/commands/alter.c
patching file src/backend/commands/analyze.c
patching file src/backend/commands/comment.c
patching file src/backend/commands/copy.c
patching file src/backend/commands/discard.c
patching file src/backend/commands/explain.c
patching file src/backend/commands/foreigncmds.c
patching file src/backend/commands/lockcmds.c
patching file src/backend/commands/tablecmds.c
Hunk #6 FAILED at 1980.
Hunk #25 succeeded at 7190 (offset 3 lines).
Hunk #26 succeeded at 7795 (offset 3 lines).
Hunk #27 succeeded at 7812 (offset 3 lines).
Hunk #28 succeeded at 7843 (offset 3 lines).
1 out of 28 hunks FAILED -- saving rejects to file
src/backend/commands/tablecmds.c.rej
patching file src/backend/commands/vacuum.c
patching file src/backend/executor/Makefile
patching file src/backend/executor/execAmi.c


*** src/backend/commands/tablecmds.c
--- src/backend/commands/tablecmds.c
*** renameatt(Oid myrelid,
*** 1980,1989 
-- 1993,2003 
-><-->ereport(ERROR,
 <><--><--><-->(errcode(ERRCODE_WRONG_OBJECT_TYPE),
! <><--><--><--> errmsg("\"%s\" is not a table, view,
composite type,index or foreign table", < missing space before
index
 <><--><--><--><--><-->RelationGetRelationName(targetrelation;

is there a some special reason to divide diff to separate parts?

I can't to compile code

execAmi.c: In function ‘ExecReScan’:
execAmi.c:186: error: ‘exprCtxt’ undeclared (first use in this function)
execAmi.c:186: error: (Each undeclared identifier is reported only once
execAmi.c:186: error: for each function it appears in.)
make[3]: *** [execAmi.o] Error 1
make[3]: Leaving directory `/home/pavel/src/pgsq

because Tom commited significant changes in executor

http://archives.postgresql.org/pgsql-committers/2010-07/msg00155.php

When I looked to documentation I miss a some tutorial for foreign
tables. There are only reference. I miss some paragraph where is
cleanly and simple specified what is possible now and whot isn't
possible. Enhancing of dblink isn't documented

Why you don't use PQescapeLiteral for escaping. Isn't
escape_param_str redundant and unsecure?

In function  pgIterate(ForeignScanState *scanstate) you are iterare
via pg result. I am thinking so using a cursor and fetching multiple
rows should be preferable.

Regards

Pavel Stehule

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


[HACKERS] security label support, part.2

2010-07-13 Thread KaiGai Kohei
The attached patch is a part of efforts to support security label
on database objects.

It adds statement support to manage security label of relations.
Right now, object labeling except for relations/columns are not
supported, because the DML permission hook is the only chance to
apply access control decision of ESP module.

It has the following syntax:
  ALTER TABLE  [ALTER [COLUMN] ]
  SECURITY LABEL TO '';

I believe Robert's refactoring on COMMENT ON code also helps to
implement security label support for various kind of object classes.
However, we need to handle relabeling on the tables particularly
because of table's inheritances, unlike any other object classes.
So, I considered we can make progress these works in progress, then
we can integrated them later.

Example:
  postgres=# CREATE TABLE t (a int, b text);
  CREATE TABLE
  postgres=# ALTER TABLE t SECURITY LABEL TO 
'system_u:object_r:sepgsql_table_t:s0';
  ALTER TABLE
  postgres=# ALTER TABLE t ALTER a SECURITY LABEL TO 
'system_u:object_r:sepgsql_table_t:s0';
  ALTER TABLE
  postgres=# ALTER TABLE t ALTER b SECURITY LABEL TO 
'system_u:object_r:sepgsql_table_t:s0:c1';
  ALTER TABLE

  [kai...@saba ~]$ runcon -l s0 psql postgres
  psql (9.1devel)
  Type "help" for help.

  postgres=# set client_min_messages = log;
  SET
  postgres=# SELECT * FROM t;
  LOG:  SELinux: denied { select } 
scontext=unconfined_u:unconfined_r:unconfined_t:s0 
tcontext=system_u:object_r:sepgsql_table_t:s0:c1 tclass=db_column name=t.b
  ERROR:  SELinux: security policy violation
  postgres=# SELECT a FROM t;
   a
  ---
  (0 rows)

Thanks,
-- 
KaiGai Kohei 


pgsql-v9.1-security-label-2.v1.patch
Description: application/octect-stream

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


[HACKERS] security label support, part.1

2010-07-13 Thread KaiGai Kohei
The attached patch is a part of efforts to support security label
on database objects.

It adds a new system catalog named pg_seclabel, that has similar
structure with pg_description.

  #define SecLabelRelationId3037

  CATALOG(pg_seclabel,3037) BKI_WITHOUT_OIDS
  {
  Oid reloid; /* OID of table containing the object */
  Oid objoid; /* OID of the object itself */
  int4subid;  /* column number, or 0 if not used */
  textlabel;  /* security label of the object */
  } FormData_pg_seclabel;

This patch provides only its definition and basic internal APIs to
get/set/delete security labels, so, we also need to apply the part.2
patch to support ALTER statement to manage security labels, in addition
to the part.1.

Right now, modular sepgsql will perform on the patch.
It can be checked out using:
  % svn checkout http://sepgsql.googlecode.com/svn/trunk/sepgsql

Thanks,
-- 
KaiGai Kohei 


pgsql-v9.1-security-label-1.v1.patch
Description: application/octect-stream

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


Re: [HACKERS] multiple -f support

2010-07-13 Thread Mark Wong
Hi all,

I took a stab at changing this up a little bit.  I pushed the logic
that David introduced down into process_file().  In doing so I changed
up the declaration of process_file() to accept an additional parameter
specifying how many files are being passed to the function.  Doing it
this way also makes use of the logic for the current single
transaction flag without turning it into dead code.

I also added a check to return EXIT_FAILURE if any error was thrown
from any of the sql files used, thus stopping execution of any further
file.  Gabrielle tells me the error echoed in this event is not clear
so there is still a little more work that needs to be done.

Regards,
Mark


psql-f-v2.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] testing plpython3u on 9.0beta3

2010-07-13 Thread Tom Lane
Chris  writes:
> So if I have to explicitly set the python interpretor, how would you ever
> have a plpython2u and plpython3u function in the same DB (regardless of the
> fact that they can't run in the same session)?

You'd have to build the two plpython.so's in separate compile operations.

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] testing plpython3u on 9.0beta3

2010-07-13 Thread Chris
So if I have to explicitly set the python interpretor, how would you ever
have a plpython2u and plpython3u function in the same DB (regardless of the
fact that they can't run in the same session)?  The manual implies you could
have them both installed since it says that there's a per session
limitation.  After specifying the python3 interpretor, I can indeed now run
plpython3u, but I (rather obviously) can't createlang plpython2u now.
I would think that the plpython section of the manual may want some
reference to that fact that that compile flag needs to be set.

Additionally, "What's New In Python
3.0 "
for the beta 3 docs on
http://www.postgresql.org/docs/9.0/static/plpython-python23.html is dead.


On Tue, Jul 13, 2010 at 4:02 PM, Peter Eisentraut  wrote:

> On tis, 2010-07-13 at 15:38 -0500, Chris wrote:
> > I'm testing beta 3 and ran across a PL/Python3u bug again.
> > This time it won't let me install it at all.
> > Kubuntu 10.04
> >
> > ./configure --with-pgport=5433 --with-python --with-ossp-uuid
> > --with-libxml
> > --with-libxslt --with-perl --prefix=/usr/local/pgsqlb3
>
> You probably need something like
>
> ./configure ... PYTHON=/usr/bin/python3 ...
>
> here.  Otherwise it picks up /usr/bin/python, which is probably Python
> 2.
>
>


-- 
Chris Spotts
rfu...@gmail.com


Re: [HACKERS] Date of 9.0 beta4

2010-07-13 Thread Bruce Momjian
Marc G. Fournier wrote:
> On Tue, 13 Jul 2010, Bruce Momjian wrote:
> 
> > If we are going to hit mid-August for Postgres 9.0 final, we will
> > probably need a final beta in the next two weeks, or go right to 9.0 RC
> > in early August.  Should we schedule 9.0 beta4 now in case we need it?
> 
> Go with 2 weeks-ish ... if there is no reason to do a beta then, we don't 
> do it ...

OK, agreed.  That would mean we wrap beta4 on July 22, with release on
July 26.  Comments?

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

  + None of us is going to be here forever. +

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


Re: [HACKERS] Date of 9.0 beta4

2010-07-13 Thread Marc G. Fournier

On Tue, 13 Jul 2010, Bruce Momjian wrote:


If we are going to hit mid-August for Postgres 9.0 final, we will
probably need a final beta in the next two weeks, or go right to 9.0 RC
in early August.  Should we schedule 9.0 beta4 now in case we need it?


Go with 2 weeks-ish ... if there is no reason to do a beta then, we don't 
do it ...



Marc G. FournierHub.Org Hosting Solutions S.A.
scra...@hub.org http://www.hub.org

Yahoo:yscrappySkype: hub.orgICQ:7615664MSN:scra...@hub.org

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


[HACKERS] Date of 9.0 beta4

2010-07-13 Thread Bruce Momjian
If we are going to hit mid-August for Postgres 9.0 final, we will
probably need a final beta in the next two weeks, or go right to 9.0 RC
in early August.  Should we schedule 9.0 beta4 now in case we need it?

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

  + None of us is going to be here forever. +

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


Re: [HACKERS] testing plpython3u on 9.0beta3

2010-07-13 Thread Peter Eisentraut
On tis, 2010-07-13 at 15:38 -0500, Chris wrote:
> I'm testing beta 3 and ran across a PL/Python3u bug again.
> This time it won't let me install it at all.
> Kubuntu 10.04
> 
> ./configure --with-pgport=5433 --with-python --with-ossp-uuid
> --with-libxml
> --with-libxslt --with-perl --prefix=/usr/local/pgsqlb3

You probably need something like

./configure ... PYTHON=/usr/bin/python3 ...

here.  Otherwise it picks up /usr/bin/python, which is probably Python
2.


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


[HACKERS] testing plpython3u on 9.0beta3

2010-07-13 Thread Chris
I'm testing beta 3 and ran across a PL/Python3u bug again.
This time it won't let me install it at all.
Kubuntu 10.04

./configure --with-pgport=5433 --with-python --with-ossp-uuid --with-libxml
--with-libxslt --with-perl --prefix=/usr/local/pgsqlb3
make
make check
sudo make install
All work fine.

However
postg...@cspotts-laptop:/usr/local/pgsql/lib$ createlang plpython3u
ERROR:  could not access file "$libdir/plpython3": No such file or directory
STATEMENT:  CREATE LANGUAGE "plpython3u";

createlang: language installation failed: ERROR:  could not access file
"$libdir/plpython3": No such file or directory




Thoughts? Ideas? Am I doing something wrong? If I can provide more detail
about something specific, let me know.

-- 
Chris Spotts
rfu...@gmail.com


Re: [HACKERS] bg worker: overview

2010-07-13 Thread Kevin Grittner
Markus Wanner  wrote:
 
> (I don't dare to add these patches to the commit fest, as this 
> refactoring doesn't have any immediate benefit for Postgres
> itself, at the moment.)
 
You could submit them as Work In Progress patches
 
-Kevin

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


Re: [HACKERS] ERROR: argument to pg_get_expr() must come from system catalogs

2010-07-13 Thread Tom Lane
Dave Page  writes:
> We had a report of the above error from a pgAdmin user testing
> 1.12.0b3 with PG 9.0b3. The (highly simplified) query below works fine
> as a superuser:

> SELECT pg_get_expr(proargdefaults, 'pg_catalog.pg_class'::regclass)
>   FROM pg_proc pr
>   LEFT OUTER JOIN pg_description des ON des.objoid=pr.oid

> Run as a regular user though, we get the error. If I remove the join,
> it works fine as the normal user. This is in a database owned by the
> regular user.

> Am I missing something obvious, or is there a bug here?

Yeah, it's a bug.  The code Heikki added in parse_expr.c isn't allowing
for the possibility of join alias Vars.  (There's another problem with
it too, which is you can trivially circumvent the check, just by
creating an operator based on pg_get_expr ...)

I wasn't terribly happy with that approach to begin with.  I think we
need to rethink.

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: argument to pg_get_expr() must come from system catalogs

2010-07-13 Thread Thom Brown
On 13 July 2010 17:01, Thom Brown  wrote:
> On 13 July 2010 17:00, Dave Page  wrote:
>> On Tue, Jul 13, 2010 at 4:56 PM, Thom Brown  wrote:
>>
>>> I works if you use pr.proargdefaults so not unresolvable.  Maybe it's
>>> because it can't tell where the column's coming from at that point?
>>
>> Hmm, so it does. It still seems like a bug though - why should it be
>> able to resolve the column based on whether you're a superuser or not,
>> just because it's not qualified?
>>
>
> Yeah see what you mean.  Seems like a bug.
>
> Thom
>

And yes, as a result PgAdmin becomes very upset when not a superuser.
:(  Kind of a show-stopper.

Thom

-- 
Sent 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: argument to pg_get_expr() must come from system catalogs

2010-07-13 Thread Thom Brown
On 13 July 2010 17:00, Dave Page  wrote:
> On Tue, Jul 13, 2010 at 4:56 PM, Thom Brown  wrote:
>
>> I works if you use pr.proargdefaults so not unresolvable.  Maybe it's
>> because it can't tell where the column's coming from at that point?
>
> Hmm, so it does. It still seems like a bug though - why should it be
> able to resolve the column based on whether you're a superuser or not,
> just because it's not qualified?
>

Yeah see what you mean.  Seems like a bug.

Thom

-- 
Sent 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: argument to pg_get_expr() must come from system catalogs

2010-07-13 Thread Dave Page
On Tue, Jul 13, 2010 at 4:56 PM, Thom Brown  wrote:

> I works if you use pr.proargdefaults so not unresolvable.  Maybe it's
> because it can't tell where the column's coming from at that point?

Hmm, so it does. It still seems like a bug though - why should it be
able to resolve the column based on whether you're a superuser or not,
just because it's not qualified?


-- 
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise Postgres 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] ERROR: argument to pg_get_expr() must come from system catalogs

2010-07-13 Thread Thom Brown
On 13 July 2010 16:50, Dave Page  wrote:
> On Tue, Jul 13, 2010 at 4:48 PM, Thom Brown  wrote:
>> On 13 July 2010 16:44, Thom Brown  wrote:
>>> On 13 July 2010 16:31, Dave Page  wrote:
 We had a report of the above error from a pgAdmin user testing
 1.12.0b3 with PG 9.0b3. The (highly simplified) query below works fine
 as a superuser:

 SELECT pg_get_expr(proargdefaults, 'pg_catalog.pg_class'::regclass)
  FROM pg_proc pr
  LEFT OUTER JOIN pg_description des ON des.objoid=pr.oid

 Run as a regular user though, we get the error. If I remove the join,
 it works fine as the normal user. This is in a database owned by the
 regular user.

 Am I missing something obvious, or is there a bug here? pg_get_expr is
 used pretty extensively in pgAdmin, so we're obviously keen to ensure
 it works :-)

>>>
>>> I tested this on both beta2 and beta3 and can confirm that it works on
>>> beta2 but produces the following error in beta3:
>>>
>>> ** Error **
>>>
>>> ERROR: argument to pg_get_expr() must come from system catalogs
>>> SQL state: 42501
>>>
>>> Thom
>>>
>>
>> And here's why:
>> http://archives.postgresql.org/pgsql-committers/2010-06/msg00259.php
>>
>> "stringToNode() and deparse_expression_pretty() crash on invalid input,
>> but we have nevertheless exposed them to users via pg_get_expr(). It would
>> be too much maintenance effort to rigorously check the input, so put a hack
>> in place instead to restrict pg_get_expr() so that the argument must come
>> from one of the system catalog columns known to contain valid expressions."
>
> Yeah, I recall that - but... the argument *is* coming from the system
> catalogs, and why is the error only thrown for a non-superuser, when
> the query includes the join?
>

I works if you use pr.proargdefaults so not unresolvable.  Maybe it's
because it can't tell where the column's coming from at that point?

Thom

-- 
Sent 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: argument to pg_get_expr() must come from system catalogs

2010-07-13 Thread Dave Page
On Tue, Jul 13, 2010 at 4:48 PM, Thom Brown  wrote:
> On 13 July 2010 16:44, Thom Brown  wrote:
>> On 13 July 2010 16:31, Dave Page  wrote:
>>> We had a report of the above error from a pgAdmin user testing
>>> 1.12.0b3 with PG 9.0b3. The (highly simplified) query below works fine
>>> as a superuser:
>>>
>>> SELECT pg_get_expr(proargdefaults, 'pg_catalog.pg_class'::regclass)
>>>  FROM pg_proc pr
>>>  LEFT OUTER JOIN pg_description des ON des.objoid=pr.oid
>>>
>>> Run as a regular user though, we get the error. If I remove the join,
>>> it works fine as the normal user. This is in a database owned by the
>>> regular user.
>>>
>>> Am I missing something obvious, or is there a bug here? pg_get_expr is
>>> used pretty extensively in pgAdmin, so we're obviously keen to ensure
>>> it works :-)
>>>
>>
>> I tested this on both beta2 and beta3 and can confirm that it works on
>> beta2 but produces the following error in beta3:
>>
>> ** Error **
>>
>> ERROR: argument to pg_get_expr() must come from system catalogs
>> SQL state: 42501
>>
>> Thom
>>
>
> And here's why:
> http://archives.postgresql.org/pgsql-committers/2010-06/msg00259.php
>
> "stringToNode() and deparse_expression_pretty() crash on invalid input,
> but we have nevertheless exposed them to users via pg_get_expr(). It would
> be too much maintenance effort to rigorously check the input, so put a hack
> in place instead to restrict pg_get_expr() so that the argument must come
> from one of the system catalog columns known to contain valid expressions."

Yeah, I recall that - but... the argument *is* coming from the system
catalogs, and why is the error only thrown for a non-superuser, when
the query includes the join?

-- 
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise Postgres 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] ERROR: argument to pg_get_expr() must come from system catalogs

2010-07-13 Thread Thom Brown
On 13 July 2010 16:44, Thom Brown  wrote:
> On 13 July 2010 16:31, Dave Page  wrote:
>> We had a report of the above error from a pgAdmin user testing
>> 1.12.0b3 with PG 9.0b3. The (highly simplified) query below works fine
>> as a superuser:
>>
>> SELECT pg_get_expr(proargdefaults, 'pg_catalog.pg_class'::regclass)
>>  FROM pg_proc pr
>>  LEFT OUTER JOIN pg_description des ON des.objoid=pr.oid
>>
>> Run as a regular user though, we get the error. If I remove the join,
>> it works fine as the normal user. This is in a database owned by the
>> regular user.
>>
>> Am I missing something obvious, or is there a bug here? pg_get_expr is
>> used pretty extensively in pgAdmin, so we're obviously keen to ensure
>> it works :-)
>>
>
> I tested this on both beta2 and beta3 and can confirm that it works on
> beta2 but produces the following error in beta3:
>
> ** Error **
>
> ERROR: argument to pg_get_expr() must come from system catalogs
> SQL state: 42501
>
> Thom
>

And here's why:
http://archives.postgresql.org/pgsql-committers/2010-06/msg00259.php

"stringToNode() and deparse_expression_pretty() crash on invalid input,
but we have nevertheless exposed them to users via pg_get_expr(). It would
be too much maintenance effort to rigorously check the input, so put a hack
in place instead to restrict pg_get_expr() so that the argument must come
from one of the system catalog columns known to contain valid expressions."

Thom

-- 
Sent 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: argument to pg_get_expr() must come from system catalogs

2010-07-13 Thread Thom Brown
On 13 July 2010 16:31, Dave Page  wrote:
> We had a report of the above error from a pgAdmin user testing
> 1.12.0b3 with PG 9.0b3. The (highly simplified) query below works fine
> as a superuser:
>
> SELECT pg_get_expr(proargdefaults, 'pg_catalog.pg_class'::regclass)
>  FROM pg_proc pr
>  LEFT OUTER JOIN pg_description des ON des.objoid=pr.oid
>
> Run as a regular user though, we get the error. If I remove the join,
> it works fine as the normal user. This is in a database owned by the
> regular user.
>
> Am I missing something obvious, or is there a bug here? pg_get_expr is
> used pretty extensively in pgAdmin, so we're obviously keen to ensure
> it works :-)
>

I tested this on both beta2 and beta3 and can confirm that it works on
beta2 but produces the following error in beta3:

** Error **

ERROR: argument to pg_get_expr() must come from system catalogs
SQL state: 42501

Thom

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


[HACKERS] ERROR: argument to pg_get_expr() must come from system catalogs

2010-07-13 Thread Dave Page
We had a report of the above error from a pgAdmin user testing
1.12.0b3 with PG 9.0b3. The (highly simplified) query below works fine
as a superuser:

SELECT pg_get_expr(proargdefaults, 'pg_catalog.pg_class'::regclass)
  FROM pg_proc pr
  LEFT OUTER JOIN pg_description des ON des.objoid=pr.oid

Run as a regular user though, we get the error. If I remove the join,
it works fine as the normal user. This is in a database owned by the
regular user.

Am I missing something obvious, or is there a bug here? pg_get_expr is
used pretty extensively in pgAdmin, so we're obviously keen to ensure
it works :-)

-- 
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise Postgres 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] explain.c: why trace PlanState and Plan trees separately?

2010-07-13 Thread Yeb Havinga

Tom Lane wrote:

Yeb Havinga  writes:
  

Tom Lane wrote:


The reason I'm on about this at the moment is that I think I see how to
get ruleutils to print PARAM_EXEC Params as the referenced expression
rather than $N ...
  
Wouldn't this obfuscate the plan more than printing subplan arguments at 
the call site?



It would if subplans could have more than one call site, but they can't.

I do intend to force qualification of Vars that are printed as a result
of param expansion;
  
Will the new referenced expression printing also be used when printing 
subplans?


If yes, I do not have to submit the latest version of a patch I made for 
subplan argument printing (discussed earlier in this thread 
http://archives.postgresql.org/pgsql-hackers/2010-02/msg01602.php)


regards,
Yeb Havinga


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


Re: [HACKERS] multibyte charater set in levenshtein function

2010-07-13 Thread Alexander Korotkov
Hi!

* levenshtein_internal() and levenshtein_less_equal_internal() are very
>  similar. Can you merge the code? We can always use less_equal_internal()
>  if the overhead is ignorable. Did you compare them?
>
With big value of max_d overhead is significant. Here is example on
american-english dictionary from Openoffice.

test=# select sum(levenshtein('qweqweqweqweqwe',word)) from words;
   sum
-
 1386456
(1 row)

Time: 195,083 ms
test=# select sum(levenshtein_less_equal('qweqweqweqweqwe',word,100)) from
words;
   sum
-
 1386456
(1 row)

Time: 317,821 ms


> * There are many "if (!multibyte_encoding)" in levenshtein_internal().
>  How about split the function into two funcs for single-byte chars and
>  multi-byte chars? (ex. levenshtein_internal_mb() ) Or, we can always
>  use multi-byte version if the overhead is small.
>
The overhead of multi-byte version was about 4 times slower. But I have
rewritten my CHAR_CMP macro with inline function. And now it's only about
1.5 times slower.

In database with muti-byte encoding:
test=# select * from words where levenshtein('qweqweqwe',word)<=5;
  id   |   word
---+--
 69053 | peewee
 69781 | pewee
 81279 | sequence
 88421 | sweetie
(4 rows)

Time: 136,742 ms

In database with single-byte encoding:
test2=# select * from words where levenshtein('qweqweqwe',word)<=5;
  id   |   word
---+--
 69053 | peewee
 69781 | pewee
 81279 | sequence
 88421 | sweetie
(4 rows)

Time: 88,471 ms

Anyway I think that overhead is not ignorable. That's why I have splited
levenshtein_internal into levenshtein_internal and levenshtein_internal_mb,
and levenshtein_less_equal_internal into levenshtein_less_equal_internal and
levenshtein_less_equal_internal_mb.


> * I prefer a struct rather than an array.  "4 * m" and "3 * m" look like
> magic
>  numbers for me. Could you name the entries with definition of a struct?
>/*
> * For multibyte encoding we'll also store array of lengths of
> * characters and array with character offsets in first string
> * in order to avoid great number of pg_mblen calls.
> */
>prev = (int *) palloc(4 * m * sizeof(int));
>
I this line of code the memory is allocated for 4 arrays: prev, curr,
offsets, char_lens. So I have joined offsets and char_lens into struct. But
I can't join prev and curr because of this trick:
temp = curr;
curr = prev;
prev = temp;

* There are some compiler warnings. Avoid them if possible.
> fuzzystrmatch.c: In function ‘levenshtein_less_equal_internal’:
> fuzzystrmatch.c:428: warning: ‘char_lens’ may be used uninitialized in
> this function
> fuzzystrmatch.c:428: warning: ‘offsets’ may be used uninitialized in
> this function
> fuzzystrmatch.c:430: warning: ‘curr_right’ may be used uninitialized
> in this function
> fuzzystrmatch.c: In function ‘levenshtein_internal’:
> fuzzystrmatch.c:222: warning: ‘char_lens’ may be used uninitialized in
> this function
>
Fixed.

* Coding style: Use "if (m == 0)" instead of "if (!m)" when the type
> of 'm' is an integer.
>
Fixed.


> * Need to fix the caution in docs.
> http://developer.postgresql.org/pgdocs/postgres/fuzzystrmatch.html
> | Caution: At present, fuzzystrmatch does not work well with
> | multi-byte encodings (such as UTF-8).
> but now levenshtein supports multi-byte encoding!  We should
> mention which function supports mbchars not to confuse users.
>
I've updated this notification. Also I've added documentation for
levenshtein_less_equal function.

* (Not an issue for the patch, but...)
>  Could you rewrite PG_GETARG_TEXT_P, VARDATA, and VARSIZE to
>  PG_GETARG_TEXT_PP, VARDATA_ANY, and VARSIZE_ANY_EXHDR?
>  Unpacking versions make the core a bit faster.
>
 Fixed.

With best regards,
Alexander Korotkov.


fuzzystrmatch-0.4.diff.gz
Description: GNU Zip compressed data

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


[HACKERS] bg worker: patch 6 of 6 - ooo messages

2010-07-13 Thread Markus Wanner
Finally, this patch adds the capability to cache out-of-order messages 
for workers within the coordinator process. Unlike the cache of jobs, 
which basically are messages as well, these messages don't trigger a 
job, but might provide additional information or data payload to a 
worker that's already processing a job.


In case of Postgres-R, the very first change set received starts an 
applicator job, while every subsequent change set as well as the final 
commit decision are such ooo-messages, which are forwarded to the same 
worker.


A parallel querying feature might want to use similar ooo messages for 
data payload, forth and back. However, simpler jobs like vacuum don't 
necessarily need this.
*** src/backend/postmaster/coordinator.c	c12c70fed2f6025831ef4ba9555af0debe063003
--- src/backend/postmaster/coordinator.c	7f25538532e2633e8fed8232b33588eb35f4f122
*** typedef struct cached_job {
*** 121,126 
--- 121,132 
  	IMessage *cj_msg;
  } cached_job;
  
+ typedef struct cached_msg {
+ 	Dlelem cm_links;
+ 	IMessage *cm_msg;
+ 	BackendId cm_backend_id;
+ } cached_msg;
+ 
  CoordinatorShmemStruct *CoordinatorShmem;
  
  /*
*** static void process_cached_jobs(co_datab
*** 156,161 
--- 162,172 
  		 BackendId backend_id);
  static void process_cached_jobs(co_database *codb);
  
+ static void process_ooo_msgs_for(co_database *codb, BackendId backend_id);
+ static void add_ooo_msg(IMessage *msg, co_database *codb,
+ 		BackendId backend_id);
+ 
+ 
  static void manage_workers(bool can_launch);
  
  static void do_start_worker(Oid dboid);
*** init_co_database(co_database *codb)
*** 271,276 
--- 282,290 
  	codb->codb_num_cached_jobs = 0;
  	DLInitList(&codb->codb_cached_jobs);
  
+ 	codb->codb_num_ooo_msgs = 0;
+ 	DLInitList(&codb->codb_ooo_msgs);
+ 
  	codb->codb_num_connected_workers = 0;
  }
  
*** cache_job(IMessage *msg, co_database *co
*** 291,296 
--- 305,328 
  	codb->codb_num_cached_jobs++;
  }
  
+ void
+ add_ooo_msg(IMessage *msg, co_database *codb, BackendId backend_id)
+ {
+ 	cached_msg *cm;
+ 
+ #ifdef COORDINATOR_DEBUG
+ 	elog(DEBUG5, "Coordinator: storing out-of-order message of type %s for database %d",
+ 		 decode_imessage_type(msg->type), codb->codb_dboid);
+ #endif
+ 
+ 	cm = palloc(sizeof(cached_msg));
+ 	DLInitElem(&cm->cm_links, cm);
+ 	cm->cm_msg = msg;
+ 	cm->cm_backend_id = backend_id;
+ 	DLAddTail(&codb->codb_ooo_msgs, &cm->cm_links);
+ 	codb->codb_num_ooo_msgs++;
+ }
+ 
  /*
   * get_idle_worker
   *
*** dispatch_job(IMessage *msg, co_database 
*** 361,375 
  void
  dispatch_job(IMessage *msg, co_database *codb)
  {
! WorkerInfo worker;
  
! 	if (codb->codb_num_idle_workers > 0)
  {
! worker = get_idle_worker(codb); 
! 		forward_job(msg, codb, worker->wi_backend_id);
  }
  	else
! 		cache_job(msg, codb);
  }
  
  /*
--- 393,436 
  void
  dispatch_job(IMessage *msg, co_database *codb)
  {
! 	bool can_deliver;
! 	BackendId target = InvalidBackendId;
  
! 	can_deliver = can_deliver_cached_job(codb, msg, &target);
! 
! 	if (can_deliver && target == InvalidBackendId)
! 		can_deliver = (codb->codb_num_idle_workers > 0);
! 
! 	if (can_deliver)
! 	{
! 		if (target == InvalidBackendId)
! 			target = get_idle_worker(codb)->wi_backend_id;
! 		forward_job(msg, codb, target);
! 	}
! 	else
! 		cache_job(msg, codb);
! }
! 
! void
! dispatch_ooo_msg(IMessage *msg, co_database *codb)
! {
! 	bool can_deliver;
! 	BackendId target = InvalidBackendId;
! 
! 	can_deliver = can_deliver_cached_job(codb, msg, &target);
! 
! 	if (can_deliver && target == InvalidBackendId)
! 		can_deliver = (codb->codb_num_idle_workers > 0);
! 
! 	if (can_deliver)
  {
! 		if (target == InvalidBackendId)
! 			target = get_idle_worker(codb)->wi_backend_id;
! 		forward_job(msg, codb, target);
! 		process_ooo_msgs_for(codb, target);
  }
  	else
! 		add_ooo_msg(msg, codb, InvalidBackendId);
  }
  
  /*
*** process_cached_jobs(co_database *codb)
*** 409,414 
--- 470,482 
  
  			forward_job(job->cj_msg, codb, target);
  			pfree(job);
+ 
+ 			/*
+ 			 * Trigger subsequent ooo messages, as the delivery of a job
+ 			 * might change the delivery status of further out-of-order
+ 			 * messages 
+ 			 */
+ 			process_ooo_msgs_for(codb, target);
  			
  			job = (cached_job*) DLGetHead(&codb->codb_cached_jobs);
  		}
*** process_cached_jobs(co_database *codb)
*** 417,422 
--- 485,569 
  	}
  }
  
+ void
+ process_ooo_msgs_for(co_database *codb, BackendId backend_id)
+ {
+ 	BackendId target;
+ 	cached_msg *cm;
+ 	bool can_deliver;
+ 
+ #ifdef COORDINATOR_DEBUG
+ 	elog(DEBUG5, "Coordinator: out-of-order messages: %d",
+ 		 codb->codb_num_ooo_msgs);
+ #endif
+ 
+ 	cm = (cached_msg*) DLGetHead(&codb->codb_ooo_msgs);
+ 	while (cm != NULL)
+ 	{
+ 		target = InvalidBackendId;
+ 		can_deliver = false;
+ 
+ 		if (cm->cm_backend_id == InvalidBackendId ||
+ 			cm->cm_backend

[HACKERS] bg worker: patch 3 of 6 - sockets

2010-07-13 Thread Markus Wanner
This patch adds the capability for the coordinator to listen on sockets 
while waiting for imessages to arrive. Before the coordinator just slept 
until a signal arrives, notifying the coordinator about an internal message.


Major caveat here: I'm using pselect(), which might still not be 
portable enough. The work-around for platforms on which a signal doesn't 
interrupt select has been removed as well. I can't think of any way to 
support platforms as broken as that.
*** src/backend/postmaster/autovacuum.c	50ad2e93982867e91a47e8ca7af5f4be8b975d8f
--- src/backend/postmaster/autovacuum.c	ca02f9e08a1dbe34ff8049c0d51cc76594fb16a0
*** static void launcher_determine_sleep(boo
*** 264,270 
  static void do_start_worker(Oid dboid);
  static Oid autovacuum_select_database(void);
  static void launcher_determine_sleep(bool can_launch, bool recursing,
! 	 struct timeval *nap);
  static void autovacuum_update_timing(Oid dbid, TimestampTz now);
  static List *get_database_list(void);
  static void rebuild_database_list(Oid newdb);
--- 264,270 
  static void do_start_worker(Oid dboid);
  static Oid autovacuum_select_database(void);
  static void launcher_determine_sleep(bool can_launch, bool recursing,
! 		 struct timespec *nap);
  static void autovacuum_update_timing(Oid dbid, TimestampTz now);
  static List *get_database_list(void);
  static void rebuild_database_list(Oid newdb);
*** AutoVacLauncherMain(int argc, char *argv
*** 748,754 
  	for (;;)
  	{
  		TimestampTz		current_time;
! 		struct timeval	nap;
  
  		/*
  		 * Emergency bailout if postmaster has died.  This is to avoid the
--- 748,758 
  	for (;;)
  	{
  		TimestampTz		current_time;
! 		struct timespec	nap;
! 		sigset_t		sigmask, oldmask;
! 		fd_set			socks;
! 		intmax_sock_id;
! 		bool			socket_ready;
  
  		/*
  		 * Emergency bailout if postmaster has died.  This is to avoid the
*** AutoVacLauncherMain(int argc, char *argv
*** 760,802 
  		can_launch = (AutoVacuumShmem->av_freeWorkers != NULL);
  		launcher_determine_sleep(can_launch, false, &nap);
  
  		/* Allow sinval catchup interrupts while sleeping */
  		EnableCatchupInterrupt();
  
  		/*
! 		 * Sleep for a while according to schedule.
  		 *
! 		 * On some platforms, signals won't interrupt the sleep.  To ensure we
! 		 * respond reasonably promptly when someone signals us, break down the
! 		 * sleep into 1-second increments, and check for interrupts after each
! 		 * nap.
  		 */
! 		while (nap.tv_sec > 0 || nap.tv_usec > 0)
  		{
- 			uint32		sleeptime;
  
! 			if (nap.tv_sec > 0)
  			{
! sleeptime = 100;
! nap.tv_sec--;
  			}
  			else
! 			{
! sleeptime = nap.tv_usec;
! nap.tv_usec = 0;
! 			}
! 			pg_usleep(sleeptime);
  
  			/*
  			 * Emergency bailout if postmaster has died.  This is to avoid the
  			 * necessity for manual cleanup of all postmaster children.
  			 */
  			if (!PostmasterIsAlive(true))
  proc_exit(1);
- 
- 			msg = IMessageCheck();
- 			if (got_SIGTERM || got_SIGHUP || got_SIGUSR2 || msg)
- break;
  		}
  
  		DisableCatchupInterrupt();
--- 764,834 
  		can_launch = (AutoVacuumShmem->av_freeWorkers != NULL);
  		launcher_determine_sleep(can_launch, false, &nap);
  
+ 		/* Initialize variables for listening on sockets */ 
+ 		FD_ZERO(&socks);
+ 		max_sock_id = 0;
+ 		socket_ready = false;
+ 
+ #ifdef COORDINATOR_DEBUG
+ 		elog(DEBUG1, "Coordinator: listening...");
+ #endif
+ 
  		/* Allow sinval catchup interrupts while sleeping */
  		EnableCatchupInterrupt();
  
  		/*
! 		 * Sleep for a while according to schedule - and possibly interrupted
! 		 * by messages from one of the sockets or by internal messages from
! 		 * background workers or normal backends.
  		 *
! 		 * Using pselect here prevents the possible loss of a singnal in
! 		 * between the last check for imessages and following select call.
! 		 * However, it requires a newish platform that supports pselect.
! 		 *
! 		 * On some platforms, signals won't interrupt select. Postgres used
! 		 * to split the nap time into one second intervals to ensure to react
! 		 * reasonably promptly for autovacuum purposes. However, for
! 		 * Postgres-R this is not tolerable, so that mechanism has been
! 		 * removed.
! 		 *
! 		 * FIXME: to support these platforms or others that don't implement
! 		 *pselect properly, another work-around like for example the
! 		 *self-pipe trick needs to be implemented. On Windows, we
! 		 *could implement pselect based on the current port's select
! 		 *method.
  		 */
! 
! 		/* FIXME: indentation */
  		{
  
! 			sigemptyset(&sigmask);
! 			sigaddset(&sigmask, SIGINT);
! 			sigaddset(&sigmask, SIGHUP);
! 			sigaddset(&sigmask, SIGUSR2);
! 			sigprocmask(SIG_BLOCK, &sigmask, &oldmask);
! 
! 			sigemptyset(&sigmask);
! 
! 			if (pselect(max_sock_id + 1, &socks, NULL, NULL, &nap,
! 		&sigmask) < 0)
  			{
! if (errno != EINTR)
! {
!

Re: [HACKERS] log files and permissions

2010-07-13 Thread Tom Lane
Itagaki Takahiro  writes:
> ...
> We should also check the value not to be something like 0699.
> How about checking it with (file_mode & ~0666) != 0 ?
> ...
> I want show_log_file_mode to print the setting value in octal format.

It seems like a whole lot of lily-gilding is going on here.  Just make
it work like unix_socket_permissions already does.  That's been there
for years and nobody has complained 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


[HACKERS] bg worker: patch 1 of 6 - permanent process

2010-07-13 Thread Markus Wanner
This patch turns the existing autovacuum launcher into an always running 
process, partly called the coordinator. If autovacuum is disabled, the 
coordinator process still gets started and keeps around, but it doesn't 
dispatch vacuum jobs. The coordinator process now uses imessages to 
communicate with background (autovacuum) workers and to trigger a vacuum 
job. So please apply the imessages patches [1] before any of the bg 
worker ones.


It also adds two new controlling GUCs: min_spare_background_workers and 
max_spare_background_workers. The autovacuum_max_workers still serves as 
a limit for the total amount of background/autovacuum workers. (It is 
going to be renamed in step 4).


Interaction with the postmaster has changed a bit. If autovacuum is 
disabled, the coordinator isn't started with 
PMSIGNAL_START_AUTOVAC_LAUNCHER anymore, instead there is an 
IMSGT_FORCE_VACUUM that any backend might want to send to the 
coordinator to prevent data loss due to XID wrap around (see changes in 
access/transam/varsup.c). The SIGUSR2 from postmaster to the coordinator 
doesn't need to be multiplexed anymore, but is only sent to inform about 
fork failures.


A note on the dependency on imessages: for just autovacuum, this message 
passing infrastructure isn't absolutely necessary and could be removed. 
However, for Postgres-R it turned out to be really helpful and I think 
chances are good another user of this background worker infrastructure 
would also want to transfer data of varying size to and from these workers.


Just as in the current version of Postgres, the background worker 
terminates immediately after having performed a vacuum job.



Open issue: if the postmaster fails to fork a new background worker, the 
coordinator still waits a whole second after receiving the SIGUSR2 
notification signal from the postmaster. That might have been fine with 
just autovacuum, but for other jobs, namely changeset application in 
Postgres-R, that's not feasible.



[1] dynshmem and imessages patch
http://archives.postgresql.org/message-id/ab0cd52a64e788f4ecb4515d1e6e4...@localhost

*** src/backend/access/transam/varsup.c	58aea89cb33fe0ee2cb3e10c0a714cd0057a8bdb
--- src/backend/access/transam/varsup.c	058cb2f268e956e37724f1d05d98946d1cc56ab4
*** GetNewTransactionId(bool isSubXact)
*** 46,51 
--- 46,52 
  GetNewTransactionId(bool isSubXact)
  {
  	TransactionId xid;
+ 	IMessage *msg;
  
  	/*
  	 * During bootstrap initialization, we return the special bootstrap
*** GetNewTransactionId(bool isSubXact)
*** 94,105 
  		LWLockRelease(XidGenLock);
  
  		/*
! 		 * To avoid swamping the postmaster with signals, we issue the autovac
  		 * request only once per 64K transaction starts.  This still gives
  		 * plenty of chances before we get into real trouble.
  		 */
  		if (IsUnderPostmaster && (xid % 65536) == 0)
! 			SendPostmasterSignal(PMSIGNAL_START_AUTOVAC_LAUNCHER);
  
  		if (IsUnderPostmaster &&
  			TransactionIdFollowsOrEquals(xid, xidStopLimit))
--- 95,109 
  		LWLockRelease(XidGenLock);
  
  		/*
! 		 * To avoid swamping the coordinator with signals, we issue the autovac
  		 * request only once per 64K transaction starts.  This still gives
  		 * plenty of chances before we get into real trouble.
  		 */
  		if (IsUnderPostmaster && (xid % 65536) == 0)
! 		{
! 			msg = IMessageCreate(IMSGT_FORCE_VACUUM, 0);
! 			IMessageActivate(msg, GetAutovacuumLauncherId());
! 		}
  
  		if (IsUnderPostmaster &&
  			TransactionIdFollowsOrEquals(xid, xidStopLimit))
*** SetTransactionIdLimit(TransactionId olde
*** 258,263 
--- 262,268 
  	TransactionId xidStopLimit;
  	TransactionId xidWrapLimit;
  	TransactionId curXid;
+ 	IMessage *msg;
  
  	Assert(TransactionIdIsNormal(oldest_datfrozenxid));
  
*** SetTransactionIdLimit(TransactionId olde
*** 341,347 
  	 */
  	if (TransactionIdFollowsOrEquals(curXid, xidVacLimit) &&
  		IsUnderPostmaster && !InRecovery)
! 		SendPostmasterSignal(PMSIGNAL_START_AUTOVAC_LAUNCHER);
  
  	/* Give an immediate warning if past the wrap warn point */
  	if (TransactionIdFollowsOrEquals(curXid, xidWarnLimit) && !InRecovery)
--- 346,355 
  	 */
  	if (TransactionIdFollowsOrEquals(curXid, xidVacLimit) &&
  		IsUnderPostmaster && !InRecovery)
! 	{
! 		msg = IMessageCreate(IMSGT_FORCE_VACUUM, 0);
! 		IMessageActivate(msg, GetAutovacuumLauncherId());
! 	}
  
  	/* Give an immediate warning if past the wrap warn point */
  	if (TransactionIdFollowsOrEquals(curXid, xidWarnLimit) && !InRecovery)

*** src/backend/postmaster/postmaster.c	13da26f5fb9a09c47a8dc3b30f0bbfdb1da0420c
--- src/backend/postmaster/postmaster.c	e6a455be46eee967cafa76cef125cedef4ba986f
*** bool		redirection_done = false;	/* stder
*** 294,301 
  
  bool		redirection_done = false;	/* stderr redirected for syslogger? */
  
- /* received START_AUTOVAC_LAUNCHER signal */
- stati

[HACKERS] bg worker: overview

2010-07-13 Thread Markus Wanner

Hi,

I've been working on modularizing Postgres-R to ease review and maybe 
allow code reuse. As threatened at the Cluster Meeting in Tokyo and 
again at CHAR(10), I'm now presenting more results of that effort: the 
background workers infrastructure module.


Postgres-R so far used custom backends to apply transactions from remote 
nodes. These were controlled by an additional coordinator process, which 
acted as a job dispatcher and obviously didn't have a client connection. 
There were obvious similarities between that and the existing autovacuum 
component, with its launcher that controls multiple worker processes.


I've combined these two components into a single, general purpose 
background worker infrastructure component, which is now capable to 
serve autovacuum as well as Postgres-R. And it might be of use for other 
purposes as well, most prominently parallel query processing. Basically 
anything that needs a backend connected to a database to do any kind of 
background processing, possibly parallelized.


Overall, this module represents quite a large portion of the Postgres-R 
patch. 15% by lines inserted (2912 vs 19332) and as much as 95% by lines 
deleted (1422 vs 1482).


With this further modularization, I hope to increase understandability 
and wish to encourage more hackers to have a look at (parts of) the 
Postgres-R source code. Of course, I highly appreciate reviews and 
discussions. And it would be very nice to see this module reused. Please 
don't hesitate to ask questions, if you need help.


(I don't dare to add these patches to the commit fest, as this 
refactoring doesn't have any immediate benefit for Postgres itself, at 
the moment.)


Regards

Markus Wanner

P.S.: git adicts, everything's up here:
http://git.postgres-r.org/?p=bgworker

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


[HACKERS] Last call for patches for the July CommitFest

2010-07-13 Thread Kevin Grittner
The CF starts the day after tomorrow.  If you've been working on a
patch and you want feedback, you must post it to the -hackers list
and create an entry in the CF application before the start of the
CommitFest, or you will likely need to wait until the September CF
for a review.
 
See this Wiki page for more information about submitting a patch:
 
http://wiki.postgresql.org/wiki/Submitting_a_Patch
 
The link to whatever CF is open to new submissions is always:
 
http://commitfest.postgresql.org/action/commitfest_view/open
 
During the review phase, the link for the CF in progress is:
 
http://commitfest.postgresql.org/action/commitfest_view/inprogress
 
-Kevin

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


Re: [HACKERS] explain.c: why trace PlanState and Plan trees separately?

2010-07-13 Thread Tom Lane
Yeb Havinga  writes:
> Tom Lane wrote:
>> The reason I'm on about this at the moment is that I think I see how to
>> get ruleutils to print PARAM_EXEC Params as the referenced expression
>> rather than $N ...

> Wouldn't this obfuscate the plan more than printing subplan arguments at 
> the call site?

It would if subplans could have more than one call site, but they can't.

I do intend to force qualification of Vars that are printed as a result
of param expansion; for example consider a standard nestloop-with-
inner-indexscan plan:

NestLoop
Seq Scan on a
Index Scan on b
Index Cond: x = a.y

If y weren't qualified to show that it's not a variable of b, this could
be confusing.  But as long as we do that, it pretty much matches our
historical behavior.  Note that CVS HEAD is printing this case as

NestLoop
Seq Scan on a
Index Scan on b
Index Cond: x = $0

which is definitely not very helpful.

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] dblink regression failure in HEAD

2010-07-13 Thread Andrew Dunstan



Stephen Frost wrote:

* Tom Lane (t...@sss.pgh.pa.us) wrote:
  

A few experiments later: I can reproduce the failure shown on pangolin
exactly if I revert the latest changes in sql/dblink.sql and
expected/dblink.out, while keeping dblink.c up to date.  So I guessed
wrong on which file was out of sync, but I say confidently that this
is a repository sync problem.



I'll check with Scott on this, sorry, but it might be an issue with the
machine and not the repository.  That's the box that he's been doing the
performance-farm work on and it's entirely possible soemthing got
screwed up there.  Of course, it could also be a repository problem,
I'll find out.


  


I just ran my validation script against the community git repo and it is 
indeed broken in just this way :-(


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] log files and permissions

2010-07-13 Thread Martin Pihlak
Itagaki Takahiro wrote:
> I think the patch is almost ready for committer except the following
> three issues:
> 
> 2010/7/13 Fujii Masao :
>>> + if (!*value || *endptr || file_mode < 0 || file_mode > 0777)
>> The sticky bit cannot be set via log_file_mode. Is this intentional? 

Yes -- I don't think there is a use case for sticky or setuid bits on log
files, even allowing execute is questionable.

> We should also check the value not to be something like 0699.
> How about checking it with (file_mode & ~0666) != 0 ?

Aha, that would ensure that the execute bit is not specified. Works for me.
The 0699 and other invalid octal values are caught by strtol()

>>> +#ifndef WIN32
>>> + chmod(filename, Log_file_mode);
>>> +#endif
>> Don't we need to check the return value of chmod()?
> 
> I prefer umask() rather than chmod() here.
> 

Converted to umask()

>>> +const char *show_log_file_mode(void);
>> You forgot to write the show_log_file_mode()? I was not able to find that
>> in the patch.
> 
> I want show_log_file_mode to print the setting value in octal format.
> 

I've now (re)added the show_log_file_mode(). It used to be there, but then
at some point I decided to display the value "as-is".

While going through it, I moved the _setmode() call for win32 to logfile_open().

regards,
Martin

*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***
*** 2789,2794  local0.*/var/log/postgresql
--- 2789,2815 

   
  
+  
+   log_file_mode (integer)
+   
+log_file_mode configuration parameter
+   
+   
+
+ On Unix systems this parameter sets the permissions for log files
+ when logging_collector is enabled. On Microsoft
+ Windows the file mode will be ignored. The value is an octal number
+ consisting of 3 digits signifying the permissions for the user, group
+ and others. Specifying execute permissions for log files results in
+ an error.
+
+
+ This parameter can only be set in the postgresql.conf
+ file or on the server command line.
+
+   
+  
+ 
   
log_rotation_age (integer)

*** a/src/backend/commands/variable.c
--- b/src/backend/commands/variable.c
***
*** 914,916  show_role(void)
--- 914,956 
  
  	return endptr + 1;
  }
+ 
+ 
+ /*
+  * LOG_FILE_MODE
+  */
+ 
+ extern int Log_file_mode;		/* in guc.c */
+ 
+ /*
+  * assign_log_file_mode: GUC assign_hook for log_file_mode
+  */
+ const char *
+ assign_log_file_mode(const char *value, bool doit, GucSource source)
+ {
+ 	char *endptr;
+ 	long file_mode = strtol(value, &endptr, 8);
+ 
+ 	if (!*value || *endptr || file_mode < 0 || (file_mode & ~0666) != 0)
+ 	{
+ 		ereport(GUC_complaint_elevel(source),
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+  errmsg("invalid value for parameter \"log_file_mode\""),
+  errhint("value must be a 3 digit octal number, with the execute bit not set")));
+ 		return NULL;
+ 	}
+ 
+ 	if (doit)
+ 		Log_file_mode = (int) file_mode;
+ 
+ 	return value;
+ }
+ 
+ const char *
+ show_log_file_mode(void)
+ {
+ 	static char buf[5];
+ 
+ 	snprintf(buf, sizeof(buf), "%04o", Log_file_mode);
+ 	return buf;
+ }
*** a/src/backend/postmaster/syslogger.c
--- b/src/backend/postmaster/syslogger.c
***
*** 73,78  int			Log_RotationSize = 10 * 1024;
--- 73,79 
  char	   *Log_directory = NULL;
  char	   *Log_filename = NULL;
  bool		Log_truncate_on_rotation = false;
+ int			Log_file_mode = 0600;
  
  /*
   * Globally visible state (used by elog.c)
***
*** 135,140  static void syslogger_parseArgs(int argc, char *argv[]);
--- 136,142 
  static void process_pipe_input(char *logbuffer, int *bytes_in_logbuffer);
  static void flush_pipe_input(char *logbuffer, int *bytes_in_logbuffer);
  static void open_csvlogfile(void);
+ static FILE *logfile_open(const char *filename, const char *mode, bool am_rotating);
  
  #ifdef WIN32
  static unsigned int __stdcall pipeThread(void *arg);
***
*** 516,530  SysLogger_Start(void)
  	 */
  	filename = logfile_getname(time(NULL), NULL);
  
! 	syslogFile = fopen(filename, "a");
! 
! 	if (!syslogFile)
! 		ereport(FATAL,
! (errcode_for_file_access(),
!  (errmsg("could not create log file \"%s\": %m",
! 		 filename;
! 
! 	setvbuf(syslogFile, NULL, LBF_MODE, 0);
  
  	pfree(filename);
  
--- 518,524 
  	 */
  	filename = logfile_getname(time(NULL), NULL);
  
! 	syslogFile = logfile_open(filename, "a", false);
  
  	pfree(filename);
  
***
*** 1004,1027  open_csvlogfile(void)
  
  	filename = logfile_getname(time(NULL), ".csv");
  
! 	fh = fopen(filename, "a");
  
! 	if (!fh)
! 		ereport(FATAL,
! (errcode_for_file_access(),
!  (errmsg("could not create log file \"%s\": %m",
! 		 filename;
  
! 	setvbuf(fh, NULL, LBF_MODE, 0);
  
! #ifdef WIN32
! 	_setmode(_fileno(fh), _O_TEXT);		/* use C

Re: [HACKERS] dblink regression failure in HEAD

2010-07-13 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote:
> A few experiments later: I can reproduce the failure shown on pangolin
> exactly if I revert the latest changes in sql/dblink.sql and
> expected/dblink.out, while keeping dblink.c up to date.  So I guessed
> wrong on which file was out of sync, but I say confidently that this
> is a repository sync problem.

I'll check with Scott on this, sorry, but it might be an issue with the
machine and not the repository.  That's the box that he's been doing the
performance-farm work on and it's entirely possible soemthing got
screwed up there.  Of course, it could also be a repository problem,
I'll find out.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] explain.c: why trace PlanState and Plan trees separately?

2010-07-13 Thread Yeb Havinga

Tom Lane wrote:

The reason I'm on about this at the moment is that I think I see how to
get ruleutils to print PARAM_EXEC Params as the referenced expression
rather than $N ...
Wouldn't this obfuscate the plan more than printing subplan arguments at 
the call site?


regards,
Yeb Havinga


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


Re: [HACKERS] multibyte-character aware support for function "downcase_truncate_identifier()"

2010-07-13 Thread Rajanikant Chirmade
On Wed, Jul 7, 2010 at 7:37 PM, Tom Lane  wrote:

> Rajanikant Chirmade  writes:
> > Every identifier is downcase & truncated by function
> > "downcase_truncate_identifier()"
> > before using it.
>
> > But since the function "downcase_truncate_identifier()" is not
> > multibyte-charecter aware,
> > it is not able to downcase some of special charecters in identifier like
> > "my_SchemÄ".
>
>




> IIRC this is intentional.  Please consult the archives for previous
> discussions.
>
>regards, tom lane
>



I got one discussion thread on same issue. But it stopped without any
conclusion.

http://archives.postgresql.org/pgsql-bugs/2006-09/msg00128.php

Thanks & Regards,
Rajanikant Chirmade.