Re: proposal: type info support functions for functions that use "any" type - Mailing list pgsql-hackers
| From | Pavel Stehule |
|---|---|
| Subject | Re: proposal: type info support functions for functions that use "any" type |
| Date | |
| Msg-id | CAFj8pRBqobQN7ZOGLmDo9j_dFff0r4UUFvCZxCMtVrGhhxEY0w@mail.gmail.com Whole thread Raw |
| In response to | Re: proposal: type info support functions for functions that use "any" type (Tom Lane <tgl@sss.pgh.pa.us>) |
| List | pgsql-hackers |
pá 31. 7. 2020 v 2:32 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:
Daniel Gustafsson <daniel@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;
With this proposal I can write a good enough implementation of the "decode" function, although it cannot be 100% compatible. It can cover probably almost all use cases.
But this design doesn't help with ANSI compatible LEAD, LAG functions. There is a different strategy - optional argument is implicitly casted to type of first argument.
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.
This patch is reduced - first version allowed similar argument list transformations like parser does with COALESCE or CASE expressions.
When arguments are transformed early, then the body of function can be thin.
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.
It is by reduced design. First implementation did a transformation of the argument list too. Then the cast was visible in the argument list.
It is true, so this patch implements an alternative way to polymorphic types. I don't think it is necessarily bad (and this functionality is available only for C language). We do it for COALESCE, CASE, GREATEST, LEAST functions and minimally due lazy evaluation we don't try to rewrite these functionality to usual functions. I would not increase the complexity of Postgres type systems or introduce some specific features used just by me. When people start to write an application on Postgres, then the current system is almost good enough. But a different situation is when a significant factor is compatibility - this is a topic that I have to solve in Orafce or issue with LAG, LEAD functions. Introducing a special polymorphic type for some specific behavior is hard and maybe unacceptable work. For me (as extension author) it can be nice to have some possibility to modify a parse tree - without useless overhead. With this possibility, some functions can be lighter and faster - because casting will be outside the function.
Regards
Pavel
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
pgsql-hackers by date: