On Mon, Oct 13, 2014 at 1:25 PM, Fabrízio de Royes Mello <fabriziome...@gmail.com> wrote: > On Sun, Oct 12, 2014 at 2:27 PM, Stephen Frost <sfr...@snowman.net> wrote: >> >> * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: >> > Sawada Masahiko wrote: >> > > Attached WIP patch adds new syntax REINEX SCHEMA which does reindexing >> > > all table of specified schema. >> > > There are syntax dose reindexing specified index, per table and per >> > > database, >> > > but we can not do reindexing per schema for now. >> > >> > It seems doubtful that there really is much use for this feature, but if >> > there is, I think a better syntax precedent is the new ALTER TABLE ALL >> > IN TABLESPACE thingy, rather than your proposed REINDEX SCHEMA. >> > Something like REINDEX TABLE ALL IN SCHEMA perhaps. >> >> Yeah, I tend to agree that we should be looking at the 'ALL IN >> TABLESPACE' and 'ALL IN SCHEMA' type of commands to keep things >> consistent. This might be an alternative for the vacuum / analyze / >> reindex database commands also.. >> > > Some review: > > 1) +1 to "REINDEX ALL IN SCHEMA name" >
Thank you for comment and reviewing! I agree with this. I will change syntax to above like that. > > 2) IMHO the logic should be exactly the same as REINDEX DATABASE, including > the transaction control. Imagine a schema with a lot of tables, you can lead > to a deadlock using just one transaction block. > Yep, it should be same as REINDEX DATABASE. IMO, we can put them into one function if they use exactly same logic. Thought? > > 3) The patch was applied to master and compile without warnings > > > 4) Typo (... does not have any table) > > + if (!reindex_schema(heapOid)) > + ereport(NOTICE, > + (errmsg("schema\"%s\" does not hava any table", > + schema->relname))); > Thanks! I will correct typo. > > 5) Missing of regression tests, please add it to > src/test/regress/sql/create_index.sql > > 6) You need to add psql complete tabs > Next version patch, that I will submit, will have 5), 6) things you pointed. > 7) I think we can add "-S / --schema" option do reindexdb in this patch too. > What do you think? > +1 to add "-S / --schema" option I was misunderstanding about that. I will make the patch which adds this option as separate patch. -- Regards, ------- Sawada Masahiko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers