Re: ALTER TABLE OWNER: change indexes

Hi all,

Currently, ALTER TABLE OWNER only changes the ownership of a table; it
does not change the ownership of any of its 'dependants' (e.g. indexes,
sequences, etc). This patch modifies ALTER TABLE OWNER to change the
ownership of any of a table's indexes to match the new owner of the
table.

I didn't change sequence ownership, since a sequence could be used by
multiple tables. Bruce: can you add a note to the TODO -- it currently
has an item regarding dropping a table's serial sequences along with the
table. When/if this behavior is implemented, the same change should be
applied to ALTER TABLE OWNER.

I wasn't sure if ALTER TABLE OWNER should notify the user that the
ownership of some indexes are also being changed; this patch follows the
behavior of DROP TABLE, which is to be silent.

Also, I needed some utility code to find the name of a relation, given
its OID. I find a suitable utility function in
src/backend/utils/adt/ruleutils.c -- however, it's a static function. I
couldn't find a good place in the tree to stick this utility function,
so I just copy-and-pasted the code I needed. If someone would like to
suggest somewhere where this code should go, speak up and I'll happily
remove the duplicated code.

Comments and criticism are welcome.

Cheers,

Neil

--
Neil Conway <neilconway(at)rogers(dot)com>
PGP Key ID: DB3C29FC

Neil Conway <nconway(at)klamath(dot)dyndns(dot)org> writes:
> Also, I needed some utility code to find the name of a relation, given
> its OID.

get_rel_name in utils/cache/lsyscache.c.

> I find a suitable utility function in
> src/backend/utils/adt/ruleutils.c -- however, it's a static function.

Oh? Looks like someone forgot about lsyscache ...

In general, lsyscache is the place for quick and dirty little catalog
access functions like this.

regards, tom lane

On Sun, 2002-02-24 at 16:35, Tom Lane wrote:
> Neil Conway <nconway(at)klamath(dot)dyndns(dot)org> writes:
> > Also, I needed some utility code to find the name of a relation, given
> > its OID.
>
> get_rel_name in utils/cache/lsyscache.c.

Thanks Tom.

> > I find a suitable utility function in
> > src/backend/utils/adt/ruleutils.c -- however, it's a static function.
>
> Oh? Looks like someone forgot about lsyscache ...

I've attached a new version of the patch that uses get_rel_name() -- I
also removed get_relation_name() from ruleutils.c and changed that file
to use get_rel_name().

Cheers,

Neil

--
Neil Conway <neilconway(at)rogers(dot)com>
PGP Key ID: DB3C29FC

BTW, I forgot to inquire about *why* you needed to look up a relation
name. As things stand I think you've coded it okay, but this will need
to change for schemas: a relation name isn't going to be a unique
identifier much longer.

It'd probably be best to redesign the ALTER TABLE routines so that
the recursive execution routine accepts a relation OID rather than a
relation name, with a front-end routine that does a one-time name-to-
OID lookup. Recursion using OID will be simpler than recursion using
name, for both child-table and index cases. And it won't break for
schemas.

Perhaps this could be done as part of the overall refactoring of the
ALTER code that someone (I forget who) was going to look at doing.

This doesn't need to be done just to make this patch acceptable, but
I thought I'd better mention that it needs to be done soon.

Another point that maybe does need immediate attention: as coded,
reassignment of ownership of a table won't affect the associated
TOAST table, if any. Should it?

regards, tom lane

Tom Lane writes:

> Another point that maybe does need immediate attention: as coded,
> reassignment of ownership of a table won't affect the associated
> TOAST table, if any. Should it?

Maybe indexes and TOAST tables shouldn't have ownership it all. (Set
relowner to 0.) Since only owners can create these things, there doesn't
seem to be a point in maintaining ownership.

--
Peter Eisentraut peter_e(at)gmx(dot)net

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> Maybe indexes and TOAST tables shouldn't have ownership it all. (Set
> relowner to 0.) Since only owners can create these things, there doesn't
> seem to be a point in maintaining ownership.

That's a good thought. At one time we allowed non-owners to create
indexes on tables, but it was correctly pointed out that this was a
bad idea (think UNIQUE index, or a functional index that always
causes an error...). I can't see a real good reason for either indexes
or TOAST tables to have a concept of ownership distinct from the parent
table.

psql's \d displays might have some trouble with this, and pg_dump might
get confused too. But fixing them would be a net improvement in
robustness so it's probably a reasonable thing to do.

regards, tom lane

On Sun, 2002-02-24 at 17:43, Tom Lane wrote:
> It'd probably be best to redesign the ALTER TABLE routines so that
> the recursive execution routine accepts a relation OID rather than a
> relation name, with a front-end routine that does a one-time name-to-
> OID lookup. Recursion using OID will be simpler than recursion using
> name, for both child-table and index cases. And it won't break for
> schemas.

You mentioned child-tables: should ALTER TABLE OWNER also recurse for
those?

BTW, a question on coding style: are large source files discouraged? I
usually like to separate my source into relatively small files, but
there are a bunch of files in the tree with 2000, 3000 or more lines of
code. Is this the preferred style, or would smaller files (divided in
logical groupings of functions, of course) be better?

> Perhaps this could be done as part of the overall refactoring of the
> ALTER code that someone (I forget who) was going to look at doing.

Do you have any more information on this? (e.g. an ML thread) I could
pick up this work if it's been dropped...

> Another point that maybe does need immediate attention: as coded,
> reassignment of ownership of a table won't affect the associated
> TOAST table, if any. Should it?

Dunno, I don't know anything about TOAST. I would think if anyone would
know, it'd be you ;-)

The revised patch is attached. It follows Tom's suggestion and does
recursion using the OID of the relation (and the sysid of the user, as
well). Since this involved basically rewriting ALTER TABLE OWNER, I may
have messed something up -- but it works, AFAICT.

Now that I've changed ALTER TABLE OWNER to use recursion-with-oid,
should I make changes to the other ALTER TABLE functions as necessary?

Comments and criticism are welcome.

Cheers,

Neil

--
Neil Conway <neilconway(at)rogers(dot)com>
PGP Key ID: DB3C29FC

Neil Conway <nconway(at)klamath(dot)dyndns(dot)org> writes:
> You mentioned child-tables: should ALTER TABLE OWNER also recurse for
> those?

Only if you want to put in an "ONLY" variant to suppress the recursion.
(This might actually be a reasonable thing to do, if so. But I don't
have a strong feeling about it.)

> BTW, a question on coding style: are large source files discouraged?

Personally I prefer source file == logical module, however big that is.
For stuff like this it's not always clear where the module boundaries
should be drawn, of course. I really hate the style where each C
function lives in a separate source file; beyond that it's all a matter
of taste to some extent.

My thought would be to group all the ALTER TABLE variants into one file,
perhaps "alter.c", separate from the other current inhabitants of
command.c. Note that alter.c could be a lot smaller than the current
sum of the ALTER routine sizes, given appropriate refactoring to
eliminate duplicate code.

>> Perhaps this could be done as part of the overall refactoring of the
>> ALTER code that someone (I forget who) was going to look at doing.

> Do you have any more information on this? (e.g. an ML thread) I could
> pick up this work if it's been dropped...

I seem to recall discussing this with Gavin Sherry or Christopher
Kings-Lynne sometime in the past six months or so, but can't find the
thread at the moment. AFAIR it hadn't really progressed beyond the
observation that there was an awful lot of duplicated code in those
routines, and that the ones that weren't duplicating it were a few
bricks shy of a load (eg, didn't support recursion but should).
So we were wondering about ways to consolidate the duplication into
some sort of common control routine.

>> Another point that maybe does need immediate attention: as coded,
>> reassignment of ownership of a table won't affect the associated
>> TOAST table, if any. Should it?

> Dunno, I don't know anything about TOAST. I would think if anyone would
> know, it'd be you ;-)

Well, see Peter's suggestion that this is all wrong because indexes
don't have meaningful ownership anyway. I think he's got a point...

regards, tom lane

On Sun, 2002-02-24 at 22:37, Tom Lane wrote:
> Neil Conway <nconway(at)klamath(dot)dyndns(dot)org> writes:
> > You mentioned child-tables: should ALTER TABLE OWNER also recurse for
> > those?
>
> Only if you want to put in an "ONLY" variant to suppress the recursion.
> (This might actually be a reasonable thing to do, if so. But I don't
> have a strong feeling about it.)

Okay, I'll do that in a later patch.

> My thought would be to group all the ALTER TABLE variants into one file,
> perhaps "alter.c", separate from the other current inhabitants of
> command.c. Note that alter.c could be a lot smaller than the current
> sum of the ALTER routine sizes, given appropriate refactoring to
> eliminate duplicate code.

Okay, I'll create alter.c and see if I can refactor some of the ALTER
code -- but that can wait for a later patch also.

> >> Another point that maybe does need immediate attention: as coded,
> >> reassignment of ownership of a table won't affect the associated
> >> TOAST table, if any. Should it?
>
> > Dunno, I don't know anything about TOAST. I would think if anyone would
> > know, it'd be you ;-)
>
> Well, see Peter's suggestion that this is all wrong because indexes
> don't have meaningful ownership anyway. I think he's got a point...

That's probably true -- in the long-run, it probably makes more sense to
remove the concept of ownership from indexes.

However, in the mean-time, I think my patch is still valid. Unless there
are any remaining problems, please apply for 7.3.

Cheers,

Neil

--
Neil Conway <neilconway(at)rogers(dot)com>
PGP Key ID: DB3C29FC

Neil Conway <nconway(at)klamath(dot)dyndns(dot)org> writes:
>> Well, see Peter's suggestion that this is all wrong because indexes
>> don't have meaningful ownership anyway. I think he's got a point...

> That's probably true -- in the long-run, it probably makes more sense to
> remove the concept of ownership from indexes.

> However, in the mean-time, I think my patch is still valid. Unless there
> are any remaining problems, please apply for 7.3.

No, I disagree. If we are intending to remove ownership from indexes,
there is no good reason to add code that will only get taken out again.
There is no bug here of sufficient importance to warrant a temporary
hack solution; the existing behavior can be lived with.

regards, tom lane

On Mon, 2002-02-25 at 18:32, Tom Lane wrote:
> > However, in the mean-time, I think my patch is still valid. Unless there
> > are any remaining problems, please apply for 7.3.
>
> No, I disagree. If we are intending to remove ownership from indexes,
> there is no good reason to add code that will only get taken out again.

Well, that's assuming that someone steps forward to actually write the
code. I haven't heard that anyone has...

And as for adding code that will only be removed, the code is finished,
tested, reviewed and ready to apply -- all the work _has_ been done.

Additionally, if someone eventually fixes the index-ownership situation,
the changes to command.c to remove the index recursion are trivial. This
patch also includes some refactoring and code cleanups that are useful
in any case.

> There is no bug here of sufficient importance to warrant a temporary
> hack solution; the existing behavior can be lived with.

I wouldn't call my solution a "temporary hack"; it's a similar idea to
code that is found throughout the tree.

Cheers,

Neil

--
Neil Conway <neilconway(at)rogers(dot)com>
PGP Key ID: DB3C29FC

Neil Conway <nconway(at)klamath(dot)dyndns(dot)org> writes:
> Additionally, if someone eventually fixes the index-ownership situation,
> the changes to command.c to remove the index recursion are trivial.

... but won't necessarily get done. More to the point, they may confuse
someone who's trying to refactor the code: without careful thought, he
might think he needs to support recursion over indexes as well as child
tables.

> This patch also includes some refactoring and code cleanups that are
> useful in any case.

Sure. Please resubmit just that part.

regards, tom lane

Neil Conway writes:

> Okay, I'll create alter.c and see if I can refactor some of the ALTER
> code -- but that can wait for a later patch also.

It might make more sense to factor the source files by the kind of objects
they act on. For instance, the CREATE/ALTER USER code shares a lot of
code, but it shares zero with, say, ALTER TABLE.

--
Peter Eisentraut peter_e(at)gmx(dot)net

On Mon, 2002-02-25 at 18:48, Tom Lane wrote:
> Neil Conway <nconway(at)klamath(dot)dyndns(dot)org> writes:
> > Additionally, if someone eventually fixes the index-ownership situation,
> > the changes to command.c to remove the index recursion are trivial.
>
> ... but won't necessarily get done. More to the point, they may confuse
> someone who's trying to refactor the code: without careful thought, he
> might think he needs to support recursion over indexes as well as child
> tables.

Not if the code includes a comment (as it does) that the recursion is
intended _only_ to support changing the ownership of any indexes which
belong to the table.

IMHO, it's not confusing at all: in the current code, indexes have
owners, and should be owned by the owner of the table they belong to.
The patch makes this consistent; without the patch, one might conclude
that there are reasonable situations in the owner of a table should not
own its indexes, which is incorrect AFAIK.

BTW, should ownership be removed from sequences as well?

> > This patch also includes some refactoring and code cleanups that are
> > useful in any case.
>
> Sure. Please resubmit just that part.

Okay, I've attached a patch which implements this.

I think it is still a bad idea to leave code that is _known_ to be
broken in the tree, waiting for a possible future enhancement that no
one has committed to writing. But it's your call -- please apply either
this patch, or the previous one (-3) as you see fit.

Cheers,

Neil

--
Neil Conway <neilconway(at)rogers(dot)com>
PGP Key ID: DB3C29FC

> I think it is still a bad idea to leave code that is _known_ to be
> broken in the tree, waiting for a possible future enhancement that no
> one has committed to writing. But it's your call -- please apply either
> this patch, or the previous one (-3) as you see fit.

We have been bitten by refusing patches that have a valid purpose,
supposing someone will come and something that may take years to do,
e.g. nocreatable permissions. If it does something useful, and can be
ripped out later, why not add it?

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

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> We have been bitten by refusing patches that have a valid purpose,
> supposing someone will come and something that may take years to do,
> e.g. nocreatable permissions. If it does something useful, and can be
> ripped out later, why not add it?

But it doesn't do anything useful, AFAICS. What significant problem
occurs if an index shows an owner different from its parent? If there's
any actual operational problem there, then I'd agree we need a fix.

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > We have been bitten by refusing patches that have a valid purpose,
> > supposing someone will come and something that may take years to do,
> > e.g. nocreatable permissions. If it does something useful, and can be
> > ripped out later, why not add it?
>
> But it doesn't do anything useful, AFAICS. What significant problem
> occurs if an index shows an owner different from its parent? If there's
> any actual operational problem there, then I'd agree we need a fix.

Oh, I see.

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

Your patch has been added to the PostgreSQL unapplied patches list at:

http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

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

Neil Conway wrote:
> On Mon, 2002-02-25 at 18:48, Tom Lane wrote:
> > Neil Conway <nconway(at)klamath(dot)dyndns(dot)org> writes:
> > > Additionally, if someone eventually fixes the index-ownership situation,
> > > the changes to command.c to remove the index recursion are trivial.
> >
> > ... but won't necessarily get done. More to the point, they may confuse
> > someone who's trying to refactor the code: without careful thought, he
> > might think he needs to support recursion over indexes as well as child
> > tables.
>
> Not if the code includes a comment (as it does) that the recursion is
> intended _only_ to support changing the ownership of any indexes which
> belong to the table.
>
> IMHO, it's not confusing at all: in the current code, indexes have
> owners, and should be owned by the owner of the table they belong to.
> The patch makes this consistent; without the patch, one might conclude
> that there are reasonable situations in the owner of a table should not
> own its indexes, which is incorrect AFAIK.
>
> BTW, should ownership be removed from sequences as well?
>
> > > This patch also includes some refactoring and code cleanups that are
> > > useful in any case.
> >
> > Sure. Please resubmit just that part.
>
> Okay, I've attached a patch which implements this.
>
> I think it is still a bad idea to leave code that is _known_ to be
> broken in the tree, waiting for a possible future enhancement that no
> one has committed to writing. But it's your call -- please apply either
> this patch, or the previous one (-3) as you see fit.
>
> Cheers,
>
> Neil
>
> --
> Neil Conway <neilconway(at)rogers(dot)com>
> PGP Key ID: DB3C29FC

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly

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

OK, the issue with this patch is that it fixes ownership of INDEXES.
The complaint is that we shouldn't have ownership of indexes. However,
we are clearly now preventing anyone except the table owner from
creating indexes, so it seems there is ownership, or at least a
restriction.

Now, we are we going with this? Can we just remove ownership of indexes
totally? And sequences? Is that the direction we want to go in? I
can't imagine it is very hard to do. Or is the problem that we should
not be reporting the owner of these items?

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

Neil Conway wrote:
> On Mon, 2002-02-25 at 18:48, Tom Lane wrote:
> > Neil Conway <nconway(at)klamath(dot)dyndns(dot)org> writes:
> > > Additionally, if someone eventually fixes the index-ownership situation,
> > > the changes to command.c to remove the index recursion are trivial.
> >
> > ... but won't necessarily get done. More to the point, they may confuse
> > someone who's trying to refactor the code: without careful thought, he
> > might think he needs to support recursion over indexes as well as child
> > tables.
>
> Not if the code includes a comment (as it does) that the recursion is
> intended _only_ to support changing the ownership of any indexes which
> belong to the table.
>
> IMHO, it's not confusing at all: in the current code, indexes have
> owners, and should be owned by the owner of the table they belong to.
> The patch makes this consistent; without the patch, one might conclude
> that there are reasonable situations in the owner of a table should not
> own its indexes, which is incorrect AFAIK.
>
> BTW, should ownership be removed from sequences as well?
>
> > > This patch also includes some refactoring and code cleanups that are
> > > useful in any case.
> >
> > Sure. Please resubmit just that part.
>
> Okay, I've attached a patch which implements this.
>
> I think it is still a bad idea to leave code that is _known_ to be
> broken in the tree, waiting for a possible future enhancement that no
> one has committed to writing. But it's your call -- please apply either
> this patch, or the previous one (-3) as you see fit.
>
> Cheers,
>
> Neil
>
> --
> Neil Conway <neilconway(at)rogers(dot)com>
> PGP Key ID: DB3C29FC

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly

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

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> OK, the issue with this patch is that it fixes ownership of INDEXES.

I thought the resubmitted patch did no such thing?

> Now, we are we going with this? Can we just remove ownership of indexes
> totally? And sequences?

How did you get from indexes to sequences? The issues are completely
different.

I'm in favor of considering that indexes and toast tables have no
separate ownership, and storing zero in pg_class.relowner for them.
However, I have not looked to see what this might break. It might
be more trouble than it's worth.

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > OK, the issue with this patch is that it fixes ownership of INDEXES.
>
> I thought the resubmitted patch did no such thing?
>
> > Now, we are we going with this? Can we just remove ownership of indexes
> > totally? And sequences?
>
> How did you get from indexes to sequences? The issues are completely
> different.

The poster mentioned it. What does it matter? I am asking.

> I'm in favor of considering that indexes and toast tables have no
> separate ownership, and storing zero in pg_class.relowner for them.
> However, I have not looked to see what this might break. It might
> be more trouble than it's worth.

Well, before we reject this patch, we should decide what we are going to
do. Of course, indexes are still in pg_class, and putting zero in there
for a user could be trouble. It may be easier to just apply the patch.
In fact, because it is pg_class, I am not sure we will ever know what
3rd party apps we will break.

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

On Tue, 2002-03-05 at 01:50, Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > > OK, the issue with this patch is that it fixes ownership of INDEXES.
> >
> > I thought the resubmitted patch did no such thing?

That's correct.

> > > Now, we are we going with this? Can we just remove ownership of indexes
> > > totally? And sequences?
> >
> > How did you get from indexes to sequences? The issues are completely
> > different.
>
> The poster mentioned it. What does it matter? I am asking.

I think ownership is still valid for sequences (after I asked about it,
I thought about it some more -- sequences can be used by multiple
tables, and their ownership is actually used by the system).

> > I'm in favor of considering that indexes and toast tables have no
> > separate ownership, and storing zero in pg_class.relowner for them.
> > However, I have not looked to see what this might break. It might
> > be more trouble than it's worth.
>
> Well, before we reject this patch, we should decide what we are going to
> do. Of course, indexes are still in pg_class, and putting zero in there
> for a user could be trouble. It may be easier to just apply the patch.

Well, the latest version of "the patch" doesn't do much -- it just
refactors the code involved, there is no change in functionality (I
removed the ownership-changing code on Tom's request). Please apply it
-- it's quite uncontroversial. If at some later date we decide to apply
the additional ownership changing code, that's simple to do.

Cheers,

Neil

--
Neil Conway <neilconway(at)rogers(dot)com>
PGP Key ID: DB3C29FC

OK, I propose that we do the index part of this patch too. No one seems
to know if seting the index user id to zero in pg_class will break
things so we may as keep it consistent. Can you send that over?

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

Neil Conway wrote:
> On Tue, 2002-03-05 at 01:50, Bruce Momjian wrote:
> > Tom Lane wrote:
> > > Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > > > OK, the issue with this patch is that it fixes ownership of INDEXES.
> > >
> > > I thought the resubmitted patch did no such thing?
>
> That's correct.
>
> > > > Now, we are we going with this? Can we just remove ownership of indexes
> > > > totally? And sequences?
> > >
> > > How did you get from indexes to sequences? The issues are completely
> > > different.
> >
> > The poster mentioned it. What does it matter? I am asking.
>
> I think ownership is still valid for sequences (after I asked about it,
> I thought about it some more -- sequences can be used by multiple
> tables, and their ownership is actually used by the system).
>
> > > I'm in favor of considering that indexes and toast tables have no
> > > separate ownership, and storing zero in pg_class.relowner for them.
> > > However, I have not looked to see what this might break. It might
> > > be more trouble than it's worth.
> >
> > Well, before we reject this patch, we should decide what we are going to
> > do. Of course, indexes are still in pg_class, and putting zero in there
> > for a user could be trouble. It may be easier to just apply the patch.
>
> Well, the latest version of "the patch" doesn't do much -- it just
> refactors the code involved, there is no change in functionality (I
> removed the ownership-changing code on Tom's request). Please apply it
> -- it's quite uncontroversial. If at some later date we decide to apply
> the additional ownership changing code, that's simple to do.
>
> Cheers,
>
> Neil
>
> --
> Neil Conway <neilconway(at)rogers(dot)com>
> PGP Key ID: DB3C29FC
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://archives.postgresql.org
>

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

On Tue, 2002-03-05 at 16:08, Bruce Momjian wrote:
>
> OK, I propose that we do the index part of this patch too. No one seems
> to know if seting the index user id to zero in pg_class will break
> things so we may as keep it consistent. Can you send that over?

Sure. The attached patch makes ALTER TABLE OWNER change the ownership of
any indexes belonging to the target table, as well as refactoring the
code involved.

Cheers,

Neil

--
Neil Conway <neilconway(at)rogers(dot)com>
PGP Key ID: DB3C29FC

Neil Conway <nconway(at)klamath(dot)dyndns(dot)org> writes:
> Sure. The attached patch makes ALTER TABLE OWNER change the ownership of
> any indexes belonging to the target table,

What about TOAST tables (and their indexes)? If you intend to take this
approach then their ownership needs to change too.

regards, tom lane

On Tue, 2002-03-05 at 18:34, Tom Lane wrote:
> Neil Conway <nconway(at)klamath(dot)dyndns(dot)org> writes:
> > Sure. The attached patch makes ALTER TABLE OWNER change the ownership of
> > any indexes belonging to the target table,
>
> What about TOAST tables (and their indexes)? If you intend to take this
> approach then their ownership needs to change too.

Well, it's not really a matter of what "I" intend to do -- the
implementation is really pretty trivial, it's just a matter of policy
(and I'm happy to go along with the core hackers on policy).

But if the general consensus is that this is the right way to go, I'll
happily write the code for TOAST tables + TOAST table indexes as well.
I'll wait to see if the index-owner patch is committed first though...

Cheers,

Neil

--
Neil Conway <neilconway(at)rogers(dot)com>
PGP Key ID: DB3C29FC

Your patch has been added to the PostgreSQL unapplied patches list at:

http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

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

Neil Conway wrote:
> On Tue, 2002-03-05 at 16:08, Bruce Momjian wrote:
> >
> > OK, I propose that we do the index part of this patch too. No one seems
> > to know if seting the index user id to zero in pg_class will break
> > things so we may as keep it consistent. Can you send that over?
>
> Sure. The attached patch makes ALTER TABLE OWNER change the ownership of
> any indexes belonging to the target table, as well as refactoring the
> code involved.
>
> Cheers,
>
> Neil
>
> --
> Neil Conway <neilconway(at)rogers(dot)com>
> PGP Key ID: DB3C29FC

[ Attachment, skipping... ]

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

Patch applied. Thanks.

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

Neil Conway wrote:
> On Tue, 2002-03-05 at 16:08, Bruce Momjian wrote:
> >
> > OK, I propose that we do the index part of this patch too. No one seems
> > to know if seting the index user id to zero in pg_class will break
> > things so we may as keep it consistent. Can you send that over?
>
> Sure. The attached patch makes ALTER TABLE OWNER change the ownership of
> any indexes belonging to the target table, as well as refactoring the
> code involved.
>
> Cheers,
>
> Neil
>
> --
> Neil Conway <neilconway(at)rogers(dot)com>
> PGP Key ID: DB3C29FC

[ Attachment, skipping... ]

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

Bruce Momjian writes:

> OK, I propose that we do the index part of this patch too. No one seems
> to know if seting the index user id to zero in pg_class will break
> things so we may as keep it consistent. Can you send that over?

What would it break? Joins against pg_user should use outer joins anyway.
I don't think this is the right direction. It's just a waste of code.

--
Peter Eisentraut peter_e(at)gmx(dot)net

Peter Eisentraut wrote:
> Bruce Momjian writes:
>
> > OK, I propose that we do the index part of this patch too. No one seems
> > to know if seting the index user id to zero in pg_class will break
> > things so we may as keep it consistent. Can you send that over?
>
> What would it break? Joins against pg_user should use outer joins anyway.
> I don't think this is the right direction. It's just a waste of code.

To me, maintaining it is very low cost and we don't know for sure people
are using outer joins for all their system table access. We only added
outer joins in pg_dump and psql in the past year or two.

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