CREATE tab completion

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

+       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

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:
/messages/by-id/87im0efqhp.fsf@wibble.ilmari.org

The rest looks good, I'll take care of that in a bit.
--
Michael

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

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

On Fri, Nov 19, 2021 at 04:19:01PM +0900, Ken Kato wrote:

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/

Thanks, I did not notice that :)
--
Michael