Tom Lane <[email protected]> 于2025年11月17日周一 01:27写道：
> Tender Wang <[email protected]> writes: > > Tom Lane <[email protected]> 于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. > Yeah, the plan of the second test case should be like below: postgres=# explain select t1.* from t1, t2 where t2.vc1 = '66' and (t1.vc1 = t2.x or t1.vc1 = '99'); QUERY PLAN ----------------------------------------------------------------------------- Nested Loop (cost=0.83..17.32 rows=2 width=5) -> Index Scan using t2_pkey on t2 (cost=0.42..8.44 rows=1 width=5) Index Cond: ((vc1)::text = '66'::text) -> Index Only Scan using t1_pkey on t1 (cost=0.42..8.87 rows=2 width=5) Index Cond: (vc1 = ANY (ARRAY[(t2.x)::text, '99'::text])) (5 rows) > > >> 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. > > So I end with v3, now with a full-dress commit message. > The v3 LGTM. -- Thanks, Tender Wang
