Hi, On Fri, Jan 10, 2025 at 10:15:52AM +0300, Nazir Bilal Yavuz wrote: > Hi, > > On Fri, 10 Jan 2025 at 04:47, Michael Paquier <mich...@paquier.xyz> wrote: > > > > On Thu, Jan 09, 2025 at 03:30:37PM +0000, Bertrand Drouvot wrote: > > > We first use an Assert in is_ioop_tracked_in_bytes() and then we return > > > a value "just" to check another Assert. I wonder if it wouldn't make more > > > sense > > > to get rid of this function and use a macro instead, something like? > > > > > > #define is_ioop_tracked_in_bytes(io_op) \ > > > ((io_op) < IOOP_NUM_TYPES && (io_op) >= IOOP_EXTEND) > > > > Indeed. Your suggestion to use a macro makes more sense to me because > > is_ioop_tracked_in_bytes() itself has an Assert(), while being itself > > only used in an assertion, as you say. Better to document the > > dependency on the ordering of IOOp, even if that's kind of hard to > > miss. > > But I agree that having a macro has more benefits, > also there already is a check for the 'io_op < IOOP_NUM_TYPES' in the > pgstat_count_io_op() function.
Yeah, I think we can remove the "io_op < IOOP_NUM_TYPE" assertion in pgstat_count_io_op() (but keep this check as part of the macro). > > That > > seems OK here. Please let me know if you have more comments. Appart from the above, LGTM. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
