Tom Lane <[email protected]> 于2025年11月16日周日 04:45写道：
> Tender Wang <[email protected]> writes: > > Tom Lane <[email protected]> 于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. > Thank you for pointing that out. I hadn’t been aware of these problems earlier. > > 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. > Agree. The v2 patch LGTM. -- Thanks, Tender Wang
