[PATCH v1] [doc] polish the comments of reloptions
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__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 0001-doc-polish-the-comments-of-reloptions.patch Description: Binary data
Re: [PATCH v1] [doc] polish the comments of reloptions
thoughts? On Tue, Aug 30, 2022 at 11:56 AM 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__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 -- Regards Junwang Zhao
Re: [PATCH v1] [doc] polish the comments of reloptions
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__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 > 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__reloption). > * (iii) add it to the appropriate options struct (perhaps StdRdOptions) > * (iv) add it to the appropriate handlin