On Fri, Jan 04, 2019 at 03:18:06PM +0300, Sergei Kornilov wrote:
> NOTICE seems unnecessary here.
>
> Unfortunally concurrently reindex loses comments, reproducer:
Yes, the NOTICE message makes little sense.
I am getting back in touch with this stuff. It has been some time but
the core of the patch has not actually changed in its base concept, so
I am still very familiar with it as the original author. There are
even typos I may have introduced a couple of years back, like
"contraint". I have not yet spent much time on that, but there are at
quick glance a bunch of things that could be retouched to get pieces
of that committable.
+ The concurrent index created during the processing has a name ending in
+ the suffix ccnew, or ccold if it is an old index definiton which we failed
+ to drop. Invalid indexes can be dropped using <literal>DROP INDEX</literal>,
+ including invalid toast indexes.
This needs <literal> markups for "ccnew" and "ccold". "definiton" is
not correct.
index_create does not actually need its extra argument with the tuple
descriptor. I think that we had better grab the column name list from
indexInfo and just pass that down to index_create() (patched on my
local branch), so it is an overkill to take a full copy of the index's
TupleDesc.
The patch, standing as-is, is close to 2k lines long, so let's cut
that first into more pieces refactoring the concurrent build code.
Here are some preliminary notes:
- WaitForOlderSnapshots() could be in its own patch.
- index_concurrently_build() and index_concurrently_set_dead() can be
in an independent patch. set_dead() had better be a wrapper on top of
index_set_state_flags actually which is able to set any kind of
flags.
- A couple of pieces in index_create() could be cut as well.
I can send patches for those things as first steps which could happen
in this commit then, and commit them as needed. This way, we reduce
the size of the main patch. Even if the main portion does not get
into v12, we'd still have base pieces to move on next.
Regarding the grammar, we tend for the last couple of years to avoid
complicating the main grammar and move on to parenthesized grammars
(see VACUUM, ANALYZE, EXPLAIN, etc). So in the same vein I think that
it would make sense to only support CONCURRENTLY within parenthesis
and just plugin that with the VERBOSE option.
Does somebody mind if I jump into the ship after so long? I was the
original author of the monster after all...
--
Michael
