Subject: Dump/Restore of non-default PKs

From: Simon Riggs <[email-masked]>
Date: Mon, 18 Apr 2022 20:59:44 +0100

At the moment you cannot create a unique index other than a btree. (As
discussed on other threads, I am pursuing unique hash indexes for
PostgreSQL, one step at a time).
You get "ERROR index foo_idx is not a btree"

According to parse_utilcmd.c line 2310, this is because it would break
pg_dump, which needs ADD CONSTRAINT to create the same kind of index
again. Fair enough.

This is needed because ADD CONSTRAINT just uses the defaults index
type. We could simply allow a GUC for
default_primary_key_access_method, but that is overkill and there
seems to be an easy and more general solution:

I propose that we change pg_dump so that when it creates a PK it does
so in 2 commands:
1. CREATE [UNIQUE] INDEX iname ...
2. ALTER TABLE .. ADD PRIMARY KEY USING INDEX iname;

Step
(1) recreates the index, respecting its AM, even if that is not a btree
(2) works and there is no problem with defaults

Doing this as 2 steps instead of one doesn't add any more time because
(2) is just a metadata-only change, not an index build.

Any objections to a patch to implement this thought?

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

---

From: "David G. Johnston" <[email-masked]>
Date: Mon, 18 Apr 2022 13:20:52 -0700

> Any objections to a patch to implement this thought?

Why not just get rid of the limitation that constraint definitions don't
support non-default methods?

I.e., add syntax to index_parameters so that the underlying index can be
defined directly.

index_parameters in UNIQUE, PRIMARY KEY, and EXCLUDE constraints are:

[ INCLUDE ( column_name [, ... ] ) ]
[ WITH ( storage_parameter [= value] [, ... ] ) ]
[ USING INDEX TABLESPACE tablespace_name ]

We should add:

[ USING INDEX METHOD index_method ]

index_method := { BTREE | GIN | GIST | HASH | SPGIST | BRIN }

David J.

---

From: Tom Lane <[email-masked]>
Date: Mon, 18 Apr 2022 16:27:29 -0400

> (2) is just a metadata-only change, not an index build.

I don't believe the claim that this adds no cost.  Maybe it's negligible
in context, but you've provided no evidence of that.  (Parallel restore,
where the ALTER would have to be a separate worker task, would probably
be the worst case here.)

Also, I assume your ambition would extend to supporting UNIQUE (but
non-PKEY) constraints, so that would have to be done this way too.

A potential advantage of doing things this way is that if we make
pg_dump treat the index and the constraint as fully independent
objects, that might allow some logic simplifications in pg_dump.
Right now I think there are various weirdnesses in there that
exist precisely because we don't want to dump them separately.

One concern is that this'd create a hard compatibility break for
loading dump output into servers that predate whenever we added
ADD PRIMARY KEY USING INDEX.  However, it looks like that syntax
is accepted back to 9.1, so probably that's no issue in practice.
Maybe a bigger concern for people who want to port to other
RDBMSes?

			regards, tom lane

---

From: Tom Lane <[email-masked]>
Date: Mon, 18 Apr 2022 16:48:07 -0400

> support non-default methods?

That approach would be doubling down on the assumption that we can always
shoehorn more custom options into SQL-standard constraint clauses, and
we'll never fall foul of shift/reduce problems or future spec additions.
I think for example that USING INDEX TABLESPACE is a blot on humanity,
and I'd be very glad to see pg_dump stop using it in favor of doing
things as Simon suggests.

			regards, tom lane

---

From: Peter Eisentraut <[email-masked]>
Date: Wed, 20 Apr 2022 22:46:35 +0200

> we'll never fall foul of shift/reduce problems or future spec additions.

When we do get the ability to create a table with a primary key with an 
underlying hash index, how would that be done?  Would the only way be

1. create the table without primary key
2. create the index
3. attach the index as primary key constraint

That doesn't sound attractive.