Re: Bug in pg_describe_object

From: Andreas Karlsson <andreas(at)proxel(dot)se>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Joel Jacobson <joel(at)gluefinance(dot)com>, Josh Berkus <josh(at)agliodbs(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr>, Jim Nasby <jim(at)nasby(dot)net>, Herrera Alvaro <alvherre(at)commandprompt(dot)com>, pgsql-hackers Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Bug in pg_describe_object
Date: 2011-01-11 23:52:20
Message-ID: 1294789940.14741.201.camel@jansson
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, 2011-01-11 at 14:01 -0500, Tom Lane wrote:
> It really shouldn't be useful to include those. Attend what it says in
> the fine manual for CREATE OPERATOR CLASS:
>
> In a FUNCTION clause, the operand data type(s) the function is
> intended to support, if different from the input data type(s) of
> the function (for B-tree and hash indexes) or the class's data
> type (for GIN and GiST indexes). These defaults are always
> correct, so there is no point in specifying op_type in a
> FUNCTION clause in CREATE OPERATOR CLASS, but the option is
> provided for consistency with the comparable syntax in ALTER
> OPERATOR FAMILY.
>
> The reason the ALTER OPERATOR FAMILY DROP syntax has to include operand
> types is that it lacks the full name/types of the referenced function.
> Since getObjectDescription *does* provide those, it doesn't serve any
> real purpose to repeat the information.
>
> regards, tom lane

Hm, that is not what I see when reading the source.

There can exist several entries in pg_amproc for one operator family
with the same short_number and function (both name and types). The only
difference is in lefttype and righttype. For example these two in
array_ops.

SELECT *, amproc::oid FROM pg_amproc WHERE oid IN (10608,10612);
amprocfamily | amproclefttype | amprocrighttype | amprocnum | amproc | amproc
--------------+----------------+-----------------+-----------+-----------+--------
2745 | 1009 | 1009 | 1 | bttextcmp | 360
2745 | 1015 | 1015 | 1 | bttextcmp | 360
(2 rows)

The reason you must specify the types in ALTER OPERATOR FAMILY DROP is
that otherwise it would not know which row of these two to drop. And
that information is not included in the current string returned by
getObjectDescription.

As I interpret it the pg_amproc entries belonging to the array_ops
family really belong to one of the opclasses (_int2_ops, _text_ops, ...)
and the lefttype and righttype are used to look up the amproc entries
based on the opcintype of the opclass of the index class in
IndexSupportInitialize. The comment in pg_amproc.h aslo seems to confirm
my view.[1]

So instead of

function 1 bttextcmp(text,text) of operator family array_ops for access method gin

or in a improved version of my stab at a patch

function 1 (text[],text[]) bttextcmp(text,text) of operator family array_ops for access method gin

it really is

function 1 bttextcmp(text,text) of operator class _text_ops for access method gin

The "imporved version" might be simpler to implement and more future
proof though if we start using pg_amproc for other lookups.

--

[1] From pg_amproc.h about lefttype and righttype.

> The primary key for this table is <amprocfamily, amproclefttype,
> amprocrighttype, amprocnum>. The "default" support functions for a
> particular opclass within the family are those with amproclefttype =
> amprocrighttype = opclass's opcintype. These are the ones loaded into the
> relcache for an index and typically used for internal index operations.
> Other support functions are typically used to handle cross-type indexable
> operators with oprleft/oprright matching the entry's amproclefttype and
> amprocrighttype. The exact behavior depends on the index AM, however, and
> some don't pay attention to non-default functions at all.

Regards,
Andreas

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2011-01-12 00:04:47 Re: Something fishy about the current Makefiles
Previous Message Andrew Dunstan 2011-01-11 23:44:53 Re: arrays as pl/perl input arguments [PATCH]