Обсуждение: Improve tab completion for REFRESH MATERIALIZED VIEW

Hi all,

Tab completion for REFRESH command is oddly with following scenario.

=# REFRESH MATERIALIZED VIEW CONCURRENTLY hoge_mv [Tab]

It shows only WITH DATA option without WITH NO DATA option.
Attached patch improves tab completion for WITH DATA/NO DATA option of
REFRESH MATERIALIZED VIEW.


Regards,

--
Masahiko Sawada

On Tue, Jan 26, 2016 at 9:52 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> Tab completion for REFRESH command is oddly with following scenario.
>
> =# REFRESH MATERIALIZED VIEW CONCURRENTLY hoge_mv [Tab]
>
> It shows only WITH DATA option without WITH NO DATA option.
> Attached patch improves tab completion for WITH DATA/NO DATA option of
> REFRESH MATERIALIZED VIEW.

Correct. Nice catch and good patch.
-- 
Michael

On Tue, Jan 26, 2016 at 10:25 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Tue, Jan 26, 2016 at 9:52 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>> Tab completion for REFRESH command is oddly with following scenario.
>>
>> =# REFRESH MATERIALIZED VIEW CONCURRENTLY hoge_mv [Tab]
>>
>> It shows only WITH DATA option without WITH NO DATA option.
>> Attached patch improves tab completion for WITH DATA/NO DATA option of
>> REFRESH MATERIALIZED VIEW.
>
> Correct. Nice catch and good patch.

The patch looks good to me.

While testing the patch, I found that
REFRESH MATERIALIZED VIEW <tab> doesn't list the materialized views.
I added the following change to the patch to fix that problem. Patch attached.

-       {"MATERIALIZED VIEW", NULL, NULL},
+       {"MATERIALIZED VIEW", NULL, &Query_for_list_of_matviews},

Regards,

--
Fujii Masao

On Tue, Jan 26, 2016 at 11:08 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
> On Tue, Jan 26, 2016 at 10:25 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> On Tue, Jan 26, 2016 at 9:52 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>>> Tab completion for REFRESH command is oddly with following scenario.
>>>
>>> =# REFRESH MATERIALIZED VIEW CONCURRENTLY hoge_mv [Tab]
>>>
>>> It shows only WITH DATA option without WITH NO DATA option.
>>> Attached patch improves tab completion for WITH DATA/NO DATA option of
>>> REFRESH MATERIALIZED VIEW.
>>
>> Correct. Nice catch and good patch.
>
> The patch looks good to me.
>
> While testing the patch, I found that
> REFRESH MATERIALIZED VIEW <tab> doesn't list the materialized views.

This is not true. Sorry for the noise...

Regards,

-- 
Fujii Masao

On Tue, Jan 26, 2016 at 8:24 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
> On Tue, Jan 26, 2016 at 11:08 PM, Fujii Masao <masao.fujii@gmail.com> wrote:

>> While testing the patch, I found that
>> REFRESH MATERIALIZED VIEW <tab> doesn't list the materialized views.
>
> This is not true. Sorry for the noise...

But CREATE MATERIALIZED VIEW <tab> doesn't list existing matviews.
I'm not clear on why it's a good idea to do so, but most kinds of
objects (including tables, views, and indexes) do so; so it seems
best to follow suit here.  I will push something shortly with the
improvements from both of you, plus a couple other MV tab
completion issues I found in testing these patches.

-- 
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Tue, Jan 26, 2016 at 8:43 AM, Kevin Grittner <kgrittn@gmail.com> wrote:

> I will push something shortly with the
> improvements from both of you, plus a couple other MV tab
> completion issues I found in testing these patches.

Done.

-- 
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Tue, Jan 26, 2016 at 11:49 PM, Kevin Grittner <kgrittn@gmail.com> wrote:
> On Tue, Jan 26, 2016 at 8:43 AM, Kevin Grittner <kgrittn@gmail.com> wrote:
>
>> I will push something shortly with the
>> improvements from both of you, plus a couple other MV tab
>> completion issues I found in testing these patches.
>
> Done.

But ISTM that CREATE MATERIALIZED VIEW <tab> doesn't list
existing matviews yet. What's worse it lists existing *views*.

This happens because words_after_create mechanism doesn't support
the case where the keyword has more than one words like "MATERIALIZED VIEW".
Probably we should improve that mechanism so that even multiple words can be
handled. Or we should just add something like the following.
      /* Complete CREATE MATERIALIZED VIEW with <name> */
+       else if (Matches3("CREATE", "MATERIALIZED", "VIEW"))
+               COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_matviews, NULL);


Regards,

-- 
Fujii Masao

On Wed, Jan 27, 2016 at 11:41 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
> But ISTM that CREATE MATERIALIZED VIEW <tab> doesn't list
> existing matviews yet. What's worse it lists existing *views*.

Yep.

> This happens because words_after_create mechanism doesn't support
> the case where the keyword has more than one words like "MATERIALIZED VIEW".

Having worked on that, I am not convinced this is worth the complication.

> Probably we should improve that mechanism so that even multiple words can be
> handled. Or we should just add something like the following.
>
>        /* Complete CREATE MATERIALIZED VIEW with <name> */
> +       else if (Matches3("CREATE", "MATERIALIZED", "VIEW"))
> +               COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_matviews, NULL);

Indeed, we do the same for tables and views as well. That would be fine.
-- 
Michael