Thread: 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@rogers.com> PGP Key ID: DB3C29FC
Attachment
Neil Conway <nconway@klamath.dyndns.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@klamath.dyndns.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@rogers.com> PGP Key ID: DB3C29FC
Attachment
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@gmx.net
Peter Eisentraut <peter_e@gmx.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@rogers.com> PGP Key ID: DB3C29FC
Attachment
Neil Conway <nconway@klamath.dyndns.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@klamath.dyndns.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@rogers.com> PGP Key ID: DB3C29FC
Neil Conway <nconway@klamath.dyndns.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@rogers.com> PGP Key ID: DB3C29FC
Neil Conway <nconway@klamath.dyndns.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@gmx.net
On Mon, 2002-02-25 at 18:48, Tom Lane wrote: > Neil Conway <nconway@klamath.dyndns.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@rogers.com> PGP Key ID: DB3C29FC
Attachment
> 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@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
Bruce Momjian <pgman@candle.pha.pa.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@candle.pha.pa.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@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
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@klamath.dyndns.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@rogers.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@postgresql.org so that your
> message can get through to the mailing list cleanly
--
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
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@klamath.dyndns.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@rogers.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@postgresql.org so that your > message can get through to the mailing list cleanly -- 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
Bruce Momjian <pgman@candle.pha.pa.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@candle.pha.pa.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@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
On Tue, 2002-03-05 at 01:50, Bruce Momjian wrote: > Tom Lane wrote: > > Bruce Momjian <pgman@candle.pha.pa.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@rogers.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@candle.pha.pa.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@rogers.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@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
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@rogers.com> PGP Key ID: DB3C29FC
Attachment
Neil Conway <nconway@klamath.dyndns.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@klamath.dyndns.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@rogers.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@rogers.com>
> PGP Key ID: DB3C29FC
[ Attachment, skipping... ]
--
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
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@rogers.com> > PGP Key ID: DB3C29FC [ Attachment, skipping... ] -- 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
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@gmx.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@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
pgman wrote: > 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. Actually, more to the point, the table owner does own the indexes. I know that is is always true, but if someone is spinning through pg_class and wants to find everything owned by a specific person, without joining to pg_index, keeping the user id correct on indexes is helpful. -- 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
Hello! On Sun, 10 Mar 2002, Bruce Momjian wrote: > > Actually, more to the point, the table owner does own the indexes. I > know that is is always true, but if someone is spinning through pg_class > and wants to find everything owned by a specific person, without joining > to pg_index, keeping the user id correct on indexes is helpful. Sometimes it is very useful to have one owner on a table and another owner on indices of that table (all from real life). BTW, compare with MySQL privileges to manage index/indices. Just my two kopecks. -- WBR, Yury Bokhoncovich, Senior System Administrator, NOC of F1 Group. Phone: +7 (3832) 106228, ext.140, E-mail: byg@center-f1.ru. Unix is like a wigwam -- no Gates, no Windows, and an Apache inside.
Yury Bokhoncovich wrote: > Hello! > > On Sun, 10 Mar 2002, Bruce Momjian wrote: > > > > > Actually, more to the point, the table owner does own the indexes. I > > know that is is always true, but if someone is spinning through pg_class > > and wants to find everything owned by a specific person, without joining > > to pg_index, keeping the user id correct on indexes is helpful. > > Sometimes it is very useful to have one owner on a table and another owner > on indices of that table (all from real life). > > BTW, compare with MySQL privileges to manage index/indices. Can you explain the advantages? We don't allow that currently. -- 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
Hello! On Mon, 11 Mar 2002, Bruce Momjian wrote: > > > Actually, more to the point, the table owner does own the indexes. I [skip] > > Sometimes it is very useful to have one owner on a table and another owner > > on indices of that table (all from real life). > > BTW, compare with MySQL privileges to manage index/indices. > Can you explain the advantages? We don't allow that currently. Sure. Though I don't think it is a high priority task. 1) There is a table with bulk data updates periodically. The best found solution to enhance performance was to drop all table's indices before data update then re-create those again. 2) I (admin) have a few tables as owner and wanna give another user (programmer) ability to create/drop any necessary indices. At the same time I do not wanna give him (her) a chance to drop/alter the table itself. BTW, If ppl want to be more similar to Oracle, remember that it also has the feature to manage indeces. -- WBR, Yury Bokhoncovich, Senior System Administrator, NOC of F1 Group. Phone: +7 (3832) 106228, ext.140, E-mail: byg@center-f1.ru. Unix is like a wigwam -- no Gates, no Windows, and an Apache inside.
Yury Bokhoncovich <byg@center-f1.ru> writes:
> 2) I (admin) have a few tables as owner and wanna give another user
> (programmer) ability to create/drop any necessary indices. At the same
> time I do not wanna give him (her) a chance to drop/alter the table
> itself.
But keep in mind that CREATE UNIQUE INDEX does alter the table, in
the sense that subsequent operations may fail where they'd not have
done before. Conversely, dropping a unique index may allow logically
invalid data to be inserted. Also, dropping an index may cause important
operations to take vastly longer than they're expected to. So giving
away the right to manipulate indexes is at the very least an opening
to denial-of-service problems.
In any case, what you are really suggesting here is that we offer a
grantable "right to create/drop indexes" on a *table*. Dangerous or
not, it could be useful. But it has nothing that I can see to do with
a notion of ownership of the indexes themselves; there's still no visible
reason to consider the indexes to have ownership independent of the
table they're on.
regards, tom lane
Hello! On Tue, 12 Mar 2002, Tom Lane wrote: [skip] > operations to take vastly longer than they're expected to. So giving > away the right to manipulate indexes is at the very least an opening > to denial-of-service problems. Yes, but this is what sysadmin is for, isn'it? > In any case, what you are really suggesting here is that we offer a > grantable "right to create/drop indexes" on a *table*. Dangerous or > not, it could be useful. But it has nothing that I can see to do with > a notion of ownership of the indexes themselves; there's still no visible > reason to consider the indexes to have ownership independent of the > table they're on. Partially agreed. It seems to be that such behaviour resembles MySQL one. How about index on a view? Thanks for a clue anyway. -- WBR, Yury Bokhoncovich, Senior System Administrator, NOC of F1 Group. Phone: +7 (3832) 106228, ext.140, E-mail: byg@center-f1.ru. Unix is like a wigwam -- no Gates, no Windows, and an Apache inside.