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.
From 2ae2c73f4ed8ddce11d2a18474cb2e9e4fe75c6d Mon Sep 17 00:00:00 2001
From: Evdokimov Ilia <ilya.evdoki...@tantorlabs.com>
Date: Thu, 20 Mar 2025 23:18:38 +0300
Subject: [PATCH v1] Remove redundant if-else in EXPLAIN by using
ExplainPropertyText
---
src/backend/commands/explain.c | 15 +++------------
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 33a16d2d8e2..c65dc9b9434 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -3625,18 +3625,9 @@ show_memoize_info(MemoizeState *mstate, List *ancestors, ExplainState *es)
separator = ", ";
}
- if (es->format != EXPLAIN_FORMAT_TEXT)
- {
- ExplainPropertyText("Cache Key", keystr.data, es);
- ExplainPropertyText("Cache Mode", mstate->binary_mode ? "binary" : "logical", es);
- }
- else
- {
- ExplainIndentText(es);
- appendStringInfo(es->str, "Cache Key: %s\n", keystr.data);
- ExplainIndentText(es);
- appendStringInfo(es->str, "Cache Mode: %s\n", mstate->binary_mode ? "binary" : "logical");
- }
+ ExplainPropertyText("Cache Key", keystr.data, es);
+ ExplainPropertyText("Cache Mode", mstate->binary_mode ? "binary" : "logical", es);
+
pfree(keystr.data);
--
2.25.1
