On 2020-09-09 15:22, Michael Paquier wrote:
On Tue, Sep 08, 2020 at 07:17:58PM -0500, Justin Pryzby wrote:
Initially I added List *params, and Michael suggested to retire
ReindexStmt->concurrent. I provided a patch to do so, initially by
leaving int
options and then, after this, removing it to "complete the thought",
and get
rid of the remnants of the "old way" of doing it. This is also how
vacuum and
explain are done.
https://www.postgresql.org/message-id/20200902022410.GA20149%40telsasoft.com
Defining a set of DefElem when parsing and then using the int
"options" with bitmasks where necessary at the beginning of the
execution looks like a good balance to me. This way, you can extend
the grammar to use things like (verbose = true), etc.
By the way, skimming through the patch set, I was wondering if we
could do the refactoring of patch 0005 as a first step
Yes, I did it with intention to put as a first patch, but wanted to get
some feedback. It's easier to refactor the last patch without rebasing
others.
until I
noticed this part:
+common_option_name:
NonReservedWord { $$ = $1; }
| analyze_keyword { $$ = "analyze"; }
This is not a good idea as you make ANALYZE an option available for
all the commands involved in the refactoring. A portion of that could
be considered though, like the use of common_option_arg.
From the grammar perspective ANY option is available for any command
that uses parenthesized option list. All the checks and validations are
performed at the corresponding command code.
This analyze_keyword is actually doing only an ANALYZE word
normalization if it's used as an option. Why it could be harmful?
Regards
--
Alexey Kondratov
Postgres Professional https://www.postgrespro.com
Russian Postgres Company
