Unused variable found in AttrDefaultFetch
Hi,
I was looking at AttrDefaultFetch and saw that the variable found is never
read.
I think it can be removed. See attached patch.
Cheers
Attachments:
def-fetch-found.patchapplication/octet-stream; name=def-fetch-found.patchDownload
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index ff7395c85b..b528d26600 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -4263,7 +4263,6 @@ AttrDefaultFetch(Relation relation)
HeapTuple htup;
Datum val;
bool isnull;
- int found;
int i;
ScanKeyInit(&skey,
@@ -4274,7 +4273,6 @@ AttrDefaultFetch(Relation relation)
adrel = table_open(AttrDefaultRelationId, AccessShareLock);
adscan = systable_beginscan(adrel, AttrDefaultIndexId, true,
NULL, 1, &skey);
- found = 0;
while (HeapTupleIsValid(htup = systable_getnext(adscan)))
{
@@ -4289,8 +4287,6 @@ AttrDefaultFetch(Relation relation)
elog(WARNING, "multiple attrdef records found for attr %s of rel %s",
NameStr(attr->attname),
RelationGetRelationName(relation));
- else
- found++;
val = fastgetattr(htup,
Anum_pg_attrdef_adbin,
On Sun, Apr 4, 2021 at 8:14 AM Zhihong Yu <zyu@yugabyte.com> wrote:
Hi,
I was looking at AttrDefaultFetch and saw that the variable found is never read.I think it can be removed. See attached patch.
+1 to remove it and the patch LGTM. For reference, below is the commit
that removed last usage of "found" variable:
commit 16828d5c0273b4fe5f10f42588005f16b415b2d8
Author: Andrew Dunstan <andrew@dunslane.net>
Date: Wed Mar 28 10:43:52 2018 +1030
Fast ALTER TABLE ADD COLUMN with a non-NULL default
Currently adding a column to a table with a non-NULL default results in
a rewrite of the table. For large tables this can be both expensive and
disruptive. This patch removes the need for the rewrite as long as the
@@ -4063,10 +4125,6 @@ AttrDefaultFetch(Relation relation)
systable_endscan(adscan);
heap_close(adrel, AccessShareLock);
-
- if (found != ndef)
- elog(WARNING, "%d attrdef record(s) missing for rel %s",
- ndef - found, RelationGetRelationName(relation));
}
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
On Sun, Apr 04, 2021 at 10:13:26AM +0530, Bharath Rupireddy wrote:
+1 to remove it and the patch LGTM.
Indeed, there is no point to keep that around. I'll go clean up that
as you propose.
--
Michael
Michael Paquier <michael@paquier.xyz> writes:
On Sun, Apr 04, 2021 at 10:13:26AM +0530, Bharath Rupireddy wrote:
+1 to remove it and the patch LGTM.
Indeed, there is no point to keep that around. I'll go clean up that
as you propose.
What Andrew was suggesting in the other thread might well result in
putting it back. I'd hold off till that decision is made.
regards, tom lane
Andrew:
Can you chime in which direction to go ?
Once consensus is reached, I can provide a new patch, if needed.
Cheers
On Sat, Apr 3, 2021 at 9:54 PM Michael Paquier <michael@paquier.xyz> wrote:
Show quoted text
On Sun, Apr 04, 2021 at 10:13:26AM +0530, Bharath Rupireddy wrote:
+1 to remove it and the patch LGTM.
Indeed, there is no point to keep that around. I'll go clean up that
as you propose.
--
Michael
On 4/4/21 9:39 AM, Zhihong Yu wrote:
Andrew:
Can you chime in which direction to go ?Once consensus is reached, I can provide a new patch, if needed.
Cheers
[ please don't top-post ]
I don't think we need a new patch. We'll clean this up one way or
another as part of the cleanup on the other thread.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Andrew:
Can you let me know which thread you were referring to?
I navigated the thread mentioned in your commit. It has been more than 3
years since the last response:
/messages/by-id/CAA8=A7-OPsGeazXxiojQNMus51odNZVn8EVNSoWZ2y9yRL+BvQ@mail.gmail.com
Can you let me know the current plan ?
Cheers
On Sun, Apr 4, 2021 at 8:13 AM Andrew Dunstan <andrew@dunslane.net> wrote:
Show quoted text
On 4/4/21 9:39 AM, Zhihong Yu wrote:
Andrew:
Can you chime in which direction to go ?Once consensus is reached, I can provide a new patch, if needed.
Cheers
[ please don't top-post ]
I don't think we need a new patch. We'll clean this up one way or
another as part of the cleanup on the other thread.cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
I found the recent thread under 'ALTER TABLE ADD COLUMN fast default' which
hasn't appeared in the message chain yet.
I will watch that thread.
Cheers
On Sun, Apr 4, 2021 at 8:47 AM Zhihong Yu <zyu@yugabyte.com> wrote:
Show quoted text
Andrew:
Can you let me know which thread you were referring to?I navigated the thread mentioned in your commit. It has been more than 3
years since the last response:/messages/by-id/CAA8=A7-OPsGeazXxiojQNMus51odNZVn8EVNSoWZ2y9yRL+BvQ@mail.gmail.com
Can you let me know the current plan ?
Cheers
On Sun, Apr 4, 2021 at 8:13 AM Andrew Dunstan <andrew@dunslane.net> wrote:
On 4/4/21 9:39 AM, Zhihong Yu wrote:
Andrew:
Can you chime in which direction to go ?Once consensus is reached, I can provide a new patch, if needed.
Cheers
[ please don't top-post ]
I don't think we need a new patch. We'll clean this up one way or
another as part of the cleanup on the other thread.cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Zhihong Yu <zyu@yugabyte.com> writes:
Andrew:
Can you let me know which thread you were referring to?
I assume he meant
/messages/by-id/31e2e921-7002-4c27-59f5-51f08404c858@2ndQuadrant.com
whih was last added to just moments ago.
regards, tom lane