Thread: CREATE tab completion
Hi hackers, I noticed that there are some tab completions missing for the following commands: -CREATE CONVERSION : missing FOR, TO, FROM -CREATE DOMAIN : missing after AS -CREATE LANGUAGE : missing after HANDLER -CREATE SCHEMA : missing AUTHORIZATION, IF NOT EXISTS -CREATE SEQUENCE : missing AS -CREATE TRANSFORM : missing after FOR I made a patch for this, so please have a look. Best wishes, -- Ken Kato Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
Attachment
On Wed, Nov 17, 2021 at 10:44:44AM +0900, Ken Kato wrote:
> I made a patch for this, so please have a look.
+ else if (Matches("CREATE", "TRANSFORM", "FOR", MatchAny)
+ COMPLETE_WITH("LANGUAGE")
+ else if (Matches("CREATE", "TRANSFORM", "FOR", MatchAny, "LANGUAGE")
Those three lines are wrong, for two different reasons and three
mistakes. You may want to compile your code before sending it :)
"CREATE [TEMP|TEMPORARY] SEQUENCE name AS" could be completed with the
supported types. There are three of them.
+ COMPLETE_WITH_QUERY(Query_for_list_of_schemas
+ " UNION SELECT 'AUTORIZATION'");
Incorrect completion here, s/AUTORIZATION/AUTHORIZATION/.
+ else if (Matches("CREATE", "CONVERSION", MatchAny))
+ COMPLETE_WITH("FOR");
Why didn't you consider DEFAULT for the set of changes with
conversions?
+ else if (Matches("CREATE", "SCHEMA"))
+ COMPLETE_WITH("AUTHORIZATION", "IF NOT EXISTS");
+ else if (Matches("CREATE", "SCHEMA", "IF", "NOT", "EXISTS"))
We don't do any completion for INE or IE in the other object types.
+ /* CREATE LANGUAGE */
+ else if (Matches("CREATE", "LANGUAGE", MatchAny))
+ COMPLETE_WITH("HANDLER");
+ else if (Matches("CREATE", "LANGUAGE", MatchAny, "HANDLER", MatchAny))
+ COMPLETE_WITH("INLINE", "VALIDATOR");
It looks like you forgot the case of "OR REPLACE" here? This is done
for triggers, for example.
--
Michael
Attachment
> + else if (Matches("CREATE", "TRANSFORM", "FOR", MatchAny)
> + COMPLETE_WITH("LANGUAGE")
> + else if (Matches("CREATE", "TRANSFORM", "FOR", MatchAny,
> "LANGUAGE")
>
> Those three lines are wrong, for two different reasons and three
> mistakes. You may want to compile your code before sending it :)
>
> + COMPLETE_WITH_QUERY(Query_for_list_of_schemas
> + " UNION SELECT
> 'AUTORIZATION'");
> Incorrect completion here, s/AUTORIZATION/AUTHORIZATION/.
Thank you for the comments.
I am sorry for the compile error and a typo. I will make sure to compile
before sending it and double check typos.
> "CREATE [TEMP|TEMPORARY] SEQUENCE name AS" could be completed with the
> supported types. There are three of them.
For this part, I did the following:
+ else if (TailMatches("CREATE", "SEQUENCE", MatchAny, "AS") ||
+ TailMatches("CREATE", "TEMP|TEMPORARY", "SEQUENCE", MatchAny,
"AS"))
+ COMPLETE_WITH("smallint", "integer", "bigint");
Am I doing this right? or Are there better ways to do it?
Best wishes,
--
Ken Kato
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Attachment
On Thu, Nov 18, 2021 at 04:25:30PM +0900, Ken Kato wrote:
> For this part, I did the following:
> + else if (TailMatches("CREATE", "SEQUENCE", MatchAny, "AS") ||
> + TailMatches("CREATE", "TEMP|TEMPORARY", "SEQUENCE", MatchAny, "AS"))
> + COMPLETE_WITH("smallint", "integer", "bigint");
>
> Am I doing this right? or Are there better ways to do it?
That looks fine per se.
> +/* CREATE SCHEMA */
> + else if (Matches("CREATE", "SCHEMA"))
> + COMPLETE_WITH("AUTHORIZATION");
> + else if (Matches("CREATE", "SCHEMA") && TailMatches("AUTHORIZATION"))
> + COMPLETE_WITH_QUERY(Query_for_list_of_roles);
The part about CREATE SCHEMA was itching me a bit, until I recalled
this recent thread which has a more complete logic:
https://www.postgresql.org/message-id/87im0efqhp.fsf@wibble.ilmari.org
The rest looks good, I'll take care of that in a bit.
--
Michael
Attachment
At Thu, 18 Nov 2021 17:17:25 +0900, Michael Paquier <michael@paquier.xyz> wrote in
> On Thu, Nov 18, 2021 at 04:25:30PM +0900, Ken Kato wrote:
> > For this part, I did the following:
> > + else if (TailMatches("CREATE", "SEQUENCE", MatchAny, "AS") ||
> > + TailMatches("CREATE", "TEMP|TEMPORARY", "SEQUENCE", MatchAny, "AS"))
> > + COMPLETE_WITH("smallint", "integer", "bigint");
> >
> > Am I doing this right? or Are there better ways to do it?
>
> That looks fine per se.
FWIW, I would be a bit perplexed to see type names suggested in
upper-cases, even if it is acceptable by the parser. If you type-in
the following phrase:
=# type boo<tab>
it is completed to "boolean" but,
=# type BOO<tab>
doesn't respond.
So we could use COMPLETE_WITH_CS instead so that CREATE SEQUENCE
behavess the same way with the existing behavior of TYPE.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
On Thu, Nov 18, 2021 at 05:20:58PM +0900, Kyotaro Horiguchi wrote: > So we could use COMPLETE_WITH_CS instead so that CREATE SEQUENCE > behavess the same way with the existing behavior of TYPE. Makes sense. Another issue I have noticed with the patch is that it forgets to apply quotes for the FROM/TO clauses of CREATE CONVERSION, making the queries fail. \encoding does not care about that but we do for this set of DDLs. So I have discarded this part. One extra issue was in CREATE TRANSFORM, where we should handle "OR REPLACE" like the others. And a last issue I had was with CREATE LANGUAGE where we would miss TRUSTED. This last one cannot be completed, but we'd better allow its completion if we can. That makes things a bit tense if you add on top of that the handling of "OR REPLACE". I have fixed (or discarded) that, and the parts for sequences, domains and transforms remained. That looked like good enough on its own, so applied those parts of the patch. -- Michael
Attachment
> I have fixed (or discarded) that, and the parts for sequences, domains > and transforms remained. That looked like good enough on its own, so > applied those parts of the patch. > -- > Michael Thank you very much! I assume Michael has committed the modified version of the patch. Therefore, I changed the status to"committed" in Commitfest 2022-01. https://commitfest.postgresql.org/36/3418/ Best wishes, -- Ken Kato Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION