Re: doc review for v13

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Justin Pryzby <pryzby(at)telsasoft(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, James Coleman <jtc331(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>
Subject: Re: doc review for v13
Date: 2020-06-12 07:48:32
Message-ID: 20200612074832.GH3362@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jun 11, 2020 at 09:37:09PM -0500, Justin Pryzby wrote:
> Some new bits,
> And some old ones.

I was looking at this patch set, and 0005 has attracted my attention
here:

> --- a/src/backend/utils/cache/relcache.c
> +++ b/src/backend/utils/cache/relcache.c
> @@ -4240,7 +4240,6 @@ AttrDefaultFetch(Relation relation)
> HeapTuple htup;
> Datum val;
> bool isnull;
> - int found;
> int i;

Since 16828d5, this variable is indeed unused. Now, the same commit
has removed the following code:
- if (found != ndef)
- elog(WARNING, "%d attrdef record(s) missing for rel %s",
- ndef - found, RelationGetRelationName(relation));

Should we actually keep this variable and have this sanity check in
place? It seems to me that it would be good to have that, so as we
can make sure that the number of default attributes cached matches
with the number of defaults actually found when scanning each
attribute. Adding in CC Andrew as the author of 16828d5 for more
input.
--
Michael

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message 李杰 (慎追) 2020-06-12 08:06:28 回复:how to create index concurrently on paritioned table
Previous Message Michael Paquier 2020-06-12 07:30:45 Re: remove some STATUS_* symbols