Re: Use opresulttype instead of calling SearchSysCache1() in match_orclause_to_indexcol()

On Sun, Nov 16, 2025 at 7:27 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Tender Wang <tndrwang@gmail.com> writes:
> > Tom Lane <tgl@sss.pgh.pa.us> 于2025年11月16日周日 04:45写道：
> >> Yeah.  In fact, I think it's outright wrong to do that here.
> >> It'd result in building a SAOP expression that lacks the RelabelType,
> >> which seems incorrect since the operator is one that expects the
> >> relabeled type.
> >>
> >> The RelabelType-stripping logic for the constExpr seems unnecessary as
> >> well, if not outright wrong.  It won't matter for an actual Const,
> >> because eval_const_expressions would have flattened the relabeled type
> >> into the Const node.  However, if we have some non-Const right-hand
> >> sides, the effect of stripping RelabelTypes could easily be to fail the
> >> transformation unnecessarily.  That'd happen if the parser had coerced
> >> all the RHS values to be the same type for application of the operator,
> >> but then we stripped some RelabelTypes and mistakenly decided that
> >> the resulting RHSes didn't match in type.
>
> > Thank you for pointing that out. I hadn’t been aware of these problems
> > earlier.
>
> I made a test script (attached) that demonstrates that these problems
> are real.  In HEAD, if you look at the logged plan tree for the first
> query, you'll see that we have a SAOP with operator texteq whose first
> input is a bare varchar-type Var, unlike what you get with a plain
> indexqual such as "vc1 = '23'".  Now texteq() doesn't care, but there
> are polymorphic functions that do care because they look at the
> exposed types of their input arguments.  Also, HEAD fails to optimize
> the second test case into a SAOP because it's fooled itself by
> stripping the RelabelType from the outer-side Var.

Thank you so much for the clarification of this subject with examples.

> >> I'm not very convinced that the type_is_rowtype checks are correct
> >> either.  I can see that we'd better forbid RECORD, because we've got
> >> no way to be sure that all the RHSes are actually the same record
> >> type.  But I don't see why it's necessary or appropriate to forbid
> >> named composite types.  I didn't change that here; maybe we should
> >> look into the discussion leading up to d4378c000.
>
> > Agree.
>
> I dug into the history a little and could not find anything except
> [1], which says
>
>     I have made some changes (attachment).
>     * if the operator expression left or right side type category is
>     {array | domain | composite}, then don't do the transformation.
>     (i am not 10% sure with composite)
>
> with no further justification than that.  There were even messages
> later in the thread questioning the need for it, but nobody did
> anything about it.  The transformation does work perfectly well
> if enabled, as shown by the second part of the attached test script.

I think another email to reference is [1].  It analyses the problems
with row expressions, but finally it mistakenly generalizes that for
composite types.  So, yes, thread didn't show any problems with
composites.

> So I end with v3, now with a full-dress commit message.

It looks very good, thank you so much for dedicating your time on fixing this.

Links.
1. https://www.postgresql.org/message-id/flat/567ED6CA.2040504%40sigaev.ru

------
Regards,
Alexander Korotkov
Supabase