Re: [PATCH] Add section headings to index types doc

[Tom Lane]

=?UTF-8?Q?J=c3=bcrgen_Purtz?=  writes:
> Attached are two patches: a) summary against master and b) standalone of 
> my current changes.

This was a bit confused ... as submitted, the patch wanted to commit
a couple of patchfiles.  I sorted it out I believe, and pushed with
a little additional fiddling of my own.

I did not commit the reindentation of existing text --- I don't think
it's worth adding "git blame" noise for.

regards, tom lane

Re: [PATCH] Add section headings to index types doc

[Jürgen Purtz]

On 23.10.20 18:08, David G. Johnston wrote:
On Fri, Oct 23, 2020 at 3:18 AM Jürgen Purtz > wrote:



and add a link to the "CREATE INDEX" command from the chapter
preamble.

is the link necessary?


I suppose it would make more sense to add it to the previous section - 
the introduction page.  I do think having a link (or more than one) to 
CREATE INDEX from the Indexes chapter is reader friendly.  Having 
links to SQL commands is never truly necessary - the reader knows a 
SQL command reference exists and the name of the command allows them 
to find the correct page.


David J.

I'm afraid I haven't grasped everything of your intentions and 
suggestions of your last two mails.


- equal operator in standalone paragraph: ok, integrated.

- shift "create index ... using HASH" to a different place: You suggest 
shifting the statement or a link to the previous (sub-)chapter "11.1 
Introduction"? But there is already a "create index" example. Please 
read my suggestion/modification in the first paragraph of the "11.2 
Index Types" page.


- "rewording hash": I don't know what is missing here. But I have added 
a few words about the nature of this index type.


Attached are two patches: a) summary against master and b) standalone of 
my current changes.


--

J. Purtz

diff --git a/doc/src/sgml/0001-Add-section-headers-to-index-types-doc_1.patch b/doc/src/sgml/0001-Add-section-headers-to-index-types-doc_1.patch
new file mode 100644
index 00..1919080bfc
--- /dev/null
+++ b/doc/src/sgml/0001-Add-section-headers-to-index-types-doc_1.patch
@@ -0,0 +1,183 @@
+>From 84522dc77afd1b8ce0bf111279302888d9d3edcb Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= 
+Date: Fri, 31 Jul 2020 10:20:48 +0100
+Subject: [PATCH] Add section headers to index types doc
+
+This makes it easier to compare the properties of different index
+types at a glance.
+
+In passing, make the index operator lists a single line each.
+---
+ doc/src/sgml/indices.sgml | 81 +++
+ 1 file changed, 39 insertions(+), 42 deletions(-)
+
+diff --git a/doc/src/sgml/indices.sgml b/doc/src/sgml/indices.sgml
+index 671299ff05..f22253f4c3 100644
+--- a/doc/src/sgml/indices.sgml
 b/doc/src/sgml/indices.sgml
+@@ -122,6 +122,9 @@
+B-tree indexes, which fit the most common situations.
+   
+ 
++  
++   B-tree
++
+   
+
+ index
+@@ -137,13 +140,9 @@
+will consider using a B-tree index whenever an indexed column is
+involved in a comparison using one of these operators:
+ 
+-   
+-<
+-<=
+-=
+->=
+->
+-   
++
++<   <=   =   >=   >
++
+ 
+Constructs equivalent to combinations of these operators, such as
+BETWEEN and IN, can also be implemented with
+@@ -172,6 +171,10 @@
+This is not always faster than a simple scan and sort, but it is
+often helpful.
+   
++  
++
++  
++   Hash
+ 
+   
+
+@@ -191,6 +194,10 @@
+ CREATE INDEX name ON table USING HASH (column);
+ 
+   
++  
++
++  
++   GiST
+ 
+   
+
+@@ -210,20 +217,9 @@
+for several two-dimensional geometric data types, which support indexed
+queries using these operators:
+ 
+-   
+-<<
+-&<
+-&>
+->>
+-<<|
+-&<|
+-|&>
+-|>>
+-@>
+-<@
+-~=
+-&&
+-   
++
++<<   &<   &>   >>   <<|   &<|   |&>   |>>   @>   <@   ~=   &&
++
+ 
+(See  for the meaning of
+these operators.)
+@@ -246,6 +242,10 @@
+In , operators that can be
+used in this way are listed in the column Ordering Operators.
+   
++  
++
++  
++   SP-GiST
+ 
+   
+
+@@ -264,14 +264,9 @@
+for two-dimensional points, which support indexed
+queries using these operators:
+ 
+-   
+-<<
+->>
+-~=
+-<@
+-<^
+->^
+-   
++
++<<   >>   ~=   <@   <^   >^
++
+ 
+(See  for the meaning of
+these operators.)
+@@ -286,6 +281,10 @@
+corresponding operator is specified in the Ordering Operators
+column in .
+   
++  
++
++  
++   GIN
+ 
+   
+
+@@ -312,12 +311,9 @@
+PostgreSQL includes a GIN operator class
+for arrays, which supports indexed queries using these operators:
+ 
+-   
+-<@
+-@>
+-=
+-&&
+-   
++
++<@   @>   =   &&
++
+ 
+(See  for the meaning of
+these operators.)
+@@ -327,6 +323,10 @@
+classes are available in the contrib collection or as separate
+projects.  For more information see .
+   
++  
++
++  
++   BRIN
+ 
+   
+
+@@ -348,18 +348,15 @@
+values in the column for each block range.  This supports indexed queries
+using these operators:
+ 
+-   
+-<
+-<=
+-=
+->=
+->
+-   
++
++<   <=   =   >=   >
++
+ 
+The BRIN operator classes included in the standard distribution are
+documented in .
+For more information see .
+   
++  
+  
+ 
+ 
+-- 
+2.27.0
+
diff --git a/doc/src/sgml/0002-Reindent-index-types-docs-after-previous-commit.patch b/doc/src/sgml/0002-Reindent-index-types-docs-after

Re: [PATCH] Add section headings to index types doc

[David G. Johnston]

On Fri, Oct 23, 2020 at 3:18 AM Jürgen Purtz  wrote:

> and add a link to the "CREATE INDEX" command from the chapter preamble.
>
> is the link necessary?
>

I suppose it would make more sense to add it to the previous section - the
introduction page.  I do think having a link (or more than one) to CREATE
INDEX from the Indexes chapter is reader friendly.  Having links to SQL
commands is never truly necessary - the reader knows a SQL command
reference exists and the name of the command allows them to find the
correct page.

David J.

Re: [PATCH] Add section headings to index types doc

[Jürgen Purtz]

On 21.10.20 23:12, David G. Johnston wrote:
On Wed, Sep 30, 2020 at 4:25 AM Dagfinn Ilmari Mannsåker 
mailto:ilm...@ilmari.org>> wrote:


Michael Paquier mailto:mich...@paquier.xyz>>
writes:

> On Mon, Aug 10, 2020 at 12:52:17PM +, Jürgen Purtz wrote:
>> The new status of this patch is: Waiting on Author
>
> This has not been answered yet, so I have marked the patch as
returned
> with feedback.

Updated patch attached, wich reformats the operator lists as requested
by Jürgen, 



A couple of things:

One, I would place the equality operator for hash inside a standalone 
synopsis just like all of the others.

ok
Two, why is hash special in having its create index command provided 
here? (I notice this isn't the fault of this patch but it stands out 
when reviewing it)

yes, this looks odd.


I would suggest rewording hash to look more like the others

ok

and add a link to the "CREATE INDEX" command from the chapter preamble.

is the link necessary?


and skips the reindentation as suggested by Tom.


Agreed
David J.


--

J. Purtz

Re: [PATCH] Add section headings to index types doc

[David G. Johnston]

On Wed, Sep 30, 2020 at 4:25 AM Dagfinn Ilmari Mannsåker 
wrote:

> Michael Paquier  writes:
>
> > On Mon, Aug 10, 2020 at 12:52:17PM +, Jürgen Purtz wrote:
> >> The new status of this patch is: Waiting on Author
> >
> > This has not been answered yet, so I have marked the patch as returned
> > with feedback.
>
> Updated patch attached, wich reformats the operator lists as requested
> by Jürgen,


A couple of things:

One, I would place the equality operator for hash inside a standalone
synopsis just like all of the others.
Two, why is hash special in having its create index command provided here?
(I notice this isn't the fault of this patch but it stands out when
reviewing it)

I would suggest rewording hash to look more like the others and add a link
to the "CREATE INDEX" command from the chapter preamble.

and skips the reindentation as suggested by Tom.
>

Agreed
David J.

Re: [PATCH] Add section headings to index types doc

[Jürgen Purtz]

On 30.09.20 14:53, Heikki Linnakangas wrote:

On 30/09/2020 14:25, Dagfinn Ilmari Mannsåker wrote:

Michael Paquier  writes:


On Mon, Aug 10, 2020 at 12:52:17PM +, Jürgen Purtz wrote:

The new status of this patch is: Waiting on Author


This has not been answered yet, so I have marked the patch as returned
with feedback.


Updated patch attached, wich reformats the operator lists as requested
by Jürgen, and skips the reindentation as suggested by Tom.


I wonder if "synopsis" is the right markup for the operator lists. I'm 
not too familiar with SGML, but the closest similar list I could find 
is this in create_operator.sgml:


   The operator name is a sequence of up to 
NAMEDATALEN-1

   (63 by default) characters from the following list:

+ - * / < > = ~ ! @ # % ^ & | ` ?



Reading up on the meaning of "literallayout" at 
https://tdg.docbook.org/tdg/4.5/literallayout.html, though, it doesn't 
sound quite right either. Maybe "" ?


- Heikki


 loses the aqua background color (in comparison to the 
existing documentation).


columns="5">< ... is very chatty: 
it needs the additional 'columns' attribute and the additional 'member' 
element.


Therefor I am in favor of the  solution as given in the last 
patch of Dagfinn.


Playing around I found another solution, which also looks quite good. 
The chapter uses operators within the text flow at different places. All 
of them are embedded in a  element (inline). Using this 
 element also for the index-specific operators, the reading of 
the page gets easier and the rendering is consistent. But the drawback 
is that these operator are no longer accentuated. Because they 
'represents' the possible access methods per index-type, one can argue 
that they should be shown in a special way, eg.: in a separate paragraph 
as in Dagfin's patch. (I suppose that this was the original intention of 
the huge number of line-breaks.) It would look like the following, but I 
don't recommend to use it:


--

Jürgen Purtz

Re: [PATCH] Add section headings to index types doc

[Heikki Linnakangas]

On 30/09/2020 14:25, Dagfinn Ilmari Mannsåker wrote:

Michael Paquier  writes:


On Mon, Aug 10, 2020 at 12:52:17PM +, Jürgen Purtz wrote:

The new status of this patch is: Waiting on Author


This has not been answered yet, so I have marked the patch as returned
with feedback.


Updated patch attached, wich reformats the operator lists as requested
by Jürgen, and skips the reindentation as suggested by Tom.


I wonder if "synopsis" is the right markup for the operator lists. I'm 
not too familiar with SGML, but the closest similar list I could find is 
this in create_operator.sgml:



   The operator name is a sequence of up to NAMEDATALEN-1
   (63 by default) characters from the following list:

+ - * / < > = ~ ! @ # % ^ & | ` ?



Reading up on the meaning of "literallayout" at 
https://tdg.docbook.org/tdg/4.5/literallayout.html, though, it doesn't 
sound quite right either. Maybe "" ?


- Heikki

Re: [PATCH] Add section headings to index types doc

[Dagfinn Ilmari Mannsåker]

Michael Paquier  writes:

> On Mon, Aug 10, 2020 at 12:52:17PM +, Jürgen Purtz wrote:
>> The new status of this patch is: Waiting on Author
>
> This has not been answered yet, so I have marked the patch as returned
> with feedback.

Updated patch attached, wich reformats the operator lists as requested
by Jürgen, and skips the reindentation as suggested by Tom.

The reindentation patch is attached separately, in case the committer
decides they want it properly indented after all.

- ilmari
-- 
- Twitter seems more influential [than blogs] in the 'gets reported in
  the mainstream press' sense at least.   - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
  to a mainstream media article.  - Calle Dybedahl

>From 84522dc77afd1b8ce0bf111279302888d9d3edcb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= 
Date: Fri, 31 Jul 2020 10:20:48 +0100
Subject: [PATCH] Add section headers to index types doc

This makes it easier to compare the properties of different index
types at a glance.

In passing, make the index operator lists a single line each.
---
 doc/src/sgml/indices.sgml | 81 +++
 1 file changed, 39 insertions(+), 42 deletions(-)

diff --git a/doc/src/sgml/indices.sgml b/doc/src/sgml/indices.sgml
index 671299ff05..f22253f4c3 100644
--- a/doc/src/sgml/indices.sgml
+++ b/doc/src/sgml/indices.sgml
@@ -122,6 +122,9 @@
B-tree indexes, which fit the most common situations.
   
 
+  
+   B-tree
+
   

 index
@@ -137,13 +140,9 @@
will consider using a B-tree index whenever an indexed column is
involved in a comparison using one of these operators:
 
-   
-<
-<=
-=
->=
->
-   
+
+<   <=   =   >=   >
+
 
Constructs equivalent to combinations of these operators, such as
BETWEEN and IN, can also be implemented with
@@ -172,6 +171,10 @@
This is not always faster than a simple scan and sort, but it is
often helpful.
   
+  
+
+  
+   Hash
 
   

@@ -191,6 +194,10 @@
 CREATE INDEX name ON table USING HASH (column);
 
   
+  
+
+  
+   GiST
 
   

@@ -210,20 +217,9 @@
for several two-dimensional geometric data types, which support indexed
queries using these operators:
 
-   
-<<
-&<
-&>
->>
-<<|
-&<|
-|&>
-|>>
-@>
-<@
-~=
-&&
-   
+
+<<   &<   &>   >>   <<|   &<|   |&>   |>>   @>   <@   ~=   &&
+
 
(See  for the meaning of
these operators.)
@@ -246,6 +242,10 @@
In , operators that can be
used in this way are listed in the column Ordering Operators.
   
+  
+
+  
+   SP-GiST
 
   

@@ -264,14 +264,9 @@
for two-dimensional points, which support indexed
queries using these operators:
 
-   
-<<
->>
-~=
-<@
-<^
->^
-   
+
+<<   >>   ~=   <@   <^   >^
+
 
(See  for the meaning of
these operators.)
@@ -286,6 +281,10 @@
corresponding operator is specified in the Ordering Operators
column in .
   
+  
+
+  
+   GIN
 
   

@@ -312,12 +311,9 @@
PostgreSQL includes a GIN operator class
for arrays, which supports indexed queries using these operators:
 
-   
-<@
-@>
-=
-&&
-   
+
+<@   @>   =   &&
+
 
(See  for the meaning of
these operators.)
@@ -327,6 +323,10 @@
classes are available in the contrib collection or as separate
projects.  For more information see .
   
+  
+
+  
+   BRIN
 
   

@@ -348,18 +348,15 @@
values in the column for each block range.  This supports indexed queries
using these operators:
 
-   
-<
-<=
-=
->=
->
-   
+
+<   <=   =   >=   >
+
 
The BRIN operator classes included in the standard distribution are
documented in .
For more information see .
   
+  
  
 
 
-- 
2.27.0

>From 2a75f535e60e89ed7e2276f4775395162f364e8c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= 
Date: Tue, 1 Sep 2020 11:48:59 +0100
Subject: [PATCH 2/2] Reindent index types docs after previous commit

The previous commit wrapped each index type descripiton in a ``
tag, but avoided reidenting for ease of review.  This commit reindents
everything, except the `` and `` tags, which
appear to need to be in column zero.
---
 doc/src/sgml/indices.sgml | 338 +++---
 1 file changed, 169 insertions(+), 169 deletions(-)

diff --git a/doc/src/sgml/indices.sgml b/doc/src/sgml/indices.sgml
index f22253f4c3..3e11d3917f 100644
--- a/doc/src/sgml/indices.sgml
+++ b/doc/src/sgml/indices.sgml
@@ -125,237 +125,237 @@
   
B-tree
 
-  
-   
-index
-B-tree
-   
-   
-B-tree
-index
-   
-   B-trees can handle equality and range queries on data that can be sorted
-   into some ordering.
-   In particular, the PostgreSQL query planner
-   will consider using a B-tree index whenever an indexed column is
-   involved in a comparison using one of these operators:
+   
+
+ index

Re: [PATCH] Add section headings to index types doc

[Michael Paquier]

On Mon, Aug 10, 2020 at 12:52:17PM +, Jürgen Purtz wrote:
> The new status of this patch is: Waiting on Author

This has not been answered yet, so I have marked the patch as returned
with feedback.
--
Michael


signature.asc
Description: PGP signature

Re: [PATCH] Add section headings to index types doc

[Jürgen Purtz]

The following review has been posted through the commitfest application:
make installcheck-world:  not tested
Implements feature:   not tested
Spec compliant:   not tested
Documentation:tested, passed

a) I'm wondering if we should apply one more change to this page. The 
line-by-line listing of operators occupies much space. What do you think about 
an inline list (in a separate line) of the operators?

old source:

<
<=
=
>=
>


new source:
   <   <=   =   >=   >
It looks nice in HTML as well as in PDF.

b) I'm in favor of the indentation of all affected lines as it is done in the 
patch.

The new status of this patch is: Waiting on Author

Re: [PATCH] Add section headings to index types doc

[Tom Lane]

ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:
> Also, for easier review, here's the `git diff -w` output, since the
>  tags caused most of the page to have to be renidented.

TBH, I'd suggest just not being anal about whether the indentation
nesting is perfect ;-).  There are certainly plenty of places in
the SGML files today where it is not.  And for something like this,
I doubt the gain is worth the loss of "git blame" tracking and
possible back-patching hazards.

I'm a compulsive neatnik when it comes to indentation of the
C code, but much less so about the SGML docs.  YMMV of course.

regards, tom lane

Re: [PATCH] Add section headings to index types doc

[Magnus Hagander]

On Mon, Aug 3, 2020 at 1:32 PM Dagfinn Ilmari Mannsåker 
wrote:

> ilm...@ilmari.org (Dagfinn Ilmari Mannsåker) writes:
>
> > Hi hackers,
> >
> > Every time I have to look up what kinds of operations each index type is
> > suitable for, I get annoyed by the index types page being virtually
> > unskimmable due to not having headings for each index type.
> >
> > Attached is a patch that adds  tags for each index type to make
> > it easier to see where the description of each one starts.
>
> Added to the next commitfest:
>
> https://commitfest.postgresql.org/29/2665/
>
> Also, for easier review, here's the `git diff -w` output, since the
>  tags caused most of the page to have to be renidented.
>
> Tangentially, does anyone know of a tool to strip whitespace changes
> from an existing diff, as if it had been generated with `-w` in the
> first place?
>

I think you can do something like:

combinediff -w 0001-Add-section-headers-to-index-types-doc.patch  /dev/null

(combinediff requires two diffs, but one can be /dev/null)

-- 
 Magnus Hagander
 Me: https://www.hagander.net/ 
 Work: https://www.redpill-linpro.com/ 

Re: [PATCH] Add section headings to index types doc

[Dagfinn Ilmari Mannsåker]

ilm...@ilmari.org (Dagfinn Ilmari Mannsåker) writes:

> Hi hackers,
>
> Every time I have to look up what kinds of operations each index type is
> suitable for, I get annoyed by the index types page being virtually
> unskimmable due to not having headings for each index type.
>
> Attached is a patch that adds  tags for each index type to make
> it easier to see where the description of each one starts.

Added to the next commitfest:

https://commitfest.postgresql.org/29/2665/

Also, for easier review, here's the `git diff -w` output, since the
 tags caused most of the page to have to be renidented.

Tangentially, does anyone know of a tool to strip whitespace changes
from an existing diff, as if it had been generated with `-w` in the
first place?

diff --git a/doc/src/sgml/indices.sgml b/doc/src/sgml/indices.sgml
index 28adaba72d..332d161547 100644
--- a/doc/src/sgml/indices.sgml
+++ b/doc/src/sgml/indices.sgml
@@ -122,6 +122,9 @@
B-tree indexes, which fit the most common situations.
   
 
+  
+   B-tree
+

 
  index
@@ -172,6 +175,10 @@
 This is not always faster than a simple scan and sort, but it is
 often helpful.

+  
+
+  
+   Hash
 

 
@@ -191,6 +198,10 @@
  CREATE INDEX name ON 
table USING HASH (column);
 

+  
+
+  
+   GiST
 

 
@@ -246,6 +257,10 @@
 In , operators that can be
 used in this way are listed in the column Ordering 
Operators.

+  
+
+  
+   SP-GiST
 

 
@@ -286,6 +301,10 @@
 corresponding operator is specified in the Ordering 
Operators
 column in .

+  
+
+  
+   GIN
 

 
@@ -327,6 +346,10 @@
 classes are available in the contrib collection or as 
separate
 projects.  For more information see .

+  
+
+  
+   BRIN
 

 
@@ -360,6 +383,7 @@
 documented in .
 For more information see .

+  
  
 

- ilmari 
-- 
"A disappointingly low fraction of the human race is,
 at any given time, on fire." - Stig Sandbeck Mathisen