Re: Bug in pg_describe_object, patch v2 - Mailing list pgsql-hackers

From Joel Jacobson
Subject Re: Bug in pg_describe_object, patch v2
Date
Msg-id AANLkTimWLP+xafqeOC2wixgsN2gDC=3RG-iBg-mCRQZA@mail.gmail.com
Whole thread Raw
In response to Re: Bug in pg_describe_object, patch v2  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Bug in pg_describe_object, patch v2  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
2011/1/16 Tom Lane <tgl@sss.pgh.pa.us>:
> Comments?

I think it's great you undertook the challenge of solving this problem
the "proper way".
I think your desire to achieve perfection in every little detail is admirable.

Your patch is according to me, not far from perfect, but could be
improved in a faw ways:

a) pg_describe_object should always include the schema in the name,
even for object in public and pg_catalog. Otherwise it's not
explicitly stated whether an object relies in pg_catalog or public. If
you would create a function in the public schema in a 8.3 database,
with a name which does not exist in pg_catalog, when converting to
version 9 in the future, which provides a few more functions in the
pg_catalog schema than 8.3, there is a risk your function would
conflict with the new function with the same name in version 9. The
pg_catalog function would then be selected by default, unless
explicitly calling it with the fully qualified name. This might or
might not be what you want. Let's say you are unfortunate and unaware
it's bad to name functions pg_* in the public schema, since it's
possible, it could possibly lead to unexpected results. Now, let's go
back to the discussion on what pg_describe_object(oid,oid,int) should
return. I strongly believe it's wiser to include the schema in the
description, even though it's not important nor interesting in most
cases. It's good to explicitly inform the end-user (who's
understanding of SQL or PostgreSQL might be limited) there is a
distinct difference between public.myfunc and pg_catalog.myfunc, and
it's better to always include the schema, than to "auto-detect" if
there are two functions with the same name, because it's highly
confusing the description depends on your "SET search_path TO"
setting.

Conclusions in summary:
*) It would be a lot more user-friendly if pg_describe_object always
returned the same text string, independent of your "search_path"
setting.
*) It would be a lot more "programmer-friendly" if pg_describe_object
returned the same text string, independent of your "search_path"
setting and at the same time always included all the unique columns of
the object (including the schema would fix it in >99% of the cases,
and your patch would fix the remaining <1% of the cases).
*) Alternatively, it could possibly be better to introduce a new
function, only returning a text string composed using only the unique
columns (and some constants to do the formatting of the nice text
string, resolved using the unique columns), if it's not feasible to
append the schema name to the description, considering people might
(unlikely, but not impossibly) programtically rely on
pg_describe_object already and parse the text string, which would
break a lot of things.

Just some thoughts...


-- 
Best regards,

Joel Jacobson
Glue Finance


pgsql-hackers by date:

Previous
From: Magnus Hagander
Date:
Subject: Re: pg_stat_replication security
Next
From: Pavel Golub
Date:
Subject: Re: Warning compiling pg_dump (MinGW, Windows XP)