Thread: Remove redundant if-else in EXPLAIN by using ExplainPropertyText
Hi hackers, 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’m attaching a patch with the fix. If anyone spots a similar case elsewhere, please let me know. [0]: https://www.postgresql.org/message-id/CAApHDvpq2gOt3gnzkd4Ud%3DwrT7YRGrF3zb29jgWzDsYEhETrOA%40mail.gmail.com -- Best regards, Ilia Evdokimov, Tantor Labs LLC.
Attachment
On Fri, 21 Mar 2025 at 09:24, Ilia Evdokimov <ilya.evdokimov@tantorlabs.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
On Fri, 21 Mar 2025 at 10:12, David Rowley <dgrowleyml@gmail.com> wrote: > The patch looks good to me and seems worth applying to master. Ok. Pushed. David