Peter Eisentraut <peter.eisentr...@enterprisedb.com> writes: > On 02.05.22 16:48, Tom Lane wrote: >> Peter Eisentraut <peter.eisentr...@enterprisedb.com> writes: >>> There are many calls to construct_array() and deconstruct_array() for >>> built-in types, for example, when dealing with system catalog columns. >>> These all hardcode the type attributes necessary to pass to these functions. >> >>> To simplify this a bit, add construct_array_builtin(), >>> deconstruct_array_builtin() as wrappers that centralize this hardcoded >>> knowledge. This simplifies many call sites and reduces the amount of >>> hardcoded stuff that is spread around. >> >>> I also considered having genbki.pl generate lookup tables for these >>> hardcoded values, similar to schemapg.h, but that ultimately seemed >>> excessive. >> +1 --- the added overhead of the switch statements is probably a >> reasonable price to pay for the notational simplification and >> bug-proofing. >> One minor coding gripe is that compilers that don't know that >> elog(ERROR) >> doesn't return will certainly generate "use of possibly-uninitialized >> variable" complaints. Suggest inserting "return NULL;" or similar into >> the default: cases. I'd also use more specific error wording to help >> people find where they need to add code when they make use of a new type; >> maybe like "type %u not supported by construct_array_builtin". > > I have pushed this with the improvements you had suggested.
I dind't pay much attention to this thread earlier, but I was struck by the duplication of the switch statement determining the elemlen, elembyval, and elemalign values between the construct and deconstruct functions. How about a common function they can both call? Something like: static void builtin_type_details(Oid elemtype, int *elemlen, bool *elembyval, char *elemalign); - ilmari