Re: proposal: new polymorphic types - commontype and commontypearray

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Greg Stark <stark(at)mit(dot)edu>, David Steele <david(at)pgmasters(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: proposal: new polymorphic types - commontype and commontypearray
Date: 2020-03-13 22:42:32
Message-ID: 24137.1584139352@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> [ anycompatible-types-20191127.patch ]

I'm starting to review this patch seriously. I've found some bugs and
things I didn't like, and the documentation certainly needs work, but
I think I can get it to a committable state before too much longer.

What I want to talk about right now is some preliminary refactoring
that I'd like to do, as shown in the 0001 patch below. (0002 is the
rest of the patch as I currently have it.) There are two main things
in it:

1. Rearrange the macros in pseudotypes.c so that we don't have any
pure-boilerplate functions that aren't built by the macros. I don't
think this should be controversial, as it's not changing anything
functionally.

2. Refactor the function signature validation logic in pg_proc.c and
pg_aggregate.c to avoid having duplicate logic between those two.
I did that by creating new functions in parse_coerce.c (for lack of
a better place) that say whether a proposed result type or aggregate
transition type is valid given a particular set of declared input types.
The reason that this might be controversial is that it forces a slightly
less precise error detail message to be issued, since the call site that's
throwing the error doesn't know exactly which rule was being violated.
(For example, before there was a specific error message about anyrange
result requiring an anyrange input, and now there isn't.)

I think this is all right, mainly because we'd probably end up with
less-precise messages anyway for the more complex rules that 0002 is
going to add. If anybody's really hot about it, we could complicate
the API, say by having the call sites pass in the primary error message
or by having the checking subroutines pass back an errdetail string.

We definitely need to do *something* about that, because it's already
the case that pg_aggregate.c is out of step with pg_proc.c about
polymorphism rules --- it's not enforcing the anyrange rule. I think
there's probably no user-reachable bug in that, because an aggregate
is constrained by its implementation functions for which the rule
would be enforced, but it still seems not good.

Thoughts?

regards, tom lane

Attachment Content-Type Size
0001-refactoring.patch text/x-diff 32.3 KB
0002-add-anycompatible.patch text/x-diff 66.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Justin Pryzby 2020-03-14 00:10:00 Re: Berserk Autovacuum (let's save next Mandrill)
Previous Message Laurenz Albe 2020-03-13 21:48:27 Re: Berserk Autovacuum (let's save next Mandrill)