mirrors/postgres
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Mark operator implementation functions as such in their comments.

Browse Source 
Historically, we've not had separate comments for built-in pg_operator
entries, but relied on the comments for the underlying functions.  The
trouble with this approach is that there isn't much of anything to suggest
to users that they'd be better off using the operators instead.  So, move
all the relevant comments into pg_operator, and give each underlying
function a comment that just says "implementation of XXX operator".
There are only about half a dozen cases where it seems reasonable to use
the underlying function interchangeably with the operator; in these cases
I left the same comment in place on the function as on the operator.

While at it, establish a policy that every built-in function and operator
entry should have a comment: there are now queries in the opr_sanity
regression test that will complain if one doesn't.  This only required
adding a dozen or two more entries than would have been there anyway.

I also spent some time trying to eliminate gratuitous inconsistencies in
the style of the comments, though it's hopeless to suppose that more won't
creep in soon enough.

Per my proposal of 2010-10-15.
This commit is contained in:
Tom Lane 2011-03-03 01:33:19 -05:00
parent 091bda0188
commit 94133a9354
6 changed files with 1676 additions and 838 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
src/bin/psql/describe.c
View File

@ -559,6 +559,19 @@ describeOperators(const char *pattern, bool showSystem)
initPQExpBuffer(&buf);
/*
* Note: before Postgres 9.1, we did not assign comments to any built-in
* operators, preferring to let the comment on the underlying function
* suffice. The coalesce() on the obj_description() calls below supports
* this convention by providing a fallback lookup of a comment on the
* operator's function. As of 9.1 there is a policy that every built-in
* operator should have a comment; so the coalesce() is no longer
* necessary so far as built-in operators are concerned. We keep it
* anyway, for now, because (1) third-party modules may still be following
* the old convention, and (2) we'd need to do it anyway when talking to a
* pre-9.1 server.
*/
printfPQExpBuffer(&buf,
"SELECT n.nspname as \"%s\",\n"
" o.oprname AS \"%s\",\n"
@ -877,7 +890,7 @@ objectDescription(const char *pattern, bool showSystem)
"n.nspname", "p.proname", NULL,
"pg_catalog.pg_function_is_visible(p.oid)");
/* Operator descriptions (only if operator has its own comment) */
/* Operator descriptions */
appendPQExpBuffer(&buf,
"UNION ALL\n"
" SELECT o.oid as oid, o.tableoid as tableoid,\n"
@ -896,7 +909,7 @@ objectDescription(const char *pattern, bool showSystem)
"n.nspname", "o.oprname", NULL,
"pg_catalog.pg_operator_is_visible(o.oid)");
/* Type description */
/* Type descriptions */
appendPQExpBuffer(&buf,
"UNION ALL\n"
" SELECT t.oid as oid, t.tableoid as tableoid,\n"
@ -942,7 +955,7 @@ objectDescription(const char *pattern, bool showSystem)
"n.nspname", "c.relname", NULL,
"pg_catalog.pg_table_is_visible(c.oid)");
/* Rule description (ignore rules for views) */
/* Rule descriptions (ignore rules for views) */
appendPQExpBuffer(&buf,
"UNION ALL\n"
" SELECT r.oid as oid, r.tableoid as tableoid,\n"
@ -964,7 +977,7 @@ objectDescription(const char *pattern, bool showSystem)
"n.nspname", "r.rulename", NULL,
"pg_catalog.pg_table_is_visible(c.oid)");
/* Trigger description */
/* Trigger descriptions */
appendPQExpBuffer(&buf,
"UNION ALL\n"
" SELECT t.oid as oid, t.tableoid as tableoid,\n"

2
src/include/catalog/catversion.h
View File

@ -53,6 +53,6 @@
*/
/* yyyymmddN */
#define CATALOG_VERSION_NO 201102281
#define CATALOG_VERSION_NO 201103031
#endif

709
src/include/catalog/pg_operator.h
View File

File diff suppressed because it is too large Load Diff

1694
src/include/catalog/pg_proc.h
View File

File diff suppressed because it is too large Load Diff

51
src/test/regress/expected/opr_sanity.out
View File

@ -324,6 +324,15 @@ WHERE proargmodes IS NOT NULL AND proargnames IS NOT NULL AND
-----+---------
(0 rows)
-- Insist that all built-in pg_proc entries have descriptions
SELECT p1.oid, p1.proname
FROM pg_proc as p1 LEFT JOIN pg_description as d
ON p1.tableoid = d.classoid and p1.oid = d.objoid and d.objsubid = 0
WHERE d.classoid IS NULL AND p1.oid <= 9999;
oid | proname
-----+---------
(0 rows)
-- **************** pg_cast ****************
-- Catch bogus values in pg_cast columns (other than cases detected by
-- oidjoins test).
@ -642,6 +651,48 @@ WHERE p1.oprjoin = p2.oid AND
-----+---------+-----+---------
(0 rows)
-- Insist that all built-in pg_operator entries have descriptions
SELECT p1.oid, p1.oprname
FROM pg_operator as p1 LEFT JOIN pg_description as d
ON p1.tableoid = d.classoid and p1.oid = d.objoid and d.objsubid = 0
WHERE d.classoid IS NULL AND p1.oid <= 9999;
oid | oprname
-----+---------
(0 rows)
-- Check that operators' underlying functions have suitable comments,
-- namely 'implementation of XXX operator'. In some cases (mostly legacy
-- duplicate names for operators) there are multiple operators referencing
-- the same pg_proc entry, and of course the function comment can only match
-- one of them; so don't print functions for which there's any matching
-- entry. This still leaves a small number of functions for which the
-- comment is intentionally different because we expect the function to be
-- used on its own as well as via the operator; generally, in these special
-- cases, the function and operator comments should match.
WITH funcdescs AS (
SELECT p.oid as p_oid, proname, o.oid::regoperator as operator,
obj_description(p.oid, 'pg_proc') as prodesc,
'implementation of ' || oprname || ' operator' as expecteddesc,
obj_description(o.oid, 'pg_operator') as oprdesc
FROM pg_proc p JOIN pg_operator o ON oprcode = p.oid
WHERE o.oid <= 9999
)
SELECT p_oid, proname, operator, prodesc, oprdesc FROM funcdescs
WHERE prodesc IS DISTINCT FROM expecteddesc
AND NOT EXISTS (SELECT 1 FROM funcdescs f2
WHERE f2.p_oid = funcdescs.p_oid
AND f2.prodesc = f2.expecteddesc)
ORDER BY 1,3;
p_oid | proname | operator | prodesc | oprdesc
-------+---------------+-------------------------+-------------------------------------+-------------------------------------
378 | array_append | ||(anyarray,anyelement) | append element onto end of array | append element onto end of array
379 | array_prepend | ||(anyelement,anyarray) | prepend element onto front of array | prepend element onto front of array
1035 | aclinsert | +(aclitem[],aclitem) | add/update ACL item | add/update ACL item
1036 | aclremove | -(aclitem[],aclitem) | remove ACL item | remove ACL item
1037 | aclcontains | @>(aclitem[],aclitem) | contains | contains
1037 | aclcontains | ~(aclitem[],aclitem) | contains | contains
(6 rows)
-- **************** pg_aggregate ****************
-- Look for illegal values in pg_aggregate fields.
SELECT ctid, aggfnoid::oid

37
src/test/regress/sql/opr_sanity.sql
View File

@ -250,6 +250,12 @@ FROM pg_proc as p1
WHERE proargmodes IS NOT NULL AND proargnames IS NOT NULL AND
array_length(proargmodes,1) <> array_length(proargnames,1);
-- Insist that all built-in pg_proc entries have descriptions
SELECT p1.oid, p1.proname
FROM pg_proc as p1 LEFT JOIN pg_description as d
ON p1.tableoid = d.classoid and p1.oid = d.objoid and d.objsubid = 0
WHERE d.classoid IS NULL AND p1.oid <= 9999;
-- **************** pg_cast ****************
@ -516,6 +522,37 @@ WHERE p1.oprjoin = p2.oid AND
p2.proargtypes[3] != 'int2'::regtype OR
p2.proargtypes[4] != 'internal'::regtype);
-- Insist that all built-in pg_operator entries have descriptions
SELECT p1.oid, p1.oprname
FROM pg_operator as p1 LEFT JOIN pg_description as d
ON p1.tableoid = d.classoid and p1.oid = d.objoid and d.objsubid = 0
WHERE d.classoid IS NULL AND p1.oid <= 9999;
-- Check that operators' underlying functions have suitable comments,
-- namely 'implementation of XXX operator'. In some cases (mostly legacy
-- duplicate names for operators) there are multiple operators referencing
-- the same pg_proc entry, and of course the function comment can only match
-- one of them; so don't print functions for which there's any matching
-- entry. This still leaves a small number of functions for which the
-- comment is intentionally different because we expect the function to be
-- used on its own as well as via the operator; generally, in these special
-- cases, the function and operator comments should match.
WITH funcdescs AS (
SELECT p.oid as p_oid, proname, o.oid::regoperator as operator,
obj_description(p.oid, 'pg_proc') as prodesc,
'implementation of ' || oprname || ' operator' as expecteddesc,
obj_description(o.oid, 'pg_operator') as oprdesc
FROM pg_proc p JOIN pg_operator o ON oprcode = p.oid
WHERE o.oid <= 9999
)
SELECT p_oid, proname, operator, prodesc, oprdesc FROM funcdescs
WHERE prodesc IS DISTINCT FROM expecteddesc
AND NOT EXISTS (SELECT 1 FROM funcdescs f2
WHERE f2.p_oid = funcdescs.p_oid
AND f2.prodesc = f2.expecteddesc)
ORDER BY 1,3;
-- **************** pg_aggregate ****************
-- Look for illegal values in pg_aggregate fields.