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
DATABASEto cause a deadlock. However, I am not in favor of simply omitting the database name after DATABASE clause
becauseof consistency. Almost all other queries involving the DATABASE clause require database name to be given
followingafter. 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
providea database name and require that it must match the current database. Not very useful option, isn’t it? But it is
stillrequired 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
reindexsystem 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