On Mon, Feb 29, 2016 at 12:58 PM, Fabrízio de Royes Mello <fabriziomello@gmail.com> wrote: > Some time ago we added [1] the infrastructure to allow different lock levels > for relation options. > > So per discussion [2] the attached patch reduce lock levels down to > ShareUpdateExclusiveLock for: > - fastupdate > - fillfactor > - gin_pending_list_limit > - seq_page_cost > - random_page_cost > - n_distinct > - n_distinct_inherited > - buffering
I am of the opinion that there needs to be comments or a README or some kind of documentation somewhere indicating the rationale for the lock levels we choose here. Just changing it without analyzing why a certain level is sufficient for safety and no higher than necessary seems poor. And the reasoning should be documented for future readers.
+1
The patch altered tests for fillfactor, so I extracted just that parameter from the patch for commit, after adding comments and docs.
Fabrízio, you've been around long enough to know that patches need docs and tests. And as Robert says, comments that show you've done the analysis to show you know the patch is safe.
Some parts of this patch could be resubmitted in a later CF with some time and attention spent on it, but it isn't in a good enough state for last CF.
--
Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
