Обсуждение: time for catalog/pg_cast.c?

I extracted from the latest multirange patch a bit that creates a new
routine CastCreate() in src/backend/catalog/pg_cast.c.  It contains the
catalog-accessing bits to create a new cast.  It seems harmless, so I
thought I'd apply it to get rid of a couple of hunks in the large patch.

(I also threw in a move of get_cast_oid from functioncmds.c to
lsyscache.c, which seems its natural place; at first I thought to put it
in catalog/pg_cast.c but really it's not a great place IMO.  This
function was invented out of whole cloth in commit fd1843ff8979.  I also
contemplated the move of CreateCast and DropCastById from functioncmds.c
to some new place, but creating a new commands/castcmds.c seemed a bit
excessive, so I left them in their current locations.)

-- 
Álvaro Herrera                            39°49'30"S 73°17'W
"The problem with the future is that it keeps turning into the present"
(Hobbes)

On 2020-Mar-09, Alvaro Herrera wrote:

> I extracted from the latest multirange patch a bit that creates a new
> routine CastCreate() in src/backend/catalog/pg_cast.c.  It contains the
> catalog-accessing bits to create a new cast.  It seems harmless, so I
> thought I'd apply it to get rid of a couple of hunks in the large patch.

I forgot to "git add" this comment addition before sending:

/*
 * ----------------------------------------------------------------
 *        CastCreate
 *
 * Forms and inserts catalog tuples for a new cast being created.
 * Caller must have already checked privileges, and done consistency
 * checks on the given datatypes and cast function (if applicable).
 *
 * 'behavior' indicates the dependency that the new cast will have on
 * its input and output types and the cast function.
 * ----------------------------------------------------------------
 */
ObjectAddress
CastCreate(Oid sourcetypeid, Oid targettypeid, Oid funcid, char castcontext,
           char castmethod, DependencyType behavior)


I think the only API consideration here is for 'castcontext', which we
pass here as the pg_type.h symbol, but could alternatively be passed as
CoercionContext enum values (from primnodes.h).  I think the pg_cast.h
char is okay, but maybe somebody has a different opinion.
We could also add some trivial asserts (like if procoid is not invalid,
then method is function, etc.), but it doesn't seem worth fussing too
much over.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

I would even say that DropCastById belongs in the new file, which is
just the attached.  However, none of the Drop.*ById or Remove.*ById
functions seem to be in backend/catalog/ at all, and moving just a
single one seems to make things even more inconsistent.  I think all
these catalog-accessing functions should be in backend/catalog/ but I'm
not in a hurry to patch half of backend/commands/ to move them all.

(I think the current arrangement is just fallout from having created the
dependency.c system to drop objects, which rid us of a bunch of bespoke
deletion-handling code.)

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2020-Mar-09, Alvaro Herrera wrote:

> I extracted from the latest multirange patch a bit that creates a new
> routine CastCreate() in src/backend/catalog/pg_cast.c.  It contains the
> catalog-accessing bits to create a new cast.  It seems harmless, so I
> thought I'd apply it to get rid of a couple of hunks in the large patch.

Pushed, thanks.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services