Обсуждение: What exactly is drop-index-concurrently-1.spec trying to test?

Peter pointed out in
http://www.postgresql.org/message-id/527C0FE9.7000600@gmx.net
that Kyotaro-san's patch to treat unique indexes as satisfying any sort
condition that they are a prefix of broke the drop-index-concurrently-1
isolation test.  The latest iterations of the patch respond to that by
changing the expected output.  However, that seems rather wrongheaded,
because AFAICT the intent of that part of the test is to demonstrate that
a seqscan has particular behavior; so if the planner starts generating an
indexscan instead, the test no longer proves anything of the kind.

What I'm wondering though is what's the point of testing that a concurrent
DROP INDEX doesn't affect a seqscan?  That seems kinda silly, so it's
tempting to address the patch's problem by just removing the steps
involving the getrow_seq query, rather than hacking it up enough so we'd
still get a seqscan plan.

What's more, the expected output shows that the other sessions are
blocking the DROP INDEX CONCURRENTLY until they complete, which means if
there *were* any odd effects like that, this test likely would fail to
expose them anyway.  I'd have thought the test would be designed to allow
the DROP to complete and then re-test that the results of the prepared
query are still sane, but it does no such thing.

So, in short, exactly what is it that this is supposed to be testing?
        regards, tom lane

On 2013-12-31 17:14:11 -0500, Tom Lane wrote:
> Peter pointed out in
> http://www.postgresql.org/message-id/527C0FE9.7000600@gmx.net
> that Kyotaro-san's patch to treat unique indexes as satisfying any sort
> condition that they are a prefix of broke the drop-index-concurrently-1
> isolation test.  The latest iterations of the patch respond to that by
> changing the expected output.  However, that seems rather wrongheaded,
> because AFAICT the intent of that part of the test is to demonstrate that
> a seqscan has particular behavior; so if the planner starts generating an
> indexscan instead, the test no longer proves anything of the kind.
> 
> What I'm wondering though is what's the point of testing that a concurrent
> DROP INDEX doesn't affect a seqscan?  That seems kinda silly, so it's
> tempting to address the patch's problem by just removing the steps
> involving the getrow_seq query, rather than hacking it up enough so we'd
> still get a seqscan plan.

The point is to show that an index scan returns the same rows a
sequential scan would, even though the index is in the process of being
dropped and has been updated *after* the DROP started. That was broken
at some point.
Now, you could argue that that would also be shown without the
sequential scan, but I think that would make understanding the faulty
output harder.

> I'd have thought the test would be designed to allow
> the DROP to complete and then re-test that the results of the prepared
> query are still sane, but it does no such thing.

We could add a permutation like this, but ISTM that it would just test
plan invalidation?

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services