Re: ALTER TYPE 3: add facility to identify further no-work cases - Mailing list pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:
> ... A side issue is that I really
> want to avoid adding a new parser keyword for this.  It doesn't bother
> me too much to add keywords for really important and user-visible
> features, but when we're adding stuff that's only going to be used by
> 0.1% of our users it's really better if we can avoid it, because it
> slows down the parser.  Maybe we could do something like:

> CREATE CAST (source_type AS target_type)
>     WITH FUNCTION function_name (argument_type, [, ...])
>     [ ANALYZE USING function_name ]
>     [ AS ASSIGNMENT | AS IMPLICIT ]

I'm not terribly thrilled with the suggestion of "ANALYZE" here, because
given the way we use that word elsewhere, people are likely to think it
means something to do with statistics collection; or at least that it
implies some deep and complicated analysis of the cast.

I suggest using a phrase based on the idea that this function tells you
whether you can skip the cast, or (if the sense is inverted) whether the
cast has to be executed.  "SKIP IF function_name" would be nice but SKIP
isn't an extant keyword either.  The best variant that I can come up
with after a minute's contemplation of kwlist.h is "NO WORK IF
function_name".  If you didn't mind inverting the sense of the result
then we could use "EXECUTE IF function_name".

> Internally, we could refer to a function of this type as a "cast analyzer".

Another possibility is to call it a "cast checker" and use CHECK USING.
But I'm not sure that's much better than ANALYZE in terms of conveying
the purpose to a newbie.

>> I believe the patch does implement Tom's suggestion, at least in spirit. �He
>> mentioned expression_planner() as the place to do it. �In principle, We could do
>> it *right there* and avoid any changes to coerce_to_target_type(), but the
>> overhead would increase. �Specifically, we would be walking an already-built
>> expression tree looking for casts to remove, probably doing a syscache lookup
>> for every cast to grab the exemptor function OID.

No no no no.  The right place to do it is during expression
simplification in eval_const_expressions().  We are already
disassembling and reassembling the tree, and looking up functions,
in that pass.  Detecting that a call is a no-op fits into that
very naturally.  Think of it as an alternative to inline_function().

One point worth making here is that eval_const_expressions() does not
currently care very much whether a function call came from cast syntax
or explicitly.  It might be worth thinking about whether we want to have
a generic this-function-call-is-a-no-op simplifier hook available for
*all* functions not just those that are casts.  I'm not sure we want to
pay the overhead of another pg_proc column, but it's something to think
about.
        regards, tom lane