Обсуждение: Lack of RelabelType is causing me pain

Joe, do you recall the reasoning for this code in parse_coerce.c?
   if (targetTypeId == ANYOID ||       targetTypeId == ANYARRAYOID ||       targetTypeId == ANYELEMENTOID)   {       /*
assumecan_coerce_type verified that implicit coercion is okay */       /* NB: we do NOT want a RelabelType here */
return node;   }
 

This is AFAICT the only case where the parser will generate an
expression tree that is not labeled with the same datatype expected
by the next-higher operator.  That is precisely the sort of mismatch
that RelabelType was invented to avoid, and I'm afraid that we have
broken some things by regressing on the explicit representation of
type coercions.

The particular case that is causing me pain right now is that in my
modified tree with support for cross-datatype index operations, cases
involving anyarray_ops indexes are blowing up.  That's because the
visible input type of an indexed comparison isn't matching the declared
righthand input type of any operator in the opclass.  But RelabelType
was put in to avoid a number of other problems that I can't recall in
detail, so I am suspicious that this shortcut breaks other things too.

I think that the reason we did this was to allow get_fn_expr_argtype()
to see the unrelabeled datatype of the input to an anyarray/anyelement-
accepting function.  Couldn't we fix that locally in that function
instead of breaking a system-wide convention?  I'm thinking that we
could simply make that function "burrow down" through any RelabelTypes
for any/anyarray/anyelement.

Comments?
        regards, tom lane

Tom Lane wrote:
> Joe, do you recall the reasoning for this code in parse_coerce.c?
> 
>     if (targetTypeId == ANYOID ||
>         targetTypeId == ANYARRAYOID ||
>         targetTypeId == ANYELEMENTOID)
>     {
>         /* assume can_coerce_type verified that implicit coercion is okay */
>         /* NB: we do NOT want a RelabelType here */
>         return node;
>     }

I see this in REL7_3_STABLE
     else if (targetTypeId == ANYOID ||              targetTypeId == ANYARRAYOID)    {    /* assume can_coerce_type
verifiedthat implicit coercion is okay */        /* NB: we do NOT want a RelabelType here */        result = node;
}

This was introduced here:
------------------------------------------
Revision 2.80 / (download) - annotate - [select for diffs] , Thu Aug 22 
00:01:42 2002 UTC (14 months, 2 weeks ago) by tgl
Changes since 2.79: +42 -19 lines
Diff to previous 2.79

Add a bunch of pseudo-types to replace the behavior formerly associated
with OPAQUE, as per recent pghackers discussion.  I still want to do 
some more work on the 'cstring' pseudo-type, but I'm going to commit the 
bulk of the changes now before the tree starts shifting under me ...
------------------------------------------

I think I just followed suit when adding ANYELEMENTOID.

> This is AFAICT the only case where the parser will generate an
> expression tree that is not labeled with the same datatype expected
> by the next-higher operator.  That is precisely the sort of mismatch
> that RelabelType was invented to avoid, and I'm afraid that we have
> broken some things by regressing on the explicit representation of
> type coercions.
> 
> The particular case that is causing me pain right now is that in my
> modified tree with support for cross-datatype index operations, cases
> involving anyarray_ops indexes are blowing up.  That's because the
> visible input type of an indexed comparison isn't matching the declared
> righthand input type of any operator in the opclass.  But RelabelType
> was put in to avoid a number of other problems that I can't recall in
> detail, so I am suspicious that this shortcut breaks other things too.
> 
> I think that the reason we did this was to allow get_fn_expr_argtype()
> to see the unrelabeled datatype of the input to an anyarray/anyelement-
> accepting function.  Couldn't we fix that locally in that function
> instead of breaking a system-wide convention?  I'm thinking that we
> could simply make that function "burrow down" through any RelabelTypes
> for any/anyarray/anyelement.

Does the RelabelType keep a record of what was relabeled (I presume from 
your description above it does)? The original code above predates 
get_fn_expr_argtype() I think, but it sounds like a reasonable approach 
to me.

Joe

Joe Conway <mail@joeconway.com> writes:
> Tom Lane wrote:
>> Joe, do you recall the reasoning for this code in parse_coerce.c?
>> [much snipped]

> Does the RelabelType keep a record of what was relabeled (I presume from 
> your description above it does)?

The RelabelType node itself doesn't, but you can look to its input node
to see the initial type.  The code I was imagining adding to
get_fn_expr_argtype would go like
while (<node is a RelabelType with output ANYELEMENT/ANYARRAY/ANY>)    node := node->input;

to chain down to the first thing that isn't a Relabel.  You can see
examples of this coding pattern in various places in the optimizer that
want to ignore binary-compatible relabelings.

> The original code above predates get_fn_expr_argtype() I think,

Oh, okay, if the coding predates 7.4 then I'm not so concerned about it.
I was afraid we'd done this as of 7.4, in which case there's no field
experience to indicate that it's really safe in corner cases.

I have found a workaround for my immediate problem with indexing
behavior, so I think we can leave parse_coerce.c as-is for the moment,
but I'm planning to keep my eyes open for any evidence that we ought to
reconsider the decision to omit RelabelType here.  When RelabelType was
put in, the intention was that it would appear *anywhere* that the
actual output of one expression didn't match the expected input type of
its parent.
        regards, tom lane