From: Justin Pryzby <pryzby@telsasoft.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: strange case of "if ((a & b))"
Date: Thu, 24 Jun 2021 21:31:11 -0500
Message-ID: <20210625023110.GI29179@telsasoft.com> (raw)
In-Reply-To: <2896014.1619635209@sss.pgh.pa.us>
References: <20210428182936.GE27406@telsasoft.com>
<2896014.1619635209@sss.pgh.pa.us>
On Wed, Apr 28, 2021 at 02:40:09PM -0400, Tom Lane wrote:
> Justin Pryzby <pryzby@telsasoft.com> writes:
> > These look strange to me - the inner parens don't do anything.
> > I wouldn't write it with 2x parens for the same reason I wouldn't write it with
> > 8x parens.
>
> Agreed, but shouldn't we just drop the excess parens rather than
> doubling down on useless notation?
I believe I got the impression from Michael that there was a style preference
to write != 0.
0002 is a bonus patch I found in my typos branch. I will hold onto it for
later if nobody wants to deal with it.
Attachments:download all (2 files, .tar.gz)0001-Avoid-double-parens.patch (text/x-diff, 5.6K) download
From 2534ff46830ab505a6a2079a1b6c0f8a1e9b2b8b Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Wed, 28 Apr 2021 14:12:49 -0500
Subject: [PATCH 1/2] Avoid double parens
git grep -l '\<if (([^(]*))' '*.c'
---
contrib/amcheck/verify_heapam.c | 6 +++---
contrib/ltree/ltree_io.c | 6 +++---
src/backend/access/transam/xact.c | 6 +++---
src/backend/commands/tablecmds.c | 2 +-
src/backend/nodes/nodeFuncs.c | 4 ++--
src/backend/replication/logical/decode.c | 2 +-
6 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.cindex a3caee7cdd..cd275bd45d 100644--- a/contrib/amcheck/verify_heapam.c+++ b/contrib/amcheck/verify_heapam.c@@ -389,13 +389,13 @@ verify_heapam(PG_FUNCTION_ARGS)
&vmbuffer);
if (skip_option == SKIP_PAGES_ALL_FROZEN)
{
- if ((mapbits & VISIBILITYMAP_ALL_FROZEN) != 0)+ if (mapbits & VISIBILITYMAP_ALL_FROZEN)
continue;
}
if (skip_option == SKIP_PAGES_ALL_VISIBLE)
{
- if ((mapbits & VISIBILITYMAP_ALL_VISIBLE) != 0)+ if (mapbits & VISIBILITYMAP_ALL_VISIBLE)
continue;
}
}
@@ -692,7 +692,7 @@ check_tuple_header(HeapCheckContext *ctx)
report_corruption(ctx,
psprintf("tuple data should begin at byte %u, but actually begins at byte %u (1 attribute, has nulls)",
expected_hoff, ctx->tuphdr->t_hoff));
- else if ((infomask & HEAP_HASNULL))+ else if (infomask & HEAP_HASNULL)
report_corruption(ctx,
psprintf("tuple data should begin at byte %u, but actually begins at byte %u (%u attributes, has nulls)",
expected_hoff, ctx->tuphdr->t_hoff, ctx->natts));
diff --git a/contrib/ltree/ltree_io.c b/contrib/ltree/ltree_io.cindex 15115cb29f..b75f35d5b5 100644--- a/contrib/ltree/ltree_io.c+++ b/contrib/ltree/ltree_io.c@@ -661,17 +661,17 @@ deparse_lquery(const lquery *in)
}
memcpy(ptr, curtlevel->name, curtlevel->len);
ptr += curtlevel->len;
- if ((curtlevel->flag & LVAR_SUBLEXEME))+ if (curtlevel->flag & LVAR_SUBLEXEME)
{
*ptr = '%';
ptr++;
}
- if ((curtlevel->flag & LVAR_INCASE))+ if (curtlevel->flag & LVAR_INCASE)
{
*ptr = '@';
ptr++;
}
- if ((curtlevel->flag & LVAR_ANYEND))+ if (curtlevel->flag & LVAR_ANYEND)
{
*ptr = '*';
ptr++;
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.cindex da8a460722..9c13a7c589 100644--- a/src/backend/access/transam/xact.c+++ b/src/backend/access/transam/xact.c@@ -2452,7 +2452,7 @@ PrepareTransaction(void)
* cases, such as a temp table created and dropped all within the
* transaction. That seems to require much more bookkeeping though.
*/
- if ((MyXactFlags & XACT_FLAGS_ACCESSEDTEMPNAMESPACE))+ if (MyXactFlags & XACT_FLAGS_ACCESSEDTEMPNAMESPACE)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot PREPARE a transaction that has operated on temporary objects")));
@@ -5565,7 +5565,7 @@ XactLogCommitRecord(TimestampTz commit_time,
xl_xinfo.xinfo |= XACT_COMPLETION_UPDATE_RELCACHE_FILE;
if (forceSyncCommit)
xl_xinfo.xinfo |= XACT_COMPLETION_FORCE_SYNC_COMMIT;
- if ((xactflags & XACT_FLAGS_ACQUIREDACCESSEXCLUSIVELOCK))+ if (xactflags & XACT_FLAGS_ACQUIREDACCESSEXCLUSIVELOCK)
xl_xinfo.xinfo |= XACT_XINFO_HAS_AE_LOCKS;
/*
@@ -5716,7 +5716,7 @@ XactLogAbortRecord(TimestampTz abort_time,
xlrec.xact_time = abort_time;
- if ((xactflags & XACT_FLAGS_ACQUIREDACCESSEXCLUSIVELOCK))+ if (xactflags & XACT_FLAGS_ACQUIREDACCESSEXCLUSIVELOCK)
xl_xinfo.xinfo |= XACT_XINFO_HAS_AE_LOCKS;
if (nsubxacts > 0)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex 4e23c7fce5..39180999fd 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -16171,7 +16171,7 @@ PreCommit_on_commit_actions(void)
* relations, we can skip truncating ON COMMIT DELETE ROWS
* tables, as they must still be empty.
*/
- if ((MyXactFlags & XACT_FLAGS_ACCESSEDTEMPNAMESPACE))+ if (MyXactFlags & XACT_FLAGS_ACCESSEDTEMPNAMESPACE)
oids_to_truncate = lappend_oid(oids_to_truncate, oc->relid);
break;
case ONCOMMIT_DROP:
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.cindex ff3dcc7b18..ae3364dfdc 100644--- a/src/backend/nodes/nodeFuncs.c+++ b/src/backend/nodes/nodeFuncs.c@@ -2390,7 +2390,7 @@ query_tree_walker(Query *query,
* don't contain actual expressions. However they do contain OIDs which
* may be needed by dependency walkers etc.
*/
- if ((flags & QTW_EXAMINE_SORTGROUP))+ if (flags & QTW_EXAMINE_SORTGROUP)
{
if (walker((Node *) query->groupClause, context))
return true;
@@ -3328,7 +3328,7 @@ query_tree_mutator(Query *query,
* may be of interest to some mutators.
*/
- if ((flags & QTW_EXAMINE_SORTGROUP))+ if (flags & QTW_EXAMINE_SORTGROUP)
{
MUTATE(query->groupClause, query->groupClause, List *);
MUTATE(query->windowClause, query->windowClause, List *);
diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.cindex 453efc51e1..34a36b22f8 100644--- a/src/backend/replication/logical/decode.c+++ b/src/backend/replication/logical/decode.c@@ -337,7 +337,7 @@ DecodeXactOp(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
ReorderBufferXidSetCatalogChanges(ctx->reorder, xid,
buf->origptr);
}
- else if ((!ctx->fast_forward))+ else if (!ctx->fast_forward)
ReorderBufferImmediateInvalidation(ctx->reorder,
invals->nmsgs,
invals->msgs);
--
2.17.0
0002-tablefunc.c-Fix-comment.patch (text/x-diff, 994B) download
From 16773708a7ab2c995fa3d7e1bba1b3626468a8d0 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 8 May 2021 08:12:46 -0500
Subject: [PATCH 2/2] tablefunc.c: Fix comment
Mentioned at: https://www.postgresql.org/message-id/CAJRYxuKmMkgHFKjuYPxThdT5Mjg%2Bg-nH5szYbS8UoVsQXzd95A%40mail.gmail.com
---
contrib/tablefunc/tablefunc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/tablefunc/tablefunc.c b/contrib/tablefunc/tablefunc.cindex 779bd4415e..52b272f298 100644--- a/contrib/tablefunc/tablefunc.c+++ b/contrib/tablefunc/tablefunc.c@@ -1480,7 +1480,7 @@ validateConnectbyTupleDesc(TupleDesc td, bool show_branch, bool show_serial)
"fifth column must be type %s",
format_type_be(INT4OID))));
- /* check that the type of the fifth column is INT4 */+ /* check that the type of the fourth column is INT4 */
if (!show_branch && show_serial &&
TupleDescAttr(td, 3)->atttypid != INT4OID)
ereport(ERROR,
--
2.17.0
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pryzby@telsasoft.com, tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org
Subject: Re: strange case of "if ((a & b))"
In-Reply-To: <20210625023110.GI29179@telsasoft.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox