Patch applied.
--------------------------------------------------------------------------- On Tue, Aug 30, 2022 at 11:56:30AM +0800, Junwang Zhao wrote: > When adding an option, we have 5 choices (bool, integer, real, enum, string), > so the comments seem stale. > > There are some sentences missing *at ShareUpdateExclusiveLock*, this > patch adds them to make the sentences complete. > > One thing I'm not sure is should we use *at ShareUpdateExclusiveLock* or > *with ShareUpdateExclusiveLock*, pls take a look. > > src/backend/access/common/reloptions.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/src/backend/access/common/reloptions.c > b/src/backend/access/common/reloptions.c > index 609329bb21..9e99868faa 100644 > --- a/src/backend/access/common/reloptions.c > +++ b/src/backend/access/common/reloptions.c > @@ -42,9 +42,9 @@ > * > * To add an option: > * > - * (i) decide on a type (integer, real, bool, string), name, default value, > - * upper and lower bounds (if applicable); for strings, consider a validation > - * routine. > + * (i) decide on a type (bool, integer, real, enum, string), name, default > + * value, upper and lower bounds (if applicable); for strings, consider a > + * validation routine. > * (ii) add a record below (or use add_<type>_reloption). > * (iii) add it to the appropriate options struct (perhaps StdRdOptions) > * (iv) add it to the appropriate handling routine (perhaps > @@ -68,24 +68,24 @@ > * since they are only used by the AV procs and don't change anything > * currently executing. > * > - * Fillfactor can be set because it applies only to subsequent changes made > to > - * data blocks, as documented in hio.c > + * Fillfactor can be set at ShareUpdateExclusiveLock because it applies only > to > + * subsequent changes made to data blocks, as documented in hio.c > * > * n_distinct options can be set at ShareUpdateExclusiveLock because they > * are only used during ANALYZE, which uses a ShareUpdateExclusiveLock, > * so the ANALYZE will not be affected by in-flight changes. Changing those > * values has no effect until the next ANALYZE, so no need for stronger lock. > * > - * Planner-related parameters can be set with ShareUpdateExclusiveLock > because > + * Planner-related parameters can be set at ShareUpdateExclusiveLock because > * they only affect planning and not the correctness of the execution. Plans > * cannot be changed in mid-flight, so changes here could not easily result > in > * new improved plans in any case. So we allow existing queries to continue > * and existing plans to survive, a small price to pay for allowing better > * plans to be introduced concurrently without interfering with users. > * > - * Setting parallel_workers is safe, since it acts the same as > - * max_parallel_workers_per_gather which is a USERSET parameter that doesn't > - * affect existing plans or queries. > + * Setting parallel_workers at ShareUpdateExclusiveLock is safe, since it > acts > + * the same as max_parallel_workers_per_gather which is a USERSET parameter > + * that doesn't affect existing plans or queries. > * > * vacuum_truncate can be set at ShareUpdateExclusiveLock because it > * is only used during VACUUM, which uses a ShareUpdateExclusiveLock, > -- > 2.33.0 > > > -- > Regards > Junwang Zhao > From 4cef73a6f7ef4b59a6cc1cd9720b4e545bc36861 Mon Sep 17 00:00:00 2001 > From: Junwang Zhao <zhjw...@gmail.com> > Date: Tue, 30 Aug 2022 11:33:14 +0800 > Subject: [PATCH v1] [doc] polish the comments of reloptions > > 1. add the missing enum type and change the order to consistent > with relopt_type > 2. add some missing ShareUpdateExclusiveLock > --- > src/backend/access/common/reloptions.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/src/backend/access/common/reloptions.c > b/src/backend/access/common/reloptions.c > index 609329bb21..9e99868faa 100644 > --- a/src/backend/access/common/reloptions.c > +++ b/src/backend/access/common/reloptions.c > @@ -42,9 +42,9 @@ > * > * To add an option: > * > - * (i) decide on a type (integer, real, bool, string), name, default value, > - * upper and lower bounds (if applicable); for strings, consider a validation > - * routine. > + * (i) decide on a type (bool, integer, real, enum, string), name, default > + * value, upper and lower bounds (if applicable); for strings, consider a > + * validation routine. > * (ii) add a record below (or use add_<type>_reloption). > * (iii) add it to the appropriate options struct (perhaps StdRdOptions) > * (iv) add it to the appropriate handling routine (perhaps > @@ -68,24 +68,24 @@ > * since they are only used by the AV procs and don't change anything > * currently executing. > * > - * Fillfactor can be set because it applies only to subsequent changes made > to > - * data blocks, as documented in hio.c > + * Fillfactor can be set at ShareUpdateExclusiveLock because it applies only > to > + * subsequent changes made to data blocks, as documented in hio.c > * > * n_distinct options can be set at ShareUpdateExclusiveLock because they > * are only used during ANALYZE, which uses a ShareUpdateExclusiveLock, > * so the ANALYZE will not be affected by in-flight changes. Changing those > * values has no effect until the next ANALYZE, so no need for stronger lock. > * > - * Planner-related parameters can be set with ShareUpdateExclusiveLock > because > + * Planner-related parameters can be set at ShareUpdateExclusiveLock because > * they only affect planning and not the correctness of the execution. Plans > * cannot be changed in mid-flight, so changes here could not easily result > in > * new improved plans in any case. So we allow existing queries to continue > * and existing plans to survive, a small price to pay for allowing better > * plans to be introduced concurrently without interfering with users. > * > - * Setting parallel_workers is safe, since it acts the same as > - * max_parallel_workers_per_gather which is a USERSET parameter that doesn't > - * affect existing plans or queries. > + * Setting parallel_workers at ShareUpdateExclusiveLock is safe, since it > acts > + * the same as max_parallel_workers_per_gather which is a USERSET parameter > + * that doesn't affect existing plans or queries. > * > * vacuum_truncate can be set at ShareUpdateExclusiveLock because it > * is only used during VACUUM, which uses a ShareUpdateExclusiveLock, > -- > 2.33.0 > -- Bruce Momjian <br...@momjian.us> https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you.