Daniel Gustafsson <dan...@yesql.se> writes:
> This patch has been bumped in CFs for the past year, with the thread stalled
> and the last review comment being in support of rejection. Tom, do you still
> feel it should be rejected in light of Pavel's latest posts?
I have seen no convincing response to the concerns I raised in my
last message in the thread [1], to wit that
1. I think the "flexibility" of letting a support function resolve the
output type in some unspecified way is mostly illusory, because if it
doesn't do it in a way that's morally equivalent to polymorphism, it's
doing it wrong. Also, I'm not that excited about improving polymorphism
in a way that is only accessible with specialized C code. The example of
Oracle-like DECODE() could be handled about as well if we had a second set
of anycompatible-style polymorphic types, that is something like
decode(expr anycompatible,
search1 anycompatible, result1 anycompatible2,
search2 anycompatible, result2 anycompatible2,
search3 anycompatible, result3 anycompatible2,
...
) returns anycompatible2;
Admittedly, you'd need to write a separate declaration for each number of
arguments you wanted to support, but they could all point at the same C
function --- which'd be a lot simpler than in this patch, since it would
not need to deal with any type coercions, only comparisons.
I also argue that to the extent that the support function is reinventing
polymorphism internally, it's going to be inferior to the parser's
version. As an example, with Pavel's sample implementation, if a
particular query needs a coercion from type X to type Y, that's nowhere
visible in the parse tree. So you could drop the cast without being told
that view so-and-so depends on it, leading to a run-time failure next time
you try to use that view. Doing the same thing with normal polymorphism,
the X-to-Y cast function would be used in the parse tree and so we'd know
about the dependency.
2. I have no faith that the proposed implementation is correct or
complete. As I complained earlier, a lot of places have special-case
handling for polymorphism, and it seems like every one of them would
need to know about this feature too. That is, to the extent that
this patch's footprint is smaller than commit 24e2885ee -- which it
is, by a lot -- I think those are bugs of omission. It will not work
to have a situation where some parts of the backend resolve a function's
result type as one thing and others resolve it as something else thanks to
failure to account for this new feature. As a concrete example, it looks
like we'd fail pretty hard if someone tried to use this facility in an
aggregate support function.
So my opinion is still what it was in January.
regards, tom lane
[1] https://www.postgresql.org/message-id/31501.1579036195%40sss.pgh.pa.us
