Re: [HACKERS] Reducing some DDL Locks to ShareLock

2008-10-07 Thread Simon Riggs

On Mon, 2008-10-06 at 18:57 -0400, Tom Lane wrote:
 Simon Riggs [EMAIL PROTECTED] writes:
  It seems possible to change some DDL commands/subcommands to use a
  ShareLock rather than an AccessExclusiveLock. Enclosed patch implements
  this reduction for CREATE TRIGGER, CREATE RULE and ALTER TABLE.
 
 What happens when two transactions try to do one of these things
 concurrently to the same table?

It should be very similar to CREATE INDEX. If they hold ShareLocks then
their locks do not conflict at relation level.

Updates of their pg_class entry should be handled non-transactionally,
just as they are with CREATE INDEX. This is the reason for the change
from reltriggers being an exact count to being a boolean, i.e. it is
actually now relhastriggers = true | false. Multiple updaters will then
change the value to the same thing.

It was an excellent question because that aspect isn't handled correctly
in the enclosed patch for subcommands, other than index-creating
constraints. 

My main focus is on these commands
* CREATE TRIGGER
* ALTER TABLE ..  ADD PRIMARY KEY
* ALTER TABLE ..  ADD FOREIGN KEY

because those are the most painful ones. We could make it work against
more, but we'd need to rewrite lots and lots of catalog update code.

So I'll make CreateTrigger() use index_update_stats() so we get the
atomic update correct.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support


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


Re: [HACKERS] Transactions and temp tables

2008-10-07 Thread Heikki Linnakangas

Emmanuel Cecchet wrote:
Instead of relying on a boolean that tells if a temp table was accessed, 
I keep a list of the Oid for the temp tables accessed in the transaction 
and at prepare commit time, I check if the relations are still valid. I 
also added a check to allow empty temp tables at prepare commit time 
(this allows to use temp tables with 'on commit delete rows' options.


I am attaching the patch and the use cases I have been using to test it. 
The test cases try to compile the various use cases that I have seen 
reported on the list.


Thanks for looking into this.

The patch allows preparing any transaction that has dropped the temp 
table, even if it wasn't created in the same transaction. Is that sane?


Also, even if the table is created and dropped in the same transaction, 
a subsequent transaction that tries to create and drop the table gets 
blocked on the lock. I suppose we could just say that that's the way it 
works, but I'm afraid it will come as a nasty surprise, making the 
feature a lot less useful.


The fixed-size array of temp table oids is an unnecessary limitation. A 
list or hash table would be better.


Let me know what you think of the patch and if it 
could be applied to 8.3 and 8.4?


Not to 8.3. We only back-patch bug-fixes, and this isn't one.

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

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


Re: [HACKERS] Shouldn't pg_settings.enumvals be array of text?

2008-10-07 Thread Magnus Hagander
Tom Lane wrote:
 Greg Smith [EMAIL PROTECTED] writes:
 When I was looking at this code for the first time recently I thought the 
 same thing Tom did here--that this was kind of odd and it should give a 
 text array back instead.  I would even volunteer to take a stab at writing 
 that change myself just to get some more practice with this section of 
 code, should be able to squeeze that in over the next month.
 
 Fine with me, but let's be sure this doesn't slide off the radar screen.
 If we forget about it and release 8.4 with the current definition of the
 column, it'll be too late to change it.

Might this be the time to add an open items for 8.4 page to the wiki?
(I'm just assuming we'll do it on the wiki and not in Bruce's mailbox
this time) I notice Greg put it on the project-specific wiki page around
GUC, but we should probably collect these things at a central location
as well.. If we want to do that, we should just ask our wiki-gurus to
create some nice templates I think :-)

//Magnus


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


Re: [PATCHES] [HACKERS] Infrastructure changes for recovery

2008-10-07 Thread Simon Riggs

On Thu, 2008-10-02 at 19:07 -0400, Andrew Dunstan wrote:

 Simon Riggs wrote:

  Just seen this patch has been bounced into November CommitFest, even
  though the new patch fixes all of the concerns raised.
 
  I'm concerned that this is going to make the final Hot Standby patch
  fairly large, which will make it even harder to review, test and
  generally get accepted.
 
  What's the best way to make this easier for you/others to review?

 The fact that it's been put on the November list doesn't mean it can't 
 be reviewed and committed before then.

But that seems unlikely to be the case.

A patch specifically marked as required for other work has been
delayed by more than 5 weeks on queue and nobody was ever assigned to
review it. That was exactly the problem CommitFests were supposed to
resolve and from my perspective this is a systemic failure. If I had
submitted the patch a month late it wouldn't have got reviewed any
earlier, yet many people would cry foul (why?). The current system means
I have to code up to the deadline, officially do nothing for a month,
then respond within hours to code reviews or have the patch rejected for
another month. It works great for minor patches, but its simply not
working for bigger features. It's just not possible to be fully
available to respond to reviews, yet at the same time not able to work
more than about 25% of the development calendar.

Luckily Tom reviewed it, but with no commit and no guidance on how to
proceed this still leaves me in a difficult position.

I'm forced now to leave much of this code behind, since I cannot now
complete Hot Standby at the same time as having bgwriter active during
recovery, if that code is at risk of causing the whole thing to be
rejected. Are the two together a risk? No. Is developing them together
harder? Yes. Do *I* trust my own code? Yes, but its reviewers that
count. Is it a good thing for Postgres to leave this code behind?
Probably not. Can I add it later? Maybe.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support


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


Re: [HACKERS] Subtransaction commits and Hot Standby

2008-10-07 Thread Simon Riggs

On Sun, 2008-10-05 at 14:51 -0400, Alvaro Herrera wrote:
 Simon Riggs wrote:
 
  OK, spent long time testing various batching scenarios for this using a
  custom test harness to simulate various spreads of xids in transaction
  trees. All looks fine now.
 
 I had a look and was mostly rephrasing some comments and the README
 (hopefully I didn't make any of them worse than they were), when I
 noticed that the code to iterate thru pages could be refactored.  I
 think the change makes the algorithm in TransactionIdSetTreeStatus
 easier to follow.

Yes, all fits on one screen when reading it now.

 I also noticed that TransactionIdSetPageStatus has a subcommit arg
 which is unexplained.  I sort-of understand the point, but I think it's
 better that you fill in the explanation in the header comment (marked
 with XXX)

I've changed the logic slightly to remove the need for the subcommit
argument. So no need to explain now.

Added an Assert to check for what should be an impossible call.

Example provided in comments for a complex update.

 I hope I didn't break the code with the new function
 set_tree_status_by_pages -- please recheck that part.

Renamed to set_status_by_pages because we never use this on the whole
tree. Added comments to say that.

Overall, cleaner and more readable now. Thanks.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support
Index: src/backend/access/transam/README
===
RCS file: /home/sriggs/pg/REPOSITORY/pgsql/src/backend/access/transam/README,v
retrieving revision 1.11
diff -c -r1.11 README
*** src/backend/access/transam/README	21 Mar 2008 13:23:28 -	1.11
--- src/backend/access/transam/README	7 Oct 2008 12:42:18 -
***
*** 341,351 
  pg_clog records the commit status for each transaction that has been assigned
  an XID.  A transaction can be in progress, committed, aborted, or
  sub-committed.  This last state means that it's a subtransaction that's no
! longer running, but its parent has not updated its state yet (either it is
! still running, or the backend crashed without updating its status).  A
! sub-committed transaction's status will be updated again to the final value as
! soon as the parent commits or aborts, or when the parent is detected to be
! aborted.
  
  Savepoints are implemented using subtransactions.  A subtransaction is a
  transaction inside a transaction; its commit or abort status is not only
--- 341,360 
  pg_clog records the commit status for each transaction that has been assigned
  an XID.  A transaction can be in progress, committed, aborted, or
  sub-committed.  This last state means that it's a subtransaction that's no
! longer running, but its parent has not updated its state yet.  It is not
! necessary to update a subtransaction's transaction status to subcommit, so we
! can just defer it until main transaction commit.  The main role of marking
! transactions as sub-committed is to provide an atomic commit protocol when
! transaction status is spread across multiple clog pages. As a result, whenever
! transaction status spreads across multiple pages we must use a two-phase commit
! protocol: the first phase is to mark the subtransactions as sub-committed, then
! we mark the top level transaction and all its subtransactions committed (in
! that order).  Thus, subtransactions that have not aborted appear as in-progress
! even when they have already finished, and the subcommit status appears as a
! very short transitory state during main transaction commit.  Subtransaction
! abort is always marked in clog as soon as it occurs.  When the transaction
! status all fit in a single CLOG page, we atomically mark them all as committed
! without bothering with the intermediate sub-commit state.
  
  Savepoints are implemented using subtransactions.  A subtransaction is a
  transaction inside a transaction; its commit or abort status is not only
Index: src/backend/access/transam/clog.c
===
RCS file: /home/sriggs/pg/REPOSITORY/pgsql/src/backend/access/transam/clog.c,v
retrieving revision 1.47
diff -c -r1.47 clog.c
*** src/backend/access/transam/clog.c	1 Aug 2008 13:16:08 -	1.47
--- src/backend/access/transam/clog.c	7 Oct 2008 12:44:14 -
***
*** 80,111 
  static bool CLOGPagePrecedes(int page1, int page2);
  static void WriteZeroPageXlogRec(int pageno);
  static void WriteTruncateXlogRec(int pageno);
  
  
  /*
!  * Record the final state of a transaction in the commit log.
   *
   * lsn must be the WAL location of the commit record when recording an async
   * commit.	For a synchronous commit it can be InvalidXLogRecPtr, since the
   * caller guarantees the commit record is already flushed in that case.  It
   * should be InvalidXLogRecPtr for abort cases, too.
   *
   * NB: this is a low-level routine and is NOT the preferred entry point
!  * for most 

Re: [HACKERS] Reducing some DDL Locks to ShareLock

2008-10-07 Thread Simon Riggs

On Tue, 2008-10-07 at 08:30 -0400, Andrew Dunstan wrote:

 Simon Riggs wrote:
 
  My main focus is on these commands
  * CREATE TRIGGER
  * ALTER TABLE ..  ADD PRIMARY KEY
  * ALTER TABLE ..  ADD FOREIGN KEY
 
  because those are the most painful ones. We could make it work against
  more, but we'd need to rewrite lots and lots of catalog update code.

 Anything that scans the table is a prime candidate. In particular, for 
 parallel pg_dump, ALTER TABLE ... ADD UNIQUE is important, as well as 
 possibly other table constraints.

Yes, I should have mentioned: today's patch (v5) does ADD UNIQUE also.

I tested a concurrent mix of ALTER PK, FK and CREATE INDEX, all fine.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support


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


Re: [HACKERS] Reducing some DDL Locks to ShareLock

2008-10-07 Thread Robert Haas
 3. The patch introduces a slight weirdness: if you create two FKs on the
 same column at the same time you end up with two constraints with
 identical names. Drop constraint then removes them both, though in other
 respects they are both valid, just not uniquely. CREATE INDEX avoids
 this by way of the unique index on relname. The equivalent index on
 pg_constraint is not unique, though *cannot* be made unique without
 breaking some corner cases of table inheritance.

Urk... this seems pretty undesirable.

...Robert

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


[HACKERS] doubts about toast_flatten_tuple_attribute/heap_form_tuple

2008-10-07 Thread Zdenek Kotala
I'm trying write tuple conversion function and now I'm hit problem with 
composite data types. Composite data types can contain inner composite 
data types. Is there any limit? And I found that heap_form_tuple calls 
 toast_flatten_tuple_attribute only one level composite type. Is it bug?



Thanks Zdenek

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


Re: [PATCHES] [HACKERS] Infrastructure changes for recovery

2008-10-07 Thread Tom Lane
Simon Riggs [EMAIL PROTECTED] writes:
 A patch specifically marked as required for other work has been
 delayed by more than 5 weeks on queue and nobody was ever assigned to
 review it. That was exactly the problem CommitFests were supposed to
 resolve and from my perspective this is a systemic failure.

To be blunt, that patch spent most of September in waiting for author
state.  Looking in the archives, I see that

* Original patch was posted on 31-Aug.

* I reviewed that patch on 8-Sep.

* You posted a revised patch on 10-Sep, but it was explicitly marked
as not ready to be actioned.

* It was not until 23-Sep that a patch was posted that you stated
you were happy with.

* I reviewed that one on 25-Sep.

* The patch now in the queue was posted on 30-Sep (all of 8 minutes
before midnight).

I don't see any systemic failure here.

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] Reducing some DDL Locks to ShareLock

2008-10-07 Thread Simon Riggs

On Tue, 2008-10-07 at 10:05 -0400, Robert Haas wrote:
  3. The patch introduces a slight weirdness: if you create two FKs on the
  same column at the same time you end up with two constraints with
  identical names. Drop constraint then removes them both, though in other
  respects they are both valid, just not uniquely. CREATE INDEX avoids
  this by way of the unique index on relname. The equivalent index on
  pg_constraint is not unique, though *cannot* be made unique without
  breaking some corner cases of table inheritance.
 
 Urk... this seems pretty undesirable.

OK, but please say what behaviour you would like in its place. 

Or are you saying you dislike this so much that you would prefer not to
be able to run ALTER TABLE concurrently?

Also, how often do you think this will be a problem? This only happens
when you have two FKs on the exact same column set. That happens seldom,
if ever, AFAICS.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support


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


Re: [PATCHES] [HACKERS] Infrastructure changes for recovery

2008-10-07 Thread Simon Riggs

On Tue, 2008-10-07 at 10:08 -0400, Tom Lane wrote:
 Simon Riggs [EMAIL PROTECTED] writes:
  A patch specifically marked as required for other work has been
  delayed by more than 5 weeks on queue and nobody was ever assigned to
  review it. That was exactly the problem CommitFests were supposed to
  resolve and from my perspective this is a systemic failure.
 
 To be blunt...

The bluntness was mine, for which I apologise. The last review uncovered
essential behaviour I was missing, no doubt about that.

I'm just grumpy because I can't see a way to do the
patch-on-patch-on-patch that I'll need to make this all work for Nov 1.
So big patch here we come. But that's just the way it is and I'll stop
honking about it.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support


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


Re: [HACKERS] Reducing some DDL Locks to ShareLock

2008-10-07 Thread Tom Lane
Simon Riggs [EMAIL PROTECTED] writes:
 On Tue, 2008-10-07 at 10:05 -0400, Robert Haas wrote:
 3. The patch introduces a slight weirdness: if you create two FKs on the
 same column at the same time you end up with two constraints with
 identical names. Drop constraint then removes them both, though in other
 respects they are both valid, just not uniquely. CREATE INDEX avoids
 this by way of the unique index on relname. The equivalent index on
 pg_constraint is not unique, though *cannot* be made unique without
 breaking some corner cases of table inheritance.
 
 Urk... this seems pretty undesirable.

 OK, but please say what behaviour you would like in its place. 

I wonder whether this could be helped if we refactored pg_constraint.
The lack of a useful pkey for it has been annoying me for awhile,
and I think it stems from a misguided choice to put table and domain
constraints into the same catalog.  Suppose that

* table constraints go into pg_relation_constraint, with a unique key
on (conrelid, conname)

* domain constraints go into pg_domain_constraint, with a unique key
on (contypid, conname)

* pg_constraint can still exist as a union view, for client
compatibility

Then the unique key would prevent concurrent creation of
identically-named constraints for the same relation.

I'm confused by your comment about inheritance --- what cases are
you thinking this would break?

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: [PATCHES] [HACKERS] Infrastructure changes for recovery

2008-10-07 Thread Alvaro Herrera
Simon Riggs wrote:

 I'm just grumpy because I can't see a way to do the
 patch-on-patch-on-patch that I'll need to make this all work for Nov 1.
 So big patch here we come. But that's just the way it is and I'll stop
 honking about it.

This is one of the problems that DVCSs are supposed to solve ... have
you tried Git?

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

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


Re: [HACKERS] doubts about toast_flatten_tuple_attribute/heap_form_tuple

2008-10-07 Thread Tom Lane
Zdenek Kotala [EMAIL PROTECTED] writes:
 I'm trying write tuple conversion function and now I'm hit problem with 
 composite data types. Composite data types can contain inner composite 
 data types. Is there any limit?

No.

 And I found that heap_form_tuple calls 
   toast_flatten_tuple_attribute only one level composite type. Is it bug?

No, because any composite value that's being presented for inclusion in
the new tuple was already flattened when it was formed.

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: [PATCHES] [HACKERS] Infrastructure changes for recovery

2008-10-07 Thread Robert Haas
 I'm just grumpy because I can't see a way to do the
 patch-on-patch-on-patch that I'll need to make this all work for Nov 1.
 So big patch here we come. But that's just the way it is and I'll stop
 honking about it.

 This is one of the problems that DVCSs are supposed to solve ... have
 you tried Git?

I think the other problem here is that the difficulty of getting the
patch landed increases more than linearly with its size.  If it's hard
to get a patch of size X landed in one CommitFest, what are the
chances of landing on three times as large, with three times as many
changes to argue about?  Getting things done in stages makes it easier
to build on earlier work without worrying that you'll be asked to go
back and redo everything.

...Robert

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


Re: [HACKERS] Reducing some DDL Locks to ShareLock

2008-10-07 Thread Simon Riggs

On Tue, 2008-10-07 at 07:21 +0100, Simon Riggs wrote:

 It was an excellent question because that aspect isn't handled correctly
 in the enclosed patch for subcommands, other than index-creating
 constraints. 
 
 My main focus is on these commands
 * CREATE TRIGGER
 * ALTER TABLE ..  ADD PRIMARY KEY
 * ALTER TABLE ..  ADD FOREIGN KEY
 * CREATE RULE

 because those are the most painful ones. We could make it work against
 more, but we'd need to rewrite lots and lots of catalog update code.

OK, patch updated, tested, working.

Points of note.

1. I've left the infrastructure there for further changes, but we can
rip that out if desired.

2. Also need to decide whether we want pg_class.reltriggers as int2 (as
implemented here) or switch to relhastriggers as boolean.

3. The patch introduces a slight weirdness: if you create two FKs on the
same column at the same time you end up with two constraints with
identical names. Drop constraint then removes them both, though in other
respects they are both valid, just not uniquely. CREATE INDEX avoids
this by way of the unique index on relname. The equivalent index on
pg_constraint is not unique, though *cannot* be made unique without
breaking some corner cases of table inheritance.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support
Index: doc/src/sgml/mvcc.sgml
===
RCS file: /home/sriggs/pg/REPOSITORY/pgsql/doc/src/sgml/mvcc.sgml,v
retrieving revision 2.69
diff -c -r2.69 mvcc.sgml
*** doc/src/sgml/mvcc.sgml	18 Feb 2007 01:21:49 -	2.69
--- doc/src/sgml/mvcc.sgml	7 Oct 2008 11:27:21 -
***
*** 504,512 
  most productnamePostgreSQL/productname commands automatically
  acquire locks of appropriate modes to ensure that referenced
  tables are not dropped or modified in incompatible ways while the
! command executes.  (For example, commandALTER TABLE/ cannot safely be
! executed concurrently with other operations on the same table, so it
! obtains an exclusive lock on the table to enforce that.)
 /para
  
 para
--- 504,512 
  most productnamePostgreSQL/productname commands automatically
  acquire locks of appropriate modes to ensure that referenced
  tables are not dropped or modified in incompatible ways while the
! command executes.  (For example, commandTRUNCATE/ 
! cannot safely be executed concurrently with other operations on the 
! same table, so it obtains an exclusive lock on the table to enforce that.)
 /para
  
 para
***
*** 647,654 
  /para
  
  para
!  Acquired by commandCREATE INDEX/command
!  (without optionCONCURRENTLY/option).
  /para
 /listitem
/varlistentry
--- 647,659 
  /para
  
  para
!  Acquired by commandCREATE INDEX/command (without 
!  optionCONCURRENTLY/option), commandCREATE TRIGGER/command, 
!  commandCREATE RULE/command (all except ON SELECT rules),
!  commandALTER TABLE/command (if all of its subcommands are one 
!  of the following subcommands: commandADD UNIQUE/command, 
!  commandADD PRIMARY KEY/command or 
!  commandADD FOREIGN KEY/command).
  /para
 /listitem
/varlistentry
***
*** 714,724 
  /para
  
  para
!  Acquired by the commandALTER TABLE/command, commandDROP
!  TABLE/command, commandTRUNCATE/command, commandREINDEX/command,
   commandCLUSTER/command, and commandVACUUM FULL/command
   commands.  This is also the default lock mode for commandLOCK
   TABLE/command statements that do not specify a mode explicitly.
  /para
 /listitem
/varlistentry
--- 719,731 
  /para
  
  para
!  Acquired by the commandDROP TABLE/command, 
!  commandTRUNCATE/command, commandREINDEX/command,
   commandCLUSTER/command, and commandVACUUM FULL/command
   commands.  This is also the default lock mode for commandLOCK
   TABLE/command statements that do not specify a mode explicitly.
+  commandALTER TABLE/command will take a lock at this level, 
+  with some exceptions, noted above under SHARE LOCK.
  /para
 /listitem
/varlistentry
Index: src/backend/commands/tablecmds.c
===
RCS file: /home/sriggs/pg/REPOSITORY/pgsql/src/backend/commands/tablecmds.c,v
retrieving revision 1.266
diff -c -r1.266 tablecmds.c
*** src/backend/commands/tablecmds.c	8 Sep 2008 00:47:40 -	1.266
--- src/backend/commands/tablecmds.c	7 Oct 2008 10:14:53 -
***
*** 249,269 
  			 Relation rel, Relation pkrel, Oid constraintOid);
  static void createForeignKeyTriggers(Relation rel, FkConstraint *fkconstraint,
  		 Oid constraintOid);
! 

Re: [HACKERS] Reducing some DDL Locks to ShareLock

2008-10-07 Thread Andrew Dunstan



Simon Riggs wrote:


My main focus is on these commands
* CREATE TRIGGER
* ALTER TABLE ..  ADD PRIMARY KEY
* ALTER TABLE ..  ADD FOREIGN KEY

because those are the most painful ones. We could make it work against
more, but we'd need to rewrite lots and lots of catalog update code.


  


Anything that scans the table is a prime candidate. In particular, for 
parallel pg_dump, ALTER TABLE ... ADD UNIQUE is important, as well as 
possibly other table constraints.


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: [PATCHES] [HACKERS] Infrastructure changes for recovery

2008-10-07 Thread Simon Riggs

On Tue, 2008-10-07 at 10:37 -0400, Alvaro Herrera wrote:
 Simon Riggs wrote:
 
  I'm just grumpy because I can't see a way to do the
  patch-on-patch-on-patch that I'll need to make this all work for Nov 1.
  So big patch here we come. But that's just the way it is and I'll stop
  honking about it.
 
 This is one of the problems that DVCSs are supposed to solve ... have
 you tried Git?

I have no doubt there's a better way, but no, I don't know it. My
braintime is devoted to databases not computing per se. Most of the time
that's a good time allocation decision.

Not convinced this is the right time to invest in side activities, but
if you think so, I'll look into it.

Anybody wanting to write or link to a Simon's Guide, most welcome. 

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support


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


Re: [PATCHES] [HACKERS] Infrastructure changes for recovery

2008-10-07 Thread Tom Lane
Robert Haas [EMAIL PROTECTED] writes:
 I'm just grumpy because I can't see a way to do the
 patch-on-patch-on-patch that I'll need to make this all work for Nov 1.
 So big patch here we come. But that's just the way it is and I'll stop
 honking about it.
 
 This is one of the problems that DVCSs are supposed to solve ... have
 you tried Git?

 I think the other problem here is that the difficulty of getting the
 patch landed increases more than linearly with its size.

Yeah.  Tools aren't really the key problem there IMHO.

I agree with Simon that it would be a good thing if this patch got
landed before he went on to the next part.  But personally I'm a bit
burned out from commitfest and am not eager to go take a third look
at the patch right now.  Does anyone else want to review it?

regards, tom lane

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


Re: [HACKERS] Reducing some DDL Locks to ShareLock

2008-10-07 Thread Simon Riggs

On Tue, 2008-10-07 at 10:35 -0400, Tom Lane wrote:
 Simon Riggs [EMAIL PROTECTED] writes:
  On Tue, 2008-10-07 at 10:05 -0400, Robert Haas wrote:
  3. The patch introduces a slight weirdness: if you create two FKs on the
  same column at the same time you end up with two constraints with
  identical names. Drop constraint then removes them both, though in other
  respects they are both valid, just not uniquely. CREATE INDEX avoids
  this by way of the unique index on relname. The equivalent index on
  pg_constraint is not unique, though *cannot* be made unique without
  breaking some corner cases of table inheritance.
  
  Urk... this seems pretty undesirable.
 
  OK, but please say what behaviour you would like in its place. 
 
 I wonder whether this could be helped if we refactored pg_constraint.
 The lack of a useful pkey for it has been annoying me for awhile,
 and I think it stems from a misguided choice to put table and domain
 constraints into the same catalog.  Suppose that
 
 * table constraints go into pg_relation_constraint, with a unique key
 on (conrelid, conname)
 
 * domain constraints go into pg_domain_constraint, with a unique key
 on (contypid, conname)
 
 * pg_constraint can still exist as a union view, for client
 compatibility
 
 Then the unique key would prevent concurrent creation of
 identically-named constraints for the same relation.

Sounds better. Doesn't make much sense as it is now.

 I'm confused by your comment about inheritance --- what cases are
 you thinking this would break?

Well, I made the index unique and got a boat load of errors. I guess I
might have misdiagnosed their cause. I'll have another look.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support


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


Re: [HACKERS] Reducing some DDL Locks to ShareLock

2008-10-07 Thread Tom Lane
Simon Riggs [EMAIL PROTECTED] writes:
 2. Also need to decide whether we want pg_class.reltriggers as int2 (as
 implemented here) or switch to relhastriggers as boolean.

I'd go for changing the column name/type.  Yeah, you will break any
clients that are still trying to manipulate reltriggers directly, but
better to break them obviously than non-obviously.  And I think a silent
change in the column semantics has significant risk of the latter.

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: [PATCHES] [HACKERS] Infrastructure changes for recovery

2008-10-07 Thread Dave Page
On Tue, Oct 7, 2008 at 4:08 PM, Simon Riggs [EMAIL PROTECTED] wrote:

 Not convinced this is the right time to invest in side activities, but
 if you think so, I'll look into it.

 Anybody wanting to write or link to a Simon's Guide, most welcome.

Heikki will be presenting a talk about GIT + PG at PGDay next week.
Might be useful.

-- 
Dave Page
EnterpriseDB UK:   http://www.enterprisedb.com

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


Re: [PATCHES] [HACKERS] Infrastructure changes for recovery

2008-10-07 Thread Heikki Linnakangas

Alvaro Herrera wrote:

Simon Riggs wrote:

I'm just grumpy because I can't see a way to do the
patch-on-patch-on-patch that I'll need to make this all work for Nov 1.
So big patch here we come. But that's just the way it is and I'll stop
honking about it.


This is one of the problems that DVCSs are supposed to solve ... have
you tried Git?


Yeah, I maintained a relforks patch + FSM patch on top of that for quite 
some time using git. It's still a bit of work, for sure, but it's possible.


The bottom line is that hot standby is a big feature, and probably a big 
patch. No amount of version control will work around that. Finishing all 
that in a few weeks is a very ambitious goal. I wish you luck, and I 
wish I could do more to help you with that; it's a feature I'd really 
like to see in 8.4. Given how many iterations just this part of the 
patch needed, and it's still not there, the little project manager in me 
says that we really need to be seeing the complete patch or patches very 
soon, and do a first round of review well before the commit fest. The 
risk that we have to drop it as unfinished in November is very high 
otherwise.


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

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


Re: [HACKERS] Weird behaviour with ALTER TABLE ... SET TABLESPACE ... statement

2008-10-07 Thread Heikki Linnakangas

Tom Lane wrote:

Heikki Linnakangas [EMAIL PROTECTED] writes:
Yeah, seems like we need to allocate a new relfilenode in the new 
tablespace.


I looked into tablecmds.c and verified that ATExecSetTableSpace doesn't
worry about selecting a new relfilenode.  I'm also noticing a number of
permissions-type checks that seem like they'd better be done in
ATPrepSetTableSpace, because we don't go through ATExecSetTableSpace
if the table requires rewriting for other reasons.


The same tests are performed in the rewriting code path in 
ATRewriteTables() and in heap_create_with_catalog().


I fixed the relfilenode allocation in 8.1-HEAD. Doesn't seem worth 
fixing in 8.0, because GetNewRelFileNode() didn't exist before 8.1, so 
we couldn't check for collisions anyway.


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

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


Re: [HACKERS] Weird behaviour with ALTER TABLE ... SET TABLESPACE ... statement

2008-10-07 Thread Guillaume Lelarge
Heikki Linnakangas a écrit :
 Tom Lane wrote:
 Heikki Linnakangas [EMAIL PROTECTED] writes:
 Yeah, seems like we need to allocate a new relfilenode in the new
 tablespace.

 I looked into tablecmds.c and verified that ATExecSetTableSpace doesn't
 worry about selecting a new relfilenode.  I'm also noticing a number of
 permissions-type checks that seem like they'd better be done in
 ATPrepSetTableSpace, because we don't go through ATExecSetTableSpace
 if the table requires rewriting for other reasons.
 
 The same tests are performed in the rewriting code path in
 ATRewriteTables() and in heap_create_with_catalog().
 
 I fixed the relfilenode allocation in 8.1-HEAD. Doesn't seem worth
 fixing in 8.0, because GetNewRelFileNode() didn't exist before 8.1, so
 we couldn't check for collisions anyway.
 

Thanks. It works great!


-- 
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.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] Reducing some DDL Locks to ShareLock

2008-10-07 Thread Tom Lane
Simon Riggs [EMAIL PROTECTED] writes:
 On Tue, 2008-10-07 at 10:35 -0400, Tom Lane wrote:
 I wonder whether this could be helped if we refactored pg_constraint.

 Sounds better. Doesn't make much sense as it is now.

I looked at the code a bit, and it seems the only place where the
current design makes any sense is in ChooseConstraintName, which
explains itself thusly:

 * Select a nonconflicting name for a new constraint.
 *
 * The objective here is to choose a name that is unique within the
 * specified namespace.  Postgres does not require this, but the SQL
 * spec does, and some apps depend on it.  Therefore we avoid choosing
 * default names that so conflict.
 *
 * Note: it is theoretically possible to get a collision anyway, if someone
 * else chooses the same name concurrently.  This is fairly unlikely to be
 * a problem in practice, especially if one is holding an exclusive lock on
 * the relation identified by name1.

(The last bit of the comment falls flat when you consider constraints
on domains...)

Note that this policy is used for system-selected constraint names;
it's not enforced against user-selected names.  We do attempt (in
ConstraintNameIsUsed) to reject duplicate user-selected constraint names
*on the same object*, but that test is not bulletproof against
concurrent additions.  The refactoring I suggested would make for
bulletproof enforcement via the unique indexes.

To preserve the same behavior for system-selected constraint names with
the new design, we'd still need to store namespace OIDs in the two new
tables (I had been thinking those columns would go away), and still have
nonunique indexes on (conname, connamespace), and probe both of the new
catalogs via these indexes to look for a match to a proposed constraint
name.  So that's a bit of a PITA but certainly doable.  Again, it's not
bulletproof against concurrent insertions, but the existing code is not
either.

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: [PATCHES] [HACKERS] Infrastructure changes for recovery

2008-10-07 Thread Simon Riggs

On Tue, 2008-10-07 at 18:19 +0300, Heikki Linnakangas wrote:

 The bottom line is that hot standby is a big feature, and probably a big 
 patch. No amount of version control will work around that. Finishing all 
 that in a few weeks is a very ambitious goal. I wish you luck, and I 
 wish I could do more to help you with that; it's a feature I'd really 
 like to see in 8.4. Given how many iterations just this part of the 
 patch needed, and it's still not there, the little project manager in me 
 says that we really need to be seeing the complete patch or patches very 
 soon, and do a first round of review well before the commit fest. The 
 risk that we have to drop it as unfinished in November is very high 
 otherwise.

Agreed and understood. The coding isn't the problem, its the risk of
being blindsided by a showstopper. Coding is quick when you know exactly
what you are trying to achieve.

That's one reason to strip out the bgwriter stuff. It's the postmaster
state change stuff that's most needed. Anyway, watch this space.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support


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


Re: [HACKERS] Shouldn't pg_settings.enumvals be array of text?

2008-10-07 Thread Greg Smith

On Tue, 7 Oct 2008, Magnus Hagander wrote:


Might this be the time to add an open items for 8.4 page to the wiki?


There's already:

http://wiki.postgresql.org/wiki/Todo:WishlistFor84

Which was aimed at being a live version of that, but was superseded by the 
CommitFest pages.


--
* Greg Smith [EMAIL PROTECTED] http://www.gregsmith.com Baltimore, MD

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


Re: [HACKERS] Shouldn't pg_settings.enumvals be array of text?

2008-10-07 Thread Magnus Hagander
Greg Smith wrote:
 On Tue, 7 Oct 2008, Magnus Hagander wrote:
 
 Might this be the time to add an open items for 8.4 page to the wiki?
 
 There's already:
 
 http://wiki.postgresql.org/wiki/Todo:WishlistFor84
 
 Which was aimed at being a live version of that, but was superseded by
 the CommitFest pages.

wishlist != open items, IMHO.

//Magnus


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


Re: [HACKERS] Shouldn't pg_settings.enumvals be array of text?

2008-10-07 Thread Alvaro Herrera
Greg Smith wrote:
 On Tue, 7 Oct 2008, Magnus Hagander wrote:

 Might this be the time to add an open items for 8.4 page to the wiki?

 There's already:

 http://wiki.postgresql.org/wiki/Todo:WishlistFor84

 Which was aimed at being a live version of that, but was superseded by 
 the CommitFest pages.

No, open items is a list of tasks that must be completed before
release, not a wishlist.  Commitfest is not it either, because
commitfest tracks submitted patches, whereas open items might not have a
patch at all.

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

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


Re: [HACKERS] Transactions and temp tables

2008-10-07 Thread Emmanuel Cecchet

Hi Heikki,

The patch allows preparing any transaction that has dropped the temp 
table, even if it wasn't created in the same transaction. Is that sane?
If you have a temp table created with an 'on commit delete rows' option 
in another transaction, it would be fine to drop it in another 
transaction. If the temp table was created without any on commit option, 
it could only cross prepare commit if it is empty and then it could be 
safely dropped in another transaction. That does not seem to insane for 
me if you need temp tables for a session.
Also, even if the table is created and dropped in the same 
transaction, a subsequent transaction that tries to create and drop 
the table gets blocked on the lock. I suppose we could just say that 
that's the way it works, but I'm afraid it will come as a nasty 
surprise, making the feature a lot less useful.
I do not get that one, if the table is dropped in the transaction the 
lock is released. Why would another transaction be blocked when trying 
to create/drop another temp table?
When I run my test cases (see attached file in previous mail), I 
create/drop multiple times the same temp table in different transactions 
and I do not experience any blocking.
The fixed-size array of temp table oids is an unnecessary limitation. 
A list or hash table would be better.

You are right, I will fix that.
Let me know what you think of the patch and if it could be applied to 
8.3 and 8.4?

Not to 8.3. We only back-patch bug-fixes, and this isn't one.

Ok understood.

Thanks for your feedback and don't hesitate to enlighten me on the 
potential locking issue I did not understand.

Emmanuel

--
Emmanuel Cecchet
FTO @ Frog Thinker 
Open Source Development  Consulting

--
Web: http://www.frogthinker.org
email: [EMAIL PROTECTED]
Skype: emmanuel_cecchet


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


Re: [HACKERS] Reducing some DDL Locks to ShareLock

2008-10-07 Thread Robert Haas
 Urk... this seems pretty undesirable.

 OK, but please say what behaviour you would like in its place.

 Or are you saying you dislike this so much that you would prefer not to
 be able to run ALTER TABLE concurrently?

Personally, yes.  I work mostly with small databases where ease of
management is a lot more important than increased concurrency, and
constraints almost always have unique names but you're not allowed to
rely on that in any queries or code because there is this one wierd
case that you probably will never hit where it might not be true
doesn't sound like ease of management to me.  However, I hope we're
not forced into that choice, because this sounds like a great feature
otherwise.

Robert

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


Re: [HACKERS] Reducing some DDL Locks to ShareLock

2008-10-07 Thread Simon Riggs

On Tue, 2008-10-07 at 11:46 -0400, Tom Lane wrote:
 Simon Riggs [EMAIL PROTECTED] writes:
  On Tue, 2008-10-07 at 10:35 -0400, Tom Lane wrote:
  I wonder whether this could be helped if we refactored pg_constraint.
 
  Sounds better. Doesn't make much sense as it is now.
 
 I looked at the code a bit, and it seems the only place where the
 current design makes any sense is in ChooseConstraintName, which
 explains itself thusly:
 
  * Select a nonconflicting name for a new constraint.
  *
  * The objective here is to choose a name that is unique within the
  * specified namespace.  Postgres does not require this, but the SQL
  * spec does, and some apps depend on it.  Therefore we avoid choosing
  * default names that so conflict.
  *
  * Note: it is theoretically possible to get a collision anyway, if someone
  * else chooses the same name concurrently.  This is fairly unlikely to be
  * a problem in practice, especially if one is holding an exclusive lock on
  * the relation identified by name1.
 
 (The last bit of the comment falls flat when you consider constraints
 on domains...)
 
 Note that this policy is used for system-selected constraint names;
 it's not enforced against user-selected names.  We do attempt (in
 ConstraintNameIsUsed) to reject duplicate user-selected constraint names
 *on the same object*, but that test is not bulletproof against
 concurrent additions.  The refactoring I suggested would make for
 bulletproof enforcement via the unique indexes.
 
 To preserve the same behavior for system-selected constraint names with
 the new design, we'd still need to store namespace OIDs in the two new
 tables (I had been thinking those columns would go away), and still have
 nonunique indexes on (conname, connamespace), and probe both of the new
 catalogs via these indexes to look for a match to a proposed constraint
 name.  So that's a bit of a PITA but certainly doable.  Again, it's not
 bulletproof against concurrent insertions, but the existing code is not
 either.

How about we put a partial unique index on instead?

Dunno if its possible, but the above begins to sound too much froth for
such a small error.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support


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


Re: [HACKERS] Reducing some DDL Locks to ShareLock

2008-10-07 Thread Tom Lane
Simon Riggs [EMAIL PROTECTED] writes:
 How about we put a partial unique index on instead?

We don't currently support partial indexes on system catalogs.
I forget what all would need to be fixed to make it happen, but I'm
pretty sure it's not trivial.  Certainly refactoring pg_constraint
would be a lot easier.

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] Reducing some DDL Locks to ShareLock

2008-10-07 Thread Greg Stark
We can't do partial indexes on system tables. I forget exactly why nut  
if you search for relevant comments it should pop up.


greg

On 7 Oct 2008, at 07:38 PM, Simon Riggs [EMAIL PROTECTED] wrote:



On Tue, 2008-10-07 at 11:46 -0400, Tom Lane wrote:

Simon Riggs [EMAIL PROTECTED] writes:

On Tue, 2008-10-07 at 10:35 -0400, Tom Lane wrote:
I wonder whether this could be helped if we refactored  
pg_constraint.



Sounds better. Doesn't make much sense as it is now.


I looked at the code a bit, and it seems the only place where the
current design makes any sense is in ChooseConstraintName, which
explains itself thusly:

* Select a nonconflicting name for a new constraint.
*
* The objective here is to choose a name that is unique within the
* specified namespace.  Postgres does not require this, but the SQL
* spec does, and some apps depend on it.  Therefore we avoid choosing
* default names that so conflict.
*
* Note: it is theoretically possible to get a collision anyway, if  
someone
* else chooses the same name concurrently.  This is fairly unlikely  
to be
* a problem in practice, especially if one is holding an exclusive  
lock on

* the relation identified by name1.

(The last bit of the comment falls flat when you consider constraints
on domains...)

Note that this policy is used for system-selected constraint names;
it's not enforced against user-selected names.  We do attempt (in
ConstraintNameIsUsed) to reject duplicate user-selected constraint  
names

*on the same object*, but that test is not bulletproof against
concurrent additions.  The refactoring I suggested would make for
bulletproof enforcement via the unique indexes.

To preserve the same behavior for system-selected constraint names  
with
the new design, we'd still need to store namespace OIDs in the two  
new
tables (I had been thinking those columns would go away), and still  
have
nonunique indexes on (conname, connamespace), and probe both of the  
new
catalogs via these indexes to look for a match to a proposed  
constraint
name.  So that's a bit of a PITA but certainly doable.  Again, it's  
not
bulletproof against concurrent insertions, but the existing code is  
not

either.


How about we put a partial unique index on instead?

Dunno if its possible, but the above begins to sound too much froth  
for

such a small error.

--
Simon Riggs   www.2ndQuadrant.com
PostgreSQL Training, Services and Support


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


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


Re: [HACKERS] query path, and rules

2008-10-07 Thread Grzegorz Jaskiewicz


On 2008-10-06, at 20:59, Grzegorz Jaskiewicz wrote:


Hey folks,

I would like to learn more about execution path for a simple query,  
that is going to be changed by a rule. I want to find out, why  
output of 'affected rows' isn't always altered properly in rules  
rewriting inserts and updates.
Can someone give me simple guide on where should I look, what  
functions is a query going through, etc, etc ?


(based on cvs head).

I might say it wrong.
I am looking for someone who could kindly walk me through query  
execution, in head code, especially in regard on how rules are  
applied. Please ?


PGP.sig
Description: This is a digitally signed message part


Re: [HACKERS] Transactions and temp tables

2008-10-07 Thread Emmanuel Cecchet

Heikki,

Here is a new version of the patch using a hash table as you suggested.
I also include the tests that I have added to the regression test suite 
to test the various scenarios.
All patches are based on Postgres 8.3.3, let me know if you want me to 
generate patch for 8.4.


Thanks in advance for your feedback,
Emmanuel


Emmanuel Cecchet wrote:

Hi Heikki,

The patch allows preparing any transaction that has dropped the temp 
table, even if it wasn't created in the same transaction. Is that sane?
If you have a temp table created with an 'on commit delete rows' 
option in another transaction, it would be fine to drop it in another 
transaction. If the temp table was created without any on commit 
option, it could only cross prepare commit if it is empty and then it 
could be safely dropped in another transaction. That does not seem to 
insane for me if you need temp tables for a session.
Also, even if the table is created and dropped in the same 
transaction, a subsequent transaction that tries to create and drop 
the table gets blocked on the lock. I suppose we could just say that 
that's the way it works, but I'm afraid it will come as a nasty 
surprise, making the feature a lot less useful.
I do not get that one, if the table is dropped in the transaction the 
lock is released. Why would another transaction be blocked when trying 
to create/drop another temp table?
When I run my test cases (see attached file in previous mail), I 
create/drop multiple times the same temp table in different 
transactions and I do not experience any blocking.
The fixed-size array of temp table oids is an unnecessary limitation. 
A list or hash table would be better.

You are right, I will fix that.
Let me know what you think of the patch and if it could be applied 
to 8.3 and 8.4?

Not to 8.3. We only back-patch bug-fixes, and this isn't one.

Ok understood.

Thanks for your feedback and don't hesitate to enlighten me on the 
potential locking issue I did not understand.

Emmanuel




--
Emmanuel Cecchet
FTO @ Frog Thinker 
Open Source Development  Consulting

--
Web: http://www.frogthinker.org
email: [EMAIL PROTECTED]
Skype: emmanuel_cecchet


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


Re: [HACKERS] Transactions and temp tables

2008-10-07 Thread David Fetter
On Tue, Oct 07, 2008 at 04:57:37PM -0400, Emmanuel Cecchet wrote:
 Heikki,

 Here is a new version of the patch using a hash table as you
 suggested.  I also include the tests that I have added to the
 regression test suite  to test the various scenarios.  All patches
 are based on Postgres 8.3.3, let me know if you want me to  generate
 patch for 8.4.

CVS TIP is the only place where new features, like this, go :)

I didn't see the attachment anyhow...

Cheers,
David.
-- 
David Fetter [EMAIL PROTECTED] http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: [EMAIL PROTECTED]

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

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


Re: [HACKERS] Transactions and temp tables

2008-10-07 Thread Emmanuel Cecchet

Hi,


On Tue, Oct 07, 2008 at 04:57:37PM -0400, Emmanuel Cecchet wrote:
  

Heikki,

Here is a new version of the patch using a hash table as you
suggested.  I also include the tests that I have added to the
regression test suite  to test the various scenarios.  All patches
are based on Postgres 8.3.3, let me know if you want me to  generate
patch for 8.4.



CVS TIP is the only place where new features, like this, go :)
  
I looked at the Wiki and it looks like I should add en entry (assuming I 
get a patch for the current CVS HEAD) to CommitFest 2008-11. Is that 
correct?

I didn't see the attachment anyhow...
  

Good point! The same with the attachments now!

Thanks,
manu
### Eclipse Workspace Patch 1.0
#P Postgres-8.3.3
Index: src/include/access/xact.h
===
RCS file: /root/cvsrepo/pgsql/src/include/access/xact.h,v
retrieving revision 1.93.2.1
diff -u -r1.93.2.1 xact.h
--- src/include/access/xact.h   4 Mar 2008 19:54:13 -   1.93.2.1
+++ src/include/access/xact.h   7 Oct 2008 20:29:47 -
@@ -18,6 +18,7 @@
 #include nodes/pg_list.h
 #include storage/relfilenode.h
 #include utils/timestamp.h
+#include postgres_ext.h
 
 
 /*
@@ -44,8 +45,8 @@
 /* Asynchronous commits */
 extern bool XactSyncCommit;
 
-/* Kluge for 2PC support */
-extern bool MyXactAccessedTempRel;
+/* List of temp tables accessed during a transaction for 2PC support */
+extern void enlistRelationIdFor2PCChecks(Oid relationId);
 
 /*
  * start- and end-of-transaction callbacks for dynamically loaded modules
Index: src/backend/access/heap/heapam.c
===
RCS file: /root/cvsrepo/pgsql/src/backend/access/heap/heapam.c,v
retrieving revision 1.249.2.2
diff -u -r1.249.2.2 heapam.c
--- src/backend/access/heap/heapam.c8 Mar 2008 21:58:07 -   
1.249.2.2
+++ src/backend/access/heap/heapam.c7 Oct 2008 20:29:47 -
@@ -870,7 +870,7 @@
 
/* Make note that we've accessed a temporary relation */
if (r-rd_istemp)
-   MyXactAccessedTempRel = true;
+   enlistRelationIdFor2PCChecks(relationId);
 
pgstat_initstats(r);
 
@@ -918,7 +918,7 @@
 
/* Make note that we've accessed a temporary relation */
if (r-rd_istemp)
-   MyXactAccessedTempRel = true;
+   enlistRelationIdFor2PCChecks(relationId);
 
pgstat_initstats(r);
 
@@ -968,7 +968,7 @@
 
/* Make note that we've accessed a temporary relation */
if (r-rd_istemp)
-   MyXactAccessedTempRel = true;
+   enlistRelationIdFor2PCChecks(relationId);
 
pgstat_initstats(r);
 
Index: src/backend/access/transam/xact.c
===
RCS file: /root/cvsrepo/pgsql/src/backend/access/transam/xact.c,v
retrieving revision 1.257.2.2
diff -u -r1.257.2.2 xact.c
--- src/backend/access/transam/xact.c   26 Apr 2008 23:35:33 -  
1.257.2.2
+++ src/backend/access/transam/xact.c   7 Oct 2008 20:29:47 -
@@ -62,13 +62,6 @@
 intCommitDelay = 0;/* precommit delay in 
microseconds */
 intCommitSiblings = 5; /* # concurrent xacts needed to 
sleep */
 
-/*
- * MyXactAccessedTempRel is set when a temporary relation is accessed.
- * We don't allow PREPARE TRANSACTION in that case.  (This is global
- * so that it can be set from heapam.c.)
- */
-bool   MyXactAccessedTempRel = false;
-
 
 /*
  * transaction states - transaction state from server perspective
@@ -206,6 +199,9 @@
  */
 static MemoryContext TransactionAbortContext = NULL;
 
+/* Hash table containing Oids of accessed temporary relations */
+HTAB   *accessedTempRel;
+
 /*
  * List of add-on start- and end-of-xact callbacks
  */
@@ -1512,7 +1508,6 @@
XactIsoLevel = DefaultXactIsoLevel;
XactReadOnly = DefaultXactReadOnly;
forceSyncCommit = false;
-   MyXactAccessedTempRel = false;
 
/*
 * reinitialize within-transaction counters
@@ -1777,6 +1772,35 @@
RESUME_INTERRUPTS();
 }
 
+/* 
+ * enlistRelationIdFor2PCChecks - enlist a relation in the list of
+ * resources to check at PREPARE COMMIT time if we are part of
+ * a 2PC transaction. The resource will be removed from the list
+ * if the table is dropped before commit.
+ * 
+ */
+void
+enlistRelationIdFor2PCChecks(Oid relationId)
+{
+   /*
+* Each time a temporary relation is accessed, it is added to the
+* accessedTempRel list. PREPARE TRANSACTION will fail if any
+* of the accessed relation is still valid (not dropped).  (This is
+* called from from heapam.c.)
+*/
+   if (accessedTempRel == NULL)
+   { // Allocate the list on-demand
+   HASHCTL ctl;
+
+   ctl.keysize = sizeof(Oid);
+   ctl.entrysize = sizeof(Oid);
+   

Re: [HACKERS] Transactions and temp tables

2008-10-07 Thread David Fetter
On Tue, Oct 07, 2008 at 06:12:14PM -0400, Emmanuel Cecchet wrote:
 Hi,

 On Tue, Oct 07, 2008 at 04:57:37PM -0400, Emmanuel Cecchet wrote:
   
 Heikki,

 Here is a new version of the patch using a hash table as you
 suggested.  I also include the tests that I have added to the
 regression test suite  to test the various scenarios.  All patches
 are based on Postgres 8.3.3, let me know if you want me to
 generate patch for 8.4.

 CVS TIP is the only place where new features, like this, go :)
   
 I looked at the Wiki and it looks like I should add en entry
 (assuming I  get a patch for the current CVS HEAD) to CommitFest
 2008-11. Is that  correct?
 
 I didn't see the attachment anyhow...
   
 Good point! The same with the attachments now!

Perhaps we did not make this clear.  The patch is a new feature.  New
features are not going into 8.3, as it has already been released.

Make a patch against CVS TIP aka 8.4, and re-submit.

Cheers,
David
-- 
David Fetter [EMAIL PROTECTED] http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: [EMAIL PROTECTED]

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

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


Re: [HACKERS] query path, and rules

2008-10-07 Thread Robert Haas
This is not a support list.  Sounds like you should consider
purchasing a commercial support contract, or you could try asking on
pgsql-general.

...Robert

On Tue, Oct 7, 2008 at 4:30 PM, Grzegorz Jaskiewicz [EMAIL PROTECTED] wrote:

 On 2008-10-06, at 20:59, Grzegorz Jaskiewicz wrote:

 Hey folks,

 I would like to learn more about execution path for a simple query, that
 is going to be changed by a rule. I want to find out, why output of
 'affected rows' isn't always altered properly in rules rewriting inserts and
 updates.
 Can someone give me simple guide on where should I look, what functions is
 a query going through, etc, etc ?

 (based on cvs head).

 I might say it wrong.
 I am looking for someone who could kindly walk me through query execution,
 in head code, especially in regard on how rules are applied. Please ?


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


Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches

2008-10-07 Thread KaiGai Kohei

KaiGai Kohei wrote:

Bruce Momjian wrote:

  2) Do we want row-level permissions at the SQL level?

Now I'm working for it and will submit patches due to the end of Oct,
if it is really required to make progress reviewing of SE-PostgreSQL
on the v8.4 development cycle.
However, the scale of its demand is unclear for me.


Yes, which is why I would like the community to answer the question
before you have to start coding things.  I will say that if we do want
it, the SE-Linux code will be 96% in separate modules and will make it
much easier to accept.


OK, I'll stop my hand today, and wait for pgsql-hacker's opinions.


I want to confirm the conclusion of this issue.

If I can understand correctly, we don't have a clear conclusion of this.
So, it is unclear for me whether the row-level permission at SQL level is
necessary to progress the reviewing process of SE-PostgreSQL, or not.

Please give me any suggestion.

Thanks,
--
KaiGai Kohei [EMAIL PROTECTED]

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


[HACKERS] Better error message for a small problem with WITH RECURSIVE

2008-10-07 Thread Tom Lane
Quick, what's wrong with this query?

regression=# with q(x) as (select 1 union all select x+1 from q where x10)
regression-# select * from q;
ERROR:  relation q does not exist
LINE 1: with q(x) as (select 1 union all select x+1 from q where x1...
 ^

The problem is that I forgot to say RECURSIVE.  But the error message
is certainly pretty unhelpful.  The code is following the SQL spec rule,
which says that for a non-recursive WITH query, the query's name isn't
in scope until after you've parsed it.  So you get does not exist
rather than something that would clue you in.

I've made this same mistake at least once a day for the past week,
and taken an unreasonable amount of time to figure it out each time :-(
So I think we need a better error message here.

We can't just monkey with the scope rules, because that could change
the meaning of queries that *are* valid, eg a query name could be a
reference to some outer-level WITH.  (Of course you'd have to be pretty
nuts to use the same query name at multiple levels of a single SELECT,
and even more nuts to arrange things so that the intended reference is
not the most closely nested one, but spec is spec.)  What we can do is
keep a list of not yet parsed WITH-names in ParseState, and check
through that list when about to fail for relation-not-found, and issue
a suitable message hinting that maybe you forgot RECURSIVE if we find
a match.

I would think this is overkill, except I've made the same darn mistake
one time too many.  It seems clear to me that a lot of other people will
make it too, and if the error message isn't more helpful a lot of time
will get wasted.  Barring loud objections, I'm gonna go change it.

regards, tom lane

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


Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches

2008-10-07 Thread Josh Berkus
KaiGai,

 If I can understand correctly, we don't have a clear conclusion of this.
 So, it is unclear for me whether the row-level permission at SQL level
 is necessary to progress the reviewing process of SE-PostgreSQL, or not.

Can you *do* the row-level permission?

-- 
--Josh

Josh Berkus
PostgreSQL
San Francisco

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


[HACKERS] parallel restore - latest WIP patch

2008-10-07 Thread Andrew Dunstan


Attached is the latest version of my parallel restore work.

In this version pg_dump has learned how to tag each item with the 
section it belongs to (data, pre-data, post-data). That removes the 
necessity for hardcoding knowledge about the boundary of the data 
section into the parallel restore routine. There is some legacy 
knowledge built into the archiver routines that means parallel restore 
can be supported on older archives.


Hardcoded avoidance of AccessExclusive lock contention is still there, 
although it might be made redundant if Simon's proposal for reducing 
certain lock strengths is accepted.


Dependencies are finally treated correctly, I believe (after many nasty 
debugging sessions).


This version seems to have the same performance characteristics as 
previously reported. It appears that the sweet spot is around the number 
of available CPUs. I did several runs using three times the number I had 
available (24 threads on 8 CPUs), and while there were no errors, 
performance did fall off. Not at all surprising.


I have not done anything yet about alternative item selection 
algorithms. There doesn't seem to have been much interest - perhaps the 
current algoithm will suffice for now.


cheers

andrew


parallel_restore_5.patch.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


Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches

2008-10-07 Thread KaiGai Kohei

Josh Berkus wrote:

KaiGai,


If I can understand correctly, we don't have a clear conclusion of this.
So, it is unclear for me whether the row-level permission at SQL level
is necessary to progress the reviewing process of SE-PostgreSQL, or not.


Can you *do* the row-level permission?


I think it is a tough work to complete within three weeks.

I may be able to make a patch, but I guess its qualities (number of bugs,
documentation, test cases and so on) are far from SE-PostgreSQL level.

Thanks,
--
KaiGai Kohei [EMAIL PROTECTED]

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


Re: [HACKERS] query path, and rules

2008-10-07 Thread Alvaro Herrera

  On 2008-10-06, at 20:59, Grzegorz Jaskiewicz wrote:
 
  Hey folks,
 
  I would like to learn more about execution path for a simple query, that
  is going to be changed by a rule. I want to find out, why output of
  'affected rows' isn't always altered properly in rules rewriting inserts 
  and
  updates.
  Can someone give me simple guide on where should I look, what functions is
  a query going through, etc, etc ?

Robert Haas escribió:
 This is not a support list.  Sounds like you should consider
 purchasing a commercial support contract, or you could try asking on
 pgsql-general.

Actually I find this to be a perfectly acceptable question for this
list.  ISTM the answer, however, is to have a look at the documentation
we have already in place ... perhaps starting with the Developer's FAQ
at http://wiki.postgresql.org/wiki/Developer_FAQ  In particular, the
question How is the source code organized?

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

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


Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches

2008-10-07 Thread Robert Haas
 Can you *do* the row-level permission?

I don't think there's any consensus on a design.

Getting consensus on a design seems to actually be one of the harder
aspects of PostgreSQL development.  The pattern seems to be:

1. Someone submits a patch.  By definition, the patch embodies some
particular design.
2. One or more members of core express concerns about the design
embodied in the patch.
3. If the concerns are fairly specific and not mutually contradictory,
the submitter revises the patch accordingly and it gets committed
(hopefully without having to discard too much of their previous work).
4. If the concerns are fairly vague and open-ended, or if they
contract each other, the patch hovers in limbo for eternity.

I'm not sure what the solution to this problem is.  The fact that a
member of core does not like a particular design for feature X does
not oblige that person to produce a better one, but it's pretty tough
to proceed without it.

In the case of SE-PostgreSQL, two members of core expressed concerns:

- Peter Eisentraut expressed concern about implementing row-level
security via SE-PostgreSQL without first implementing some sort of
SQL-based row-level security.  KaiGai expressed willingness to
implement that, but we still don't have a design, so I assume KaiGai
is going to implement... something... and hope that it turns out to be
acceptable.

- Tom Lane expressed concerns about covert channels to which KaiGai
responded, AIUI, that the patch was pretty useful even without
addressing that issue, but if Tom isn't willing to see the patch
committed otherwise, then KaiGai has to decide between giving up and
attempting to implement some form of polyinstantiation - which will
not be easy, and which is far from guaranteed to be accepted if he
does develop it.

In both cases, the lack of a clear roadmap means that a dedicated
developer is potentially putting in a lot of time and effort
developing what may end up being a complete dead-end.

...Robert

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


[HACKERS] WITH RECURSIVE ... CYCLE in vanilla SQL: issues with arrays of rows

2008-10-07 Thread Tom Lane
I looked a bit at the SQL:2008 spec for a CYCLE clause for WITH
RECURSIVE.  It is interesting to see that it is just syntactic sugar,
because *they spell out how to expand it into regular SQL*.  More,
they defined it in such a way that it's hard to optimize at all,
because the path column is exposed to the user; you don't really
have any choice about how to do it.  There are some ugly and unnecessary
choices in there too, like insisting that the cycle mark column be
char(1).

So I am not feeling very excited about implementing the syntax per se
(and I note that DB2 doesn't seem to have done so either).  Instead
we should document some examples of how to do cycle detection at the
SQL level.  However, it would be nice if the spec's approach to cycle
detection actually worked well in Postgres.  There are a couple of
things we seem to be missing, according to some experiments I just
did with trying to translate the spec's code into Postgres:

* The spec assumes that ARRAY[ROW(some columns)] works, ie, that you can
have an array of an anonymous record type.  We don't allow that right
now, but it seems like a useful thing to have --- at least as a
transient value within a query.  I'm not sure there's a case for
allowing such things to go to disk.

* The spec writes this to detect whether a row of an anonymous record
type is present in an array of that same anonymous record type:
ROW(some columns) IN (SELECT P.* FROM TABLE(array variable) P) 
We haven't got the TABLE() syntax; you can sort of emulate it with a SRF
but only for arrays of named rowtypes.  For an anonymous rowtype,
it's very unclear to me how the rowtype would be communicated at
parse time so that the P.* notation could be expanded properly.

* Instead of the above, we could try to make
ROW(some columns) = ANY (array variable)
work.  This is shorter than the above syntax and would presumably have
a lot less overhead too.  But it doesn't work right now, not even for
named rowtypes much less anonymous ones.

I'm thinking that addressing these pieces would be a generally good
thing to do, above and beyond potential uses in recursive queries.

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] Transactions and temp tables

2008-10-07 Thread Emmanuel Cecchet

Hi,

Here are the patches for 8.4 (1 patch for the code and 1 patch for the 
regression tests).


Thanks in advance for your feedback,
Emmanuel

David Fetter wrote:

On Tue, Oct 07, 2008 at 06:12:14PM -0400, Emmanuel Cecchet wrote:
  

Hi,



On Tue, Oct 07, 2008 at 04:57:37PM -0400, Emmanuel Cecchet wrote:
  
  

Heikki,

Here is a new version of the patch using a hash table as you
suggested.  I also include the tests that I have added to the
regression test suite  to test the various scenarios.  All patches
are based on Postgres 8.3.3, let me know if you want me to
generate patch for 8.4.


CVS TIP is the only place where new features, like this, go :)
  
  

I looked at the Wiki and it looks like I should add en entry
(assuming I  get a patch for the current CVS HEAD) to CommitFest
2008-11. Is that  correct?



I didn't see the attachment anyhow...
  
  

Good point! The same with the attachments now!



Perhaps we did not make this clear.  The patch is a new feature.  New
features are not going into 8.3, as it has already been released.

Make a patch against CVS TIP aka 8.4, and re-submit.

Cheers,
David
  


--
Emmanuel Cecchet
FTO @ Frog Thinker 
Open Source Development  Consulting

--
Web: http://www.frogthinker.org
email: [EMAIL PROTECTED]
Skype: emmanuel_cecchet

### Eclipse Workspace Patch 1.0
#P Postgres-HEAD
Index: src/test/regress/parallel_schedule
===
RCS file: /root/cvsrepo/pgsql/src/test/regress/parallel_schedule,v
retrieving revision 1.46
diff -u -r1.46 parallel_schedule
--- src/test/regress/parallel_schedule  24 Nov 2007 19:49:23 -  1.46
+++ src/test/regress/parallel_schedule  7 Oct 2008 23:41:14 -
@@ -90,3 +90,11 @@
 
 # run tablespace by itself
 test: tablespace
+
+# 
+# 2PC related tests
+# 
+test: 2pc_persistent_table 2pc_on_delete_rows_transaction 2pc_on_commit_drop 
2pc_temp_table_transaction 2pc_temp_table_rollback 2pc_temp_table_savepoint 
2pc_temp_table_failure
+test: 2pc_dirty_buffer_check 
+test: 2pc_temp_table_session
+test: 2pc_on_delete_rows_session
\ No newline at end of file
Index: src/test/regress/serial_schedule
===
RCS file: /root/cvsrepo/pgsql/src/test/regress/serial_schedule,v
retrieving revision 1.43
diff -u -r1.43 serial_schedule
--- src/test/regress/serial_schedule24 Nov 2007 20:41:35 -  1.43
+++ src/test/regress/serial_schedule7 Oct 2008 23:41:14 -
@@ -115,3 +115,13 @@
 test: xml
 test: stats
 test: tablespace
+test: 2pc_persistent_table
+test: 2pc_on_delete_rows_transaction
+test: 2pc_on_commit_drop
+test: 2pc_temp_table_transaction
+test: 2pc_temp_table_rollback
+test: 2pc_temp_table_savepoint
+test: 2pc_dirty_buffer_check 
+test: 2pc_temp_table_session
+test: 2pc_on_delete_rows_session
+test: 2pc_temp_table_failure
\ No newline at end of file
Index: src/test/regress/sql/2pc_on_delete_rows_transaction.sql
===
RCS file: src/test/regress/sql/2pc_on_delete_rows_transaction.sql
diff -N src/test/regress/sql/2pc_on_delete_rows_transaction.sql
--- /dev/null   1 Jan 1970 00:00:00 -
+++ src/test/regress/sql/2pc_on_delete_rows_transaction.sql 1 Jan 1970 
00:00:00 -
@@ -0,0 +1,7 @@
+-- Temp table with ON DELETE ROWS option (transaction scope)
+begin;
+create temp table foo(x int) on commit delete rows;
+insert into foo values(1);
+prepare transaction 'onCommitDeleteRowsTempTableShouldSucceed';
+commit prepared 'onCommitDeleteRowsTempTableShouldSucceed';
+drop table foo;
Index: src/test/regress/expected/2pc_on_commit_drop.out
===
RCS file: src/test/regress/expected/2pc_on_commit_drop.out
diff -N src/test/regress/expected/2pc_on_commit_drop.out
--- /dev/null   1 Jan 1970 00:00:00 -
+++ src/test/regress/expected/2pc_on_commit_drop.out1 Jan 1970 00:00:00 
-
@@ -0,0 +1,6 @@
+-- Temp table with ON COMMIT DROP option
+begin;
+create temp table foo(x int) on commit drop;
+insert into foo values(1);
+prepare transaction 'onCommitDropTempTableShouldSucceed';
+commit prepared 'onCommitDropTempTableShouldSucceed';
Index: src/test/regress/expected/2pc_temp_table_transaction.out
===
RCS file: src/test/regress/expected/2pc_temp_table_transaction.out
diff -N src/test/regress/expected/2pc_temp_table_transaction.out
--- /dev/null   1 Jan 1970 00:00:00 -
+++ src/test/regress/expected/2pc_temp_table_transaction.out1 Jan 1970 
00:00:00 -
@@ -0,0 +1,7 @@
+-- Transaction-scope dropped temp table use case
+begin;
+create temp table foo(x int);
+insert into foo values(1);
+drop table foo;
+prepare transaction 'dropTempTableShouldSucceed';
+commit prepared 'dropTempTableShouldSucceed';
Index: src/test/regress/sql/2pc_temp_table_failure.sql

Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches

2008-10-07 Thread KaiGai Kohei

Robert Haas wrote:

Can you *do* the row-level permission?


I don't think there's any consensus on a design.


Yes, unfortunatelly.
No one replied to my proposed design:
  http://marc.info/?l=pgsql-hackersm=12470930544w=2


Getting consensus on a design seems to actually be one of the harder
aspects of PostgreSQL development.  The pattern seems to be:

1. Someone submits a patch.  By definition, the patch embodies some
particular design.
2. One or more members of core express concerns about the design
embodied in the patch.
3. If the concerns are fairly specific and not mutually contradictory,
the submitter revises the patch accordingly and it gets committed
(hopefully without having to discard too much of their previous work).
4. If the concerns are fairly vague and open-ended, or if they
contract each other, the patch hovers in limbo for eternity.

I'm not sure what the solution to this problem is.  The fact that a
member of core does not like a particular design for feature X does
not oblige that person to produce a better one, but it's pretty tough
to proceed without it.


Yes, I have had similar experiences in Linux kernel development for
several years.
The amount of time to get consensus is one of the reason why I want
to move the SQL based row-level security to v8.5 development cycle.


In the case of SE-PostgreSQL, two members of core expressed concerns:

- Peter Eisentraut expressed concern about implementing row-level
security via SE-PostgreSQL without first implementing some sort of
SQL-based row-level security.  KaiGai expressed willingness to
implement that, but we still don't have a design, so I assume KaiGai
is going to implement... something... and hope that it turns out to be
acceptable.


Its conclusion is actually not clear for me.
The proposed SQL based row-level security assumes what he pointed out
is right, but I don't know the reason why it should be placed prior.

I guess he intend to share the code for row-level security, thus it
should be implemented prior to the OS specific feature. But most of
SE-PostgreSQL code is separated from core implementation and invoked
via security hooks, so it is unsuitable to share code with other
facilities.

In my preference, I want to pay my efforts for OS-independent row-level
security in the v8.5 cycle with enough days, and want to concentrate for
SE-PostgreSQL in the v8.4 cycle, as I mentioned before.
But I will do, if it is unacceptable.


- Tom Lane expressed concerns about covert channels to which KaiGai
responded, AIUI, that the patch was pretty useful even without
addressing that issue, but if Tom isn't willing to see the patch
committed otherwise, then KaiGai has to decide between giving up and
attempting to implement some form of polyinstantiation - which will
not be easy, and which is far from guaranteed to be accepted if he
does develop it.


Its conclusion it not clear.

I think the polyinstantiation is too much requirements, as prior major
commercial RDBMS with row-level security (like Oracle Label Security)
does not care about such kind of covert channels.
It is quite natural to describe such kind of limitation/specification
on the documentation explicitly to help end-user's decision.


In both cases, the lack of a clear roadmap means that a dedicated
developer is potentially putting in a lot of time and effort
developing what may end up being a complete dead-end.


I believe such a situation will help nobody get happy.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei [EMAIL PROTECTED]

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


Re: [HACKERS] About postgresql8.3.3 build in MS VS2005

2008-10-07 Thread iihero
Thanks. Now the header file include issues resolved. I fetch the latest code
and no such issues.

But I found new issues now.  (the latest code from cvs)
1. file : contrib\fuzzystrmatch\dmetaphone.c,
  line: 1040 and line: 464, both look like as below,
  case '?:
  There is no the matched single quote, and the content is repeated. This
cause build always failed for fuzzystrmatch.

2. I can't find the debug version libraries of libxml and libxslt, zlib,
iconv.  And failed to build it on windows platform respectively.
Thus I can't build the debug version of postgresql8.4dev.

Could the depend libraries (release and debug version) and header files be
tar balled in a place?

Is there any way?



2008/10/6 Tom Lane [EMAIL PROTECTED]

 Magnus Hagander [EMAIL PROTECTED] writes:
  Do you mean the include order of Platform SDK possibly influenced the
  build result?

  it could be. At first look ISTM that the error comes from not finding
  the definition of HANDLE, which is certainly very central to the
  Platform SDK.

 You should be looking at the first few errors.  I'm suspicious that a
 system include file isn't being found where expected.  Some compilers
 will bull ahead anyway, but a huge number of consequent errors can be
 expected if the needed declarations haven't been read ...

regards, tom lane



Re: [HACKERS] Better error message for a small problem with WITH RECURSIVE

2008-10-07 Thread Gregory Stark

Tom Lane [EMAIL PROTECTED] writes:

 What we can do is keep a list of not yet parsed WITH-names in ParseState,
 and check through that list when about to fail for relation-not-found, and
 issue a suitable message hinting that maybe you forgot RECURSIVE if we find
 a match.

 I would think this is overkill, except I've made the same darn mistake
 one time too many.  It seems clear to me that a lot of other people will
 make it too, and if the error message isn't more helpful a lot of time
 will get wasted.  Barring loud objections, I'm gonna go change it.

Perhaps it would be sufficient to just check if we're inside a non-recursive
WITH without bothering to check if the name matches?

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Ask me about EnterpriseDB's Slony Replication support!

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


Re: [HACKERS] query path, and rules

2008-10-07 Thread Grzegorz Jaskiewicz


On 2008-10-08, at 01:57, Alvaro Herrera wrote:




Actually I find this to be a perfectly acceptable question for this
list.  ISTM the answer, however, is to have a look at the  
documentation

we have already in place ... perhaps starting with the Developer's FAQ
at http://wiki.postgresql.org/wiki/Developer_FAQ  In particular, the
question How is the source code organized?


Thanks, I'll read that through, and next time I'll just try to be more  
specific :)




PGP.sig
Description: This is a digitally signed message part