Re: Improve code in tidbitmap.c - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Improve code in tidbitmap.c
Date
Msg-id 30708.1384558680@sss.pgh.pa.us
Whole thread Raw
In response to Re: Improve code in tidbitmap.c  ("Etsuro Fujita" <fujita.etsuro@lab.ntt.co.jp>)
List pgsql-hackers
"Etsuro Fujita" <fujita.etsuro@lab.ntt.co.jp> writes:
> I'd like to do the complementary explanation of this.
> In tbm_union_page() and tbm_intersect_page() in tidbitmap.c, WORDS_PER_PAGE
> is used in the scan of a lossy chunk, instead of WORDS_PER_CHUNK, where
> these macros are defined as:

> /* number of active words for an exact page: */
> #define WORDS_PER_PAGE  ((MAX_TUPLES_PER_PAGE - 1) / BITS_PER_BITMAPWORD +
> 1)
> /* number of active words for a lossy chunk: */
> #define WORDS_PER_CHUNK  ((PAGES_PER_CHUNK - 1) / BITS_PER_BITMAPWORD + 1)

> Though the use of WORDS_PER_PAGE in the scan of a lossy chunk is logically
> correct since these macros implicitly satisfy that WORDS_PER_CHUNK <
> WORDS_PER_PAGE, I think WORDS_PER_CHUNK should be used in the scan of a
> lossy chunk for code readability and maintenance.  So, I submitted a patch
> working in such a way in an earlier email.

This is a bug fix, not a performance improvement (any improvement would
be welcome, but that's not the point).  There's absolutely nothing
guaranteeing that WORDS_PER_CHUNK is less than WORDS_PER_PAGE, and if
it were the other way around, the code would be outright broken.  It's
pure luck that it was merely inefficient.

Committed, thanks for finding it!
        regards, tom lane



pgsql-hackers by date:

Previous
From: Christopher Browne
Date:
Subject: Re: Extra functionality to createuser
Next
From: David Rowley
Date:
Subject: Re: strncpy is not a safe version of strcpy