Horton

Thread

Commits

  1. Add test for CREATE INDEX CONCURRENTLY with not-so-immutable predicate

  2. Make index_set_state_flags() transactional

  1. Assertion on create index concurrently

    Andrei Lepikhov <a.lepikhov@postgrespro.ru> — 2021-06-26T18:48:45Z

    I found one annoying assertion in the CREATE INDEX CONCURRENTLY 
operation. It will happen if you add into WHERE part of CREATE INDEX 
CONCURRENTLY some function that will make transactional update (see an 
example in the attached patch).

It looks like a hack, but it lead to assertion and restart of the 
instance, that isn't good.
I suggest to perform error, not assertion in this case (see the patch).

This problem affects PG 9.6 - 13. The master branch was fixed 9 months 
ago by commit 83158f7, that made this operation transactional, but still.

-- 
regards,
Andrey Lepikhov
Postgres Professional

  2. Re: Assertion on create index concurrently

    Michael Paquier <michael@paquier.xyz> — 2021-06-27T01:35:49Z

    On Sat, Jun 26, 2021 at 09:48:45PM +0300, Andrey Lepikhov wrote:
> I found one annoying assertion in the CREATE INDEX CONCURRENTLY operation.
> It will happen if you add into WHERE part of CREATE INDEX CONCURRENTLY some
> function that will make transactional update (see an example in the attached
> patch).
> 
> It looks like a hack, but it lead to assertion and restart of the instance,
> that isn't good.
> I suggest to perform error, not assertion in this case (see the patch).
> 
> This problem affects PG 9.6 - 13. The master branch was fixed 9 months ago
> by commit 83158f7, that made this operation transactional, but still.

This looks like a good argument for back-patching 83158f7 all the way
down, as 813fb03 exists since 9.4.  Any thoughts from others?
--
Michael

  3. Re: Assertion on create index concurrently

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-06-27T15:37:16Z

    Michael Paquier <michael@paquier.xyz> writes:
> On Sat, Jun 26, 2021 at 09:48:45PM +0300, Andrey Lepikhov wrote:
>> This problem affects PG 9.6 - 13. The master branch was fixed 9 months ago
>> by commit 83158f7, that made this operation transactional, but still.

> This looks like a good argument for back-patching 83158f7 all the way
> down, as 813fb03 exists since 9.4.  Any thoughts from others?

+1 ... 83158f7 has survived long enough to have reasonable confidence
in it.

			regards, tom lane

  4. Re: Assertion on create index concurrently

    Andrei Lepikhov <a.lepikhov@postgrespro.ru> — 2021-06-27T19:33:44Z

    On 27/6/21 18:37, Tom Lane wrote:
> Michael Paquier <michael@paquier.xyz> writes:
>> On Sat, Jun 26, 2021 at 09:48:45PM +0300, Andrey Lepikhov wrote:
>>> This problem affects PG 9.6 - 13. The master branch was fixed 9 months ago
>>> by commit 83158f7, that made this operation transactional, but still.
> 
>> This looks like a good argument for back-patching 83158f7 all the way
>> down, as 813fb03 exists since 9.4.  Any thoughts from others?
> 
> +1 ... 83158f7 has survived long enough to have reasonable confidence
> in it.
Technically, 83158f7 is trivial patch. I think, some efforts is required 
only for PG9.6. I prepared a patch for this version.

-- 
regards,
Andrey Lepikhov
Postgres Professional

  5. Re: Assertion on create index concurrently

    Michael Paquier <michael@paquier.xyz> — 2021-06-28T02:19:48Z

    On Sun, Jun 27, 2021 at 10:33:44PM +0300, Andrey Lepikhov wrote:
> Technically, 83158f7 is trivial patch. I think, some efforts is required
> only for PG9.6. I prepared a patch for this version.

Thanks.  This version had two problems:
- The comment on top of index_set_state_flags() was not updated, still
mentioning CatalogTupleUpdate().
- CatalogUpdateIndexes() does the job for the index updates, and using
it is more consistent with the other areas of index.c in
REL9_6_STABLE.

Just to keep a cleaner history, I have split your test case into a
separate commit, and applied that and the backpatch of 83158f7.
--
Michael