pgsql: Drop index behind pg_upgrade test issue.

Started by Peter Geogheganover 5 years ago5 messagescomitters
Jump to latest

Drop index behind pg_upgrade test issue.

The vacuum_cleanup_index_scale_factor storage parameter was set in a
btree index that was previously left behind in the regression test
database. As a result, the index gets tested within pg_dump and
pg_restore tests, as well as pg_upgrade testing. This won't work when
upgrading to Postgres 14, though, because the storage parameter was
removed on that version by commit 9f3665fb.

Fix the test failure by dropping the index in question.

Per buildfarm member crake.

Discussion: /messages/by-id/CAH2-WzmeXYBWdhF7BMhNjhq9exsk=E1ohqBFAwzPdXJZ1XDMUA@mail.gmail.com
Backpatch: 11-12 only

Branch
------
REL_11_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/006c52668ab3c93b4e18a5c53a51f28faa9603a2

Modified Files
--------------
src/test/regress/expected/btree_index.out | 1 +
src/test/regress/sql/btree_index.sql | 1 +
2 files changed, 2 insertions(+)

#2Laurenz Albe
laurenz.albe@cybertec.at
In reply to: Peter Geoghegan (#1)
Re: pgsql: Drop index behind pg_upgrade test issue.

On Thu, 2021-03-11 at 03:02 +0000, Peter Geoghegan wrote:

The vacuum_cleanup_index_scale_factor storage parameter was set in a
btree index that was previously left behind in the regression test
database. As a result, the index gets tested within pg_dump and
pg_restore tests, as well as pg_upgrade testing. This won't work when
upgrading to Postgres 14, though, because the storage parameter was
removed on that version by commit 9f3665fb.

Fix the test failure by dropping the index in question.

I'd take that as an indication that Michael had a point with his
complaint[1]/messages/by-id/a3Ko3nKnBuVq@paquier.xyz. Perhaps it would be better to implement one of his
ideas than to remove the test.

Yours,
Laurenz Albe

[1]: /messages/by-id/a3Ko3nKnBuVq@paquier.xyz

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Laurenz Albe (#2)
Re: pgsql: Drop index behind pg_upgrade test issue.

Laurenz Albe <laurenz.albe@cybertec.at> writes:

On Thu, 2021-03-11 at 03:02 +0000, Peter Geoghegan wrote:

Fix the test failure by dropping the index in question.

I'd take that as an indication that Michael had a point with his
complaint[1]. Perhaps it would be better to implement one of his
ideas than to remove the test.

Strongly agree. I hadn't realized that the patch actually *removed*
the index reloption, rather than just turning it into a no-op. I think
that's a complete nonstarter. It will break pg_upgrade scenarios where
users have indexes with that option set. It's an especially bad idea
to have done it in a stable branch, but I doubt we can get away with
it even in HEAD.

regards, tom lane

In reply to: Tom Lane (#3)
Re: pgsql: Drop index behind pg_upgrade test issue.

On Thu, Mar 11, 2021 at 8:27 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Strongly agree. I hadn't realized that the patch actually *removed*
the index reloption, rather than just turning it into a no-op. I think
that's a complete nonstarter. It will break pg_upgrade scenarios where
users have indexes with that option set. It's an especially bad idea
to have done it in a stable branch, but I doubt we can get away with
it even in HEAD.

But I haven't removed the index reloption (or the GUC of the same
name) on the 13 branch. I just made it a no-op there, and removed the
documentation.

Evidently there is a consensus that the reloption should not have been
removed on HEAD. I'll discuss that on the other -committers thread
now.

--
Peter Geoghegan

In reply to: Laurenz Albe (#2)
Re: pgsql: Drop index behind pg_upgrade test issue.

On Thu, Mar 11, 2021 at 1:36 AM Laurenz Albe <laurenz.albe@cybertec.at> wrote:

I'd take that as an indication that Michael had a point with his
complaint[1]. Perhaps it would be better to implement one of his
ideas than to remove the test.

The Postgres 11 and 12 "drop index" commits are now reverted.

Thanks
--
Peter Geoghegan