Re: [HACKERS] Tracking latest timeline in standby mode

2011-12-07 Thread senthilnathan
We are using 9.1.,

We have a set up like a master and 2 standby servers. M -- > S1,S2 . Both
standby S1 and S2 share the same archive. Master will have an Virtual IP.
Both stand by servers will be replicated using this virtual ip.

Assume the master fails,using our heart beat mechanism Virtual IP bound to
S1(if S1 is ahead or equal to S2 XLOG).,

Is it required to copy the time line history file that is generated at time
of S1 promotion as master to the archive directory of S2 for replication to
work (i.e S1(new master) to S2.)

Without doing this history file copy from S1 to S2, S2 keeps throwing the
following error message.,

2011-12-07 17:29:46 IST::@:[18879]:FATAL:  could not receive data from WAL
stream: server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.

cp: cannot stat `../archive/00010005': No such file or
directory
2011-12-07 17:29:49 IST::@:[18875]:LOG:  record with zero length at
0/5D8FFC0
cp: cannot stat `../archive/00010005': No such file or
directory
cp: cannot stat `../archive/0002.history': No such file or directory
2011-12-07 17:29:49 IST::@:[20362]:FATAL:  timeline 2 of the primary does
not match recovery target timeline 1
cp: cannot stat `../archive/00010005': No such file or
directory
cp: cannot stat `../archive/00010005': No such file or
directory
cp: cannot stat `../archive/0002.history': No such file or directory
2011-12-07 17:29:54 IST::@:[20367]:FATAL:  timeline 2 of the primary does
not match recovery target timeline 1
cp: cannot stat `../archive/00010005': No such file or
directory
cp: cannot stat `../archive/00010005': No such file or
directory
cp: cannot stat `../archive/0002.history': No such file or directory


 

 

--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/Tracking-latest-timeline-in-standby-mode-tp3238829p5057733.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

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


[HACKERS] Lots of FSM-related fragility in transaction commit

2011-12-07 Thread Tom Lane
Joseph Shraibman reported some rather unpleasant behavior that
seems to be due to trying to drop a table whose FSM file has got
no permissions:
http://archives.postgresql.org/pgsql-general/2011-12/msg00246.php

One question is how the file got that way, and whether Postgres did
anything wrong to cause it to not have permissions.  But what I'm on the
warpath about now is the abysmally bad error response.  What ought to
happen if we fail to delete a file during a DROP TABLE is a WARNING,
no more.  Not a PANIC that recurs during crash recovery.

I tried to reproduce this, and found a different but equally bad
behavior:

regression=# create table foo as select * from tenk1;
SELECT 1
regression=# delete from foo where unique1%10 = 0;
DELETE 1000
regression=# vacuum foo;
VACUUM
regression=# select 'foo'::regclass::oid;
  oid  
---
 52860
(1 row)

[ go and "chmod 000 52860_fsm" ]

regression=# drop table foo;
WARNING:  AbortTransaction while in COMMIT state
ERROR:  could not open file "base/27666/52860_fsm": Permission denied
PANIC:  cannot abort transaction 7413, it was already committed
ERROR:  could not open file "base/27666/52860_fsm": Permission denied
PANIC:  cannot abort transaction 7413, it was already committed
The connection to the server was lost. Attempting reset: Failed.

I thought that removing the unreadable file would let me restart,
but after "rm 52860_fsm" and trying again to start the server,
there's a different problem:

LOG:  database system was interrupted while in recovery at 2011-12-08 00:56:11 
EST
HINT:  This probably means that some data is corrupted and you will have to use 
the last backup for recovery.
LOG:  database system was not properly shut down; automatic recovery in progress
LOG:  consistent recovery state reached at 0/5D71BA8
LOG:  redo starts at 0/5100050
WARNING:  page 18 of relation base/27666/52860 is uninitialized
CONTEXT:  xlog redo visible: rel 1663/27666/52860; blk 18
PANIC:  WAL contains references to invalid pages
CONTEXT:  xlog redo visible: rel 1663/27666/52860; blk 18
LOG:  startup process (PID 14507) was terminated by signal 6
LOG:  aborting startup due to startup process failure

Note that this isn't even the same symptom Shraibman hit, since
apparently he was failing on replaying the commit record.  How is it
that the main table file managed to have uninitialized pages?
I suspect that "redo visible" WAL processing is breaking one or more of
the fundamental WAL rules, perhaps by not generating a full-page image
when it needs to.

So this is really a whole lot worse than our behavior was in pre-FSM
days, and it needs to get fixed.

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] documentation issue - extensions

2011-12-07 Thread Pavel Stehule
2011/12/8 Robert Haas :
> On Wed, Dec 7, 2011 at 4:37 PM, Pavel Stehule  wrote:
>> 2011/12/7 Kevin Grittner :
>>> Pavel Stehule  wrote:
>>>
 I can upgrade - it's not problem - just it is surprise for me -
 because documentation is not related to mayor version. Maybe this
 issue can be documented somewhere
>>>
>>> Don't the release notes, mentioning that the bug is fixed in 9.1.2,
>>> cover that?
>>
>> It is little bit difficult detect this issue as bug
>
> It's not a bug.  It's an not-forward-compatible behavior change in a
> minor release.

so this can be mentioned in documentation elsewhere than release notes
- the best is near related examples - it is different behave than
postgresql's user expect.

I understand to reason now - and it has sense - but it is surprising -
not all has newer postgres - this version is mostly in development
environments than production in this moment.

Regards

Pavel


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

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


Re: [HACKERS] pg_dump --exclude-table-data

2011-12-07 Thread Andrew Dunstan



On 11/04/2011 10:21 AM, Robert Haas wrote:



A slightly updated patch is attached, the main change being that I removed
use of a short option and only support the long name option. "-D" didn't
seem sufficiently mnemonic to me. I'll add this to the November commitfest,
but I'd like to get it committed ASAP as it will simplify setting up the
-pre and -post data patches.

Instead of:

do NOT dump data for the named table(s)

How about:

dump only schema for the named table(s)



I have no great objection to the wording change.



I'm also a bit concerned about the relationship between this and the
existing -s option.  It seems odd that you use --schema-only to get
the behavior database-wide, and --exclude-table-data to get it for
just one table.  Is there some way we can make that a bit more
consistent?




It's consistent, and was designed to be, with the --exclude-table 
option. I'm not sure what you want it to look like instead. But TBH I'm 
more interested in getting the functionality than in how it's spelled.


cheers

andrew



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


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

2011-12-07 Thread Robert Haas
On Sat, Dec 3, 2011 at 3:19 AM, Kohei KaiGai  wrote:
> I rebased my patch set. New functions in pg_proc.h prevented to apply
> previous revision cleanly. Here is no functional changes.

I was thinking that my version of this (attached to an email from
earlier today) might be about ready to commit.  But while I was
trolling through the archives on this problem trying to figure out who
to credit, I found an old complaint of Tom's that we never fixed, and
which represents a security exposure for this patch:

rhaas=# create table foo (a integer);
CREATE TABLE
rhaas=# insert into foo select generate_series(1,10);
INSERT 0 10
rhaas=# insert into foo values (1);
INSERT 0 1
rhaas=# analyze foo;
ANALYZE
rhaas=# create view safe_foo with (security_barrier) as select * from
foo where a > 5;
CREATE VIEW
rhaas=# grant select on safe_foo to bob;
GRANT

Secure in the knowledge that Bob will only be able to see rows where a
is 6 or higher, we go to bed.  But Bob finds a way to outsmart us:

rhaas=> create or replace function leak(integer,integer) returns
boolean as $$begin raise notice 'leak % %', $1, $2; return false;
end$$ language plpgsql;
CREATE FUNCTION
rhaas=> create operator !! (procedure = leak, leftarg = integer,
rightarg = integer, restrict = eqsel);
CREATE OPERATOR
rhaas=> explain select * from safe_foo where a !! 0;
NOTICE:  leak 1 0
 QUERY PLAN
-
 Subquery Scan on safe_foo  (cost=0.00..2.70 rows=1 width=4)
   Filter: (safe_foo.a !! 0)
   ->  Seq Scan on foo  (cost=0.00..1.14 rows=6 width=4)
 Filter: (a > 5)
(4 rows)

OOPS.  The *executor* has been persuaded not to apply the
possibly-nefarious operator !! to the data until after applying the
security-critical qual "a > 5".  But the *planner* has no such
compunctions, and has cheerfully leaked the most common value in the
table, which the user wasn't supposed to see.  I guess it's hopeless
to suppose that we're going to completely conceal the list of MCVs
from the user, since it might change the plan - and even if
ProcessUtility_hook or somesuch is used to disable EXPLAIN, the user
can still try to ferret out the MCVs via a timing attack.  That having
been said, the above behavior doesn't sit well with me: letting the
user probe for MCVs via a timing attack or a plan change is one thing;
printing them out on request is a little bit too convenient for my
taste.  :-(

-- 
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] documentation issue - extensions

2011-12-07 Thread Robert Haas
On Wed, Dec 7, 2011 at 4:37 PM, Pavel Stehule  wrote:
> 2011/12/7 Kevin Grittner :
>> Pavel Stehule  wrote:
>>
>>> I can upgrade - it's not problem - just it is surprise for me -
>>> because documentation is not related to mayor version. Maybe this
>>> issue can be documented somewhere
>>
>> Don't the release notes, mentioning that the bug is fixed in 9.1.2,
>> cover that?
>
> It is little bit difficult detect this issue as bug

It's not a bug.  It's an not-forward-compatible behavior change in a
minor release.

-- 
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] [PATCH] Caching for stable expressions with constant arguments v3

2011-12-07 Thread Tom Lane
Marti Raudsepp  writes:
> Let me rephrase that as a question: Does it seem worthwhile to add a
> new argument to ExecInitExpr to handle those two cases?

Possibly.  Another way would be to keep its API as-is and introduce a
different function name for the other behavior.  I would think that
we'd always know for any given caller which behavior we need, so a
flag as such isn't notationally helpful.

> Does relying
> on the PlanState argument being NULL seem like a bad idea for any
> reason?

Yes, that seemed like a pretty horrid idea when I read your description,
but I hadn't got round to looking at just how awful it might be.

regards, tom lane

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


Re: [HACKERS] [PATCH] Caching for stable expressions with constant arguments v3

2011-12-07 Thread Marti Raudsepp
On Wed, Dec 7, 2011 at 00:29, Marti Raudsepp  wrote:
> ExecInitExpr enables the cache when its 'PlanState *parent' attribute
> isn't NULL
[...]
> On the other hand, a few places lose caching support this way since
> they don't go through the planner:
> * Column defaults in a COPY FROM operation. Common use case is
> 'timestamp default now()'
> This might be a significant loss in some data-loading scenarios.
> * ALTER TABLE t ALTER col TYPE x USING some_expr(); No big loss here.

Let me rephrase that as a question: Does it seem worthwhile to add a
new argument to ExecInitExpr to handle those two cases? Does relying
on the PlanState argument being NULL seem like a bad idea for any
reason?

PS: I forgot to mention that 2 test cases covering the two above query
types are deliberately left failing in the v4-wip patch.

Regards,
Marti

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


Re: [HACKERS] ecmascript 5 DATESTYLE

2011-12-07 Thread Pavel Stehule
2011/12/7 ben hockey :
>
> On Dec 7, 2011, at 3:56 PM, Peter Eisentraut wrote:
>
> On tis, 2011-12-06 at 15:15 -0500, Tom Lane wrote:
>
> TBH, I think that inventing a new datestyle setting "ECMA" would be a
>
> more appropriate investment of effort.
>
>
> So we'd have a setting called "ECMA" that's really ISO, and a setting
> called "ISO" that's really SQL, and a setting called "SQL" that's really
> Postgres, and a setting called "Postgres" that's also Postgres but
> different.
>
>
> ...and a setting called "XSD" that's also ISO.
>
> for now i'm backing away from the ECMA option - what i was thinking of would
> be exactly the same as "XSD" except rather than a timezone of '+00:00' it
> would be a 'Z'.  from some quick searching, it seems that XSD should be
> capable of understanding 'Z' rather than '+00:00' so if i was going to do
> anything i'd work towards making that change to 'XSD'.
>
> however, as it turns out, the constraint i have that is requiring me to use
> 'Z' is not actually from ECMAScript 5 but from json-schema
> (http://tools.ietf.org/html/draft-zyp-json-schema-03#section-5.23).  XSD is
> fully compatible with ECMAScript 5 date time string format
> (http://es5.github.com/#x15.9.1.15)  so i'm going to sit on this again for a
> little while and think some more.  maybe try to convince json-schema to
> relax their definition of date-time format.
>
> i'll be back when i have a clear picture of what i think makes the most
> sense.

please do it - we still would to have JSON support, so date style can
be processed together

Regards

Pavel

>
> thanks,
>
> ben...

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


Re: [HACKERS] documentation issue - extensions

2011-12-07 Thread Pavel Stehule
2011/12/7 Kevin Grittner :
> Pavel Stehule  wrote:
>
>> I can upgrade - it's not problem - just it is surprise for me -
>> because documentation is not related to mayor version. Maybe this
>> issue can be documented somewhere
>
> Don't the release notes, mentioning that the bug is fixed in 9.1.2,
> cover that?

It is little bit difficult detect this issue as bug

Regards

Pavel
>
> -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] ecmascript 5 DATESTYLE

2011-12-07 Thread ben hockey


On Dec 7, 2011, at 3:56 PM, Peter Eisentraut wrote:


On tis, 2011-12-06 at 15:15 -0500, Tom Lane wrote:

TBH, I think that inventing a new datestyle setting "ECMA" would be a
more appropriate investment of effort.


So we'd have a setting called "ECMA" that's really ISO, and a setting
called "ISO" that's really SQL, and a setting called "SQL" that's  
really

Postgres, and a setting called "Postgres" that's also Postgres but
different.



...and a setting called "XSD" that's also ISO.

for now i'm backing away from the ECMA option - what i was thinking of  
would be exactly the same as "XSD" except rather than a timezone of  
'+00:00' it would be a 'Z'.  from some quick searching, it seems that  
XSD should be capable of understanding 'Z' rather than '+00:00' so if  
i was going to do anything i'd work towards making that change to 'XSD'.


however, as it turns out, the constraint i have that is requiring me  
to use 'Z' is not actually from ECMAScript 5 but from json-schema (http://tools.ietf.org/html/draft-zyp-json-schema-03#section-5.23 
).  XSD is fully compatible with ECMAScript 5 date time string format (http://es5.github.com/#x15.9.1.15 
)  so i'm going to sit on this again for a little while and think some  
more.  maybe try to convince json-schema to relax their definition of  
date-time format.


i'll be back when i have a clear picture of what i think makes the  
most sense.


thanks,

ben...

Re: [HACKERS] ecmascript 5 DATESTYLE

2011-12-07 Thread Peter Eisentraut
On tis, 2011-12-06 at 15:15 -0500, Tom Lane wrote:
> TBH, I think that inventing a new datestyle setting "ECMA" would be a
> more appropriate investment of effort.

So we'd have a setting called "ECMA" that's really ISO, and a setting
called "ISO" that's really SQL, and a setting called "SQL" that's really
Postgres, and a setting called "Postgres" that's also Postgres but
different.



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


Re: [HACKERS] documentation issue - extensions

2011-12-07 Thread Kevin Grittner
Pavel Stehule  wrote:
 
> I can upgrade - it's not problem - just it is surprise for me -
> because documentation is not related to mayor version. Maybe this
> issue can be documented somewhere
 
Don't the release notes, mentioning that the bug is fixed in 9.1.2,
cover that?
 
-Kevin

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


Re: [HACKERS] documentation issue - extensions

2011-12-07 Thread Pavel Stehule
2011/12/7 Tom Lane :
> Pavel Stehule  writes:
>> 2011/12/7 Tom Lane :
>>> Pavel Stehule  writes:
 I have a problem with line that contains backslash statement
>
>>> Are you testing in an up-to-date server?  We added the ability to handle
>>> that post-9.1.0.
>
>> it was tested on 9.1.1
>
> [ checks release notes... ]  Well, we added it in 9.1.2, so there you are.

I can upgrade - it's not problem - just it is surprise for me -
because documentation is not related to mayor version. Maybe this
issue can be documented somewhere

Regards

Pavel
>
>                        regards, tom lane

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


Re: [HACKERS] documentation issue - extensions

2011-12-07 Thread Tom Lane
Pavel Stehule  writes:
> 2011/12/7 Tom Lane :
>> Pavel Stehule  writes:
>>> I have a problem with line that contains backslash statement

>> Are you testing in an up-to-date server?  We added the ability to handle
>> that post-9.1.0.

> it was tested on 9.1.1

[ checks release notes... ]  Well, we added it in 9.1.2, so there you are.

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] documentation issue - extensions

2011-12-07 Thread Pavel Stehule
2011/12/7 Tom Lane :
> Pavel Stehule  writes:
>> I am trying to create simple extension according to doc
>> http://www.postgresql.org/docs/9.1/interactive/extend-extensions.html
>
>> I have a problem with line that contains backslash statement
>
>> -- complain if script is sourced in psql, rather than via CREATE
>> EXTENSION
>> \echo Use "CREATE EXTENSION pair" to load this file. \quit
>
>> this content causes a error
>
>> postgres=# create extension gdlib;
>> ERROR:  syntax error at or near "\"
>
> Are you testing in an up-to-date server?  We added the ability to handle
> that post-9.1.0.
>

it was tested on 9.1.1
>                        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] const correctness

2011-12-07 Thread Peter Eisentraut
On ons, 2011-11-09 at 21:15 +, Thomas Munro wrote:
> I've attached a new patch, which simply adds the keyword 'const' in
> lots of places, no new functions etc.  This version generates no
> warnings under -Wcast-qual (now that I've read Peter E's thread and
> been inspired to fix up some places that previously cast away const)
> for all code under backend/nodes.  To achieve that I had to stray
> outside backend/nodes and change get_leftop and get_rightop (from
> clauses.h).

Patch committed.


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


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

2011-12-07 Thread Robert Haas
On Wed, Dec 7, 2011 at 1:45 PM, Tom Lane  wrote:
>> One other possibility that comes to mind is that, instead of adding
>> "bool security_view" to the RTE, we could instead add a new RTEKind,
>> something like RTE_SECURITY_VIEW.  That would mean going through and
>> finding all the places that refer to RTE_SUBQUERY and adjusting them
>> to handle RTE_SECURITY_VIEW in either the same way or differently as
>> may be appropriate.  The possible advantage of this approach is that
>> it doesn't bloat the RTE structure (and stored rules that use it) with
>> an additional attribute that (I think) will always be false - because
>> security_barrier can only be set on a subquery RTE after rewriting has
>> happened, and stored rules are haven't been rewritten yet.  It might
>> also force people to think a bit more carefully about how security
>> views should be handled during future code changes, which could also
>> be viewed as a plus.
>
> Hmm.  The question is whether the places where we need to care about
> this would naturally be looking at RTEKind anyway.  If they are, or many
> are, then I think this might be a good idea.  However if a lot of the
> action is elsewhere then I don't know if we get much leverage from the
> new RTEKind.  I haven't read the patch lately so can't opine on that.

*reads through the code*

It looks to me like most places that look at RTE_SUBQUERY really have
no reason to care about this. So probably it's just as well to have a
separate flag for it.

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

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


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

2011-12-07 Thread Tom Lane
Robert Haas  writes:
> Having looked at this more, I'm starting to believe KaiGai has this
> part right after all.

Yeah, you have a point.  The rewriter is intentionally trying to make an
expanded view look just the same as an in-line SELECT-in-FROM, and we
need it to be easier to distinguish them.

> One other possibility that comes to mind is that, instead of adding
> "bool security_view" to the RTE, we could instead add a new RTEKind,
> something like RTE_SECURITY_VIEW.  That would mean going through and
> finding all the places that refer to RTE_SUBQUERY and adjusting them
> to handle RTE_SECURITY_VIEW in either the same way or differently as
> may be appropriate.  The possible advantage of this approach is that
> it doesn't bloat the RTE structure (and stored rules that use it) with
> an additional attribute that (I think) will always be false - because
> security_barrier can only be set on a subquery RTE after rewriting has
> happened, and stored rules are haven't been rewritten yet.  It might
> also force people to think a bit more carefully about how security
> views should be handled during future code changes, which could also
> be viewed as a plus.

Hmm.  The question is whether the places where we need to care about
this would naturally be looking at RTEKind anyway.  If they are, or many
are, then I think this might be a good idea.  However if a lot of the
action is elsewhere then I don't know if we get much leverage from the
new RTEKind.  I haven't read the patch lately so can't opine on that.

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] [PATCH] pg_test_fsync: Delete temporary file when aborted by a signal

2011-12-07 Thread Marti Raudsepp
Hi list,

I found a 'pg_test_fsync.out' file in my $PGDATA, which was probably
left around because I aborted pg_test_fsync with ^C back when setting
up the server.

Here's a patch to delete that file via a signal handler for
SIGINT/SIGTERM/SIGHUP.

Not tested on Windows, but should work according to MSDN documentation.

Regards,
Marti
From 5531c177bf108b840563e5aecbf3321fe7efbf87 Mon Sep 17 00:00:00 2001
From: Marti Raudsepp 
Date: Wed, 7 Dec 2011 18:40:58 +0200
Subject: [PATCH] pg_test_fsync: Delete temporary file when aborted by a
 signal

---
 contrib/pg_test_fsync/pg_test_fsync.c |   20 
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/contrib/pg_test_fsync/pg_test_fsync.c b/contrib/pg_test_fsync/pg_test_fsync.c
index 3791f5a..5fe6228 100644
--- a/contrib/pg_test_fsync/pg_test_fsync.c
+++ b/contrib/pg_test_fsync/pg_test_fsync.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "getopt_long.h"
 #include "access/xlogdefs.h"
@@ -44,6 +45,7 @@ static void test_sync(int writes_per_op);
 static void test_open_syncs(void);
 static void test_open_sync(const char *msg, int writes_size);
 static void test_file_descriptor_sync(void);
+static void signal_cleanup(int sig);
 
 #ifdef HAVE_FSYNC_WRITETHROUGH
 static int	pg_fsync_writethrough(int fd);
@@ -59,6 +61,14 @@ main(int argc, char *argv[])
 
 	handle_args(argc, argv);
 
+	/* Prevent leaving behind the test file */
+	signal(SIGINT, signal_cleanup);
+	signal(SIGTERM, signal_cleanup);
+#ifdef SIGHUP
+	/* Not defined on win32 */
+	signal(SIGHUP, signal_cleanup);
+#endif
+
 	prepare_buf();
 
 	test_open();
@@ -490,6 +500,16 @@ test_non_sync(void)
 	print_elapse(start_t, stop_t);
 }
 
+static void
+signal_cleanup(int signum)
+{
+	/* Delete the file if it exists. Ignore errors */
+	unlink(filename);
+	/* Finish incomplete line on stdout */
+	puts("");
+	exit(signum);
+}
+
 #ifdef HAVE_FSYNC_WRITETHROUGH
 
 static int
-- 
1.7.8


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


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

2011-12-07 Thread Robert Haas
On Mon, Sep 26, 2011 at 3:59 PM, Tom Lane  wrote:
> Kohei KaiGai  writes:
>> Sorry, are you saying the current (in other words, rte->security_barrier
>> stores the state of reloption) approach is not a good idea?
>
> Yes.  I think the same as Robert: the way to handle this is to store it
> in RelOptInfo for the duration of planning, and pull it from the
> catalogs during planner startup (cf plancat.c).

Having looked at this more, I'm starting to believe KaiGai has this
part right after all.  The trouble is that the rewriter does this:

/*
 * Now, plug the view query in as a subselect, replacing the relation's
 * original RTE.
 */
rte = rt_fetch(rt_index, parsetree->rtable);
rte->rtekind = RTE_SUBQUERY;
rte->relid = InvalidOid;
rte->subquery = rule_action;
rte->inh = false;   /* must not be set for a subquery */

In other words, by the time the planner comes along and tries to
decide whether or not it should flatten this subquery, the view has
already been rewritten into a subquery - and that subquery is in most
respects indistinguishable from a subquery that the user wrote
directly.  There is one difference: the permission check that would
have been done against the view gets attached to the OLD entry in the
subquery's range table.  It would probably be possible to make this
work by having the code paths that need to know whether or not a given
subquery originated from a security-barrier-enabled view do that same
trick: peek down into the OLD entry in the subquery rangetable,
extract the view OID from there, and go check its reloptions.  But
that seems awfully complicated and error-prone, hence my feeling that
just flagging the subquery explicitly is probably a better approach.

One other possibility that comes to mind is that, instead of adding
"bool security_view" to the RTE, we could instead add a new RTEKind,
something like RTE_SECURITY_VIEW.  That would mean going through and
finding all the places that refer to RTE_SUBQUERY and adjusting them
to handle RTE_SECURITY_VIEW in either the same way or differently as
may be appropriate.  The possible advantage of this approach is that
it doesn't bloat the RTE structure (and stored rules that use it) with
an additional attribute that (I think) will always be false - because
security_barrier can only be set on a subquery RTE after rewriting has
happened, and stored rules are haven't been rewritten yet.  It might
also force people to think a bit more carefully about how security
views should be handled during future code changes, which could also
be viewed as a plus.

I'm attaching my current version of KaiGai's patch (with substantial
cleanup of the comments and documentation, and some other changes) for
reference.

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


leaky-views-20111207.patch
Description: Binary data

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


Re: [HACKERS] pg_restore --no-post-data and --post-data-only

2011-12-07 Thread Andrew Dunstan



On 12/07/2011 11:31 AM, Tom Lane wrote:

Josh Berkus  writes:

Note that this feature has the odd effect that some constraints are loaded at 
the same time as the tables and some are loaded with the post-data.  This is 
consistent with how text-mode pg_dump has always worked, but will seem odd to 
the user.  This also raises the possibility of a future pg_dump/pg_restore 
optimization.

That does seem odd.  Why do we do it that way?

Beats me.

Performance, mostly --- we prefer to apply checks during the original
data load if possible, but for indexes and FK constraints it's faster to
apply them later.  Also, we can separate constraints from the original
table declaration if it's necessary to break a reference circularity.
This isn't something that would be wise to whack around.





Yeah, and if we did want to change it that should be a TODO and not hold 
up this feature.


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] IDLE in transaction introspection

2011-12-07 Thread Scott Mead
On Tue, Dec 6, 2011 at 6:38 AM, Magnus Hagander  wrote:

> On Sat, Nov 19, 2011 at 02:55, Scott Mead  wrote:
> >
> > On Thu, Nov 17, 2011 at 11:58 AM, Scott Mead  wrote:
> >>
> >> On Wed, Nov 16, 2011 at 4:09 PM, Scott Mead  wrote:
> >>>
> >>>
> >>>
> >>> On Tue, Nov 15, 2011 at 1:18 PM, Robert Treat  wrote:
> 
>  On Tue, Nov 15, 2011 at 12:00 PM, Greg Smith 
>  wrote:
>  > On 11/15/2011 09:44 AM, Scott Mead wrote:
>  >>
>  >> Fell off the map last week, here's v2 that:
>  >>  * RUNNING => active
>  >>  * all states from CAPS to lower case
>  >>
>  >
>  > This looks like what I was hoping someone would add here now.  Patch
>  > looks
>  > good, only issue I noticed was a spaces instead of a tab goof where
>  > you set
>  > beentry_>st_state at line 2419 in src/backend/postmaster/pgstat.c
>  >
>  > Missing pieces:
>  >
>  > -There is one regression test that uses pg_stat_activity that is
>  > broken now.
>  > -The documentation should list the new field and all of the states
> it
>  > might
>  > include.  That's a serious doc update from the minimal info
> available
>  > right
>  > now.
> >
> >
> > V3 Attached:
> >
> >   * Updates Documentation
> > -- Adds a separate table to cover each column of pg_stat_activity
>
> I like that a lot - we should've done taht years ago :-)
>
> For consistency, either both datname and usename should refer to their
> respective oid, or none of them.
>

> application_name  should probably have a link to the libpq
> documentation for said parameter.
>
> "field is not set" should probably be "field is NULL"
>
>
Thanks for pointing these out.


> "Boolean, if the query is waiting because of a block / lock" reads
> really strange to me. "Boolean indicating if" would be a good start,
> but I'm not sure what you're trying to say with "block / lock" at all?
>

Yeah, me neither.  What about:
   "Boolean indicating if a query is in a wait state due to a conflict with
another backend."


>
> The way the list of states is built up looks really strange. I think
> it should be built out of  like other such places
> instead - but I admit to not having checked what that actually looks
> like in the output.
>

Agreed.  I'll look at 

>
> The use of single quotes in the descriptions, things like "This is the
> 'state' of" seems wrong. If we should use anything, it should be
> double quotes, but why do we need double quotes around that? And the
> reference to "think time" is just strange, IMO.
>
> Yeah, that's a 'Scottism' (see, I used it again :-).  I'll clean that up


> In some other cases, the single quotes should probably be replaced
> with .
>
> NB: should probably be .
>
>
I am actually looking for some helping in describing the "
function call" state.  Any takers?


>
>
> >   * Adds 'state_change' (timestamp) column
> > -- Tracks when the 'state' column is updated independently
> >of when the 'query' column is updated
>
> Very useful.
>
>
> >   * renames procpid => pid
>
> Shouldn't we do this consistently if we do it? It's change in
> pg_stat_get_activity() but not pg_stat_get_wal_senders()? I think we
> either change in both functions, or none of them
>

Agreed


>
> >   * Bug Fixes
> > -- If a backend had never before issued a query, another
> > session looking at pg_stat_activity would print  not
> > enabled>
> > in the query column.
> > -- query_start was being updated on a 'state' change, now only
> updated
> > on query
> >change
> >
> >   * Fixed 'rules' regression failure
> > -- Added new columns for pg_stat_activity
>
> Also, I think state=-1 needs a symbolic value. STATE_UNDEFINED or
> something like that.
>

Agreed.


>
> There's also a lot of random trailing whitespace in the patch - "git
> diff" (which you seem to be using already, that's why I mention it)
> will happily alert you about that - they should be removed. Or the
> committer can clean that up of course, but it makes for less work ;)
>
> Thanks for pointing that out.


>
> The code is starting to look really good,  but I think the docs
> definitely need another round of work.
>
>
Yeah, I figured they would.  Thanks for going through them!


--
Scott Mead
  OpenSCG, http://www.openscg.com

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


Re: [HACKERS] pg_restore --no-post-data and --post-data-only

2011-12-07 Thread Tom Lane
Josh Berkus  writes:
>>> Note that this feature has the odd effect that some constraints are loaded 
>>> at the same time as the tables and some are loaded with the post-data.  
>>> This is consistent with how text-mode pg_dump has always worked, but will 
>>> seem odd to the user.  This also raises the possibility of a future 
>>> pg_dump/pg_restore optimization.

>> That does seem odd.  Why do we do it that way?

> Beats me.

Performance, mostly --- we prefer to apply checks during the original
data load if possible, but for indexes and FK constraints it's faster to
apply them later.  Also, we can separate constraints from the original
table declaration if it's necessary to break a reference circularity.
This isn't something that would be wise to whack around.

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] documentation issue - extensions

2011-12-07 Thread Tom Lane
Pavel Stehule  writes:
> I am trying to create simple extension according to doc
> http://www.postgresql.org/docs/9.1/interactive/extend-extensions.html

> I have a problem with line that contains backslash statement

> -- complain if script is sourced in psql, rather than via CREATE
> EXTENSION
> \echo Use "CREATE EXTENSION pair" to load this file. \quit

> this content causes a error

> postgres=# create extension gdlib;
> ERROR:  syntax error at or near "\"

Are you testing in an up-to-date server?  We added the ability to handle
that post-9.1.0.

regards, tom lane

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


Re: [HACKERS] Measuring relation free space

2011-12-07 Thread Jaime Casanova
On Mon, Nov 28, 2011 at 5:40 AM, Greg Smith  wrote:
>
>> Unless I am missing something, all indexes are handled via a procedure
>> designed for BTree indices, "GetBTRelationFreeSpace".  I don't know
>> that the ultimate behavior of this is wrong, but it seems unusual.  If
>> I get some more time, I will try to explore what is actually going on
>> when called on other types of indexes.
>
>
> This I think I'll punt back toward Jaime, as well as asking "did you have a
> plan for TOAST here?"
>

for indexes. it seems pageinspect only deals with btree indexes and i
neglected to put a similar limitation on this function... now, because
the free space is calculated using PageGetFreeSpace() for indexes it
should be doing the right thing for all kind of indexes, i only put
the function there because i was trying to avoid to create a new file.
But if the function is right for all kind of indexes that's maybe
enough to create a new file and rename the helper function so is
obvious that it can manage all kind of indexes

for toast tables. a simple test here seems to show that is as easy as
to add toast tables in the supported objects and treat them as normal
pages...

or there is something i'm missing?

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

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


Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-07 Thread Robert Haas
On Wed, Dec 7, 2011 at 10:58 AM, Peter Geoghegan  wrote:
> On 7 December 2011 15:15, Robert Haas  wrote:
>> On Wed, Dec 7, 2011 at 10:09 AM, Tom Lane  wrote:
>>> But it would still have to be prepared for detoasting,
>>> so in the end I was unenthused.  Anyone who feels like testing could try
>>> to prove me wrong about it though.
>>
>> I think that'd definitely be worth investigating (although I'm not
>> sure I have the time to do it myself any time real soon).
>
> I'll at least take a look at it. Sorting text is a fairly common case.
> I'm not hugely enthused about spending too much time on work that will
> only be useful if collate_is_c.

Well, text_pattern_ops is not exactly an uncommon case, but sure.  And
there might be some benefit for other collations, too.

>> Cool.  Peter, can you rebase your patch and integrate it into the
>> sortsupport framework that's now committed?
>
> Yes, I'd be happy to, though I don't think I'm going to be getting
> around to it this side of Friday. Since it isn't a blocker, I assume
> that's okay.

Sounds fine to me.

-- 
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] Inlining comparators as a performance optimisation

2011-12-07 Thread Peter Geoghegan
On 7 December 2011 15:15, Robert Haas  wrote:
> On Wed, Dec 7, 2011 at 10:09 AM, Tom Lane  wrote:
>> But it would still have to be prepared for detoasting,
>> so in the end I was unenthused.  Anyone who feels like testing could try
>> to prove me wrong about it though.
>
> I think that'd definitely be worth investigating (although I'm not
> sure I have the time to do it myself any time real soon).

I'll at least take a look at it. Sorting text is a fairly common case.
I'm not hugely enthused about spending too much time on work that will
only be useful if collate_is_c.

>> I don't believe that #2 blocks progress on #3
>> anyway.  I think #3 is in Peter's court, or yours if you want to do it.
>>
>> (BTW, I agree with your comments yesterday about trying to break down
>> the different aspects of what Peter did, and put as many of them as we
>> can into the non-inlined code paths.)

I'm confident that we should have everything for the simple case of
ordering by a single int4 and int8 column, and I think you'd probably
agree with that - they're extremely common cases. Anything beyond that
will need to be justified, probably in part by running additional
benchmarks.

> Cool.  Peter, can you rebase your patch and integrate it into the
> sortsupport framework that's now committed?

Yes, I'd be happy to, though I don't think I'm going to be getting
around to it this side of Friday. Since it isn't a blocker, I assume
that's okay.

The rebased revision will come complete with a well thought-out
rationale for my use of inlining specialisations, that takes account
of the trade-off against binary bloat that Tom highlighted. I wasn't
ignoring that issue, but I did fail to articulate my thoughts there,
mostly because I felt the need to do some additional research to
justify my position.

-- 
Peter Geoghegan       http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services

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


Re: [HACKERS] review: CHECK FUNCTION statement

2011-12-07 Thread Albe Laurenz
Pavel Stehule wrote:
>> The syntax error messages are still inadequate; all I can get is
>> 'syntax error at or near "%s"'.  They should be more detailed.
> 
> this system is based on error messages that generates a plpgsql engine
> or bison engine. I can correct only a few percent from these messages
> :(
> 
> internally I didn't wrote a compiler or plpgsql checker - this is just
> tool that can emit some plpgsql interpret subprocess - and when these
> subprocesses raises exceptions, then takes their messages.

I see.

>> I think that at least the documentation should be improved before
>> I am ready to set this as "ready for committer".
> 
> please, can you send a correction to documentation or error messages?
> 
> I am not able to write documentation

I'll give it a try.

Yours,
Laurenz Albe

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


Re: [HACKERS] review: CHECK FUNCTION statement

2011-12-07 Thread Pavel Stehule
2011/12/7 Albe Laurenz :
> Pavel Stehule wrote:
>> there is a updated patch.
>>
>> it support multi check, options and custom check functions are not
>> supported yet. I don't plan to implement custom check functions in
>> this round - I has not any example of usage - but we have agreement on
>> syntax and behave, so this should not be problem. I changed reporting
>> - from exception to warnings.
>
> The patch applies and builds cleanly.
>
> The syntax error messages are still inadequate; all I can get is
> 'syntax error at or near "%s"'.  They should be more detailed.

this system is based on error messages that generates a plpgsql engine
or bison engine. I can correct only a few percent from these messages
:(

internally I didn't wrote a compiler or plpgsql checker - this is just
tool that can emit some plpgsql interpret subprocess - and when these
subprocesses raises exceptions, then takes their messages.

>
> Many other messages and code comments are in bad English.
>
> It might be a good idea to add some regression tests for the
> CHECK FUNCTION ALL variants.
>
> Functionality:
> --
>
> I noticed an oddity:
>
> postgres=# CHECK FUNCTION ALL;
> ERROR:  syntax error at or near ";"
> LINE 1: CHECK FUNCTION ALL;
>                          ^
> postgres=# CHECK FUNCTION ALL IN LANGUAGE plpgsql;
> NOTICE:  nothing to check
> postgres=# CHECK FUNCTION ALL IN SCHEMA pg_catalog;
> [prints lots of NOTICEs]
>
> According to the syntax diagram and my intuition CHECK FUNCTION ALL
> without additional clauses should work.

this is question - this will check all functions in postgres.It's 2421
function, so one criterium as minimum should be good idea.

We can remove buildin functions from list - so it will check all
function in database.

>
> Regarding the syntax: I know I suggested it myself, but after several
> times of typing "IN LANGUAGE plpgsql" I think that omitting the "IN"
> would be better and more like other commands (e.g. CREATE FUNCTION).
>

IN should be syntactic sugar

> It is a pity that the CHECK FUNCTION ALL variants will not check
> trigger functions, but I understand the difficulty -- it would
> require checking all trigger functions on all tables where they
> occur in a trigger.
>
> I think that the checker function should be shown in psql's
> \dL+ output.
>
> Barring these little gripes, the functionality seems "ready for
> committer" from my point of view.
>
> Code review:
> 
>
> I do not feel competent for a thorough code review.
>
> Documentation:
> --
>
> This is where I see the greatest shortcomings.
>
> - The documentation for the system catalog pg_pltemplate should be
>  extended to include tmplchecker.
> - The documentation patch for CREATE LANGUAGE is plain wrong and
>  contains a syntax error.
> - CHECK FUNCTION and CHECK TRIGGER should be treated as different
>  SQL statements.  It is misleading to have CHECK TRIGGER listed
>  under CHECK FUNCTION.  If they have to be together, the statement
>  should be called "CHECK" and not "CHECK TRIGGER", but I think
>  they should be separate.
> - There is still no documentation patch for plhandler.sgml.
>
>
> I think that at least the documentation should be improved before
> I am ready to set this as "ready for committer".

please, can you send a correction to documentation or error messages?

I am not able to write documentation

Regards

Pavel
>
> Yours,
> Laurenz Albe

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


Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-07 Thread Robert Haas
On Wed, Dec 7, 2011 at 10:09 AM, Tom Lane  wrote:
> There's some stuff that's debatable according to this criterion --- in
> particular, I wondered whether it'd be worth having a fast path for
> bttextcmp, especially if we pre-tested the collate_is_c condition and
> had a separate version that just hardwired the memcmp code path.  (The
> idea of doing that was one reason I insisted on collation being known at
> the setup step.)  But it would still have to be prepared for detoasting,
> so in the end I was unenthused.  Anyone who feels like testing could try
> to prove me wrong about it though.

I think that'd definitely be worth investigating (although I'm not
sure I have the time to do it myself any time real soon).

>> Are you planning to do anything about #2 or #3?
>
> I am willing to do #2, but not right now; I feel what I need to do next
> is go review SPGist.

Yeah, makes sense.  That one seems likely to be a challenge to absorb.

> I don't believe that #2 blocks progress on #3
> anyway.  I think #3 is in Peter's court, or yours if you want to do it.
>
> (BTW, I agree with your comments yesterday about trying to break down
> the different aspects of what Peter did, and put as many of them as we
> can into the non-inlined code paths.)

Cool.  Peter, can you rebase your patch and integrate it into the
sortsupport framework that's now committed?

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

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


Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-07 Thread Tom Lane
Robert Haas  writes:
> On Tue, Dec 6, 2011 at 8:46 PM, Tom Lane  wrote:
>> 1. Adding sortsupport infrastructure for more datatypes.
>> 2. Revising nbtree and related code to use this infrastructure.
>> 3. Integrating Peter's work into this framework.
>> 
>> I'll try to take care of #1 for at least a few key datatypes before
>> I commit, but I think #2 is best done as a separate patch, so I'll
>> postpone that till later.

> I see you've committed a chunk of this now.  Does it make sense to do
> #1 for every data type we support, or should we be more selective than
> that?

Basically, I tried to do #1 for every datatype for which the comparator
was cheap enough that reducing the call overhead seemed likely to make a
useful difference.  I'm not in favor of adding sortsupport functions
where this is not true, as I think it'll be useless code and catalog
bloat.  I don't want to add 'em for cruft like abstime either.

There's some stuff that's debatable according to this criterion --- in
particular, I wondered whether it'd be worth having a fast path for
bttextcmp, especially if we pre-tested the collate_is_c condition and
had a separate version that just hardwired the memcmp code path.  (The
idea of doing that was one reason I insisted on collation being known at
the setup step.)  But it would still have to be prepared for detoasting,
so in the end I was unenthused.  Anyone who feels like testing could try
to prove me wrong about it though.

> Are you planning to do anything about #2 or #3?

I am willing to do #2, but not right now; I feel what I need to do next
is go review SPGist.  I don't believe that #2 blocks progress on #3
anyway.  I think #3 is in Peter's court, or yours if you want to do it.

(BTW, I agree with your comments yesterday about trying to break down
the different aspects of what Peter did, and put as many of them as we
can into the non-inlined code paths.)

regards, tom lane

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


Re: [HACKERS] review: CHECK FUNCTION statement

2011-12-07 Thread Albe Laurenz
Pavel Stehule wrote:
> there is a updated patch.
> 
> it support multi check, options and custom check functions are not
> supported yet. I don't plan to implement custom check functions in
> this round - I has not any example of usage - but we have agreement on
> syntax and behave, so this should not be problem. I changed reporting
> - from exception to warnings.

The patch applies and builds cleanly.

The syntax error messages are still inadequate; all I can get is
'syntax error at or near "%s"'.  They should be more detailed.

Many other messages and code comments are in bad English.

It might be a good idea to add some regression tests for the
CHECK FUNCTION ALL variants.

Functionality:
--

I noticed an oddity:

postgres=# CHECK FUNCTION ALL;
ERROR:  syntax error at or near ";"
LINE 1: CHECK FUNCTION ALL;
  ^
postgres=# CHECK FUNCTION ALL IN LANGUAGE plpgsql;
NOTICE:  nothing to check
postgres=# CHECK FUNCTION ALL IN SCHEMA pg_catalog;
[prints lots of NOTICEs]

According to the syntax diagram and my intuition CHECK FUNCTION ALL
without additional clauses should work.

Regarding the syntax: I know I suggested it myself, but after several
times of typing "IN LANGUAGE plpgsql" I think that omitting the "IN"
would be better and more like other commands (e.g. CREATE FUNCTION).

It is a pity that the CHECK FUNCTION ALL variants will not check
trigger functions, but I understand the difficulty -- it would
require checking all trigger functions on all tables where they
occur in a trigger.

I think that the checker function should be shown in psql's
\dL+ output.

Barring these little gripes, the functionality seems "ready for
committer" from my point of view.

Code review:


I do not feel competent for a thorough code review.

Documentation:
--

This is where I see the greatest shortcomings.

- The documentation for the system catalog pg_pltemplate should be
  extended to include tmplchecker.
- The documentation patch for CREATE LANGUAGE is plain wrong and
  contains a syntax error.
- CHECK FUNCTION and CHECK TRIGGER should be treated as different
  SQL statements.  It is misleading to have CHECK TRIGGER listed
  under CHECK FUNCTION.  If they have to be together, the statement
  should be called "CHECK" and not "CHECK TRIGGER", but I think
  they should be separate.
- There is still no documentation patch for plhandler.sgml.


I think that at least the documentation should be improved before
I am ready to set this as "ready for committer".

Yours,
Laurenz Albe

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


Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-07 Thread Robert Haas
On Tue, Dec 6, 2011 at 8:46 PM, Tom Lane  wrote:
> 1. Adding sortsupport infrastructure for more datatypes.
> 2. Revising nbtree and related code to use this infrastructure.
> 3. Integrating Peter's work into this framework.
>
> I'll try to take care of #1 for at least a few key datatypes before
> I commit, but I think #2 is best done as a separate patch, so I'll
> postpone that till later.

I see you've committed a chunk of this now.  Does it make sense to do
#1 for every data type we support, or should we be more selective than
that?  My gut feeling would be to add it across the board and
introduce an opr_sanity check for it.  The general utility of adding
it to deprecated types like abstime is perhaps questionable, but it
strikes me that the value of making everything consistent probably
outweighs the cost of a few extra lines of code.

Are you planning to do anything about #2 or #3?

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

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


Re: [HACKERS] pg_stat_statements with query tree based normalization

2011-12-07 Thread Marti Raudsepp
On Wed, Dec 7, 2011 at 03:19, Peter Geoghegan  wrote:
> The results are...taking the median value of each set of runs as
> representative, my patch appears to run marginally faster than head.
> Of course, there is no reason to believe that it should, and I'm
> certain that the difference can be explained by noise, even though
> I've naturally strived to minimise noise.

You should use the t-test to distinguish whether two data sets show a
consistent difference or whether it's just noise. Excel/OpenOffice
have the TTEST() macro for this purpose. For statistics doofuses like
me, just pick mode=2 and type=3 as that's the most conservative.

If the TTEST result is less than 0.05 then you have 95% certainty that
the two dataset are consistently different. If not, you need more
consistent data.

More information here:
http://www.gifted.uconn.edu/siegle/research/t-test/t-test.html

Regards,
Marti

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


Re: [HACKERS] pg_restore --no-post-data and --post-data-only

2011-12-07 Thread Josh Berkus

>> Note that this feature has the odd effect that some constraints are loaded 
>> at the same time as the tables and some are loaded with the post-data.  This 
>> is consistent with how text-mode pg_dump has always worked, but will seem 
>> odd to the user.  This also raises the possibility of a future 
>> pg_dump/pg_restore optimization.
> 
> That does seem odd.  Why do we do it that way?

Beats me.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.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] RangeVarGetRelid()

2011-12-07 Thread Noah Misch
On Tue, Dec 06, 2011 at 04:02:31PM -0500, Robert Haas wrote:
> On Mon, Dec 5, 2011 at 2:09 AM, Noah Misch  wrote:
> >> ? ? ? ? ? ? ? AcceptInvalidationMessages();
> >
> > The above call can go away, now.
> 
> Doesn't that still protect us against namespace-shadowing issues?
> RangeVarGetRelid doesn't actually AcceptInvalidationMessages() at the
> top.

It narrows the window for race conditions of that genesis, but isn't doing so
an anti-feature?  Even if not, doing that _only_ in RemoveRelations() is odd.

FWIW, this is fairly independent of your latest patches -- I just happened to
notice it due to the incidental quotation.

> Attached please find a patch with some more fixes on this same general
> theme.  This one tackles renaming of relations, columns, and triggers;
> and changing the schema of relations.  In these cases, the current
> code does a permissions check before locking the table (which is good)
> and uses RangeVarGetRelid() to guard against "cache lookup failure"
> errors caused by concurrent DDL (also good).  However, if the referent
> of the name changes during the lock wait, we don't recheck
> permissions; we just allow the rename or schema change on the basis
> that the user had permission to do it to the relation that formerly
> had that name.  While this is pretty minor as security concerns go, it
> seems best to clean it up, so this patch does that.

I'd suggest limiting callback functions to checks that benefit greatly from
preceding the lock acquisition.  In these cases, that's only the target object
ownership check; all other checks can wait for the lock.  Writing correct
callback code is relatively tricky; we have no lock, so the available catalog
entries can change arbitrarily often as the function operates.  It's worth the
trouble of navigating that hazard to keep the mob from freely locking all
objects.  However, if the owner of "some_table" writes "ALTER VIEW some_table
RENAME TO foo", I see no commensurate benefit from reporting the relkind
mismatch before locking the relation.  This would also permit sharing a
callback among almost all the call sites you've improved so far.  Offhand, I
think only ReindexIndex() would retain a bespoke callback.

This patch removes two of the three callers of CheckRelationOwnership(),
copying some of its logic to two other sites.  I'd suggest fixing the third
caller (standard_ProcessUtility() for CREATE INDEX) in this same patch.  That
way, we can either delete the function now or adjust it to permit continued
sharing of that code under the new regime.

I like how this patch reduces the arbitrary division of authorization checks
between alter.c and tablecmds.c.

Thanks,
nm

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


Re: [HACKERS] Configuration include directory

2011-12-07 Thread Marti Raudsepp
On Wed, Nov 16, 2011 at 06:53, Greg Smith  wrote:
> -Considers all names in that directory that end with *.conf  [Discussion
> concluded more flexibility here would be of limited value relative to how it
> complicates the implementation]

I'd suggest also excluding hidden files -- files that start with a
dot. That's how glob/fnmatch functions work and most "include all
files" implementations are based on that.

Regards,
Marti

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


[HACKERS] documentation issue - extensions

2011-12-07 Thread Pavel Stehule
Hello

I am trying to create simple extension according to doc
http://www.postgresql.org/docs/9.1/interactive/extend-extensions.html

I have a problem with line that contains backslash statement

-- complain if script is sourced in psql, rather than via CREATE
EXTENSION
\echo Use "CREATE EXTENSION pair" to load this file. \quit

this content causes a error

postgres=# create extension gdlib;
ERROR:  syntax error at or near "\"

without this line extension is created fine.

Is documentation correct?

Regards

Pavel

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


Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-07 Thread Peter Geoghegan
On 7 December 2011 03:45, Robert Haas  wrote:
> In this regard, I think Heikki's remarks upthread are worth some
> thought.  If inlining is a win just because it avoids saving and
> restoring registers or allows better instruction scheduling, then
> inlining is the (probably?) the only way to get the benefit.  But if
> most of the benefit is in having a separate path for the
> single-sort-key case, we can do that without duplicating the qsort()
> code and get the benefit for every data type without much code bloat.
> I'd like to see us dig into that a little, so that we get the broadest
> possible benefit out of this work.  It doesn't bother me that not
> every optimization will apply to every case, and I don't object to
> optimizations that are intrinsically narrow (within some reasonable
> limits).  But I'd rather not take what could be a fairly broad-based
> optimization and apply it only narrowly, all things being equal.

I think we're in agreement then.

It's important to realise, though I'm sure you do, that once you've
done what I did with sorting single scanKey integers, that's it -
there isn't really any way that I can think of to speed up quick
sorting further, other than parallelism which has major challenges,
and isn't particularly appropriate at this granularity. The real
significance of inlining is, well, that's it, that's all there is -
after inlining, I predict that the well will run dry, or practically
dry. Inlining actually is a pretty great win when you look at sorting
in isolation, but it's just that there's been a number of other
significant wins in my patch, and of course there is overhead from a
number of other areas, so perhaps it's easy to lose sight of that.

-- 
Peter Geoghegan       http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services

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


Re: [HACKERS] Configuration include directory

2011-12-07 Thread Greg Smith

On 11/17/2011 11:03 AM, Tom Lane wrote:

So as long as the include-directory code path doesn't
interfere with tracking that nesting depth, I don't think it needs
any extra protection against include-the-same-directory.
   


That was the theory in Magnus's original patch, and I don't believe 
anything has broken that part; I did glance at it.  Since I have a pile 
of good feedback from Noah now, I'll specifically test this as part of 
submitting the next patch update.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us


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


Re: [HACKERS] [COMMITTERS] pgsql: plpython: Add SPI cursor support

2011-12-07 Thread Jan Urbański
On 07/12/11 11:16, Jan Urbański wrote:
> On 06/12/11 19:33, Jan Urbański wrote:
>> On 06/12/11 19:23, Tom Lane wrote:
>>> Peter Eisentraut  writes:
 plpython: Add SPI cursor support
>>>
>>> Buildfarm member narwhal does not like this patch.  It looks like
>>> "PyObject_SelfIter" is not a compile-time constant on its version
>>> of python (2.5, apparently).
>>
>> I'll try to dig around to see what can be causing that...
> 
> Seems that PyObject_GenericGetIter has been renamed to PyObject_SelfIter
> at some point:
> 
> http://hg.python.org/cpython/rev/fd5ef7003469
> 
> I'm trying to muster whatever mercurial-foo I have to check if there's
> been a 2.5 version tagged without that patch...

So no, the renaming happened on Mar 17 2003 and 2.5 was tagged on
Sep 18 2006. The source tarball for 2.5 contains PyObject_SelfIter.

Both fennec and mastodon build OK with Python 2.5. Is it possible that
narwhal's Python installation is in some way broken?

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


Re: [HACKERS] [COMMITTERS] pgsql: plpython: Add SPI cursor support

2011-12-07 Thread Jan Urbański
On 06/12/11 19:33, Jan Urbański wrote:
> On 06/12/11 19:23, Tom Lane wrote:
>> Peter Eisentraut  writes:
>>> plpython: Add SPI cursor support
>>
>> Buildfarm member narwhal does not like this patch.  It looks like
>> "PyObject_SelfIter" is not a compile-time constant on its version
>> of python (2.5, apparently).
> 
> I'll try to dig around to see what can be causing that...

Seems that PyObject_GenericGetIter has been renamed to PyObject_SelfIter
at some point:

http://hg.python.org/cpython/rev/fd5ef7003469

I'm trying to muster whatever mercurial-foo I have to check if there's
been a 2.5 version tagged without that patch...

Jan

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


Re: [HACKERS] [DOCS] Moving tablespaces

2011-12-07 Thread Magnus Hagander
On Wed, Dec 7, 2011 at 10:05, Magnus Hagander  wrote:
> On Tue, Dec 6, 2011 at 17:07, Tom Lane  wrote:
>> Magnus Hagander  writes:
>>> There is some nice precedent in the CREATE TABLESPACE command (though
>>> dependent on HAVE_SYMLINK and not HAVE_READLINK), so I'm just going to
>>> copy the error message from there.
>>
>> Fair enough.
>>
>> Looking at the existing readlink use in port/exec.c, it strikes me that
>> another thing you'd better do is include a check for buffer overrun,
>> ie the test needs to be more like
>>
>>                rllen = readlink(fname, link_buf, sizeof(link_buf));
>>                if (rllen < 0 || rllen >= sizeof(link_buf))
>>                        ... fail ...
>
> Seems reasonable, yeah. I'll go put a similar check in the
> basebackup.c file as well when I'm done here.

To close this thread (hopefully): Fixed and applied.

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

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


Re: [HACKERS] [DOCS] Moving tablespaces

2011-12-07 Thread Magnus Hagander
On Tue, Dec 6, 2011 at 17:07, Tom Lane  wrote:
> Magnus Hagander  writes:
>> There is some nice precedent in the CREATE TABLESPACE command (though
>> dependent on HAVE_SYMLINK and not HAVE_READLINK), so I'm just going to
>> copy the error message from there.
>
> Fair enough.
>
> Looking at the existing readlink use in port/exec.c, it strikes me that
> another thing you'd better do is include a check for buffer overrun,
> ie the test needs to be more like
>
>                rllen = readlink(fname, link_buf, sizeof(link_buf));
>                if (rllen < 0 || rllen >= sizeof(link_buf))
>                        ... fail ...

Seems reasonable, yeah. I'll go put a similar check in the
basebackup.c file as well when I'm done here.


> Also, you're assuming that the result is already null-terminated,
> which is incorrect.

No, I'm not - I'm MemSet()ing the whole buffer to 0 before I start.
But I'll change that to work the same way as the on in port/exec.c,
for consistency.


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

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


Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-12-07 Thread Yeb Havinga

On 2011-12-06 17:58, Kevin Grittner wrote:

Kevin Grittner  wrote:

If there are no objections, I suggest that Yeb implement the mixed
notation for cursor parameters.


Hearing no objections -- Yeb, are you OK with doing this, and do you
feel this is doable for this CF?


It is not a large change, I'll be able to do it tomorrow if nothing 
unexpected happens, or monday otherwise.


regards,
Yeb


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