Обсуждение: 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