Re: [PATCH] Added TRANSFORM FOR for COMMENT tab completion

From: Ken Kato <katouknl(at)oss(dot)nttdata(dot)com>
To: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
Cc: Shinya Kato <Shinya11(dot)Kato(at)oss(dot)nttdata(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Suraj Khamkar <khamkarsuraj(dot)b(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] Added TRANSFORM FOR for COMMENT tab completion
Date: 2021-11-04 10:18:03
Message-ID: 950d37180f3afc6697389ba9e5f5c3f0@oss.nttdata.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

>>> +   else if (Matches("COMMENT", "ON", "PROCEDURAL"))
>>> +       COMPLETE_WITH("LANGUAGE");
>>> +   else if (Matches("COMMENT", "ON", "PROCEDURAL", "LANGUAGE"))
>>> +       COMPLETE_WITH_QUERY(Query_for_list_of_languages);
>>> I don't think that there is much point in being this picky either
>>> with
>>> the usage of PROCEDURAL, as we already complete a similar and simpler
>>> grammar with LANGUAGE.  I would just remove this part of the patch.
>> In my opinion, it is written in the documentation, so tab-completion
>> of "PROCEDURAL"is good.
>> How about a completion with "LANGUAGE" and "PROCEDURAL LANGUAGE", like
>> "PASSWORD" and "ENCRYPTED PASSWORD" in CREATE ROLE?

I kept LANGUAGE and PROCEDURAL LANGUAGE just like PASSWORD and ENCRYPTED
PASSWORD.

>>> +   else if (Matches("COMMENT", "ON", "OPERATOR"))
>>> +       COMPLETE_WITH("CLASS", "FAMILY");
>>> Isn't this one wrong?  Operators can have comments, and we'd miss
>>> them.  This is mentioned upthread, but it seems to me that we'd
>>> better
>>> drop this part of the patch if the operator naming part cannot be
>>> solved easily.
>> As you said, it may be misleading.
>> I agree to drop it.

Hearing all the opinions given, I decided not to support OPERATOR CLASS
or FAMILY in COMMENT.
Therefore, I drooped Query_for_list_of_operator_class_index_methods as
well.

> +static const SchemaQuery Query_for_list_of_text_search_configurations
> = {
>
> We already have Query_for_list_of_ts_configurations in tab-complete.c.
> Do we really need both queries? Or we can drop either of them?

Thank you for pointing out!
I didn't notice that there already exists
Query_for_list_of_ts_configurations,
so I changed TEXT SEARCH completion with using Query_for_list_of_ts_XXX.

I made the changes to the points above and updated the patch.

--
Best wishes,

Ken Kato
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

Attachment Content-Type Size
comment_tab_complete.patch text/x-diff 5.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2021-11-04 10:23:54 Re: partial heap only tuples
Previous Message Daniel Gustafsson 2021-11-04 10:10:31 Re: Make Append Cost aware of some run time partition prune case