Thread: Naming of network_ops vs. inet_ops for SP-GIST

Naming of network_ops vs. inet_ops for SP-GIST

From
PG Doc comments form
Date:
The following documentation comment has been logged on the website:

Page: https://www.postgresql.org/docs/14/spgist-builtin-opclasses.html
Description:

I wanted to add an SP-GIST index for an inet field ip_address

In https://www.postgresql.org/docs/14/spgist-builtin-opclasses.html
network_ops is stated as the built-in opclass for (inet, inet)

On this basis, on this version of pgsql:
psql (15.1 (Debian 15.1-1.pgdg110+1), server 14.6 (Debian
14.6-1.pgdg110+1))

I entered the following command:
# create index concurrently ip_address_spgist_ban_by_ip on ban_by_ip using
spgist (ip_address network_ops);
ERROR:  operator class "network_ops" does not exist for access method
"spgist"

However, this worked:
# create index concurrently ip_address_spgist_ban_by_ip on ban_by_ip using
spgist (ip_address inet_ops);
CREATE INDEX

This created the index:
"ip_address_spgist_ban_by_ip" spgist (ip_address)
which worked as expected:
# EXPLAIN ANALYZE select * from ban_by_ip where ip_address >>= '1.2.3.4' and
now() < banuntil_datetime;
                                                               QUERY PLAN

----------------------------------------------------------------------------------------------------------------------------------------
 Index Scan using ip_address_spgist_ban_by_ip on ban_by_ip  (cost=0.14..2.57
rows=1 width=32) (actual time=0.013..0.013 rows=0 loops=1)
   Index Cond: (ip_address >>= '1.2.3.4'::inet)
   Filter: (now() < banuntil_datetime)
 Planning Time: 0.149 ms
 Execution Time: 0.027 ms
(5 rows)

I notice inet_ops, not network_ops, is mentioned in the docs for 13. Perhaps
it was renamed to network_ops in 15 but not 14?

Re: Naming of network_ops vs. inet_ops for SP-GIST

From
"David G. Johnston"
Date:
Michael, please see below:

On Tue, Jan 24, 2023 at 12:44 PM PG Doc comments form <noreply@postgresql.org> wrote:
The following documentation comment has been logged on the website:

Page: https://www.postgresql.org/docs/14/spgist-builtin-opclasses.html
Description:

I wanted to add an SP-GIST index for an inet field ip_address

In https://www.postgresql.org/docs/14/spgist-builtin-opclasses.html
network_ops is stated as the built-in opclass for (inet, inet)
[...]
I notice inet_ops, not network_ops, is mentioned in the docs for 13. Perhaps
it was renamed to network_ops in 15 but not 14?

I'm fairly certain this is a simple typo while performing some refactoring work here:


doc/src/sgml/spgist.sgml
L105 + <entry valign="middle" morerows="10"><literal>network_ops</literal></entry>
L185 -  <entry><literal>inet_ops</literal></entry>

David J.

Re: Naming of network_ops vs. inet_ops for SP-GIST

From
Tom Lane
Date:
PG Doc comments form <noreply@postgresql.org> writes:
> I wanted to add an SP-GIST index for an inet field ip_address
> In https://www.postgresql.org/docs/14/spgist-builtin-opclasses.html
> network_ops is stated as the built-in opclass for (inet, inet)
> I entered the following command:
> # create index concurrently ip_address_spgist_ban_by_ip on ban_by_ip using
> spgist (ip_address network_ops);
> ERROR:  operator class "network_ops" does not exist for access method
> "spgist"

Hmm.  There's some confusion here, because network_ops is the name of
the operator *family*:

=# \dAf spgist
          List of operator families
   AM   | Operator family | Applicable types 
--------+-----------------+------------------
 spgist | box_ops         | box
 spgist | kd_point_ops    | point
 spgist | network_ops     | inet
 spgist | poly_ops        | polygon
 spgist | quad_point_ops  | point
 spgist | range_ops       | anyrange
 spgist | text_ops        | text
(7 rows)

but inet_ops is the name of the operator *class*:

n=# \dAc spgist
                    List of operator classes
   AM   | Input type | Storage type | Operator class | Default? 
--------+------------+--------------+----------------+----------
 spgist | anyrange   |              | range_ops      | yes
 spgist | box        |              | box_ops        | yes
 spgist | inet       |              | inet_ops       | yes
 spgist | point      |              | kd_point_ops   | no
 spgist | point      |              | quad_point_ops | yes
 spgist | polygon    | box          | poly_ops       | yes
 spgist | text       |              | text_ops       | yes
(7 rows)

This naming was evidently chosen to match btree, which has both
inet_ops and cidr_ops opclasses within its network_ops family.
spgist only supports inet_ops (and there's not really a reason
to change that, since it will in fact work for cidr too).
But you have to use the class name not the family name when
explicitly selecting an index's opclass.

> I notice inet_ops, not network_ops, is mentioned in the docs for 13. Perhaps
> it was renamed to network_ops in 15 but not 14?

Hmm, apparently somebody decided that the family name was more
appropriate to show here, since the operators are tied to an
opfamily not just an opclass.  But the table header still says
"operator classes", so that's incorrect.

We could change the table header, but I'm not sure that that is a
useful direction to take, because people need to use the class name
for index creation but there are few cases where non-developers
need be concerned with family names.

I wonder whether we shouldn't just revert this table to
showing opclass names, and avert our eyes from the theoretical
inconsistency.  Michael, looks like it was your 7a1cd5260
that changed it; what do you think?

            regards, tom lane



Re: Naming of network_ops vs. inet_ops for SP-GIST

From
Laurence Parry
Date:
> This naming was evidently chosen to match btree, 
> which has both inet_ops and cidr_ops opclasses
> within its network_ops family.
> spgist only supports inet_ops

FWIW, the documentation for GIST has inet_ops in the equivalent table, so it was extra-confusing because I thought SP-GIST's inconsistency must have a reason - though I see now using '\dAf gist' suggests it's similar.

The index in question was replacing a default btree - we'd started using the table to block by range; when reviewing indexes, I found it wasn't used anymore. (I didn't know btree *had* cidr_ops, but that may be for the best, as it didn't help with 'CIDR contains IP'.)

Best regards,
--
Laurence 'GreenReaper' Parry
greenreaper.co.uk - Inkbunny.net

Re: Naming of network_ops vs. inet_ops for SP-GIST

From
Michael Paquier
Date:
On Tue, Jan 24, 2023 at 03:22:44PM -0500, Tom Lane wrote:
> I wonder whether we shouldn't just revert this table to
> showing opclass names, and avert our eyes from the theoretical
> inconsistency.  Michael, looks like it was your 7a1cd5260
> that changed it; what do you think?

Yes, the docs should be fixed here.  The intention is not to show the
operator families but the names of the opclasses.  I can only spot one
difference in SpGiST for network_ops -> inet_ops as of the report.
BRIN, GIN and GiST look to be clean after a second lookup.

I don't have a strong opinion about the naming inconsistency between
the opclass name and the opfamily name in this case, though, couldn't
it create more problems than actually fix something?

Anyway, attached is a patch for the docs.  Thoughts?
--
Michael

Attachment

Re: Naming of network_ops vs. inet_ops for SP-GIST

From
Tom Lane
Date:
Michael Paquier <michael@paquier.xyz> writes:
> I don't have a strong opinion about the naming inconsistency between
> the opclass name and the opfamily name in this case, though, couldn't
> it create more problems than actually fix something?

Well, it's been like that from day one and people haven't complained.
I think changing it now would add more confusion than it subtracts.

> Anyway, attached is a patch for the docs.  Thoughts?

Works for me.

            regards, tom lane



Re: Naming of network_ops vs. inet_ops for SP-GIST

From
Michael Paquier
Date:
On Tue, Jan 24, 2023 at 10:38:28PM -0500, Tom Lane wrote:
> Works for me.

Thanks for checking, applied that.
--
Michael

Attachment