On Wed, Nov 19, 2014 at 1:37 AM, Simon Riggs <si...@2ndquadrant.com> wrote: > On 23 October 2014 00:21, Alvaro Herrera <alvhe...@2ndquadrant.com> wrote: > >>> Attached patch is latest version patch I modified above. >>> Also, I noticed I had forgotten to add the patch regarding document of >>> reindexdb. >> >> Please don't use pg_catalog in the regression test. That way we will >> need to update the expected file whenever a new catalog is added, which >> seems pointless. Maybe create a schema with a couple of tables >> specifically for this, instead. > > These patches look fine to me. Don't see anybody objecting either. > > Are you looking to commit them, or shall I?
IMO, SCHEMA is just but fine, that's more consistent with the existing syntax we have for database and tables. Btw, there is an error in this patch, there are no ACL checks on the schema for the user doing the REINDEX, so any user is able to perform a REINDEX on any schema. Here is an example for a given user, let's say "poo'": => create table foo.g (a int); ERROR: 42501: permission denied for schema foo LOCATION: aclcheck_error, aclchk.c:3371 => reindex schema foo ; NOTICE: 00000: table "foo.c" was reindexed LOCATION: ReindexDatabaseOrSchema, indexcmds.c:1930 REINDEX A regression test to check that would be good as well. Also, ISTM that it is awkward to modify the values of do_user and do_system like that in ReindexDatabase for two reasons: 1) They should be set in gram.y 2) This patch passes as a new argument of ReindexDatabase the object type, something that is overlapping with what do_user and do_system are aimed for. Why not simply defining a new object type OBJECT_SYSTEM? As this patch introduces a new object category for REINDEX, this patch seems to be a good occasion to remove the boolean dance in REINDEX at the cost of having a new object type for the concept of a system, which would be a way to define the system catalogs as a whole. Another thing, ReindexDatabaseOrSchema should be renamed to ReindexObject. So, I think that we need to think a bit more here. We are not far from smth that could be committed, so marking as "Waiting on Author" for now. Thoughts? -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
