Re: [REVIEW] Re: Compression of full-page-writes - Mailing list pgsql-hackers

From Rahila Syed
Subject Re: [REVIEW] Re: Compression of full-page-writes
Date
Msg-id CAH2L28uL+iHvYjHaM+0R1QEcewp_RB7Dzipf5Q1QnZm8zmDP_w@mail.gmail.com
Whole thread Raw
In response to Re: [REVIEW] Re: Compression of full-page-writes  (Michael Paquier <michael.paquier@gmail.com>)
Responses Re: [REVIEW] Re: Compression of full-page-writes  (Michael Paquier <michael.paquier@gmail.com>)
List pgsql-hackers

>        if (!fullPageWrites)
>        {
>               WALInsertLockAcquireExclusive();
>                Insert->fullPageWrites = fullPageWrites;
>                WALInsertLockRelease();
>        }
>      

As fullPageWrites is not a boolean isnt it better to change the if condition as fullPageWrites == FULL_PAGE_WRITES_OFF? As it is done in the if condition above, this seems to be a miss.

>doPageWrites = (Insert->fullPageWrites || Insert->forcePageWrites);

IIUC, doPageWrites is true when fullPageWrites is either 'on' or  'compress'
Considering Insert -> fullPageWrites is an int now, I think its better to explicitly write the above as ,

doPageWrites = (Insert -> fullPageWrites != FULL_PAGE_WRITES_OFF || Insert->forcePageWrites)

The patch attached has the above changes. Also, it initializes doPageCompression in InitXLOGAccess as per earlier discussion.
 
I have attached the changes separately as changes.patch.

Thank you,

Rahila Syed

Attachment

pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: why is PG_AUTOCONF_FILENAME is pg_config_manual.h?
Next
From: Ian Barwick
Date:
Subject: Testing DDL deparsing support