Re: Some RELKIND macro refactoring

On 2021-Aug-25, Michael Paquier wrote:

> On Tue, Aug 24, 2021 at 12:01:33PM +0200, Peter Eisentraut wrote:
> > While analyzing this again, I think I found an existing mistake. The
> > handling of RELKIND_PARTITIONED_INDEX in RelationGetNumberOfBlocksInFork()
> > seems to be misplaced. See attached patch.

Agreed, that's a mistake.

> Right. This maps with RELKIND_HAS_STORAGE(). Makes me wonder whether
> is would be better to add a check on RELKIND_HAS_STORAGE() in this
> area, even if that's basically the same as the Assert() already used
> in this code path.

Well, the patch replaces the switch on individual relkind values with if
tests on RELKIND_HAS_FOO macros. I suppose we'd have

Assert(RELKIND_HAS_STORAGE(relkind));

so the function would not even be called for partitioned indexes. (In a
quick scan of 'git grep RelationGetNumberOfBlocks' I see nothing that
would obviously call this function on a partitioned index.)

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"XML!" Exclaimed C++. "What are you doing here? You're not a programming
language."
"Tell that to the people who use me," said XML.
https://burningbird.net/the-parable-of-the-languages/