Thread: Comment in src/backend/commands/vacuumlazy.c

Comment in src/backend/commands/vacuumlazy.c

From
Amit Langote
Date:
Hi,

Note the following comment in src/backend/commands/vacuumlazy.c:lazy_scan_heap()

1088     /* If no indexes, make log report that lazy_vacuum_heap
would've made */
1089     if (vacuumed_pages)
1090         ereport(elevel,

Just wondering if it would read better as:

1088     /* Make the log report that lazy_vacuum_heap would've made
had there been no indexes */

Is that correct?

--
Amit



Re: Comment in src/backend/commands/vacuumlazy.c

From
Robert Haas
Date:
On Mon, Mar 24, 2014 at 12:28 AM, Amit Langote <amitlangote09@gmail.com> wrote:
> Hi,
>
> Note the following comment in src/backend/commands/vacuumlazy.c:lazy_scan_heap()
>
> 1088     /* If no indexes, make log report that lazy_vacuum_heap
> would've made */
> 1089     if (vacuumed_pages)
> 1090         ereport(elevel,
>
> Just wondering if it would read better as:
>
> 1088     /* Make the log report that lazy_vacuum_heap would've made
> had there been no indexes */
>
> Is that correct?

No.  Your rewrite means the opposite of what the comment means now.
vacuumed_pages will be non-zero only if the relation does NOT have
indexes.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: Comment in src/backend/commands/vacuumlazy.c

From
Amit Langote
Date:
On Mon, Mar 31, 2014 at 11:44 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Mon, Mar 24, 2014 at 12:28 AM, Amit Langote <amitlangote09@gmail.com> wrote:
>> Hi,
>>
>> Note the following comment in src/backend/commands/vacuumlazy.c:lazy_scan_heap()
>>
>> 1088     /* If no indexes, make log report that lazy_vacuum_heap
>> would've made */
>> 1089     if (vacuumed_pages)
>> 1090         ereport(elevel,
>>
>> Just wondering if it would read better as:
>>
>> 1088     /* Make the log report that lazy_vacuum_heap would've made
>> had there been no indexes */
>>
>> Is that correct?
>
> No.  Your rewrite means the opposite of what the comment means now.
> vacuumed_pages will be non-zero only if the relation does NOT have
> indexes.
>

You are correct. It does sound opposite of what is actually happening:

Somewhere in c:lazy_scan_heap(),

941         /*
942          * If there are no indexes then we can vacuum the page right now
943          * instead of doing a second scan.
944          */
945         if (nindexes == 0 &&
946             vacrelstats->num_dead_tuples > 0)
947         {
948             /* Remove tuples from heap */
949             lazy_vacuum_page(onerel, blkno, buf, 0, vacrelstats, &vmbuffer);
950             has_dead_tuples = false;
951
952             /*
953              * Forget the now-vacuumed tuples, and press on, but be careful
954              * not to reset latestRemovedXid since we want that value to be
955              * valid.
956              */
957             vacrelstats->num_dead_tuples = 0;
958             vacuumed_pages++;
959         }


Sorry about the noise.

--
Amit