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

Tender Wang <tndrwang@gmail.com> writes:
> Tom Lane <tgl@sss.pgh.pa.us> 于2025年9月20日周六 00:16写道：
>> I don't understand what purpose this check serves at all.
>> We are looking at an arm of an OR clause, so it had better
>> yield boolean.

> Yeah, this check doesn't need any more. I removed this check in the
> attached patch.

> In match_index_to_operand(), the indexExpr has been ignored, so I removed
> below check, too.
> - if (IsA(indexExpr, RelabelType))
> - indexExpr = (Node *) ((RelabelType *) indexExpr)->arg;

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.

So I removed both of those in v2, attached.

>> In general, this code looks like a mess.  There are a lot of
>> Asserts that might be okay in development but probably should
>> not have got committed, and the comments need work.

> These assertions were removed by me, too.
> I didn’t modify these code comments since English isn’t my native language,
> and I’d appreciate your help with them.

Here's a v2 with some further cleanup work.  One notable item
is that I moved the type_is_rowtype checks, which don't seem
to need to be done more than once.

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.