Re: Tab completion for ALTER COLUMN SET STATISTICS

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Tab completion for ALTER COLUMN SET STATISTICS
Date: 2015-09-28 15:48:22
Message-ID: CA+TgmoZcVbLF-TpEt0zfxXkOu3n+kSQinJtggSkWjhMzH3RY4Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, Sep 26, 2015 at 7:24 AM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Sat, Sep 26, 2015 at 7:18 AM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> wrote:
>> If I have "alter table foo alter COLUMN bar SET STATISTICS" in the line
>> buffer,
>> it tab completes to add " TO", which is not legal.
>>
>> The attached patch makes it not tab complete anything at all, which is at
>> least not actively misleading.
>> I thought of having it complete "-1", "<integer>" so that it gives a clue
>> about what is needed, but I didn't see any precedence for non-literal
>> clue-giving and I did not want to try to create new precedence.
>
> +1 for the way you are doing it in your patch.

Before we take that approach, can I back up and ask whether we
shouldn't instead narrow the rule that's inserting TO? I think that
completion is coming from here:

else if (pg_strcasecmp(prev2_wd, "SET") == 0 &&
pg_strcasecmp(prev4_wd, "UPDATE") != 0 &&
pg_strcasecmp(prev_wd, "TABLESPACE") != 0 &&
pg_strcasecmp(prev_wd, "SCHEMA") != 0 &&
prev_wd[strlen(prev_wd) - 1] != ')' &&
prev_wd[strlen(prev_wd) - 1] != '=' &&
pg_strcasecmp(prev4_wd, "DOMAIN") != 0)
COMPLETE_WITH_CONST("TO");

Now, that is basically an incredibly broad production: every time the
second-most recent word is SET, complete with TO. It looks to me like
this has already been patched around multiple times to make it
slightly narrower. Those exceptions were added by three different
commits, in 2004, 2010, and 2012.

Maybe it's time to back up and make the whole thing a lot narrower.
Like, if second-most-recent word of the command is also the FIRST word
of the command, this is probably right. And there may be a few other
situations where it's right. But in general, SET is used in lots of
places and the fact that we've seen one recently isn't enough to
decide in TO.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2015-09-28 16:04:44 Re: Doubt in pgbench TPS number
Previous Message Fabien COELHO 2015-09-28 15:43:17 Re: pgbench stats per script & other stuff