Sorry for the late answer,
On Tue, Jul 23, 2019 at 9:38 AM Michael Paquier <mich...@paquier.xyz> wrote:
>
> On Mon, Jul 22, 2019 at 02:40:19PM +0200, Julien Rouhaud wrote:
> > Right, so I kept the long option. Also this comment was outdated, as
> > the --jobs is now just ignored with a list of indexes, so I fixed that
> > too.
>
> + if (!parallel)
> + {
> + if (user_list == NULL)
> + {
> + /*
> + * Create a dummy list with an empty string, as user requires an
> + * element.
> + */
> + process_list = pg_malloc0(sizeof(SimpleStringList));
> + simple_string_list_append(process_list, "");
> + }
> + }
> This part looks a bit hacky and this is needed because we don't have a
> list of objects when doing a non-parallel system or database reindex.
> The deal is that we just want a list with one element: the database of
> the connection. Wouldn't it be more natural to assign the database
> name here using PQdb(conn)? Then add an assertion at the top of
> run_reindex_command() checking for a non-NULL name?Good point, fixed it that way. > > > I considered this, but it would require to adapt all code that declare > > SimpleStringList stack variable (vacuumdb.c, clusterdb.c, > > createuser.c, pg_dumpall.c and pg_dump.c), so it looked like too much > > trouble to avoid 2 local variables here and 1 in vacuumdb.c. I don't > > have a strong opinion here, so I can go for it if you prefer. > > Okay. This is a tad wider than the original patch proposal, and this > adds two lines. So let's discard that for now and keep it simple. Ok! > >> +$node->issues_sql_like([qw(reindexdb -j2)], > >> + qr/statement: REINDEX TABLE public.test1/, > >> + 'Global and parallel reindex will issue per-table REINDEX'); > >> Would it make sense to have some tests for schemas here? > >> > >> One of my comments in [1] has not been answered. What about > >> the decomposition of a list of schemas into a list of tables when > >> using the parallel mode? > > > > I did that in attached v6, and added suitable regression tests. > > The two tests for "reindexdb -j2" can be combined into a single call, > checking for both commands to be generated in the same output. The > second command triggering a reindex on two schemas cannot be used to > check for the generation of both s1.t1 and s2.t2 as the ordering may > not be guaranteed. The commands arrays also looked inconsistent with > the rest. Attached is an updated patch with some format modifications > and the fixes for the tests. Attached v8 based on your v7 + the fix for the dummy part.
reindex_parallel_v8.diff
Description: Binary data
