On Fri, Mar 8, 2024 at 10:13 AM David Rowley <dgrowle...@gmail.com> wrote:
> The fix could also be to use deparseConst() in appendOrderByClause() > and have that handle Const EquivalenceMember instead. I'd rather just > skip them. To me, that seems less risky than ensuring deparseConst() > handles all Const types correctly. I've looked at this patch a bit. I once wondered why we don't check pathkey->pk_eclass->ec_has_const with EC_MUST_BE_REDUNDANT to see if the pathkey is not needed. Then I realized that a child member would not be marked as constant even if the child expr is a Const, as explained in add_child_rel_equivalences(). BTW, I wonder if it is possible that we have a pseudoconstant expression that is not of type Const. In such cases we would need to check 'bms_is_empty(pull_varnos(em_expr))' instead of 'IsA(em_expr, Const)'. However, I'm unable to think of such an expression in this context. The patch looks good to me otherwise. Thanks Richard
