Hi guys,

I've been looking at CLUSTER today. I've put together a patch which
recreates all the indices which current CLUSTER drops and copies relacl
from the old pg_class tuple and puts it in the new one. 

I was working on updating pg_inherits to handle the new OID when it
occured to me that pg_inherits is not the only system table
corrupted. pg_trigger, pg_rewrite (and therefore views) and pg_description
need to be updated as well. It seems like the easiest thing to do would be
to update the new relation to have to OID of the old relation. Is there
any reason why we would not want to do this?

Gavin


Sun, 23 Sep 2001 22:59:42 +1000 (EST)
Gavin Sherry <swm@linuxworld.com.au>
CLUSTER TODO item
> Hi guys,
> 
> I've been looking at CLUSTER today. I've put together a patch which
> recreates all the indices which current CLUSTER drops and copies relacl
> from the old pg_class tuple and puts it in the new one. 
> 
> I was working on updating pg_inherits to handle the new OID when it
> occured to me that pg_inherits is not the only system table
> corrupted. pg_trigger, pg_rewrite (and therefore views) and pg_description
> need to be updated as well. It seems like the easiest thing to do would be
> to update the new relation to have to OID of the old relation. Is there
> any reason why we would not want to do this?

We did strange things with this in the past because we didn't have
pg_class.relfilenode.  Now that we do, you can just recreate the heap
table using a different oid filename and update relfilenode when you are
done.  Keep the same oid.  Of course, as you noted, the old indexes have
to be recreated because the heap has changed.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Sun, 23 Sep 2001 09:58:47 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: CLUSTER TODO item
>> I've been looking at CLUSTER today. I've put together a patch which
>> recreates all the indices which current CLUSTER drops and copies relacl
>> from the old pg_class tuple and puts it in the new one. 

This is entirely the wrong way to go at it.

> We did strange things with this in the past because we didn't have
> pg_class.relfilenode.  Now that we do, you can just recreate the heap
> table using a different oid filename and update relfilenode when you are
> done.

Yes.  CLUSTER should not do one single thing to the system catalogs,
except heap_update the pg_class rows for the table and indexes with
new relfilenode values.  That is the facility that a rewrite of CLUSTER
was waiting for, so now that we have it, it's pointless to put more
work into the old CLUSTER implementation.

Note: I'm not convinced that relfilenode and pg_class.oid are each
used in exactly the right spots.  Once we have cases where they can
differ, we may well find some bugs to flush out.  But that needs to
happen anyway, so don't let it dissuade you from doing CLUSTER the
right way.

			regards, tom lane

Sun, 23 Sep 2001 17:04:34 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: CLUSTER TODO item 
On Sun, 23 Sep 2001, Tom Lane wrote:
> 
> Note: I'm not convinced that relfilenode and pg_class.oid are each
> used in exactly the right spots.  Once we have cases where they can
> differ, we may well find some bugs to flush out.  But that needs to
> happen anyway, so don't let it dissuade you from doing CLUSTER the
> right way.

I think I may have broken stuff. I'm not sure. I've fiddled a fair bit but
I'm still segfaulting in the storage manager. It might be because I'm
heap_creating and then just stealing relfilenode - I don't know.

Anyway, a patch is attached which clusters and then recreates the indexes
- but then segfaults.

Am I going about this all wrong?

Thanks

Gavin
Mon, 24 Sep 2001 15:19:10 +1000 (EST)
Gavin Sherry <swm@linuxworld.com.au>
Re: CLUSTER TODO item 

Can I get a status on this?


> On Sun, 23 Sep 2001, Tom Lane wrote:
> > 
> > Note: I'm not convinced that relfilenode and pg_class.oid are each
> > used in exactly the right spots.  Once we have cases where they can
> > differ, we may well find some bugs to flush out.  But that needs to
> > happen anyway, so don't let it dissuade you from doing CLUSTER the
> > right way.
> 
> I think I may have broken stuff. I'm not sure. I've fiddled a fair bit but
> I'm still segfaulting in the storage manager. It might be because I'm
> heap_creating and then just stealing relfilenode - I don't know.
> 
> Anyway, a patch is attached which clusters and then recreates the indexes
> - but then segfaults.
> 
> Am I going about this all wrong?
> 
> Thanks
> 
> Gavin

Content-Description: 

[ Attachment, skipping... ]

> 
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
> 
> http://www.postgresql.org/users-lounge/docs/faq.html

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Thu, 11 Oct 2001 16:37:33 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: CLUSTER TODO item
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Can I get a status on this?

It's not gonna happen for 7.2, I think ...

			regards, tom lane

Thu, 11 Oct 2001 20:10:38 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: CLUSTER TODO item 
On Thu, 11 Oct 2001, Tom Lane wrote:

> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Can I get a status on this?
> 
> It's not gonna happen for 7.2, I think ...
> 
> 			regards, tom lane

I'd love it to go out with 7.2 but I've had no time to work on this patch
lately. The reason I need time is that, after having fiddled a fair bit,
I've decided that there really needs to be support for the creation of a
new relfilenode in the storage manager.

The current patch works like this:

Create new heap (heap_create())
Copy tuples from old heap to new heap via index scan
Form a new pg_class tuple
simple_heap_update()
update catalogue indices
rebuild existing indices

This causes an overflow in the localbuf.c so I guess this is wrong
(included in patch on 24/sep) =). I've looked at various combinations of
this:

memcpy() the old Relation into a new Relation, update smgrunlink() the old
Relation and heap_storage_create() the new relation. This dies because
smgrunlink only schedules the drop, where as heap_storage_create() actually 
creates a file on the file system (open() returns with EEXIST).

I've also tried just copying the structure but heap_open() relies on OID
not relfilenode.

I'm probably going about it the wrong way, but it strikes me that there
needs to be a way to abstract the relfilenode from OID in the heap access 
code so that one can easily manipulate the heap on disk without having to 
play with OIDs. 

I would have included code examples/clearer description but the box I
don't have access to the box I created the patches on atm =(.

Gavin


Fri, 12 Oct 2001 13:35:30 +1000 (EST)
Gavin Sherry <swm@linuxworld.com.au>
Re: CLUSTER TODO item 

Is there an updated version of this patch for 7.3?

---------------------------------------------------------------------------

Gavin Sherry wrote:
> On Thu, 11 Oct 2001, Tom Lane wrote:
> 
> > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > Can I get a status on this?
> > 
> > It's not gonna happen for 7.2, I think ...
> > 
> > 			regards, tom lane
> 
> I'd love it to go out with 7.2 but I've had no time to work on this patch
> lately. The reason I need time is that, after having fiddled a fair bit,
> I've decided that there really needs to be support for the creation of a
> new relfilenode in the storage manager.
> 
> The current patch works like this:
> 
> Create new heap (heap_create())
> Copy tuples from old heap to new heap via index scan
> Form a new pg_class tuple
> simple_heap_update()
> update catalogue indices
> rebuild existing indices
> 
> This causes an overflow in the localbuf.c so I guess this is wrong
> (included in patch on 24/sep) =). I've looked at various combinations of
> this:
> 
> memcpy() the old Relation into a new Relation, update smgrunlink() the old
> Relation and heap_storage_create() the new relation. This dies because
> smgrunlink only schedules the drop, where as heap_storage_create() actually 
> creates a file on the file system (open() returns with EEXIST).
> 
> I've also tried just copying the structure but heap_open() relies on OID
> not relfilenode.
> 
> I'm probably going about it the wrong way, but it strikes me that there
> needs to be a way to abstract the relfilenode from OID in the heap access 
> code so that one can easily manipulate the heap on disk without having to 
> play with OIDs. 
> 
> I would have included code examples/clearer description but the box I
> don't have access to the box I created the patches on atm =(.
> 
> Gavin
> 
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster
> 

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Fri, 22 Feb 2002 11:15:11 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: CLUSTER TODO item

Gavin, do you have an updated patch you want applied to 7.3?


---------------------------------------------------------------------------

Gavin Sherry wrote:
> On Thu, 11 Oct 2001, Tom Lane wrote:
> 
> > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > Can I get a status on this?
> > 
> > It's not gonna happen for 7.2, I think ...
> > 
> > 			regards, tom lane
> 
> I'd love it to go out with 7.2 but I've had no time to work on this patch
> lately. The reason I need time is that, after having fiddled a fair bit,
> I've decided that there really needs to be support for the creation of a
> new relfilenode in the storage manager.
> 
> The current patch works like this:
> 
> Create new heap (heap_create())
> Copy tuples from old heap to new heap via index scan
> Form a new pg_class tuple
> simple_heap_update()
> update catalogue indices
> rebuild existing indices
> 
> This causes an overflow in the localbuf.c so I guess this is wrong
> (included in patch on 24/sep) =). I've looked at various combinations of
> this:
> 
> memcpy() the old Relation into a new Relation, update smgrunlink() the old
> Relation and heap_storage_create() the new relation. This dies because
> smgrunlink only schedules the drop, where as heap_storage_create() actually 
> creates a file on the file system (open() returns with EEXIST).
> 
> I've also tried just copying the structure but heap_open() relies on OID
> not relfilenode.
> 
> I'm probably going about it the wrong way, but it strikes me that there
> needs to be a way to abstract the relfilenode from OID in the heap access 
> code so that one can easily manipulate the heap on disk without having to 
> play with OIDs. 
> 
> I would have included code examples/clearer description but the box I
> don't have access to the box I created the patches on atm =(.
> 
> Gavin
> 
> 

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Fri, 22 Feb 2002 13:06:22 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: CLUSTER TODO item
Hi Bruce,

I put this on hold since I sent that email. This is basically because the
API doesn't lend itself well to attaching new relfilenodes to existing
heaps. I definately want to put this patch in but just need to find time
to modify the API to support this correctly.

Gavin


On Fri, 22 Feb 2002, Bruce Momjian wrote:

> 
> Is there an updated version of this patch for 7.3?
> 
> ---------------------------------------------------------------------------
> 
> Gavin Sherry wrote:
> > On Thu, 11 Oct 2001, Tom Lane wrote:
> > 
> > > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > > Can I get a status on this?
> > > 
> > > It's not gonna happen for 7.2, I think ...
> > > 
> > > 			regards, tom lane
> > 
> > I'd love it to go out with 7.2 but I've had no time to work on this patch
> > lately. The reason I need time is that, after having fiddled a fair bit,
> > I've decided that there really needs to be support for the creation of a
> > new relfilenode in the storage manager.
> > 
> > The current patch works like this:
> > 
> > Create new heap (heap_create())
> > Copy tuples from old heap to new heap via index scan
> > Form a new pg_class tuple
> > simple_heap_update()
> > update catalogue indices
> > rebuild existing indices
> > 
> > This causes an overflow in the localbuf.c so I guess this is wrong
> > (included in patch on 24/sep) =). I've looked at various combinations of
> > this:
> > 
> > memcpy() the old Relation into a new Relation, update smgrunlink() the old
> > Relation and heap_storage_create() the new relation. This dies because
> > smgrunlink only schedules the drop, where as heap_storage_create() actually 
> > creates a file on the file system (open() returns with EEXIST).
> > 
> > I've also tried just copying the structure but heap_open() relies on OID
> > not relfilenode.
> > 
> > I'm probably going about it the wrong way, but it strikes me that there
> > needs to be a way to abstract the relfilenode from OID in the heap access 
> > code so that one can easily manipulate the heap on disk without having to 
> > play with OIDs. 
> > 
> > I would have included code examples/clearer description but the box I
> > don't have access to the box I created the patches on atm =(.
> > 
> > Gavin
> > 
> > 
> > ---------------------------(end of broadcast)---------------------------
> > TIP 4: Don't 'kill -9' the postmaster
> > 
> 
> 


Sat, 23 Feb 2002 12:26:12 +1100 (EST)
Gavin Sherry <swm@linuxworld.com.au>
Re: CLUSTER TODO item

Gavin, please continue working on this feature and continue discussion
the hackers list.

---------------------------------------------------------------------------

Gavin Sherry wrote:
> On Thu, 11 Oct 2001, Tom Lane wrote:
> 
> > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > Can I get a status on this?
> > 
> > It's not gonna happen for 7.2, I think ...
> > 
> > 			regards, tom lane
> 
> I'd love it to go out with 7.2 but I've had no time to work on this patch
> lately. The reason I need time is that, after having fiddled a fair bit,
> I've decided that there really needs to be support for the creation of a
> new relfilenode in the storage manager.
> 
> The current patch works like this:
> 
> Create new heap (heap_create())
> Copy tuples from old heap to new heap via index scan
> Form a new pg_class tuple
> simple_heap_update()
> update catalogue indices
> rebuild existing indices
> 
> This causes an overflow in the localbuf.c so I guess this is wrong
> (included in patch on 24/sep) =). I've looked at various combinations of
> this:
> 
> memcpy() the old Relation into a new Relation, update smgrunlink() the old
> Relation and heap_storage_create() the new relation. This dies because
> smgrunlink only schedules the drop, where as heap_storage_create() actually 
> creates a file on the file system (open() returns with EEXIST).
> 
> I've also tried just copying the structure but heap_open() relies on OID
> not relfilenode.
> 
> I'm probably going about it the wrong way, but it strikes me that there
> needs to be a way to abstract the relfilenode from OID in the heap access 
> code so that one can easily manipulate the heap on disk without having to 
> play with OIDs. 
> 
> I would have included code examples/clearer description but the box I
> don't have access to the box I created the patches on atm =(.
> 
> Gavin
> 
> 

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Sun, 24 Feb 2002 22:54:47 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: CLUSTER TODO item
I put some work i to this last night. Here's how it works.

1) sanity checks
2) save information (IndexInfo *) about indices on the relation being
clustered.
3) build an uncatalogued heap so that the API can be used without too much
fuss and get a new relfilenode 
(heap_create_with_catalog(...,RELKIND_UNCATALOGED,...)
4) scan the index, inserting tuples in index order into the uncatalogued
relation (ie, insert data into new relfilenode).
5) update relcache/system with new relfilenode for clustered relation
6) drop/create indices, given (2)
7) smgrunlink() old relfilenode
8) Remove uncatalogued relation from relation from relcache

There are two issues here. One is minor: At step three I should create a
new toast table entry, and then attach it to the clustered relation at
step five.

The second point is that postgres doesn't seem to like me creating
uncataloged relations and then doing RelationForgetRelation(). I will look
a little harder though.

Gavin


On Sun, 24 Feb 2002, Bruce Momjian wrote:

> 
> Gavin, please continue working on this feature and continue discussion
> the hackers list.
> 
> ---------------------------------------------------------------------------
> 
> Gavin Sherry wrote:
> > On Thu, 11 Oct 2001, Tom Lane wrote:
> > 
> > > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > > Can I get a status on this?
> > > 
> > > It's not gonna happen for 7.2, I think ...
> > > 
> > > 			regards, tom lane
> > 
> > I'd love it to go out with 7.2 but I've had no time to work on this patch
> > lately. The reason I need time is that, after having fiddled a fair bit,
> > I've decided that there really needs to be support for the creation of a
> > new relfilenode in the storage manager.
> > 
> > The current patch works like this:
> > 
> > Create new heap (heap_create())
> > Copy tuples from old heap to new heap via index scan
> > Form a new pg_class tuple
> > simple_heap_update()
> > update catalogue indices
> > rebuild existing indices
> > 
> > This causes an overflow in the localbuf.c so I guess this is wrong
> > (included in patch on 24/sep) =). I've looked at various combinations of
> > this:
> > 
> > memcpy() the old Relation into a new Relation, update smgrunlink() the old
> > Relation and heap_storage_create() the new relation. This dies because
> > smgrunlink only schedules the drop, where as heap_storage_create() actually 
> > creates a file on the file system (open() returns with EEXIST).
> > 
> > I've also tried just copying the structure but heap_open() relies on OID
> > not relfilenode.
> > 
> > I'm probably going about it the wrong way, but it strikes me that there
> > needs to be a way to abstract the relfilenode from OID in the heap access 
> > code so that one can easily manipulate the heap on disk without having to 
> > play with OIDs. 
> > 
> > I would have included code examples/clearer description but the box I
> > don't have access to the box I created the patches on atm =(.
> > 
> > Gavin
> > 
> > 
> 
> 


Mon, 25 Feb 2002 15:14:53 +1100 (EST)
Gavin Sherry <swm@linuxworld.com.au>
Re: CLUSTER TODO item