Re: Allowing REINDEX to have an optional name
Cary Huang <cary.huang@highgo.ca>
From: Cary Huang <cary.huang@highgo.ca>
To: pgsql-hackers@lists.postgresql.org
Cc: Simon Riggs <simon.riggs@enterprisedb.com>
Date: 2022-05-27T19:08:51Z
Lists: pgsql-hackers
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation: tested, passed Hello The patch applies and tests fine and I think this patch has good intentions to prevent the default behavior of REINDEX DATABASE to cause a deadlock. However, I am not in favor of simply omitting the database name after DATABASE clause because of consistency. Almost all other queries involving the DATABASE clause require database name to be given following after. For example, ALTER DATABASE [dbname]. Being able to omit database name for REINDEX DATABASE seems inconsistent to me. The documentation states that REINDEX DATABASE only works on the current database, but it still requires the user to provide a database name and require that it must match the current database. Not very useful option, isn’t it? But it is still required from the user to stay consistent with other DATABASE clauses. Maybe the best way is to keep the query clause as is (with the database name still required) and simply don’t let it reindex system catalog to prevent deadlock. At the end, give user a notification that system catalogs have not been reindexed, and tell them to use REINDEX SYSTEM instead. Cary Huang ----------------- HighGo Software Canada www.highgo.ca
Commits
-
Tweak a bit the new TAP tests of REINDEX DATABASE/SYSTEM
- 2b99ce10c2bc 16.0 landed
-
Rework logic and simplify syntax of REINDEX DATABASE/SYSTEM
- 2cbc3c17a5c1 16.0 landed
-
Add more tests for REINDEX DATABASE/SYSTEM with relfilenode changes
- 5fb5b6c4c176 16.0 landed