| From: | David Rowley <dgrowleyml(at)gmail(dot)com> |
|---|---|
| To: | Ilia Evdokimov <ilya(dot)evdokimov(at)tantorlabs(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Remove redundant if-else in EXPLAIN by using ExplainPropertyText |
| Date: | 2025-03-20 21:12:27 |
| Message-ID: | CAApHDvo+HL+suyFwDc90DOuqr6BsDHN09kbymGPdjshhpK1Vwg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Fri, 21 Mar 2025 at 09:24, Ilia Evdokimov
<ilya(dot)evdokimov(at)tantorlabs(dot)com> wrote:
> Thanks to David [0], we found that the if and else branches contained
> equivalent code. Since ExplainPropertyText already handles non-text
> formats, the extra condition is unnecessary.
>
> I reviewed other files related to EXPLAIN where similar patterns might
> exist, but this was the only instance I found.
I looked at; git grep -E "appendStringInfo.*\\n" --
src/backend/commands/explain.c and all those results are appending
multiple options. No single option ones.
The patch looks good to me and seems worth applying to master. I feel
this should be fixed to avoid misleading future patches that add new
properties to Memoize into copying the same pattern.
David
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Mark Dilger | 2025-03-20 21:14:20 | Re: Index AM API cleanup |
| Previous Message | Tom Lane | 2025-03-20 21:08:54 | Re: [PoC] Federated Authn/z with OAUTHBEARER |