Horton

Thread

Commits

  1. Simplify EXPLAIN code for Memoize

  1. Remove redundant if-else in EXPLAIN by using ExplainPropertyText

    Ilia Evdokimov <ilya.evdokimov@tantorlabs.com> — 2025-03-20T20:24:14Z

    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.

  2. Re: Remove redundant if-else in EXPLAIN by using ExplainPropertyText

    David Rowley <dgrowleyml@gmail.com> — 2025-03-20T21:12:27Z

    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

  3. Re: Remove redundant if-else in EXPLAIN by using ExplainPropertyText

    David Rowley <dgrowleyml@gmail.com> — 2025-03-21T00:41:09Z

    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