Mark a reloption as indexterm

Started by Fujii Masaoabout 7 years ago17 messagesdocs
Jump to latest
#1Fujii Masao
masao.fujii@gmail.com

Hi,

I'd like to propose to mark reloptions as indexterms, like GUC,
so that users can more easily search the pages describing
a reloption in document. Attached is the patch which does this.
Is this helpful? Thought?

Regards,

--
Fujii Masao

Attachments:

reloption-doc-v1.patchapplication/octet-stream; name=reloption-doc-v1.patchDownload+196-196
#2Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Fujii Masao (#1)
Re: Mark a reloption as indexterm

On 2019-Apr-10, Fujii Masao wrote:

Hi,

I'd like to propose to mark reloptions as indexterms, like GUC,
so that users can more easily search the pages describing
a reloption in document. Attached is the patch which does this.
Is this helpful? Thought?

+1 for adding index entries to all reloptions. I'm not sure what you're
achieving by splitting the text for some existing index entries in two
and putting two words in the <secondary> that were part of the
<primary>, though. I'd just put the whole text in <primary> (obviously
the option name must be the first word of that).

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#3Fujii Masao
masao.fujii@gmail.com
In reply to: Alvaro Herrera (#2)
Re: Mark a reloption as indexterm

On Wed, Apr 10, 2019 at 4:11 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

On 2019-Apr-10, Fujii Masao wrote:

Hi,

I'd like to propose to mark reloptions as indexterms, like GUC,
so that users can more easily search the pages describing
a reloption in document. Attached is the patch which does this.
Is this helpful? Thought?

+1 for adding index entries to all reloptions. I'm not sure what you're
achieving by splitting the text for some existing index entries in two
and putting two words in the <secondary> that were part of the
<primary>, though. I'd just put the whole text in <primary> (obviously
the option name must be the first word of that).

Indeed. Attached is the updated version of the patch.

Regards,

--
Fujii Masao

Attachments:

reloption-doc-v2.patchapplication/octet-stream; name=reloption-doc-v2.patchDownload+160-160
#4Michael Paquier
michael@paquier.xyz
In reply to: Fujii Masao (#3)
Re: Mark a reloption as indexterm

On Fri, Apr 12, 2019 at 12:33:45PM +0900, Fujii Masao wrote:

Indeed. Attached is the updated version of the patch.

On top of what Alvaro has mentioned, it seems to me that we should
make the difference between table-level configuration parameter and
index-level configuration parameters, and also add <primary> markups
to create_index.sgml. If you take the example of fillfactor, it
applies to both indexes and tables, but with your patch you just
define "configuration parameter", and point to only CREATE TABLE.
--
Michael

#5Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Fujii Masao (#3)
Re: Mark a reloption as indexterm

On 2019-Apr-12, Fujii Masao wrote:

On Wed, Apr 10, 2019 at 4:11 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

On 2019-Apr-10, Fujii Masao wrote:

Hi,

I'd like to propose to mark reloptions as indexterms, like GUC,
so that users can more easily search the pages describing
a reloption in document. Attached is the patch which does this.
Is this helpful? Thought?

+1 for adding index entries to all reloptions. I'm not sure what you're
achieving by splitting the text for some existing index entries in two
and putting two words in the <secondary> that were part of the
<primary>, though. I'd just put the whole text in <primary> (obviously
the option name must be the first word of that).

Indeed. Attached is the updated version of the patch.

Hmm, actually, I now see you were originally proposing the words
"storage parameter" for the fillfactor index entries, but for v2 you
instead copied the "configuration parameter" words that was in some
other of the older entries. I think "configuration parameter" is wrong
(we use that for GUCs, and it seems to me that it would be confusing to
mix both things), and we should use the words "storage parameter" for
all of these, don't you think?

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#6Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Michael Paquier (#4)
Re: Mark a reloption as indexterm

On 2019-Apr-12, Michael Paquier wrote:

On Fri, Apr 12, 2019 at 12:33:45PM +0900, Fujii Masao wrote:

Indeed. Attached is the updated version of the patch.

On top of what Alvaro has mentioned, it seems to me that we should
make the difference between table-level configuration parameter and
index-level configuration parameters, and also add <primary> markups
to create_index.sgml. If you take the example of fillfactor, it
applies to both indexes and tables, but with your patch you just
define "configuration parameter", and point to only CREATE TABLE.

Are you suggesting that it should show "index storage parameters" and
"table storage parameters"? I'm not sure about that myself ...
particularly considering that certain parameters might apply to some
index AMs and not others.

BTW what about the index-specific options such as, say, BRIN's
pages_per_range? I know other AMs have their own reloptions ...

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#7Michael Paquier
michael@paquier.xyz
In reply to: Alvaro Herrera (#6)
Re: Mark a reloption as indexterm

On Thu, Apr 11, 2019 at 11:57:32PM -0400, Alvaro Herrera wrote:

Are you suggesting that it should show "index storage parameters" and
"table storage parameters"? I'm not sure about that myself ...
particularly considering that certain parameters might apply to some
index AMs and not others.

Yes, that's exactly what I was suggesting: putting all the
index-related parameters into an index bucket, without caring about
the AM involved.
--
Michael

#8Fujii Masao
masao.fujii@gmail.com
In reply to: Alvaro Herrera (#5)
Re: Mark a reloption as indexterm

On Fri, Apr 12, 2019 at 12:54 PM Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

On 2019-Apr-12, Fujii Masao wrote:

On Wed, Apr 10, 2019 at 4:11 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

On 2019-Apr-10, Fujii Masao wrote:

Hi,

I'd like to propose to mark reloptions as indexterms, like GUC,
so that users can more easily search the pages describing
a reloption in document. Attached is the patch which does this.
Is this helpful? Thought?

+1 for adding index entries to all reloptions. I'm not sure what you're
achieving by splitting the text for some existing index entries in two
and putting two words in the <secondary> that were part of the
<primary>, though. I'd just put the whole text in <primary> (obviously
the option name must be the first word of that).

Indeed. Attached is the updated version of the patch.

Hmm, actually, I now see you were originally proposing the words
"storage parameter" for the fillfactor index entries, but for v2 you
instead copied the "configuration parameter" words that was in some
other of the older entries. I think "configuration parameter" is wrong
(we use that for GUCs, and it seems to me that it would be confusing to
mix both things), and we should use the words "storage parameter" for
all of these, don't you think?

So you are suggesting to use
<primary><varname>xxx</varname> storage parameter</primary> for
reltoption and
<primary><varname>xxx</varname> configuration parameter</primary> for GUC?

If we do that, the following two lines are in the index.

xxx configuration parameter, XXX
xxx storage parameter, Storage Parameter

OTOH, originally I thought that the following style is smarter.

xxx
configuration parameter, XXX
storage parameter, Storage Parameter

Regards,

--
Fujii Masao

#9Fujii Masao
masao.fujii@gmail.com
In reply to: Michael Paquier (#7)
Re: Mark a reloption as indexterm

On Fri, Apr 12, 2019 at 1:09 PM Michael Paquier <michael@paquier.xyz> wrote:

On Thu, Apr 11, 2019 at 11:57:32PM -0400, Alvaro Herrera wrote:

Are you suggesting that it should show "index storage parameters" and
"table storage parameters"? I'm not sure about that myself ...
particularly considering that certain parameters might apply to some
index AMs and not others.

Yes, that's exactly what I was suggesting: putting all the
index-related parameters into an index bucket, without caring about
the AM involved.

+1

While reading the doc for index reloptins, I found that there is
no information about the type for each index reloption in the doc.
Probably it's worth adding that information, like the doc for table
reloptions have.

Regards,

--
Fujii Masao

#10Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Fujii Masao (#8)
Re: Mark a reloption as indexterm

On 2019-Apr-12, Fujii Masao wrote:

OTOH, originally I thought that the following style is smarter.

xxx
configuration parameter, XXX
storage parameter, Storage Parameter

Ah. Well, I like this style. Let's do that.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#11Fujii Masao
masao.fujii@gmail.com
In reply to: Alvaro Herrera (#10)
Re: Mark a reloption as indexterm

On Sat, Apr 13, 2019 at 1:30 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

On 2019-Apr-12, Fujii Masao wrote:

OTOH, originally I thought that the following style is smarter.

xxx
configuration parameter, XXX
storage parameter, Storage Parameter

Ah. Well, I like this style. Let's do that.

So I used <secondary> tag again for the above style if both reloption
and guc with the same parameter name exist. Attached is the updated
version of the patch. This patch also marks index-reloption as indexterm.

Barring any objections, I will commit this patch.

Regards,

--
Fujii Masao

Attachments:

reloption-doc-v3.patchapplication/octet-stream; name=reloption-doc-v3.patchDownload+261-260
#12Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Fujii Masao (#11)
Re: Mark a reloption as indexterm

On 2019-Apr-16, Fujii Masao wrote:

On Sat, Apr 13, 2019 at 1:30 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

On 2019-Apr-12, Fujii Masao wrote:

OTOH, originally I thought that the following style is smarter.

xxx
configuration parameter, XXX
storage parameter, Storage Parameter

Ah. Well, I like this style. Let's do that.

So I used <secondary> tag again for the above style if both reloption
and guc with the same parameter name exist. Attached is the updated
version of the patch. This patch also marks index-reloption as indexterm.

I checked the HTML output. For autovacuum it says "configuration
parameters" rather than "configuration parameter". Other than that, it
looks good to me. (I didn't check that all storage options were covered.)

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#13Fujii Masao
masao.fujii@gmail.com
In reply to: Alvaro Herrera (#12)
Re: Mark a reloption as indexterm

On Tue, Apr 16, 2019 at 12:35 AM Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

On 2019-Apr-16, Fujii Masao wrote:

On Sat, Apr 13, 2019 at 1:30 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

On 2019-Apr-12, Fujii Masao wrote:

OTOH, originally I thought that the following style is smarter.

xxx
configuration parameter, XXX
storage parameter, Storage Parameter

Ah. Well, I like this style. Let's do that.

So I used <secondary> tag again for the above style if both reloption
and guc with the same parameter name exist. Attached is the updated
version of the patch. This patch also marks index-reloption as indexterm.

I checked the HTML output.

Thanks for the review!

For autovacuum it says "configuration
parameters" rather than "configuration parameter". Other than that, it
looks good to me. (I didn't check that all storage options were covered.)

Good catch! Seems "configuration parameters" had been used for autovacuum
parameter since old version. I agree to replace it with "configuration
parameter"
for the sake of consistency. But I think that it's better to do that by
the separate patch because they are separate things.

Regards,

--
Fujii Masao

#14Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Fujii Masao (#13)
Re: Mark a reloption as indexterm

On 2019-Apr-16, Fujii Masao wrote:

On Tue, Apr 16, 2019 at 12:35 AM Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

For autovacuum it says "configuration
parameters" rather than "configuration parameter". Other than that, it
looks good to me. (I didn't check that all storage options were covered.)

Good catch! Seems "configuration parameters" had been used for autovacuum
parameter since old version. I agree to replace it with "configuration
parameter"
for the sake of consistency. But I think that it's better to do that by
the separate patch because they are separate things.

OK, no objection :-)

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#15Michael Paquier
michael@paquier.xyz
In reply to: Fujii Masao (#11)
Re: Mark a reloption as indexterm

On Tue, Apr 16, 2019 at 12:14:01AM +0900, Fujii Masao wrote:

So I used <secondary> tag again for the above style if both reloption
and guc with the same parameter name exist. Attached is the updated
version of the patch. This patch also marks index-reloption as indexterm.

Barring any objections, I will commit this patch.

Thanks Fujii-san. This looks fine to me.
--
Michael

#16Fujii Masao
masao.fujii@gmail.com
In reply to: Alvaro Herrera (#14)
Re: Mark a reloption as indexterm

On Tue, Apr 16, 2019 at 1:15 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

On 2019-Apr-16, Fujii Masao wrote:

On Tue, Apr 16, 2019 at 12:35 AM Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

For autovacuum it says "configuration
parameters" rather than "configuration parameter". Other than that, it
looks good to me. (I didn't check that all storage options were covered.)

Good catch! Seems "configuration parameters" had been used for autovacuum
parameter since old version. I agree to replace it with "configuration
parameter"
for the sake of consistency. But I think that it's better to do that by
the separate patch because they are separate things.

OK, no objection :-)

While reading config.sgml again, I found that there are both
"autovacuum configuration parameters" and "autovacuum configuration
parameter", i.e., the plural and the singular. The latter indicates
the autovacuum GUC itself. OTOH, the former seems to indicate
all the autovacuum-related GUC parameters. So the plural form is used.

So now I'm thinking that we would not need to fix that plural form of
the index term.

Regards,

--
Fujii Masao

#17Fujii Masao
masao.fujii@gmail.com
In reply to: Michael Paquier (#15)
Re: Mark a reloption as indexterm

On Tue, Apr 16, 2019 at 12:26 PM Michael Paquier <michael@paquier.xyz> wrote:

On Tue, Apr 16, 2019 at 12:14:01AM +0900, Fujii Masao wrote:

So I used <secondary> tag again for the above style if both reloption
and guc with the same parameter name exist. Attached is the updated
version of the patch. This patch also marks index-reloption as indexterm.

Barring any objections, I will commit this patch.

Thanks Fujii-san. This looks fine to me.

Thanks for the review! Committed!

Regards,

--
Fujii Masao