Re: BUG #19098: Can't create unique gist index, where pg_indexes says that WITHOUT OVERLAPS does exacly that

Поиск
Список
Период
Сортировка
От Paul A Jungwirth
Тема Re: BUG #19098: Can't create unique gist index, where pg_indexes says that WITHOUT OVERLAPS does exacly that
Дата
Msg-id CA+renyU4jKCxrtASJpssZmfrkWhi-+Q_PF__jxt8E23T755SPQ@mail.gmail.com
обсуждение исходный текст
Ответ на Re: BUG #19098: Can't create unique gist index, where pg_indexes says that WITHOUT OVERLAPS does exacly that  (Matthias van de Meent <boekewurm+postgres@gmail.com>)
Список pgsql-bugs
On Fri, Oct 31, 2025 at 4:22 AM Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
> Thanks for the CC, and sorry for the delayed response. I've been
> travelling, and will be for another week or so.

Thanks for the detailed response! I have an early patch to address this
(attached). It's not finished, but I thought I'd share it before the
commitfest deadline in case anyone wanted to take a look.

> Well, strictly speaking, GIST itself doesn't do unique indexes. There
> are hacked unique constraints which rely on the GIST AM and that
> marked those GIST indexes as unique, but GIST itself doesn't know
> about or validate any of the uniqueness - the uniqueness is instead
> guaranteed by higher level systems. Which is also my main issue with
> the new code - it is secretly an exclusion constraint under the hood,
> so instead of leveraging the AM to guarantee the uniqueness constraint
> it instead relies on repeated calls into the AM to validate the
> constraint.
>
> I'm not a fan of this disjoint uniqueness handling. It's the main
> reason why we can't build these indexes concurrently, and if we want
> the *index* to be considered unique, we should also make the *index*
> guarantee that uniqueness, not some higher code.

My understanding is that the main motivation behind exclusion
constraints was for temporal tables, so it seemed that was the right
tool to build temporal PK & UNIQUE constraints on top of. I see your
point about a layering violation, but on the other hand it doesn't seem
like indexes are strictly "lower" than constraints in general. On
pg_index we have indisexclusion and indisprimary. But this patch does
let us move the work to where it belongs (see below).

> > I would really like to support this workflow, but there are some
> > bigger pieces missing.
> >
> > Today we can't concurrently create an exclusion constraint (which is
> > what temporal PKs really are). There have been attempts in 2012 and
> > 2019 to at least concurrently REINDEX, but neither was successful (for
> > exclusion constraints).[1, 2]
>
> I don't see why we would need to support concurrent creation of
> exclusion constraints to be able to rebuild indexes concurrently.
>
> CREATE CONSTRAINT USING INDEX ... NOT VALID + VALIDATE CONSTRAINT
> works for that, right?

Good point. Creating the index concurrently, even if you have to
validate the constraint afterward, reduces most of the locking time.

> > Could we at least support the non-concurrent case? That is within
> > reach if we address a few things:
> >
> > First, an exclusion constraint's index alone won't enforce the
> > constraint, unlike a unique index. Similarly a temporal unique index
> > is strictly more restrictive than a regular unique index, so it has to
> > know that it's a temporal index.
>
> I really dislike the term "temporal index", as there is nothing
> temporal about the index. Presumably it's unique, with WITHOUT
> OVERLAPS behaviour, but none of that has anything to do with temporal
> - only uniqueness and non-unitary data types.

The reason I've used that term is because WITHOUT OVERLAPS has an extra
check beyond regular exclusion constraints: it has to forbid empty
ranges/multiranges. But I agree "temporal" is too narrow. We could talk
about "without overlaps indexes" instead.

> > To support arbitrary exclusion
> > constraints, we would have to move pg_constraint.conexclop over to
> > pg_index (or something like that). That seems messy.
>
> Yes, that'd be messy. But I'm not sure why we would need to move over
> pg_constraint.conexclop -- we're working with something called unique
> indexes here, right?

It depends if we want to enforce "without overlaps" as well. The
standard calls those "unique". And the most practical reason for this
work is to allow concurrently creating the index they need, and then
creating the constraint with a short lock time. If we had more than a
bitvector we could even support arbitrary exclusion constraints.

> > On the other hand, to enforce just temporal indexes, we don't need any
> > new columns on pg_index. We can identify temporal indexes already
> > because they have both indisunique and indisexclusion. So GiST could
> > do the right thing here. Perhaps we would have a new can_temporal
> > property for index AMs.
>
> In my opinion, if an index is marked unique, then that index ('s AM)
> should be responsible for fully validating that uniqueness; this would
> include the WITHOUT OVERLAPS support that was added to UNIQUE
> constraints in PG 18 (though apparently it isn't).
>
> AFAIK, the pg_index data should be sufficient to support this, and
> that should allow us to remove the hack that made indexes exist that a
> user can't create. We can validate whether the index's opclasses
> support this check and whether the index can actually validate
> uniqueness, but I find it weird that I can interact (in C) with an
> index marked unique without that index making sure the checks
> happened. Btree does it, so why not GIST?
>
> > Also we need syntax on CREATE INDEX to say what you want. The SQL
> > standard doesn't offer any help here. It gives syntax for WITHOUT
> > OVERLAPS in primary key and unique constraints, but it says nothing
> > about WITHOUT OVERLAPS on indexes.
> >
> > One option is to add WITHOUT OVERLAPS syntax to the index column list.
> > But those are full expressions (unlike with constraints), and you can
> > give optional opclasses and collations. There might be parser
> > conflicts.
>
> I don't think there would be a parser conflict there: the only
> production it might conflict with is the opclass' name, but that's
> either a single word or a quoted identifier and followed by a
> parenthesized list (or the end of the index attribute syntax); whilst
> WITHOUT OVERLAPS is always exactly two unquoted words. I don't think
> there is a way to confuse these two; at least not yet.

I tried adding an optional WITHOUT OVERLAPS modifier to index elements
in gram.y, but I did get parser conflicts. I tried several different
positions wrt the opclass and other modifiers, but there was always at
least one conflict.

> > Maybe we should use the existing opclass_parameter syntax instead.
>
> I don't like that. Opclasses shouldn't have to deal with determining
> whether WITHOUT OVERLAPS is applied, they only should provide the
> operator used for those checks and/or determining whether the value is
> considered "empty" i.e. whether the value can overlap with another
> (same) value. Both of which should be unique (no alternative
> implementations) for any given opclass.
>
> > These get stored in pg_attribute.attoptions (for the index rel). That
> > seems safer and more extensible. It avoids future conflicts with the
> > standard.
>
> I'd rather have this added to pg_index as either a bit in e.g.
> indoption (the new bit indicating the attribute should use WITHOUT
> OVERLAPS), or as new array indicating the operators used for checking
> uniqueness.
> I don't think this issue warrants new additions to pg_attribute, which
> is already a very wide structure.

That new array is what I meant by moving conexclop to pg_index (or at
least adding its equivalent). I kind of like having a bit in indoption.
I was actually doing that in a very early version of the temporal PKs
patch. I took it out because indoption seemed to be for *storage*
options. But there is plenty of room there, and I don't see any reason
not to expand its use. Well one reason: it seems like this column is
reserved for opclasses to do what they want with, and WITHOUT OVERLAPS
is core code. Is that a problem?

> > (Just the opclass *name* is not sufficient, since the point is you
> > have to know to use overlaps semantics, not equality, and forbid empty
> > values.)
>
> Shouldn't that be implemented by the AM and delegated to the opclass
> with a single support function call? I don't see why we need to
> determine at runtime which operator is responsible for exclude
> constraints -- do you have an example where a single opclass (not:
> opfamily) needs to support multiple WITHOUT OVERLAPS -backing
> operators?
>
> > Once we have this syntax, pg_get_indexdef can use it. Likewise we
> > could allow USING INDEX for a WITHOUT OVERLAPS constraint (as long as
> > the index was enforcing temporal semantics).
> >
> > Whatever we do here, we should also consider how it affects
> > pg_indexam_has_property, pg_index_has_property, and
> > pg_index_column_has_property. What does the false mean from
> > `pg_indexam_has_property((select oid from pg_am where amname =
> > 'gist'), 'can_unique')`? We really want a "maybe" or "sometimes"
> > answer. Should we return NULL and support 'can_unique' in the other
> > more-specific columns?
>
> Why? Btree has can_unique, but that doesn't mean every index is a
> unique index. Surely we can add an IndexAM callback that validates
> that a given index is actually something that this index AM can
> support; or make ambuild/ambuildempty raise if it can't actually
> construct that empty index with the given index definition.
>
> > Note that GiST theoretically can support unique indexes,[3] but we
> > would need a way to know which operator (or strategy number) the
> > opclass wanted for equality.
>
> That's only a support function away, though. AFAIK, there is only one
> natural operator for equality for any opclass, and only one natural
> WITHOUT OVERLAPS behaviour. If the user wants different equality or
> WITHOUT OVERLAPS behaviour, then they can use a different opclass. See
> e.g. btree's text_ops vs text_pattern_ops.

Okay.

> > I don't think there are any other
> > blockers. But we've solved that now with the CompareType enum.[4] So
> > that gives us a path to make GiST be unequivocally true for
> > can_unique. Then the has_property functions don't need big changes.
> > And it would solve the misleading error message.
> >
> > You still couldn't pre-create the index *concurrently*, but all the
> > other pieces would be in place for the non-locking workflow.
> >
> > So here is a proposed sequence of work:
> >
> > - Add an opclass_parameter so you can say without_overlaps = true.
> > Only the last column of the index allows that (at least for now), and
> > there must be an overlaps operator.
>
> I've argued this before, but any passing of knowledge about which
> attribute of the index is being compared/operated on _in the opclass'
> code_ should be considered a layering violation. So how would an
> opclass know that it was given the final key attribute of the index
> and error out if it wasn't?
>
> > - If an index has that property, enforce the exclusion constraint
> > rules and forbid empty ranges/multiranges.
>
> How do you propose to pass this information on to the index AM?
>
> > - Update pg_get_indexdef to output the right syntax to create an
> > independent temporal index.
> > - Allow USING INDEX for WITHOUT OVERLAPS. It must be a temporal index.
>
> What's a "temporal index" in this context? Do you mean "unique index,
> with some attributes using the more strict WITHOUT OVERLAPS
> behaviour"?
>
> > - Support can_unique in GiST indexes using CompareType. (Perhaps this
> > is of low practical value, but the discrepancies bug me.)
>
> What do you mean by this? Do you have a reference to discussions that
> touched this topic?

This was based on a few hallway conversations at PGConfs. Even if you
could have a unique GiST index (and you can today, effectively, with an
exclusion constraint that is just `(id WITH =)`, it is probably
strictly worse than a B-Tree index. That's what I mean by low practical
value. But being able to build the index separately from the constraint
does give it a use.

> > - Add can_temporal to index AMs. Make GiST report true. Check this
> > property instead of hard-coding GiST in our WITHOUT OVERLAPS code.
>
> +1; though IMO that should be implemented regardless of earlier operations.

Okay.

To start, the patch here lets you create unique GiST indexes. If the
index is for a WITHOUT OVERLAPS constraint, they check for overlaps
instead of equality. (I was originally planning to defer that part, but
it was almost no extra code.)

So now I can do this:

```
[v19devel:5432][454633] postgres=# create table t (id int4range,
valid_at daterange, primary key (id, valid_at without overlaps));
CREATE TABLE
[v19devel:5432][454633] postgres=# select pg_get_indexdef('t_pkey'::regclass);
                         pg_get_indexdef
------------------------------------------------------------------
 CREATE UNIQUE INDEX t_pkey ON public.t USING gist (id, valid_at)
(1 row)

[v19devel:5432][454633] postgres=# alter table t drop constraint t_pkey;
ALTER TABLE
[v19devel:5432][454633] postgres=# CREATE UNIQUE INDEX t_pkey ON
public.t USING gist (id, valid_at);
CREATE INDEX
[v19devel:5432][454633] postgres=# \d t
                   Table "public.t"
  Column  |   Type    | Collation | Nullable | Default
----------+-----------+-----------+----------+---------
 id       | int4range |           | not null |
 valid_at | daterange |           | not null |
Indexes:
    "t_pkey" UNIQUE, gist (id, valid_at)
```

So now pg_get_indexdef's output doesn't crash. And you get back the
same *on disk* index. But its enforcement isn't quite what you started
with. Since there was no `WITHOUT OVERLAPS`, it will forbid equality
but not forbid overlaps. Still I think this gets us closer to the end.
If using indoption is okay, we can change pg_getindexdef to do that.
And if the index sees that option, it should set indisexclusion.

The next step is to permit USING INDEX for a WITHOUT OVERLAPS
constraint. That would only be permitted for indexes with
indisexclusion and an appropriate indoption.

There are still some big missing pieces from the current patch. From
the commit message:

    I haven't dealt with MVCC issues yet. When we find a not-yet-committed
    conflicting tuple, we need to wait and re-check after that transaction has
    finished. See nbtree for how this needs to work.

    Also I'm trying to handle NULLS NOT DISTINCT correctly. In nbtree the code
    assumes that nulls are never equal. So is that handled outside of
specific IAMs?

I also need to think more about concurrency issues and locking. The
original GiST research paper envisioned unique indexes (and didn't
think they needed a lot of extra consideration), and as far as I can
tell we already have the locks we need after traversing to the leaf
page. But I want to go through it more carefully.

Also I think that check_exclusion_or_unique_constraint can now skip
most of its work, if the constraint is WITHOUT OVERLAPS. I think I will
have a new patch with that improvement soon.

Rebased to 85d5bd308b.

Yours,

--
Paul              ~{:-)
pj@illuminatedcomputing.com

Вложения

В списке pgsql-bugs по дате отправления: