On 2020-03-30 21:34, Justin Pryzby wrote:
On Mon, Mar 30, 2020 at 09:02:22PM +0300, Alexey Kondratov wrote:
Hmm, I went through the well known to me SQL commands in Postgres and
a bit
more. Parenthesized options list is mostly used in two common cases:
There's also ANALYZE(VERBOSE), REINDEX(VERBOSE).
There was debate a year ago [0] as to whether to make "reindex
CONCURRENTLY" a
separate command, or to use parenthesized syntax "REINDEX
(CONCURRENTLY)". I
would propose to support that now (and implemented that locally).
I am fine with allowing REINDEX (CONCURRENTLY), but then we will have to
support both syntaxes as we already do for VACUUM. Anyway, if we agree
to add parenthesized options to REINDEX/CLUSTER, then it should be done
as a separated patch before the current patch set.
..and explain(...)
- In the beginning for boolean options only, e.g. VACUUM
You're right that those are currently boolean, but note that
explain(FORMAT ..)
is not boolean.
Yep, I forgot EXPLAIN, this is a good example.
.. and create table (LIKE ..)
LIKE is used in the table definition, so it is a slightly different
case.
Putting it into the WITH (...) options list looks like an option to
me.
However, doing it only for VACUUM will ruin the consistency, while
doing it
for CLUSTER and REINDEX is not necessary, so I do not like it either.
It's not necessary but I think it's a more flexible way to add new
functionality (requiring no changes to the grammar for vacuum, and for
REINDEX/CLUSTER it would allow future options to avoid changing the
grammar).
If we use parenthesized syntax for vacuum, my proposal is to do it for
REINDEX, and
consider adding parenthesized syntax for cluster, too.
To summarize, currently I see only 2 + 1 extra options:
1) Keep everything with syntax as it is in 0001-0002
2) Implement tail syntax for VACUUM, but with limitation for VACUUM
FULL of
the entire database + TABLESPACE change
3) Change TABLESPACE to a fully reserved word
+ 4) Use parenthesized syntax for all three.
Note, I mentioned that maybe VACUUM/CLUSTER should support not only
"TABLESPACE
foo" but also "INDEX TABLESPACE bar" (I would use that, too). I think
that
would be easy to implement, and for sure it would suggest using () for
both.
(For sure we don't want to implement "VACUUM t TABLESPACE foo" now, and
then
later implement "INDEX TABLESPACE bar" and realize that for consistency
we
cannot parenthesize it.
Michael ? Alvaro ? Robert ?
Yes, I would be glad to hear other opinions too, before doing this
preliminary refactoring.
--
Alexey Kondratov
Postgres Professional https://www.postgrespro.com
Russian Postgres Company
