Subject: Unused parameters & co in code

From: Michael Paquier <[email-masked]>
Date: Wed, 30 Jan 2019 16:33:17 +0900

Hi all,

I just got a run with CFLAGS with the following options:
-Wunused-function -Wunused-variable -Wunused-const-variable
 -Wno-unused-result -Wunused-parameter -Wunused-macros

And while this generates a lot of noise for callbacks and depending on
the environment used, this is also pointing to things which could be
simplified:

Unused arguments in functions:
heap_getsysattr() => tupleDesc
brin_start_evacuating_page() => idxRel
UpdateIndexRelation() => parentIndexOid
SerializeReindexState() => maxsize
ginRedoInsertEntry() => isLeaf
ginReadTuple() => ginstate, attnum
startScanKey() => ginstate
resolve_unique_index_expr() => heapRel
gistGetNodeBuffer() => giststate
heap_tuple_needs_freeze() => buf
log_heap_visible() => rnode
transformSQLValueFunction() => pstate
HeapTupleSatisfiesSelf() => snapshot, buffer
HeapTupleSatisfiesAny() (Style enforced by HeapTupleSatisfiesVisibility)
_hash_get_newbucket_from_oldbucket => rel
RelationPutHeapTuple => relation
checkNameSpaceConflicts => pstate
toast_delete_datum => rel
visibilitymap_clear => rel
_bt_relbuf => rel (Quite some cleanup here for the btree code)
ReindexPartitionedIndex => parentIdx
assignOperTypes => typeinfo
storeProcedures => amoid
dropOperators => amoid
dropProcedures => amoid
ATExecColumnDefault => lockmode
rename_constraint_internal => recursing
ATExecDropIdentity => recursing
ATPrepSetStatistics => colNum, newValue, lockmode
ATExecAlterColumnGenericOptions => lockmode
ATPostAlterTypeParse => lockmode
ATExecEnableDisableRule => lockmode
sendAuthRequest => port (Quite some simplification)
get_qual_from_partbound => rel
alloc_edge_table, free_edge_table, gimme_edge, remove_gene => root
set_tablefunc_pathlist => rte
cost_bitmap_and_node
cost_gather_merge
cost_gather
eclass_useful_for_merging => root
initial_cost_hashjoin
clear_external_function_hash => filehandle (could be extended)
pg_SASL_init => payloadlen
reached_end_position => timeline, segment_finished
syncTargetDirectory => argv0
parseAclItem => name
describeAggregates => verbose
blackholeNoticeProcessor => arg

There are also some unused constants and macros:
PREFER in regcomp.c
STATHDRSIZE in tsvector_op.c
NAMESPACE_SQLXML in xml.c (here for documentation)
messageEquals in parallel.c

For some of these functions, the format is determined by the context,
take HeapTupleSatisfiesSelf or some planner routines.  For others, it
usually comes from some code deleted which did not actually correct
the related function.  Anyway, simplifying all that means usually a
back-patch pain.  However, in some cases, like for example _bt_relbuf,
ReindexPartitionedIndex or sendAuthRequest it can simplify the code as
there is no need to move some extra arguments around from an upper
layer.  Fixing all of these makes little sense, still it seems to me
that the cases where we can get extra simplifications for other code
paths calling the functions makes sense and that we should fix them.

Thoughts?
--
Michael

---

From: Alvaro Herrera <[email-masked]>
Date: Wed, 30 Jan 2019 08:14:05 -0300

> ReindexPartitionedIndex => parentIdx

I'd not change this one, as we will surely want to do something useful
eventually.  Not that it matters, but it'd be useless churn.

> blackholeNoticeProcessor => arg

This one's signature is enforced by PQsetNoticeProcessor.

> _bt_relbuf => rel (Quite some cleanup here for the btree code)

If this is a worthwhile cleanup, let's see a patch.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

---

From: Peter Eisentraut <[email-masked]>
Date: Wed, 30 Jan 2019 12:33:47 +0100

> -Wunused-function -Wunused-variable -Wunused-const-variable

These are part of -Wall.

>  -Wno-unused-result

What is the purpose of this?

> -Wunused-parameter

I think it's worth cleaning this up a bit, but it needs to be done by
hand, unless you want to add a large number of pg_attribute_unused()
decorations.

> -Wunused-macros

I have looked into that in the past.  There are a few that were
genuinely left behind accidentally, but most are there for completeness
or symmetry and don't look like they should be removed.  Also you get
junk from flex and perl and the like.  Needs to be addressed case by
case.  I can dig up my old branch and make some proposals.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

---

From: Tom Lane <[email-masked]>
Date: Wed, 30 Jan 2019 09:41:04 -0500

Michael Paquier <[email-masked]> writes:
> ... while this generates a lot of noise for callbacks and depending on

I'd definitely take this on a case-by-case basis.  In the planner
functions you mentioned, for instance, I'd be pretty hesitant to remove
the "root" parameter even if it happens not to be needed today.
We'd probably just end up putting it back in the future, because almost
everything in the planner needs that.  I'd only consider removing it in
cases where there was a solid reason to require the function not to need
it ever (as for instance what I just did to flatten_join_alias_vars).

In cases where we can get simplifications of calling layers, and
it doesn't seem likely that we'd have to undo it in future, then
probably it's worth the trouble to change.

			regards, tom lane

---

From: Michael Paquier <[email-masked]>
Date: Thu, 31 Jan 2019 09:30:06 +0900

> it ever (as for instance what I just did to flatten_join_alias_vars).

Definitely agreed, this is a case-by-case.  For the callbacks and
hooks it makes no sense, the planner ones and a couple of layers like
shared memory calculation are just here for symmetry, so most of the
report is really noise (I deliberately discarded anything related to
bison and generated code of course).

> probably it's worth the trouble to change.

Some of these are in tablecmds.c, and visibly worth the trouble.  The
ones in the btree, gin and gist code also could do for some cleanup at
quick sight.  And these are places where we complain a lot about the
complexity of the code.
--
Michael

---

From: Michael Paquier <[email-masked]>
Date: Thu, 31 Jan 2019 09:35:50 +0900

>> -Wunused-function -Wunused-variable -Wunused-const-variable

The ones selected already generate a lot of junk, so increasing the
output is not really a good idea.  What I wanted to find out are the
spots where we could be able to simplify the code for any unused
parameter.  As you mention, some parameters are here for symmetry in
the declaration, which makes sense in some cases, but for some other
cases I think that we may be able to reduce logic complexity, and this
gives hints about that.

> What is the purpose of this?

Not really useful actually as we don't mark anything with
warn_unused_result.

> case.  I can dig up my old branch and make some proposals.

Thanks.  Maybe I missed some of them.  Some macros, like the xml one,
are here for documentation purposes, so removing such things does not
make sense either.
--
Michael

---

From: Michael Paquier <[email-masked]>
Date: Thu, 31 Jan 2019 15:47:59 +0900

> eventually.  Not that it matters, but it'd be useless churn.

A lot of these things assume that the unused arguments may be useful
in the future.

> If this is a worthwhile cleanup, let's see a patch.

This cleanup is disappointing, so it can be discarded.

I looked at all the references I have spotted yesterday, and most of
them are not really worth the trouble, still there are some which
could be cleaned up.  Attached is a set of patches which do some
cleanup here and there, I don't propose all of them for commit, some
of them are attached just for reference:
- 0001 cleans up port in SendAuthRequest.  This one is disappointing,
so I am fine to discard it.
- 0002 works on _bt_relbuf, whose callers don't actually benefit from
the cleanup as the relation worked on is always used for different
reasons, so it can be discarded.
- 0003 works on the code of GIN, which simplifies at least the code,
so it could be applied.  This removes more than changed.
- 0004 also cleans some code for clause parsing, with a negative line
output.
- 0005 is for pg_rewind, which is some stuff I introduced, so I'd like
to clean up my mess and the change is recent :)
- 0006 is for tablecmds.c, and something I would like to apply because
it reduces some confusion with some recursion arguments which are not
needed for constraint handling and inheritance.  Most of the complains
come from lockmode not being used but all the AtPrep and AtExec
routines are rather symmetric so I am not bothering about that.
--
Michael
From 2b86907b218798545e17e5f6cbc985f8368387b5 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email-masked]>
Date: Thu, 31 Jan 2019 14:20:33 +0900
Subject: [PATCH 1/6] Simplify SendAuthRequest

---
 src/backend/libpq/auth.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 20fe98fb82..f92f78e17a 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -43,8 +43,8 @@
  * Global authentication functions
  *----------------------------------------------------------------
  */
-static void sendAuthRequest(Port *port, AuthRequest areq, const char *extradata,
-				int extralen);
+static void sendAuthRequest(AuthRequest areq, const char *extradata,
+							int extralen);
 static void auth_failed(Port *port, int status, char *logdetail);
 static char *recv_password_packet(Port *port);
 
@@ -523,7 +523,7 @@ ClientAuthentication(Port *port)
 
 		case uaGSS:
 #ifdef ENABLE_GSS
-			sendAuthRequest(port, AUTH_REQ_GSS, NULL, 0);
+			sendAuthRequest(AUTH_REQ_GSS, NULL, 0);
 			status = pg_GSS_recvauth(port);
 #else
 			Assert(false);
@@ -532,7 +532,7 @@ ClientAuthentication(Port *port)
 
 		case uaSSPI:
 #ifdef ENABLE_SSPI
-			sendAuthRequest(port, AUTH_REQ_SSPI, NULL, 0);
+			sendAuthRequest(AUTH_REQ_SSPI, NULL, 0);
 			status = pg_SSPI_recvauth(port);
 #else
 			Assert(false);
@@ -603,7 +603,7 @@ ClientAuthentication(Port *port)
 		(*ClientAuthentication_hook) (port, status);
 
 	if (status == STATUS_OK)
-		sendAuthRequest(port, AUTH_REQ_OK, NULL, 0);
+		sendAuthRequest(AUTH_REQ_OK, NULL, 0);
 	else
 		auth_failed(port, status, logdetail);
 }
@@ -613,7 +613,7 @@ ClientAuthentication(Port *port)
  * Send an authentication request packet to the frontend.
  */
 static void
-sendAuthRequest(Port *port, AuthRequest areq, const char *extradata, int extralen)
+sendAuthRequest(AuthRequest areq, const char *extradata, int extralen)
 {
 	StringInfoData buf;
 
@@ -740,7 +740,7 @@ CheckPasswordAuth(Port *port, char **logdetail)
 	int			result;
 	char	   *shadow_pass;
 
-	sendAuthRequest(port, AUTH_REQ_PASSWORD, NULL, 0);
+	sendAuthRequest(AUTH_REQ_PASSWORD, NULL, 0);
 
 	passwd = recv_password_packet(port);
 	if (passwd == NULL)
@@ -841,7 +841,7 @@ CheckMD5Auth(Port *port, char *shadow_pass, char **logdetail)
 		return STATUS_ERROR;
 	}
 
-	sendAuthRequest(port, AUTH_REQ_MD5, md5Salt, 4);
+	sendAuthRequest(AUTH_REQ_MD5, md5Salt, 4);
 
 	passwd = recv_password_packet(port);
 	if (passwd == NULL)
@@ -895,7 +895,7 @@ CheckSCRAMAuth(Port *port, char *shadow_pass, char **logdetail)
 	/* Put another '\0' to mark that list is finished. */
 	appendStringInfoChar(&sasl_mechs, '\0');
 
-	sendAuthRequest(port, AUTH_REQ_SASL, sasl_mechs.data, sasl_mechs.len);
+	sendAuthRequest(AUTH_REQ_SASL, sasl_mechs.data, sasl_mechs.len);
 	pfree(sasl_mechs.data);
 
 	/*
@@ -1000,9 +1000,9 @@ CheckSCRAMAuth(Port *port, char *shadow_pass, char **logdetail)
 			elog(DEBUG4, "sending SASL challenge of length %u", outputlen);
 
 			if (result == SASL_EXCHANGE_SUCCESS)
-				sendAuthRequest(port, AUTH_REQ_SASL_FIN, output, outputlen);
+				sendAuthRequest(AUTH_REQ_SASL_FIN, output, outputlen);
 			else
-				sendAuthRequest(port, AUTH_REQ_SASL_CONT, output, outputlen);
+				sendAuthRequest(AUTH_REQ_SASL_CONT, output, outputlen);
 
 			pfree(output);
 		}
@@ -1220,7 +1220,7 @@ pg_GSS_recvauth(Port *port)
 			elog(DEBUG4, "sending GSS response token of length %u",
 				 (unsigned int) port->gss->outbuf.length);
 
-			sendAuthRequest(port, AUTH_REQ_GSS_CONT,
+			sendAuthRequest(AUTH_REQ_GSS_CONT,
 							port->gss->outbuf.value, port->gss->outbuf.length);
 
 			gss_release_buffer(&lmin_s, &port->gss->outbuf);
@@ -1472,7 +1472,7 @@ pg_SSPI_recvauth(Port *port)
 			port->gss->outbuf.length = outbuf.pBuffers[0].cbBuffer;
 			port->gss->outbuf.value = outbuf.pBuffers[0].pvBuffer;
 
-			sendAuthRequest(port, AUTH_REQ_GSS_CONT,
+			sendAuthRequest(AUTH_REQ_GSS_CONT,
 							port->gss->outbuf.value, port->gss->outbuf.length);
 
 			FreeContextBuffer(outbuf.pBuffers[0].pvBuffer);
@@ -2103,7 +2103,7 @@ pam_passwd_conv_proc(int num_msg, const struct pam_message **msg,
 					 * let's go ask the client to send a password, which we
 					 * then stuff into PAM.
 					 */
-					sendAuthRequest(pam_port_cludge, AUTH_REQ_PASSWORD, NULL, 0);
+					sendAuthRequest(AUTH_REQ_PASSWORD, NULL, 0);
 					passwd = recv_password_packet(pam_port_cludge);
 					if (passwd == NULL)
 					{
@@ -2300,7 +2300,7 @@ CheckBSDAuth(Port *port, char *user)
 	int			retval;
 
 	/* Send regular password request to client, and get the response */
-	sendAuthRequest(port, AUTH_REQ_PASSWORD, NULL, 0);
+	sendAuthRequest(AUTH_REQ_PASSWORD, NULL, 0);
 
 	passwd = recv_password_packet(port);
 	if (passwd == NULL)
@@ -2561,7 +2561,7 @@ CheckLDAPAuth(Port *port)
 			port->hba->ldapport = LDAP_PORT;
 	}
 
-	sendAuthRequest(port, AUTH_REQ_PASSWORD, NULL, 0);
+	sendAuthRequest(AUTH_REQ_PASSWORD, NULL, 0);
 
 	passwd = recv_password_packet(port);
 	if (passwd == NULL)
@@ -2910,7 +2910,7 @@ CheckRADIUSAuth(Port *port)
 	}
 
 	/* Send regular password request to client, and get the response */
-	sendAuthRequest(port, AUTH_REQ_PASSWORD, NULL, 0);
+	sendAuthRequest(AUTH_REQ_PASSWORD, NULL, 0);
 
 	passwd = recv_password_packet(port);
 	if (passwd == NULL)
-- 
2.20.1

From 8b0fe44806ecc1f080990f38349e20e388b5b209 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email-masked]>
Date: Thu, 31 Jan 2019 14:27:39 +0900
Subject: [PATCH 2/6] Simplify _bt_relbuf

---
 src/backend/access/nbtree/nbtinsert.c | 48 ++++++++++----------
 src/backend/access/nbtree/nbtpage.c   | 64 +++++++++++++--------------
 src/backend/access/nbtree/nbtree.c    |  8 ++--
 src/backend/access/nbtree/nbtsearch.c | 14 +++---
 src/backend/access/nbtree/nbtutils.c  |  2 +-
 src/include/access/nbtree.h           |  2 +-
 6 files changed, 69 insertions(+), 69 deletions(-)

diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c
index 5c2b8034f5..71249a24ec 100644
--- a/src/backend/access/nbtree/nbtinsert.c
+++ b/src/backend/access/nbtree/nbtinsert.c
@@ -191,7 +191,7 @@ top:
 			}
 			else
 			{
-				_bt_relbuf(rel, buf);
+				_bt_relbuf(buf);
 
 				/*
 				 * Something did not work out. Just forget about the cached
@@ -256,7 +256,7 @@ top:
 		if (TransactionIdIsValid(xwait))
 		{
 			/* Have to wait for the other guy ... */
-			_bt_relbuf(rel, buf);
+			_bt_relbuf(buf);
 
 			/*
 			 * If it's a speculative insertion, wait for it to finish (ie. to
@@ -295,7 +295,7 @@ top:
 	else
 	{
 		/* just release the buffer */
-		_bt_relbuf(rel, buf);
+		_bt_relbuf(buf);
 	}
 
 	/* be tidy */
@@ -429,7 +429,7 @@ _bt_check_unique(Relation rel, IndexTuple itup, Relation heapRel,
 					if (checkUnique == UNIQUE_CHECK_PARTIAL)
 					{
 						if (nbuf != InvalidBuffer)
-							_bt_relbuf(rel, nbuf);
+							_bt_relbuf(nbuf);
 						*is_unique = false;
 						return InvalidTransactionId;
 					}
@@ -444,7 +444,7 @@ _bt_check_unique(Relation rel, IndexTuple itup, Relation heapRel,
 					if (TransactionIdIsValid(xwait))
 					{
 						if (nbuf != InvalidBuffer)
-							_bt_relbuf(rel, nbuf);
+							_bt_relbuf(nbuf);
 						/* Tell _bt_doinsert to wait... */
 						*speculativeToken = SnapshotDirty.speculativeToken;
 						return xwait;
@@ -499,8 +499,8 @@ _bt_check_unique(Relation rel, IndexTuple itup, Relation heapRel,
 					 * cause deadlocks.
 					 */
 					if (nbuf != InvalidBuffer)
-						_bt_relbuf(rel, nbuf);
-					_bt_relbuf(rel, buf);
+						_bt_relbuf(nbuf);
+					_bt_relbuf(buf);
 
 					{
 						Datum		values[INDEX_MAX_KEYS];
@@ -591,7 +591,7 @@ _bt_check_unique(Relation rel, IndexTuple itup, Relation heapRel,
 									RelationGetRelationName(rel))));
 
 	if (nbuf != InvalidBuffer)
-		_bt_relbuf(rel, nbuf);
+		_bt_relbuf(nbuf);
 
 	return InvalidTransactionId;
 }
@@ -761,7 +761,7 @@ _bt_findinsertloc(Relation rel,
 
 			rblkno = lpageop->btpo_next;
 		}
-		_bt_relbuf(rel, buf);
+		_bt_relbuf(buf);
 		buf = rbuf;
 		movedright = true;
 		vacuumed = false;
@@ -944,7 +944,7 @@ _bt_insertonpg(Relation rel,
 			if (metad->btm_fastlevel >= lpageop->btpo.level)
 			{
 				/* no update wanted */
-				_bt_relbuf(rel, metabuf);
+				_bt_relbuf(metabuf);
 				metabuf = InvalidBuffer;
 			}
 		}
@@ -1063,10 +1063,10 @@ _bt_insertonpg(Relation rel,
 
 		/* release buffers */
 		if (BufferIsValid(metabuf))
-			_bt_relbuf(rel, metabuf);
+			_bt_relbuf(metabuf);
 		if (BufferIsValid(cbuf))
-			_bt_relbuf(rel, cbuf);
-		_bt_relbuf(rel, buf);
+			_bt_relbuf(cbuf);
+		_bt_relbuf(buf);
 
 		/*
 		 * If we decided to cache the insertion target block, then set it now.
@@ -1526,11 +1526,11 @@ _bt_split(Relation rel, Buffer buf, Buffer cbuf, OffsetNumber firstright,
 
 	/* release the old right sibling */
 	if (!P_RIGHTMOST(ropaque))
-		_bt_relbuf(rel, sbuf);
+		_bt_relbuf(sbuf);
 
 	/* release the child */
 	if (!isleaf)
-		_bt_relbuf(rel, cbuf);
+		_bt_relbuf(cbuf);
 
 	/* split's done */
 	return rbuf;
@@ -1838,9 +1838,9 @@ _bt_insert_parent(Relation rel,
 		/* create a new root node and update the metapage */
 		rootbuf = _bt_newroot(rel, buf, rbuf);
 		/* release the split buffers */
-		_bt_relbuf(rel, rootbuf);
-		_bt_relbuf(rel, rbuf);
-		_bt_relbuf(rel, buf);
+		_bt_relbuf(rootbuf);
+		_bt_relbuf(rbuf);
+		_bt_relbuf(buf);
 	}
 	else
 	{
@@ -1867,7 +1867,7 @@ _bt_insert_parent(Relation rel,
 			stack->bts_offset = InvalidOffsetNumber;
 			stack->bts_btentry = InvalidBlockNumber;
 			stack->bts_parent = NULL;
-			_bt_relbuf(rel, pbuf);
+			_bt_relbuf(pbuf);
 		}
 
 		/* get high key from left page == lower bound for new right page */
@@ -1892,7 +1892,7 @@ _bt_insert_parent(Relation rel,
 		 * Now we can unlock the right child. The left child will be unlocked
 		 * by _bt_insertonpg().
 		 */
-		_bt_relbuf(rel, rbuf);
+		_bt_relbuf(rbuf);
 
 		/* Check for error only after writing children */
 		if (pbuf == InvalidBuffer)
@@ -1951,7 +1951,7 @@ _bt_finish_split(Relation rel, Buffer lbuf, BTStack stack)
 
 		was_root = (metad->btm_root == BufferGetBlockNumber(lbuf));
 
-		_bt_relbuf(rel, metabuf);
+		_bt_relbuf(metabuf);
 	}
 	else
 		was_root = false;
@@ -2072,12 +2072,12 @@ _bt_getstackbuf(Relation rel, BTStack stack, int access)
 		 */
 		if (P_RIGHTMOST(opaque))
 		{
-			_bt_relbuf(rel, buf);
+			_bt_relbuf(buf);
 			return InvalidBuffer;
 		}
 		blkno = opaque->btpo_next;
 		start = InvalidOffsetNumber;
-		_bt_relbuf(rel, buf);
+		_bt_relbuf(buf);
 	}
 }
 
@@ -2256,7 +2256,7 @@ _bt_newroot(Relation rel, Buffer lbuf, Buffer rbuf)
 	END_CRIT_SECTION();
 
 	/* done with metapage */
-	_bt_relbuf(rel, metabuf);
+	_bt_relbuf(metabuf);
 
 	pfree(left_item);
 	pfree(right_item);
diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c
index 1d72fe5408..de7462b091 100644
--- a/src/backend/access/nbtree/nbtpage.c
+++ b/src/backend/access/nbtree/nbtpage.c
@@ -175,7 +175,7 @@ _bt_update_meta_cleanup_info(Relation rel, TransactionId oldestBtpoXact,
 
 	if (!needsRewrite)
 	{
-		_bt_relbuf(rel, metabuf);
+		_bt_relbuf(metabuf);
 		return;
 	}
 
@@ -217,7 +217,7 @@ _bt_update_meta_cleanup_info(Relation rel, TransactionId oldestBtpoXact,
 	}
 
 	END_CRIT_SECTION();
-	_bt_relbuf(rel, metabuf);
+	_bt_relbuf(metabuf);
 }
 
 /*
@@ -298,7 +298,7 @@ _bt_getroot(Relation rel, int access)
 			/* OK, accept cached page as the root */
 			return rootbuf;
 		}
-		_bt_relbuf(rel, rootbuf);
+		_bt_relbuf(rootbuf);
 		/* Cache is stale, throw it away */
 		if (rel->rd_amcache)
 			pfree(rel->rd_amcache);
@@ -333,7 +333,7 @@ _bt_getroot(Relation rel, int access)
 		/* If access = BT_READ, caller doesn't want us to create root yet */
 		if (access == BT_READ)
 		{
-			_bt_relbuf(rel, metabuf);
+			_bt_relbuf(metabuf);
 			return InvalidBuffer;
 		}
 
@@ -354,7 +354,7 @@ _bt_getroot(Relation rel, int access)
 			 * over again.  (Is that really true? But it's hardly worth trying
 			 * to optimize this case.)
 			 */
-			_bt_relbuf(rel, metabuf);
+			_bt_relbuf(metabuf);
 			return _bt_getroot(rel, access);
 		}
 
@@ -431,7 +431,7 @@ _bt_getroot(Relation rel, int access)
 		LockBuffer(rootbuf, BT_READ);
 
 		/* okay, metadata is correct, release lock on it */
-		_bt_relbuf(rel, metabuf);
+		_bt_relbuf(metabuf);
 	}
 	else
 	{
@@ -541,7 +541,7 @@ _bt_gettrueroot(Relation rel)
 	/* if no root page initialized yet, fail */
 	if (metad->btm_root == P_NONE)
 	{
-		_bt_relbuf(rel, metabuf);
+		_bt_relbuf(metabuf);
 		return InvalidBuffer;
 	}
 
@@ -634,7 +634,7 @@ _bt_getrootheight(Relation rel)
 		 */
 		if (metad->btm_root == P_NONE)
 		{
-			_bt_relbuf(rel, metabuf);
+			_bt_relbuf(metabuf);
 			return 0;
 		}
 
@@ -643,7 +643,7 @@ _bt_getrootheight(Relation rel)
 		 */
 		_bt_cachemetadata(rel, metad);
 
-		_bt_relbuf(rel, metabuf);
+		_bt_relbuf(metabuf);
 	}
 
 	metad = (BTMetaPageData *) rel->rd_amcache;
@@ -801,7 +801,7 @@ _bt_getbuf(Relation rel, BlockNumber blkno, int access)
 					return buf;
 				}
 				elog(DEBUG2, "FSM returned nonrecyclable page");
-				_bt_relbuf(rel, buf);
+				_bt_relbuf(buf);
 			}
 			else
 			{
@@ -881,7 +881,7 @@ _bt_relandgetbuf(Relation rel, Buffer obuf, BlockNumber blkno, int access)
  * Lock and pin (refcount) are both dropped.
  */
 void
-_bt_relbuf(Relation rel, Buffer buf)
+_bt_relbuf(Buffer buf)
 {
 	UnlockReleaseBuffer(buf);
 }
@@ -1103,7 +1103,7 @@ _bt_is_page_halfdead(Relation rel, BlockNumber blk)
 	opaque = (BTPageOpaque) PageGetSpecialPointer(page);
 
 	result = P_ISHALFDEAD(opaque);
-	_bt_relbuf(rel, buf);
+	_bt_relbuf(buf);
 
 	return result;
 }
@@ -1180,7 +1180,7 @@ _bt_lock_branch_parent(Relation rel, BlockNumber child, BTStack stack,
 			if (P_RIGHTMOST(opaque) || P_ISROOT(opaque) ||
 				P_INCOMPLETE_SPLIT(opaque))
 			{
-				_bt_relbuf(rel, pbuf);
+				_bt_relbuf(pbuf);
 				return false;
 			}
 
@@ -1188,7 +1188,7 @@ _bt_lock_branch_parent(Relation rel, BlockNumber child, BTStack stack,
 			*rightsib = opaque->btpo_next;
 			leftsib = opaque->btpo_prev;
 
-			_bt_relbuf(rel, pbuf);
+			_bt_relbuf(pbuf);
 
 			/*
 			 * Like in _bt_pagedel, check that the left sibling is not marked
@@ -1216,10 +1216,10 @@ _bt_lock_branch_parent(Relation rel, BlockNumber child, BTStack stack,
 				if (lopaque->btpo_next == parent &&
 					P_INCOMPLETE_SPLIT(lopaque))
 				{
-					_bt_relbuf(rel, lbuf);
+					_bt_relbuf(lbuf);
 					return false;
 				}
-				_bt_relbuf(rel, lbuf);
+				_bt_relbuf(lbuf);
 			}
 
 			/*
@@ -1239,7 +1239,7 @@ _bt_lock_branch_parent(Relation rel, BlockNumber child, BTStack stack,
 		else
 		{
 			/* Unsafe to delete */
-			_bt_relbuf(rel, pbuf);
+			_bt_relbuf(pbuf);
 			return false;
 		}
 	}
@@ -1320,7 +1320,7 @@ _bt_pagedel(Relation rel, Buffer buf)
 						 errmsg("index \"%s\" contains a half-dead internal page",
 								RelationGetRelationName(rel)),
 						 errhint("This can be caused by an interrupted VACUUM in version 9.3 or older, before upgrade. Please REINDEX it.")));
-			_bt_relbuf(rel, buf);
+			_bt_relbuf(buf);
 			return ndeleted;
 		}
 
@@ -1348,7 +1348,7 @@ _bt_pagedel(Relation rel, Buffer buf)
 			/* Should never fail to delete a half-dead page */
 			Assert(!P_ISHALFDEAD(opaque));
 
-			_bt_relbuf(rel, buf);
+			_bt_relbuf(buf);
 			return ndeleted;
 		}
 
@@ -1413,10 +1413,10 @@ _bt_pagedel(Relation rel, Buffer buf)
 						P_INCOMPLETE_SPLIT(lopaque))
 					{
 						ReleaseBuffer(buf);
-						_bt_relbuf(rel, lbuf);
+						_bt_relbuf(lbuf);
 						return ndeleted;
 					}
-					_bt_relbuf(rel, lbuf);
+					_bt_relbuf(lbuf);
 				}
 
 				/* we need an insertion scan key for the search, so build one */
@@ -1426,7 +1426,7 @@ _bt_pagedel(Relation rel, Buffer buf)
 								   IndexRelationGetNumberOfKeyAttributes(rel),
 								   itup_scankey, false, &lbuf, BT_READ, NULL);
 				/* don't need a pin on the page */
-				_bt_relbuf(rel, lbuf);
+				_bt_relbuf(lbuf);
 
 				/*
 				 * Re-lock the leaf page, and start over, to re-check that the
@@ -1438,7 +1438,7 @@ _bt_pagedel(Relation rel, Buffer buf)
 
 			if (!_bt_mark_page_halfdead(rel, buf, stack))
 			{
-				_bt_relbuf(rel, buf);
+				_bt_relbuf(buf);
 				return ndeleted;
 			}
 		}
@@ -1462,7 +1462,7 @@ _bt_pagedel(Relation rel, Buffer buf)
 
 		rightsib = opaque->btpo_next;
 
-		_bt_relbuf(rel, buf);
+		_bt_relbuf(buf);
 
 		/*
 		 * Check here, as calling loops will have locks held, preventing
@@ -1663,7 +1663,7 @@ _bt_mark_page_halfdead(Relation rel, Buffer leafbuf, BTStack stack)
 
 	END_CRIT_SECTION();
 
-	_bt_relbuf(rel, topparent);
+	_bt_relbuf(topparent);
 	return true;
 }
 
@@ -1786,7 +1786,7 @@ _bt_unlink_halfdead_page(Relation rel, Buffer leafbuf, bool *rightsib_empty)
 		{
 			/* step right one page */
 			leftsib = opaque->btpo_next;
-			_bt_relbuf(rel, lbuf);
+			_bt_relbuf(lbuf);
 
 			/*
 			 * It'd be good to check for interrupts here, but it's not easy to
@@ -1804,7 +1804,7 @@ _bt_unlink_halfdead_page(Relation rel, Buffer leafbuf, bool *rightsib_empty)
 				{
 					/* we have only a pin on target, but pin+lock on leafbuf */
 					ReleaseBuffer(buf);
-					_bt_relbuf(rel, leafbuf);
+					_bt_relbuf(leafbuf);
 				}
 				else
 				{
@@ -1912,7 +1912,7 @@ _bt_unlink_halfdead_page(Relation rel, Buffer leafbuf, bool *rightsib_empty)
 			if (metad->btm_fastlevel > targetlevel + 1)
 			{
 				/* no update wanted */
-				_bt_relbuf(rel, metabuf);
+				_bt_relbuf(metabuf);
 				metabuf = InvalidBuffer;
 			}
 		}
@@ -2057,19 +2057,19 @@ _bt_unlink_halfdead_page(Relation rel, Buffer leafbuf, bool *rightsib_empty)
 
 	/* release metapage */
 	if (BufferIsValid(metabuf))
-		_bt_relbuf(rel, metabuf);
+		_bt_relbuf(metabuf);
 
 	/* release siblings */
 	if (BufferIsValid(lbuf))
-		_bt_relbuf(rel, lbuf);
-	_bt_relbuf(rel, rbuf);
+		_bt_relbuf(lbuf);
+	_bt_relbuf(rbuf);
 
 	/*
 	 * Release the target, if it was not the leaf block.  The leaf is always
 	 * kept locked.
 	 */
 	if (target != leafblkno)
-		_bt_relbuf(rel, buf);
+		_bt_relbuf(buf);
 
 	return true;
 }
diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index 98917de2ef..b5053269fb 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -839,7 +839,7 @@ _bt_vacuum_needs_cleanup(IndexVacuumInfo *info)
 			result = true;
 	}
 
-	_bt_relbuf(info->index, metabuf);
+	_bt_relbuf(metabuf);
 	return result;
 }
 
@@ -1061,7 +1061,7 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 		LockBufferForCleanup(buf);
 		_bt_checkpage(rel, buf);
 		_bt_delitems_vacuum(rel, buf, NULL, 0, vstate.lastBlockVacuumed);
-		_bt_relbuf(rel, buf);
+		_bt_relbuf(buf);
 	}
 
 	MemoryContextDelete(vstate.pagedelcontext);
@@ -1149,7 +1149,7 @@ restart:
 			!P_ISLEAF(opaque) ||
 			opaque->btpo_cycleid != vstate->cycleid)
 		{
-			_bt_relbuf(rel, buf);
+			_bt_relbuf(buf);
 			return;
 		}
 	}
@@ -1350,7 +1350,7 @@ restart:
 		/* pagedel released buffer, so we shouldn't */
 	}
 	else
-		_bt_relbuf(rel, buf);
+		_bt_relbuf(buf);
 
 	/*
 	 * This is really tail recursion, but if the compiler is too stupid to
diff --git a/src/backend/access/nbtree/nbtsearch.c b/src/backend/access/nbtree/nbtsearch.c
index 92832237a8..2afd71bf28 100644
--- a/src/backend/access/nbtree/nbtsearch.c
+++ b/src/backend/access/nbtree/nbtsearch.c
@@ -297,7 +297,7 @@ _bt_moveright(Relation rel,
 			if (P_INCOMPLETE_SPLIT(opaque))
 				_bt_finish_split(rel, buf, stack);
 			else
-				_bt_relbuf(rel, buf);
+				_bt_relbuf(buf);
 
 			/* re-acquire the lock in the right mode, and re-check */
 			buf = _bt_getbuf(rel, blkno, access);
@@ -1526,7 +1526,7 @@ _bt_readnextpage(IndexScanDesc scan, BlockNumber blkno, ScanDirection dir)
 			/* nope, keep going */
 			if (scan->parallel_scan != NULL)
 			{
-				_bt_relbuf(rel, so->currPos.buf);
+				_bt_relbuf(so->currPos.buf);
 				status = _bt_parallel_seize(scan, &blkno);
 				if (!status)
 				{
@@ -1537,7 +1537,7 @@ _bt_readnextpage(IndexScanDesc scan, BlockNumber blkno, ScanDirection dir)
 			else
 			{
 				blkno = opaque->btpo_next;
-				_bt_relbuf(rel, so->currPos.buf);
+				_bt_relbuf(so->currPos.buf);
 			}
 		}
 	}
@@ -1585,7 +1585,7 @@ _bt_readnextpage(IndexScanDesc scan, BlockNumber blkno, ScanDirection dir)
 			/* Done if we know there are no matching keys to the left */
 			if (!so->currPos.moreLeft)
 			{
-				_bt_relbuf(rel, so->currPos.buf);
+				_bt_relbuf(so->currPos.buf);
 				_bt_parallel_done(scan);
 				BTScanPosInvalidate(so->currPos);
 				return false;
@@ -1633,7 +1633,7 @@ _bt_readnextpage(IndexScanDesc scan, BlockNumber blkno, ScanDirection dir)
 			 */
 			if (scan->parallel_scan != NULL)
 			{
-				_bt_relbuf(rel, so->currPos.buf);
+				_bt_relbuf(so->currPos.buf);
 				status = _bt_parallel_seize(scan, &blkno);
 				if (!status)
 				{
@@ -1703,14 +1703,14 @@ _bt_walk_left(Relation rel, Buffer buf, Snapshot snapshot)
 		/* if we're at end of tree, release buf and return failure */
 		if (P_LEFTMOST(opaque))
 		{
-			_bt_relbuf(rel, buf);
+			_bt_relbuf(buf);
 			break;
 		}
 		/* remember original page we are stepping left from */
 		obknum = BufferGetBlockNumber(buf);
 		/* step left */
 		blkno = lblkno = opaque->btpo_prev;
-		_bt_relbuf(rel, buf);
+		_bt_relbuf(buf);
 		/* check for interrupts while we're not holding any buffer lock */
 		CHECK_FOR_INTERRUPTS();
 		buf = _bt_getbuf(rel, blkno, BT_READ);
diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c
index 2c05fb5e45..2cc02a30d5 100644
--- a/src/backend/access/nbtree/nbtutils.c
+++ b/src/backend/access/nbtree/nbtutils.c
@@ -1789,7 +1789,7 @@ _bt_killitems(IndexScanDesc scan)
 		else
 		{
 			/* Modified while not pinned means hinting is not safe. */
-			_bt_relbuf(scan->indexRelation, buf);
+			_bt_relbuf(buf);
 			return;
 		}
 	}
diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h
index 4fb92d60a1..f2310f337b 100644
--- a/src/include/access/nbtree.h
+++ b/src/include/access/nbtree.h
@@ -545,7 +545,7 @@ extern void _bt_checkpage(Relation rel, Buffer buf);
 extern Buffer _bt_getbuf(Relation rel, BlockNumber blkno, int access);
 extern Buffer _bt_relandgetbuf(Relation rel, Buffer obuf,
 				 BlockNumber blkno, int access);
-extern void _bt_relbuf(Relation rel, Buffer buf);
+extern void _bt_relbuf(Buffer buf);
 extern void _bt_pageinit(Page page, Size size);
 extern bool _bt_page_recyclable(Page page);
 extern void _bt_delitems_delete(Relation rel, Buffer buf,
-- 
2.20.1

From 07d94d809b1c2a273b0aebaf710bc858f1c2269e Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email-masked]>
Date: Thu, 31 Jan 2019 14:32:43 +0900
Subject: [PATCH 3/6] Simplify gin code

---
 src/backend/access/gin/ginentrypage.c | 3 +--
 src/backend/access/gin/ginget.c       | 5 ++---
 src/backend/access/gin/gininsert.c    | 2 +-
 src/backend/access/gin/ginvacuum.c    | 4 ++--
 src/include/access/gin_private.h      | 3 +--
 5 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/src/backend/access/gin/ginentrypage.c b/src/backend/access/gin/ginentrypage.c
index 4889de2a4f..977b96d34c 100644
--- a/src/backend/access/gin/ginentrypage.c
+++ b/src/backend/access/gin/ginentrypage.c
@@ -160,8 +160,7 @@ GinFormTuple(GinState *ginstate,
  * in *nitems.
  */
 ItemPointer
-ginReadTuple(GinState *ginstate, OffsetNumber attnum, IndexTuple itup,
-			 int *nitems)
+ginReadTuple(IndexTuple itup, int *nitems)
 {
 	Pointer		ptr = GinGetPosting(itup);
 	int			nipd = GinGetNPosting(itup);
diff --git a/src/backend/access/gin/ginget.c b/src/backend/access/gin/ginget.c
index b18ae2b3ed..9a4dc1b4d7 100644
--- a/src/backend/access/gin/ginget.c
+++ b/src/backend/access/gin/ginget.c
@@ -294,7 +294,7 @@ collectMatchBitmap(GinBtreeData *btree, GinBtreeStack *stack,
 			ItemPointer ipd;
 			int			nipd;
 
-			ipd = ginReadTuple(btree->ginstate, scanEntry->attnum, itup, &nipd);
+			ipd = ginReadTuple(itup, &nipd);
 			tbm_add_tuples(scanEntry->matchBitmap, ipd, nipd, false);
 			scanEntry->predictNumberResult += GinGetNPosting(itup);
 			pfree(ipd);
@@ -452,8 +452,7 @@ restartScanEntry:
 							  snapshot);
 			if (GinGetNPosting(itup) > 0)
 			{
-				entry->list = ginReadTuple(ginstate, entry->attnum, itup,
-										   &entry->nlist);
+				entry->list = ginReadTuple(itup, &entry->nlist);
 				entry->predictNumberResult = entry->nlist;
 
 				entry->isFinished = false;
diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c
index 524ac5be8b..0721859b59 100644
--- a/src/backend/access/gin/gininsert.c
+++ b/src/backend/access/gin/gininsert.c
@@ -67,7 +67,7 @@ addItemPointersToLeafTuple(GinState *ginstate,
 	key = gintuple_get_key(ginstate, old, &category);
 
 	/* merge the old and new posting lists */
-	oldItems = ginReadTuple(ginstate, attnum, old, &oldNPosting);
+	oldItems = ginReadTuple(old, &oldNPosting);
 
 	newItems = ginMergeItemPointers(items, nitem,
 									oldItems, oldNPosting,
diff --git a/src/backend/access/gin/ginvacuum.c b/src/backend/access/gin/ginvacuum.c
index dfe885b101..701b2dae08 100644
--- a/src/backend/access/gin/ginvacuum.c
+++ b/src/backend/access/gin/ginvacuum.c
@@ -127,7 +127,7 @@ typedef struct DataPageDeleteStack
  */
 static void
 ginDeletePage(GinVacuumState *gvs, BlockNumber deleteBlkno, BlockNumber leftBlkno,
-			  BlockNumber parentBlkno, OffsetNumber myoff, bool isParentRoot)
+			  BlockNumber parentBlkno, OffsetNumber myoff)
 {
 	Buffer		dBuffer;
 	Buffer		lBuffer;
@@ -301,7 +301,7 @@ ginScanToDelete(GinVacuumState *gvs, BlockNumber blkno, bool isRoot,
 		if (me->leftBlkno != InvalidBlockNumber && !GinPageRightMost(page))
 		{
 			Assert(!isRoot);
-			ginDeletePage(gvs, blkno, me->leftBlkno, me->parent->blkno, myoff, me->parent->isRoot);
+			ginDeletePage(gvs, blkno, me->leftBlkno, me->parent->blkno, myoff);
 			meDelete = true;
 		}
 	}
diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h
index e56aaa17ab..90ee1b1039 100644
--- a/src/include/access/gin_private.h
+++ b/src/include/access/gin_private.h
@@ -210,8 +210,7 @@ extern void ginPrepareEntryScan(GinBtree btree, OffsetNumber attnum,
 					Datum key, GinNullCategory category,
 					GinState *ginstate);
 extern void ginEntryFillRoot(GinBtree btree, Page root, BlockNumber lblkno, Page lpage, BlockNumber rblkno, Page rpage);
-extern ItemPointer ginReadTuple(GinState *ginstate, OffsetNumber attnum,
-			 IndexTuple itup, int *nitems);
+extern ItemPointer ginReadTuple(IndexTuple itup, int *nitems);
 
 /* gindatapage.c */
 extern ItemPointer GinDataLeafPageGetItems(Page page, int *nitems, ItemPointerData advancePast);
-- 
2.20.1

From 69ad58c04dad1850def59536f87e6c8c7577beb4 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email-masked]>
Date: Thu, 31 Jan 2019 15:37:22 +0900
Subject: [PATCH 4/6] Simplify some code in clause parsing

---
 src/backend/parser/parse_clause.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index c6ce1011e2..36601a1513 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -94,8 +94,7 @@ static TargetEntry *findTargetlistEntrySQL99(ParseState *pstate, Node *node,
 						 List **tlist, ParseExprKind exprKind);
 static int get_matching_location(int sortgroupref,
 					  List *sortgrouprefs, List *exprs);
-static List *resolve_unique_index_expr(ParseState *pstate, InferClause *infer,
-						  Relation heapRel);
+static List *resolve_unique_index_expr(ParseState *pstate, InferClause *infer);
 static List *addTargetToGroupList(ParseState *pstate, TargetEntry *tle,
 					 List *grouplist, List *targetlist, int location);
 static WindowClause *findWindowClause(List *wclist, const char *name);
@@ -3017,8 +3016,7 @@ get_matching_location(int sortgroupref, List *sortgrouprefs, List *exprs)
  * to infer which unique index to use.
  */
 static List *
-resolve_unique_index_expr(ParseState *pstate, InferClause *infer,
-						  Relation heapRel)
+resolve_unique_index_expr(ParseState *pstate, InferClause *infer)
 {
 	List	   *result = NIL;
 	ListCell   *l;
@@ -3167,8 +3165,7 @@ transformOnConflictArbiter(ParseState *pstate,
 					  false, false, true);
 
 		if (infer->indexElems)
-			*arbiterExpr = resolve_unique_index_expr(pstate, infer,
-													 pstate->p_target_relation);
+			*arbiterExpr = resolve_unique_index_expr(pstate, infer);
 
 		/*
 		 * Handling inference WHERE clause (for partial unique index
-- 
2.20.1

From 69060f72818969fd1e84b85e0183d39a72d9be1e Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email-masked]>
Date: Thu, 31 Jan 2019 14:48:49 +0900
Subject: [PATCH 5/6] Simplify some pg_rewind code

---
 src/bin/pg_rewind/pg_rewind.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index 7ccde5c87f..aa753bb315 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -38,7 +38,7 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli,
 static void digestControlFile(ControlFileData *ControlFile, char *source,
 				  size_t size);
 static void updateControlFile(ControlFileData *ControlFile);
-static void syncTargetDirectory(const char *argv0);
+static void syncTargetDirectory(void);
 static void sanityChecks(void);
 static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex);
 
@@ -380,7 +380,7 @@ main(int argc, char **argv)
 	updateControlFile(&ControlFile_new);
 
 	pg_log(PG_PROGRESS, "syncing target data directory\n");
-	syncTargetDirectory(argv[0]);
+	syncTargetDirectory();
 
 	printf(_("Done!\n"));
 
@@ -715,7 +715,7 @@ updateControlFile(ControlFileData *ControlFile)
  * the overall amount of IO noticeably.
  */
 static void
-syncTargetDirectory(const char *argv0)
+syncTargetDirectory(void)
 {
 	if (!do_sync || dry_run)
 		return;
-- 
2.20.1

From eeaa9854f0512a33c17ab32cb031dc1bb4f52fb5 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email-masked]>
Date: Thu, 31 Jan 2019 15:35:08 +0900
Subject: [PATCH 6/6] Simplify a bit code of tablecmds.c

---
 src/backend/commands/tablecmds.c | 32 +++++++++++++-------------------
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 35a9ade059..1cc10dba1f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -307,8 +307,7 @@ static void MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel
 static void StoreCatalogInheritance(Oid relationId, List *supers,
 						bool child_is_partition);
 static void StoreCatalogInheritance1(Oid relationId, Oid parentOid,
-						 int32 seqNumber, Relation inhRelation,
-						 bool child_is_partition);
+						int32 seqNumber, bool child_is_partition);
 static int	findAttrByName(const char *attributeName, List *schema);
 static void AlterIndexNamespaces(Relation classRel, Relation rel,
 					 Oid oldNspOid, Oid newNspOid, ObjectAddresses *objsMoved);
@@ -316,7 +315,7 @@ static void AlterSeqNamespaces(Relation classRel, Relation rel,
 				   Oid oldNspOid, Oid newNspOid, ObjectAddresses *objsMoved,
 				   LOCKMODE lockmode);
 static ObjectAddress ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd,
-					  bool recurse, bool recursing, LOCKMODE lockmode);
+					  LOCKMODE lockmode);
 static ObjectAddress ATExecValidateConstraint(Relation rel, char *constrName,
 						 bool recurse, bool recursing, LOCKMODE lockmode);
 static int transformColumnNameList(Oid relId, List *colList,
@@ -376,8 +375,7 @@ static ObjectAddress ATExecAddIdentity(Relation rel, const char *colName,
 static ObjectAddress ATExecSetIdentity(Relation rel, const char *colName,
 				  Node *def, LOCKMODE lockmode);
 static ObjectAddress ATExecDropIdentity(Relation rel, const char *colName, bool missing_ok, LOCKMODE lockmode);
-static void ATPrepSetStatistics(Relation rel, const char *colName, int16 colNum,
-					Node *newValue, LOCKMODE lockmode);
+static void ATPrepSetStatistics(Relation rel, const char *colName, LOCKMODE lockmode);
 static ObjectAddress ATExecSetStatistics(Relation rel, const char *colName, int16 colNum,
 					Node *newValue, LOCKMODE lockmode);
 static ObjectAddress ATExecSetOptions(Relation rel, const char *colName,
@@ -442,7 +440,7 @@ static ObjectAddress ATExecClusterOn(Relation rel, const char *indexName,
 				LOCKMODE lockmode);
 static void ATExecDropCluster(Relation rel, LOCKMODE lockmode);
 static bool ATPrepChangePersistence(Relation rel, bool toLogged);
-static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
+static void ATPrepSetTableSpace(AlteredTableInfo *tab,
 					const char *tablespacename, LOCKMODE lockmode);
 static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode);
 static void ATExecSetTableSpaceNoStorage(Relation rel, Oid newTableSpace);
@@ -2606,7 +2604,7 @@ StoreCatalogInheritance(Oid relationId, List *supers,
 	{
 		Oid			parentOid = lfirst_oid(entry);
 
-		StoreCatalogInheritance1(relationId, parentOid, seqNumber, relation,
+		StoreCatalogInheritance1(relationId, parentOid, seqNumber,
 								 child_is_partition);
 		seqNumber++;
 	}
@@ -2620,8 +2618,7 @@ StoreCatalogInheritance(Oid relationId, List *supers,
  */
 static void
 StoreCatalogInheritance1(Oid relationId, Oid parentOid,
-						 int32 seqNumber, Relation inhRelation,
-						 bool child_is_partition)
+						 int32 seqNumber, bool child_is_partition)
 {
 	ObjectAddress childobject,
 				parentobject;
@@ -2986,7 +2983,6 @@ rename_constraint_internal(Oid myrelid,
 						   const char *oldconname,
 						   const char *newconname,
 						   bool recurse,
-						   bool recursing,
 						   int expected_parents)
 {
 	Relation	targetrelation = NULL;
@@ -3040,7 +3036,7 @@ rename_constraint_internal(Oid myrelid,
 				if (childrelid == myrelid)
 					continue;
 
-				rename_constraint_internal(childrelid, InvalidOid, oldconname, newconname, false, true, numparents);
+				rename_constraint_internal(childrelid, InvalidOid, oldconname, newconname, false, numparents);
 			}
 		}
 		else
@@ -3128,7 +3124,6 @@ RenameConstraint(RenameStmt *stmt)
 								   stmt->newname,
 								   (stmt->relation &&
 									stmt->relation->inh),	/* recursive? */
-								   false,	/* recursing? */
 								   0 /* expected inhcount */ );
 
 }
@@ -3792,7 +3787,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
 		case AT_SetStatistics:	/* ALTER COLUMN SET STATISTICS */
 			ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
 			/* Performs own permission checks */
-			ATPrepSetStatistics(rel, cmd->name, cmd->num, cmd->def, lockmode);
+			ATPrepSetStatistics(rel, cmd->name, lockmode);
 			pass = AT_PASS_MISC;
 			break;
 		case AT_SetOptions:		/* ALTER COLUMN SET ( options ) */
@@ -3897,7 +3892,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
 			ATSimplePermissions(rel, ATT_TABLE | ATT_MATVIEW | ATT_INDEX |
 								ATT_PARTITIONED_INDEX);
 			/* This command never recurses */
-			ATPrepSetTableSpace(tab, rel, cmd->name, lockmode);
+			ATPrepSetTableSpace(tab, cmd->name, lockmode);
 			pass = AT_PASS_MISC;	/* doesn't actually matter */
 			break;
 		case AT_SetRelOptions:	/* SET (...) */
@@ -4164,7 +4159,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
 											   lockmode);
 			break;
 		case AT_AlterConstraint:	/* ALTER CONSTRAINT */
-			address = ATExecAlterConstraint(rel, cmd, false, false, lockmode);
+			address = ATExecAlterConstraint(rel, cmd, lockmode);
 			break;
 		case AT_ValidateConstraint: /* VALIDATE CONSTRAINT */
 			address = ATExecValidateConstraint(rel, cmd->name, false, false,
@@ -6373,7 +6368,7 @@ ATExecDropIdentity(Relation rel, const char *colName, bool missing_ok, LOCKMODE
  * ALTER TABLE ALTER COLUMN SET STATISTICS
  */
 static void
-ATPrepSetStatistics(Relation rel, const char *colName, int16 colNum, Node *newValue, LOCKMODE lockmode)
+ATPrepSetStatistics(Relation rel, const char *colName, LOCKMODE lockmode)
 {
 	/*
 	 * We do our own permission checking because (a) we want to allow SET
@@ -8076,7 +8071,7 @@ CloneFkReferencing(Relation pg_constraint, Relation parentRel,
  */
 static ObjectAddress
 ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd,
-					  bool recurse, bool recursing, LOCKMODE lockmode)
+					  LOCKMODE lockmode)
 {
 	Constraint *cmdcon;
 	Relation	conrel;
@@ -11055,7 +11050,7 @@ ATExecDropCluster(Relation rel, LOCKMODE lockmode)
  * ALTER TABLE SET TABLESPACE
  */
 static void
-ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel, const char *tablespacename, LOCKMODE lockmode)
+ATPrepSetTableSpace(AlteredTableInfo *tab, const char *tablespacename, LOCKMODE lockmode)
 {
 	Oid			tablespaceId;
 
@@ -11990,7 +11985,6 @@ CreateInheritance(Relation child_rel, Relation parent_rel)
 	StoreCatalogInheritance1(RelationGetRelid(child_rel),
 							 RelationGetRelid(parent_rel),
 							 inhseqno + 1,
-							 catalogRelation,
 							 parent_rel->rd_rel->relkind ==
 							 RELKIND_PARTITIONED_TABLE);
 
-- 
2.20.1

---

From: Antonin Houska <[email-masked]>
Date: Thu, 31 Jan 2019 10:15:21 +0100

> the environment used, this is also pointing to things which could be

This reminds me that I reported some unused arguments too:

https://www.postgresql.org/message-id/6702.1473067599@localhost

SnapBuildBuildSnapshot() was fixed in commit 6c2003f8a1b, but the other two
are still there.

-- 
Antonin Houska
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26, A-2700 Wiener Neustadt
Web: https://www.cybertec-postgresql.com