Обсуждение: StrategyAM for IndexAMs

I'm preparing the way for a later patch that would allow unique hash
indexes to be primary keys. There are various parts to the problem. I
was surprised at how many times we hardcode BTREE_AM_OID and
associated BT Strategy Numbers in many parts of the code, so have been
looking for ways to reconcile how Hash and Btree strategies work and
potentially remove hardcoding. There are various comments that say we
need a way to be able to define which Strategy Numbers are used by
indexams.

I came up with a rather simple way: the indexam just says "I'm like a
btree", which allows you to avoid adding hundreds of operator classes
for the new index, since that is cumbersome and insecure.

Specifically, we add a "strategyam" field to the IndexAmRoutine that
allows an indexam to declare whether it is like a btree, like a hash
index or another am. This then allows us to KEEP the hardcoded
BTREE_AM_OID tests, but point them at the strategyam rather than the
relam, which can be cached in various places as needed. No catalog
changes needed.

I've coded this up and it works fine.

The attached patch is still incomplete because we use this in a few
places and they all need to be checked. So before I do that, it seems
sensible to agree the approach.

(Obviously, there are hundreds of places where BTEqualStrategyNumber
is hardcoded, and this doesn't change that at all, in case that wasn't
clear).

Comments welcome on this still WIP patch.

-- 
Simon Riggs                http://www.EnterpriseDB.com/

On Tue, 19 Jul 2022 at 18:56, Simon Riggs <simon.riggs@enterprisedb.com> wrote:
>
> I'm preparing the way for a later patch that would allow unique hash
> indexes to be primary keys. There are various parts to the problem. I
> was surprised at how many times we hardcode BTREE_AM_OID and
> associated BT Strategy Numbers in many parts of the code, so have been
> looking for ways to reconcile how Hash and Btree strategies work and
> potentially remove hardcoding. There are various comments that say we
> need a way to be able to define which Strategy Numbers are used by
> indexams.
>
> I came up with a rather simple way: the indexam just says "I'm like a
> btree", which allows you to avoid adding hundreds of operator classes
> for the new index, since that is cumbersome and insecure.

I'm fairly certain that you can't (and don't want to) make a hash
index look like a btree index, considering that of the btree
operations only equality checks make sense in the hash context, and
that you can't do ordered retrieval (incl. no min/max), which are
major features of btree.

With that in mind, could you tell whether this patch is related to the
effort of hash-based unique primary keys (apart from inspiration
during development), and if so, how?

> Specifically, we add a "strategyam" field to the IndexAmRoutine that
> allows an indexam to declare whether it is like a btree, like a hash
> index or another am. This then allows us to KEEP the hardcoded
> BTREE_AM_OID tests, but point them at the strategyam rather than the
> relam, which can be cached in various places as needed. No catalog
> changes needed.
>
> I've coded this up and it works fine.
>
> The attached patch is still incomplete because we use this in a few
> places and they all need to be checked. So before I do that, it seems
> sensible to agree the approach.
>
> (Obviously, there are hundreds of places where BTEqualStrategyNumber
> is hardcoded, and this doesn't change that at all, in case that wasn't
> clear).
>
> Comments welcome on this still WIP patch.

I think this is a great step in the right direction, fixing one of the
issues with core index AMs, issues I also complained about earlier
[0].

Thanks,

Matthias van de Meent

[0] https://www.postgresql.org/message-id/CAEze2Wg8QhpOnHoqPNB-AaexGX4Zaij%3D4TT0kaMhF_6T5FXxmQ%40mail.gmail.com

On Fri, 22 Jul 2022 at 10:23, Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
>
> On Tue, 19 Jul 2022 at 18:56, Simon Riggs <simon.riggs@enterprisedb.com> wrote:
> >
> > I'm preparing the way for a later patch that would allow unique hash
> > indexes to be primary keys. There are various parts to the problem. I
> > was surprised at how many times we hardcode BTREE_AM_OID and
> > associated BT Strategy Numbers in many parts of the code, so have been
> > looking for ways to reconcile how Hash and Btree strategies work and
> > potentially remove hardcoding. There are various comments that say we
> > need a way to be able to define which Strategy Numbers are used by
> > indexams.
> >
> > I came up with a rather simple way: the indexam just says "I'm like a
> > btree", which allows you to avoid adding hundreds of operator classes
> > for the new index, since that is cumbersome and insecure.
>
> I'm fairly certain that you can't (and don't want to) make a hash
> index look like a btree index, considering that of the btree
> operations only equality checks make sense in the hash context, and
> that you can't do ordered retrieval (incl. no min/max), which are
> major features of btree.

"like a $INDEX_TYPE" is wrong. What I really mean is "use the operator
strategy numbering same as $INDEX_TYPE".

> With that in mind, could you tell whether this patch is related to the
> effort of hash-based unique primary keys (apart from inspiration
> during development), and if so, how?

There are lots of places that are hardcoded BTREE_AM_OID, with a mix
of purposes. It's hard to tackle one without getting drawn in to fix
the others.

> > Specifically, we add a "strategyam" field to the IndexAmRoutine that
> > allows an indexam to declare whether it is like a btree, like a hash
> > index or another am. This then allows us to KEEP the hardcoded
> > BTREE_AM_OID tests, but point them at the strategyam rather than the
> > relam, which can be cached in various places as needed. No catalog
> > changes needed.
> >
> > I've coded this up and it works fine.
> >
> > The attached patch is still incomplete because we use this in a few
> > places and they all need to be checked. So before I do that, it seems
> > sensible to agree the approach.
> >
> > (Obviously, there are hundreds of places where BTEqualStrategyNumber
> > is hardcoded, and this doesn't change that at all, in case that wasn't
> > clear).
> >
> > Comments welcome on this still WIP patch.
>
> I think this is a great step in the right direction, fixing one of the
> issues with core index AMs, issues I also complained about earlier
> [0].
>
> [0] https://www.postgresql.org/message-id/CAEze2Wg8QhpOnHoqPNB-AaexGX4Zaij%3D4TT0kaMhF_6T5FXxmQ%40mail.gmail.com

Guess we're thinking along similar lines. I was unaware of your recent
post; these days I don't read Hackers apart from what I'm working on.


--
Simon Riggs                http://www.EnterpriseDB.com/