Обсуждение: Show sequences owned by
The attached patch makes the \d output for psql on a sequence show which table/column owns the sequence. The table already showed the dependency the other way through the default value, but going from sequence back to table was not possible. Comments/reviews? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Вложения
On Fri, Nov 4, 2011 at 9:01 AM, Magnus Hagander <magnus@hagander.net> wrote: > The attached patch makes the \d output for psql on a sequence show > which table/column owns the sequence. The table already showed the > dependency the other way through the default value, but going from > sequence back to table was not possible. > > Comments/reviews? Seems reasonable. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Magnus,
* Magnus Hagander (magnus@hagander.net) wrote:
> The attached patch makes the \d output for psql on a sequence show
> which table/column owns the sequence. The table already showed the
> dependency the other way through the default value, but going from
> sequence back to table was not possible.
Seems reasonable.
> Comments/reviews?
Not sure if that 'goto error_return;' handles this correctly, but it
would seem like you're missing the possibility that a sequence isn't
owned by any table/column..? Or that it could be depended upon by more
than one table/column? Both of those happen and are perfectly valid
situations for a sequence to be in..
Thanks,
Stephen
On Fri, Nov 4, 2011 at 15:09, Stephen Frost <sfrost@snowman.net> wrote: > Magnus, > > * Magnus Hagander (magnus@hagander.net) wrote: >> The attached patch makes the \d output for psql on a sequence show >> which table/column owns the sequence. The table already showed the >> dependency the other way through the default value, but going from >> sequence back to table was not possible. > > Seems reasonable. > >> Comments/reviews? > > Not sure if that 'goto error_return;' handles this correctly, but it > would seem like you're missing the possibility that a sequence isn't > owned by any table/column..? Or that it could be depended upon by more > than one table/column? Both of those happen and are perfectly valid > situations for a sequence to be in.. If there is noone owning it at all, it just falls through the if/else block and ignores it if that happens (PQntuples() returns 0). Is there really a case for multiple sequences to own it? How would you go about making that happen? ALTER SEQUENCE.. OWNED BY.. only takes one table, afaics? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes:
> The attached patch makes the \d output for psql on a sequence show
> which table/column owns the sequence. The table already showed the
> dependency the other way through the default value, but going from
> sequence back to table was not possible.
> Comments/reviews?
The join conditions are far from adequate. You can *not* just check the
objid, you *must* check classid (and refclassid) to avoid being fooled
by duplicate OIDs in different system catalogs. You've also not held
to psql's normal conventions about fully qualifying names to avoid
making assumptions about the search_path.
regards, tom lane
On Fri, Nov 4, 2011 at 15:19, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Magnus Hagander <magnus@hagander.net> writes: >> The attached patch makes the \d output for psql on a sequence show >> which table/column owns the sequence. The table already showed the >> dependency the other way through the default value, but going from >> sequence back to table was not possible. > >> Comments/reviews? > > The join conditions are far from adequate. You can *not* just check the > objid, you *must* check classid (and refclassid) to avoid being fooled Uh, it does check classid. Or are you saying it's checked the wrong way? But it's not checking refclassid, that's true - and should be fixed. > by duplicate OIDs in different system catalogs. You've also not held > to psql's normal conventions about fully qualifying names to avoid > making assumptions about the search_path. Will fix. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
* Magnus Hagander (magnus@hagander.net) wrote:
> If there is noone owning it at all, it just falls through the if/else
> block and ignores it if that happens (PQntuples() returns 0).
Ah, right, but 'result' is still non-zero, ok.
> Is there really a case for multiple sequences to own it? How would you
> go about making that happen? ALTER SEQUENCE.. OWNED BY.. only takes
> one table, afaics?
I just noticed it was pulling from pg_depend and we could be creating
multiple dependencies on a single sequence by having two tables use it
as a default value. If that situation doesn't cause a problem for this,
then that's fine. :) Couldn't remember if we distinguished 'owned by'
from 'dependend upon' for seqeunces.
Thanks,
Stephen
On Fri, Nov 4, 2011 at 15:29, Stephen Frost <sfrost@snowman.net> wrote:
> * Magnus Hagander (magnus@hagander.net) wrote:
>> If there is noone owning it at all, it just falls through the if/else
>> block and ignores it if that happens (PQntuples() returns 0).
>
> Ah, right, but 'result' is still non-zero, ok.
Yes, that's a regular libpq result set...
>> Is there really a case for multiple sequences to own it? How would you
>> go about making that happen? ALTER SEQUENCE.. OWNED BY.. only takes
>> one table, afaics?
>
> I just noticed it was pulling from pg_depend and we could be creating
> multiple dependencies on a single sequence by having two tables use it
> as a default value. If that situation doesn't cause a problem for this,
> then that's fine. :) Couldn't remember if we distinguished 'owned by'
> from 'dependend upon' for seqeunces.
I tried that now to be sure, and to confirm, this is the scenario:
CREATE TABLE seqtest (a SERIAL PRIMARY KEY);
CREATE TABLE seqtest2 (a int NOT NULL DEFAULT
nextval('seqtest_a_seq'::regclass);
In this case, we end up with just one entry in pg_depend, which refers
to seqtest.a.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes:
> On Fri, Nov 4, 2011 at 15:19, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> The join conditions are far from adequate. �You can *not* just check the
>> objid, you *must* check classid (and refclassid) to avoid being fooled
> Uh, it does check classid. Or are you saying it's checked the wrong way?
Oh, sheesh, not enough caffeine. I was expecting to see it written as
part of the ON condition --- I always think of objid and classid as
being two parts of the join key for pg_depend queries. You should write
it as classid='pg_catalog.pg_class'::pg_catalog.regclass, but at least
it's there.
> But it's not checking refclassid, that's true - and should be fixed.
Yeah.
regards, tom lane
Stephen Frost <sfrost@snowman.net> writes:
> I just noticed it was pulling from pg_depend and we could be creating
> multiple dependencies on a single sequence by having two tables use it
> as a default value. If that situation doesn't cause a problem for this,
> then that's fine. :) Couldn't remember if we distinguished 'owned by'
> from 'dependend upon' for seqeunces.
Yeah, we do, via the deptype. The check for deptype = 'a' is the
correct thing here.
Still, I'm not terribly comfortable with having multiple matches be
treated as a reason to fail the entire \d command. It'd likely be
better to just not add a footer if you get an unexpected number of
matches.
regards, tom lane
On Fri, Nov 4, 2011 at 15:44, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Stephen Frost <sfrost@snowman.net> writes: >> I just noticed it was pulling from pg_depend and we could be creating >> multiple dependencies on a single sequence by having two tables use it >> as a default value. If that situation doesn't cause a problem for this, >> then that's fine. :) Couldn't remember if we distinguished 'owned by' >> from 'dependend upon' for seqeunces. > > Yeah, we do, via the deptype. The check for deptype = 'a' is the > correct thing here. > > Still, I'm not terribly comfortable with having multiple matches be > treated as a reason to fail the entire \d command. It'd likely be > better to just not add a footer if you get an unexpected number of > matches. Ok. Updated patch attached that does this, and the proper schema qualifications. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Вложения
Magnus Hagander <magnus@hagander.net> writes:
> Updated patch attached that does this, and the proper schema qualifications.
I'd schema-qualify the quote_ident and regclass names too.
Also, just as a matter of style, I think it'd be better to assign short
aliases to the table names ("pg_catalog.pg_class c" etc) and use those.
I forget what the letter of the SQL standard is about whether an
un-aliased schema-qualified table name can be referenced in the query
without schema-qualifying the reference, but I'm pretty sure that not
doing so is at least frowned on.
regards, tom lane
On Fri, Nov 4, 2011 at 16:04, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> Updated patch attached that does this, and the proper schema qualifications.
>
> I'd schema-qualify the quote_ident and regclass names too.
>
> Also, just as a matter of style, I think it'd be better to assign short
> aliases to the table names ("pg_catalog.pg_class c" etc) and use those.
> I forget what the letter of the SQL standard is about whether an
> un-aliased schema-qualified table name can be referenced in the query
> without schema-qualifying the reference, but I'm pretty sure that not
> doing so is at least frowned on.
Fixed, and applied.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/