From: | Richard Guo <guofenglinux(at)gmail(dot)com> |
---|---|
To: | David Rowley <dgrowleyml(at)gmail(dot)com> |
Cc: | Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Michał Kłeczek <michal(at)kleczek(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Invalid query generated by postgres_fdw with UNION ALL and ORDER BY |
Date: | 2024-03-08 10:14:00 |
Message-ID: | CAMbWs4--3qi82XToCvfygf-uP83pjuqXGS7w3XV_RmGJTYMbcw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Mar 8, 2024 at 10:13 AM David Rowley <dgrowleyml(at)gmail(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Stephen Frost | 2024-03-08 10:17:56 | Re: Proposal to add page headers to SLRU pages |
Previous Message | Bharath Rupireddy | 2024-03-08 10:06:30 | Re: Add new error_action COPY ON_ERROR "log" |