Macros bundling RELKIND_* conditions

From: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Macros bundling RELKIND_* conditions
Date: 2017-05-29 07:25:52
Message-ID: CAFjFpRcfzs+yst6YBCseD_orEcDNuAr9GUTraZ5GC=AvCYh55Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,
We saw a handful bugs reported because RELKIND_PARTITIONED_TABLE was
not added to appropriate conditions on relkind. One such report is
[1]. On that thread I suggested that we encapsulate these conditions
as macros. Here's excerpt of my mail.

--
I noticed, that
after we introduced RELKIND_PARTITIONED_TABLE, we required to change a
number of conditions to include this relkind. We missed some places in
initial commits and fixed those later. I am wondering whether we
should creates macros clubbing relevant relkinds together based on the
purpose of the tests e.g. IS_RELKIND_HAS_STORAGE(). When a new relkind
is added, one can examine these macros to check whether the new
relkind fits in the given macro. If all those macros are placed
together, there is a high chance that we will not miss any place in
the initial commit itself.

For example, if we had a macro IS_RELKIND_WITH_STATS defined as
#define IS_RELKIND_WITH_STATS(relkind) \
((relkind) == RELKIND_RELATION || \
(relkind) == RELKIND_MATVIEW)

and CreateStatistics() and getExtendedStatistics() had following conditions
if (!IS_RELKIND_WITH_STATS(rel->rd_rel->relkind)) and if
(!IS_RELKIND_WITH_STATS(tabinfo->relkind)) resp. The patch would be
just adding
(relkind) == RELKIND_FOREIGN_TABLE || \
(relkind) == RELKIND_PARTITIONED_TABLE)

to that macro without requiring to find out where all we need to add
those two relkinds for statistics purposes.
-- excerpt ends

Peter Eisentraut thought that idea is worth a try. I gave it a try on
my way back from PGCon. Attached is a series of patches, one per
macro. This isn't a complete series but will give an idea of what's
involved. It might be possible to change switch cases at some places
to use if else with these macros. But I haven't done any changes
towards that.

[1] https://www.postgresql.org/message-id/CAKJS1f-BmGo410bh5RSPZUvOO0LhmHL2NYmdrC_Jm8pk_FfyCA%40mail.gmail.com
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Attachment Content-Type Size
pg_relkind_macros_patches_v1.tar.gzip application/octet-stream 4.4 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message tushar 2017-05-29 08:18:28 pg_resetwal is broken if run from v10 against older version of PG data directory
Previous Message Tom Lane 2017-05-29 07:04:09 "cannot specify finite value after UNBOUNDED" ... uh, why?