Remove some unnecessary if-condition
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t43058psql -h localhost -U postgresBuilt from patchset v1 (message #1), July 27, 2026 at 06:20 PM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t43058_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t43058_1 && git checkout t43058_1Patchset v1 (message #1) is on t43058_1
Hi
I found some likely unnecessary if-condition in code.
1. Some check in else branch seems unnecessary.
In (/src/backend/replication/logical/reorderbuffer.c)
① @@ -4068,7 +4068,7 @@ ReorderBufferToastAppendChunk(ReorderBuffer *rb, ReorderBufferTXN *txn,
bool found;
if (!found)
{
...
}
else if (found && chunk_seq != ent->last_chunk_seq + 1)
...
The check of "found" in else if branch seems unnecessary.
② (/src/backend/utils/init/postinit.c)
@@ -924,11 +924,8 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
bool bootstrap = IsBootstrapProcessingMode();
if (bootstrap)
{
...
}
else if(...)
{...}
else
{
if (!bootstrap)
{
...
}
}
The check of "bootstrap" in else branch seems unnecessary.
2.In (/src/interfaces/ecpg/compatlib/informix.c)
@@ -944,7 +944,7 @@ rupshift(char *str)
for (len--; str[len] && str[len] == ' '; len--);
The first "str[len]" seems unnecessary since " str[len] == ' '" will check it as well.
Do you think we should remove these if-condition for code clean ?
Best regards,
houzj
On Fri, Oct 9, 2020 at 6:29 AM Hou, Zhijie <houzj.fnst@cn.fujitsu.com> wrote:
Hi
I found some likely unnecessary if-condition in code.
1. Some check in else branch seems unnecessary.
In (/src/backend/replication/logical/reorderbuffer.c)
① @@ -4068,7 +4068,7 @@ ReorderBufferToastAppendChunk(ReorderBuffer *rb, ReorderBufferTXN *txn,bool found;
if (!found)
{
...
}
else if (found && chunk_seq != ent->last_chunk_seq + 1)
...The check of "found" in else if branch seems unnecessary.
② (/src/backend/utils/init/postinit.c)
@@ -924,11 +924,8 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,bool bootstrap = IsBootstrapProcessingMode();
if (bootstrap)
{
...
}
else if(...)
{...}
else
{
if (!bootstrap)
{
...
}
}The check of "bootstrap" in else branch seems unnecessary.
2.In (/src/interfaces/ecpg/compatlib/informix.c)
@@ -944,7 +944,7 @@ rupshift(char *str)for (len--; str[len] && str[len] == ' '; len--);
The first "str[len]" seems unnecessary since " str[len] == ' '" will check it as well.
Do you think we should remove these if-condition for code clean ?
To me it looks good to clean up the conditions as you have done in the
patch. Please add this to commitfest so that it's not forgotten. I
have verified the code and indeed the conditions you are removing are
unnecessary. So the patch can be marked as CFP right away.
--
Best Wishes,
Ashutosh Bapat
To me it looks good to clean up the conditions as you have done in the patch.
Please add this to commitfest so that it's not forgotten. I have verified
the code and indeed the conditions you are removing are unnecessary. So
the patch can be marked as CFP right away.
Thank you for reviewing! added it to commitfest
https://commitfest.postgresql.org/30/2760/
Best regards,
houzj
On Mon, Oct 12, 2020 at 11:42:31AM +0000, Hou, Zhijie wrote:
Thank you for reviewing! added it to commitfest
https://commitfest.postgresql.org/30/2760/
- if (!bootstrap)
- {
- pgstat_bestart();
- CommitTransactionCommand();
- }
+ pgstat_bestart();
+ CommitTransactionCommand();
FWIW, I prefer the original style here. The if/elif dance is quite
long here so it can be easy to miss by reading the code that no
transaction commit should happen in bootstrap mode as this is
conditioned only by the top of the if logic.I would also keep the code in reorderbuffer.c in its original shape,
because it does not actually hurt and changing it could introduce some
back-patching hazard, even if that be a conflict easy to fix.
There may be a point for the bit in informix.c, but similarly when you
think about back-patching I'd just keep it as it is.
--
Michael