Proposal : REINDEX xxx VERBOSE

On Mon, Feb 2, 2015 at 8:31 PM, Sawada Masahiko <sawada.mshk@gmail.com> wrote:

Attached patch adds VERBOSE option to REINDEX commands.
Please give me feedbacks.

This could provide useful feedback to users. Now, I think that it may
be better to provide the keyword VERBOSE before the type of object
reindexed as REINDEX [ VERBOSE ] object. In any case, at quick sight,
the table completion for REINDEX is broken with your patch because by
typing REINDEX VERBOSE you would show the list of objects and once
again VERBOSE.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Feb 2, 2015 at 9:21 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Mon, Feb 2, 2015 at 8:31 PM, Sawada Masahiko <sawada.mshk@gmail.com> wrote:

Attached patch adds VERBOSE option to REINDEX commands.
Please give me feedbacks.

This could provide useful feedback to users.

Thanks.

Now, I think that it may
be better to provide the keyword VERBOSE before the type of object
reindexed as REINDEX [ VERBOSE ] object.

Actually, my first WIP version of patch added VERBOSE word at before
type of object.
I'm feeling difficult about that the position of VERBOSE word in
REINDEX statement.

In any case, at quick sight,
the table completion for REINDEX is broken with your patch because by
typing REINDEX VERBOSE you would show the list of objects and once
again VERBOSE.

I have also rebased the tab-completion source, I think it's not happen.
In my environment, it does not show list of object and VERBOSE again
after typing REINDEX VERBOSE.

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

Sawada Masahiko <sawada.mshk@gmail.com> writes:

On Mon, Feb 2, 2015 at 9:21 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

Now, I think that it may
be better to provide the keyword VERBOSE before the type of object
reindexed as REINDEX [ VERBOSE ] object.

Actually, my first WIP version of patch added VERBOSE word at before
type of object.
I'm feeling difficult about that the position of VERBOSE word in
REINDEX statement.

The way that FORCE was added to REINDEX was poorly thought out; let's not
double down on that with another option added without any consideration
for future expansion. I'd be happier if we adopted something similar to
the modern syntax for VACUUM and EXPLAIN, ie, comma-separated options in
parentheses.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Feb 3, 2015 at 12:32 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Sawada Masahiko <sawada.mshk@gmail.com> writes:

On Mon, Feb 2, 2015 at 9:21 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

Now, I think that it may
be better to provide the keyword VERBOSE before the type of object
reindexed as REINDEX [ VERBOSE ] object.

Actually, my first WIP version of patch added VERBOSE word at before
type of object.
I'm feeling difficult about that the position of VERBOSE word in
REINDEX statement.

The way that FORCE was added to REINDEX was poorly thought out; let's not
double down on that with another option added without any consideration
for future expansion. I'd be happier if we adopted something similar to
the modern syntax for VACUUM and EXPLAIN, ie, comma-separated options in
parentheses.

I understood.
I'm imagining new REINDEX syntax are followings.
- REINDEX (INDEX, VERBOSE) hoge_idx;
- REINDEX (TABLE) hoge_table;

i.g., I will add following syntax format,
REINDEX ( { INDEX | TABLE | SCHEMA | SYSTEM | DATABASE } , [VERBOSE] )
name [FORCE];

Thought?

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

On Tue, Feb 3, 2015 at 9:09 AM, Sawada Masahiko <sawada.mshk@gmail.com>
wrote:

On Tue, Feb 3, 2015 at 12:32 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Sawada Masahiko <sawada.mshk@gmail.com> writes:

On Mon, Feb 2, 2015 at 9:21 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

Now, I think that it may
be better to provide the keyword VERBOSE before the type of object
reindexed as REINDEX [ VERBOSE ] object.

Actually, my first WIP version of patch added VERBOSE word at before
type of object.
I'm feeling difficult about that the position of VERBOSE word in
REINDEX statement.

The way that FORCE was added to REINDEX was poorly thought out; let's

not

double down on that with another option added without any consideration
for future expansion. I'd be happier if we adopted something similar to
the modern syntax for VACUUM and EXPLAIN, ie, comma-separated options in
parentheses.

I understood.
I'm imagining new REINDEX syntax are followings.
- REINDEX (INDEX, VERBOSE) hoge_idx;
- REINDEX (TABLE) hoge_table;

i.g., I will add following syntax format,
REINDEX ( { INDEX | TABLE | SCHEMA | SYSTEM | DATABASE } , [VERBOSE] )
name [FORCE];

Thought?

I don't think the keyworks INDEX, TABLE, SCHEMA, SYSTEM and DATABASE are
options... they are part of the command IMHO.

Maybe something like:

REINDEX { INDEX | TABLE | SCHEMA | SYSTEM | DATABASE } [( [ FORCE ],
[VERBOSE] ) ] name;

And maintain the old syntax for compatibility of course.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL

Sawada Masahiko <sawada.mshk@gmail.com> writes:

On Tue, Feb 3, 2015 at 12:32 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

The way that FORCE was added to REINDEX was poorly thought out; let's not
double down on that with another option added without any consideration
for future expansion. I'd be happier if we adopted something similar to
the modern syntax for VACUUM and EXPLAIN, ie, comma-separated options in
parentheses.

I understood.
I'm imagining new REINDEX syntax are followings.
- REINDEX (INDEX, VERBOSE) hoge_idx;
- REINDEX (TABLE) hoge_table;

i.g., I will add following syntax format,
REINDEX ( { INDEX | TABLE | SCHEMA | SYSTEM | DATABASE } , [VERBOSE] )
name [FORCE];

Well, the object type is not an optional part of the command. It's
*necessary*. I was thinking more like

REINDEX { INDEX | TABLE | etc } name [ ( option [, option ...] ) ]

option := FORCE | VERBOSE

We'd still keep the historical syntax where you can write FORCE outside
parens, but it'd be deprecated.

Where to insert the parenthesized option list is a judgment call,
but I'd lean to keeping it at the end where FORCE used to be.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2015-02-03 10:20:03 -0500, Tom Lane wrote:

Sawada Masahiko <sawada.mshk@gmail.com> writes:

On Tue, Feb 3, 2015 at 12:32 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

The way that FORCE was added to REINDEX was poorly thought out; let's not
double down on that with another option added without any consideration
for future expansion. I'd be happier if we adopted something similar to
the modern syntax for VACUUM and EXPLAIN, ie, comma-separated options in
parentheses.

I understood.
I'm imagining new REINDEX syntax are followings.
- REINDEX (INDEX, VERBOSE) hoge_idx;
- REINDEX (TABLE) hoge_table;

i.g., I will add following syntax format,
REINDEX ( { INDEX | TABLE | SCHEMA | SYSTEM | DATABASE } , [VERBOSE] )
name [FORCE];

Well, the object type is not an optional part of the command. It's
*necessary*. I was thinking more like

REINDEX { INDEX | TABLE | etc } name [ ( option [, option ...] ) ]

option := FORCE | VERBOSE

We'd still keep the historical syntax where you can write FORCE outside
parens, but it'd be deprecated.

Why would we allow force inside the parens, given it's a backward compat
only thing afaik? Don't get me wrong, I'm not at all against a
extensible syntax, I just don't see a point in further cargo culting
FORCE.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andres Freund <andres@2ndquadrant.com> writes:

On 2015-02-03 10:20:03 -0500, Tom Lane wrote:

Well, the object type is not an optional part of the command. It's
*necessary*. I was thinking more like

REINDEX { INDEX | TABLE | etc } name [ ( option [, option ...] ) ]

option := FORCE | VERBOSE

We'd still keep the historical syntax where you can write FORCE outside
parens, but it'd be deprecated.

Why would we allow force inside the parens, given it's a backward compat
only thing afaik? Don't get me wrong, I'm not at all against a
extensible syntax, I just don't see a point in further cargo culting
FORCE.

Ah, I'd forgotten that that option was now a no-op. Yeah, there's no
reason to support it in the new syntax.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2/3/15 9:20 AM, Tom Lane wrote:

i.g., I will add following syntax format,
REINDEX ( { INDEX | TABLE | SCHEMA | SYSTEM | DATABASE } , [VERBOSE] )
name [FORCE];

Well, the object type is not an optional part of the command. It's
*necessary*. I was thinking more like

REINDEX { INDEX | TABLE | etc } name [ ( option [, option ...] ) ]

VACUUM puts the options before the table name, so ISTM it'd be best to
keep that with REINDEX. Either REINDEX (options) {INDEX | ...} or
REINDEX {INDEX | ...} (options).
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Jim Nasby <Jim.Nasby@bluetreble.com> writes:

On 2/3/15 9:20 AM, Tom Lane wrote:

Well, the object type is not an optional part of the command. It's
*necessary*. I was thinking more like
REINDEX { INDEX | TABLE | etc } name [ ( option [, option ...] ) ]

VACUUM puts the options before the table name, so ISTM it'd be best to
keep that with REINDEX. Either REINDEX (options) {INDEX | ...} or
REINDEX {INDEX | ...} (options).

Well, I really really don't like the first of those. IMO the command name
is "REINDEX INDEX" etc, so sticking something in the middle of that is
bogus.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Feb 3, 2015 at 8:26 PM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:

On 2/3/15 9:20 AM, Tom Lane wrote:

i.g., I will add following syntax format,
REINDEX ( { INDEX | TABLE | SCHEMA | SYSTEM | DATABASE } , [VERBOSE] )
name [FORCE];

Well, the object type is not an optional part of the command. It's
*necessary*. I was thinking more like

REINDEX { INDEX | TABLE | etc } name [ ( option [, option ...] ) ]

VACUUM puts the options before the table name, so ISTM it'd be best to

keep that with REINDEX. Either REINDEX (options) {INDEX | ...} or REINDEX
{INDEX | ...} (options).

Makes sense... +1

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL

On 2/3/15 5:08 PM, Tom Lane wrote:

Jim Nasby <Jim.Nasby@bluetreble.com> writes:

On 2/3/15 9:20 AM, Tom Lane wrote:

Well, the object type is not an optional part of the command. It's
*necessary*. I was thinking more like
REINDEX { INDEX | TABLE | etc } name [ ( option [, option ...] ) ]

VACUUM puts the options before the table name, so ISTM it'd be best to
keep that with REINDEX. Either REINDEX (options) {INDEX | ...} or
REINDEX {INDEX | ...} (options).

Well, I really really don't like the first of those. IMO the command name
is "REINDEX INDEX" etc, so sticking something in the middle of that is
bogus.

Actually, is there a reason we can't just accept all 3? Forcing people
to remember exact ordering of options has always struck me as silly.
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Jim Nasby <Jim.Nasby@BlueTreble.com> writes:

On 2/3/15 5:08 PM, Tom Lane wrote:

Jim Nasby <Jim.Nasby@bluetreble.com> writes:

VACUUM puts the options before the table name, so ISTM it'd be best to
keep that with REINDEX. Either REINDEX (options) {INDEX | ...} or
REINDEX {INDEX | ...} (options).

Well, I really really don't like the first of those. IMO the command name
is "REINDEX INDEX" etc, so sticking something in the middle of that is
bogus.

Actually, is there a reason we can't just accept all 3? Forcing people
to remember exact ordering of options has always struck me as silly.

And that's an even worse idea. Useless "flexibility" in syntax tends to
lead to unfortunate consequences like having to reserve keywords.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Feb 4, 2015 at 2:44 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Jim Nasby <Jim.Nasby@BlueTreble.com> writes:

On 2/3/15 5:08 PM, Tom Lane wrote:

Jim Nasby <Jim.Nasby@bluetreble.com> writes:

VACUUM puts the options before the table name, so ISTM it'd be best to
keep that with REINDEX. Either REINDEX (options) {INDEX | ...} or
REINDEX {INDEX | ...} (options).

Well, I really really don't like the first of those. IMO the command name
is "REINDEX INDEX" etc, so sticking something in the middle of that is
bogus.

Actually, is there a reason we can't just accept all 3? Forcing people
to remember exact ordering of options has always struck me as silly.

And that's an even worse idea. Useless "flexibility" in syntax tends to
lead to unfortunate consequences like having to reserve keywords.

As per discussion, it seems to good with
REINDEX { INDEX | TABLE | etc } name [ ( option [, option ...] ) ]
or
REINDEX { INDEX | TABLE | etc } [ (option [, optoin ...] ) ] name
i.g., the options of reindex(VERBOSE and FORCE) are put at before or
after object name.

Because other maintenance command put option at before object name, I
think the latter is better.

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

Hello,

As per discussion, it seems to good with
REINDEX { INDEX | TABLE | etc } name [ ( option [, option ...] ) ]
or
REINDEX { INDEX | TABLE | etc } [ (option [, optoin ...] ) ] name
i.g., the options of reindex(VERBOSE and FORCE) are put at before or
after object name.

Because other maintenance command put option at before object name, I
think the latter is better.

The phrase "{INDEX | TABLE |..} name" seems to me indivisible as
target specification. IMHO, the options for VACUUM and so is
placed *just after* command name, not *before* the target.

If this is right, the syntax would be like this.

REINDEX [ (option [, option ...] ) ] {INDEX | TABLE | etc } name

What do you think about this?

regares,

--
Kyotaro Horiguchi
NTT Open Source Software Center

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes:

The phrase "{INDEX | TABLE |..} name" seems to me indivisible as
target specification. IMHO, the options for VACUUM and so is
placed *just after* command name, not *before* the target.

If this is right, the syntax would be like this.

REINDEX [ (option [, option ...] ) ] {INDEX | TABLE | etc } name

What do you think about this?

I think this is wrong and ugly. INDEX etc are part of the command name.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane-2 wrote

Kyotaro HORIGUCHI &lt;

horiguchi.kyotaro@.co

&gt; writes:

The phrase "{INDEX | TABLE |..} name" seems to me indivisible as
target specification. IMHO, the options for VACUUM and so is
placed *just after* command name, not *before* the target.

If this is right, the syntax would be like this.

REINDEX [ (option [, option ...] ) ] {INDEX | TABLE | etc } name

What do you think about this?

I think this is wrong and ugly. INDEX etc are part of the command name.

I can argue either position...

I lean toward those not being part of the command name because:

The documentation lists only "REINDEX" as a command while "DROP" gets a
separate entry for each type of object it is able to drop; mainly because
the behavior of each command could/does differ based upon the target while
REINDEX will still simply cause indexes to be rebuilt and the modifier's
purposes is to aid in the selection of target indexes.

REINDEX [ ( VERBOSE ) ] { INDEX | TABLE | DATABASE | SYSTEM } name

That said, the entire notes section is written like the writer also believed
that "REINDEX INDEX" is a command in its own right...

VACUUM is a good comparison command and, besides, putting VERBOSE after the
entire thing just doesn't seem right - though that is the only other option
that would work for me. When you read other commands with pre and post
options the wording usually flows reasonably well (IF EXISTS being, for me,
an exception - it reads better after the name, not before, but I
digress...). REINDEX ( VERBOSE ) /target/ reads well to me; I'm already
sold that "TABLE name" and "INDEX name" are target specifiers.

David J.

--
View this message in context: http://postgresql.nabble.com/Proposal-REINDEX-xxx-VERBOSE-tp5836377p5836782.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Feb 4, 2015 at 8:24 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes:

The phrase "{INDEX | TABLE |..} name" seems to me indivisible as
target specification. IMHO, the options for VACUUM and so is
placed *just after* command name, not *before* the target.

If this is right, the syntax would be like this.

REINDEX [ (option [, option ...] ) ] {INDEX | TABLE | etc } name

What do you think about this?

I think this is wrong and ugly. INDEX etc are part of the command name.

I don't think so. I think they're part of the target. We have one
manual page is for REINDEX, not five separate reference pages for
REINDEX INDEX, REINDEX TABLE, REINDEX SCHEMA, REINDEX DATABASE, and
REINDEX SYSTEM. If we really wanted to, we could probably even
support this:

REINDEX INDEX foo, TABLE bar, TABLE baz;

We've got a mix of styles for extensible options right now:

EXPLAIN [ ( option [, ...] ) ] statement
COPY table_name [ ( column_name [, ...] ) ]
FROM { 'filename' | PROGRAM 'command' | STDIN }
[ [ WITH ] ( option [, ...] ) ]
VACUUM [ ( { FULL | FREEZE | VERBOSE | ANALYZE } [, ...] ) ] [
table_name [ (column_name [, ...] ) ] ]

So COPY puts the options at the very end, but EXPLAIN and VACUUM put
them right after the command name. I prefer the latter style and
would vote to adopt it here.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:

We've got a mix of styles for extensible options right now:

That we do.

So COPY puts the options at the very end, but EXPLAIN and VACUUM put
them right after the command name. I prefer the latter style and
would vote to adopt it here.

Meh. Options-at-the-end seems by far the most sensible style to me.
The options-right-after-the-keyword style is a mess, both logically
and from a parsing standpoint, and the only reason we have it at all
is historical artifact (ask Bruce about the origins of VACUUM ANALYZE
over a beer sometime).

Still, I can't help noticing that I'm being outvoted. I'll shut up now.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers