Horton

Thread

Commits

  1. Push lpp variable closer to usage in heapgetpage()

  2. psql: Improve tab-completion for MERGE.

  1. [PATCH]Feature improvement for MERGE tab completion

    bt22kawamotok <bt22kawamotok@oss.nttdata.com> — 2022-09-09T02:18:13Z

    Hi!

I created a patch for improving MARGE tab completion.
Currently there is a problem with "MERGE INTO dst as d Using src as s ON 
d.key = s.key WHEN <tab>" is typed, "MATCHED" and "NOT MATCHED" is not 
completed.
There is also a problem that typing "MERGE INTO a AS <tab>" completes 
"USING".
This patch solves the above problems.

Regards,
Kotaro Kawamoto

  2. Re: [PATCH]Feature improvement for MERGE tab completion

    Shinya Kato <shinya11.kato@oss.nttdata.com> — 2022-09-09T11:55:43Z

    On 2022-09-09 11:18, bt22kawamotok wrote:

> I created a patch for improving MARGE tab completion.
> Currently there is a problem with "MERGE INTO dst as d Using src as s
> ON d.key = s.key WHEN <tab>" is typed, "MATCHED" and "NOT MATCHED" is
> not completed.
> There is also a problem that typing "MERGE INTO a AS <tab>" completes 
> "USING".
> This patch solves the above problems.

Thanks for the patch!

	else if (TailMatches("USING", MatchAny, "ON", MatchAny, "WHEN"))
		COMPLETE_WITH("MATCHED", "NOT MATCHED");
	else if (TailMatches("USING", MatchAny, "AS", MatchAny, "ON", MatchAny, 
"WHEN"))
		COMPLETE_WITH("MATCHED", "NOT MATCHED");
	else if (TailMatches("USING", MatchAny, MatchAny, "ON", MatchAny, 
"WHEN"))
		COMPLETE_WITH("MATCHED", "NOT MATCHED");

I thought it would be better to describe this section as follows, 
summarizing the conditions

	else if (TailMatches("USING", MatchAny, "ON", MatchAny, "WHEN") ||
			 TailMatches("USING", MatchAny, "AS", MatchAny, "ON", MatchAny, 
"WHEN") ||
			 TailMatches("USING", MatchAny, MatchAny, "ON", MatchAny, "WHEN"))
		COMPLETE_WITH("MATCHED", "NOT MATCHED");

There are similar redundancies in the tab completion of MERGE statement, 
so why not fix that as well?

-- 
Regards,

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

  3. Re: [PATCH]Feature improvement for MERGE tab completion

    bt22kawamotok <bt22kawamotok@oss.nttdata.com> — 2022-09-12T06:53:46Z

    > 	else if (TailMatches("USING", MatchAny, "ON", MatchAny, "WHEN"))
> 		COMPLETE_WITH("MATCHED", "NOT MATCHED");
> 	else if (TailMatches("USING", MatchAny, "AS", MatchAny, "ON",
> MatchAny, "WHEN"))
> 		COMPLETE_WITH("MATCHED", "NOT MATCHED");
> 	else if (TailMatches("USING", MatchAny, MatchAny, "ON", MatchAny, 
> "WHEN"))
> 		COMPLETE_WITH("MATCHED", "NOT MATCHED");
> 
> I thought it would be better to describe this section as follows,
> summarizing the conditions
> 
> 	else if (TailMatches("USING", MatchAny, "ON", MatchAny, "WHEN") ||
> 			 TailMatches("USING", MatchAny, "AS", MatchAny, "ON", MatchAny, 
> "WHEN") ||
> 			 TailMatches("USING", MatchAny, MatchAny, "ON", MatchAny, "WHEN"))
> 		COMPLETE_WITH("MATCHED", "NOT MATCHED");
> 
> There are similar redundancies in the tab completion of MERGE
> statement, so why not fix that as well?

Thanks for your review.

A new patch has been created to reflect the changes you indicated.
I also found a problem that "DO NOTHING" is not completed when type 
"WHEN MATCHED <tab>", so I fixed it as well.

Regards,

Kotaro Kawamoto

  4. Re: [PATCH]Feature improvement for MERGE tab completion

    Shinya Kato <shinya11.kato@oss.nttdata.com> — 2022-09-12T08:03:25Z

    On 2022-09-12 15:53, bt22kawamotok wrote:
>> 	else if (TailMatches("USING", MatchAny, "ON", MatchAny, "WHEN"))
>> 		COMPLETE_WITH("MATCHED", "NOT MATCHED");
>> 	else if (TailMatches("USING", MatchAny, "AS", MatchAny, "ON",
>> MatchAny, "WHEN"))
>> 		COMPLETE_WITH("MATCHED", "NOT MATCHED");
>> 	else if (TailMatches("USING", MatchAny, MatchAny, "ON", MatchAny, 
>> "WHEN"))
>> 		COMPLETE_WITH("MATCHED", "NOT MATCHED");
>> 
>> I thought it would be better to describe this section as follows,
>> summarizing the conditions
>> 
>> 	else if (TailMatches("USING", MatchAny, "ON", MatchAny, "WHEN") ||
>> 			 TailMatches("USING", MatchAny, "AS", MatchAny, "ON", MatchAny, 
>> "WHEN") ||
>> 			 TailMatches("USING", MatchAny, MatchAny, "ON", MatchAny, "WHEN"))
>> 		COMPLETE_WITH("MATCHED", "NOT MATCHED");
>> 
>> There are similar redundancies in the tab completion of MERGE
>> statement, so why not fix that as well?
> 
> Thanks for your review.
> 
> A new patch has been created to reflect the changes you indicated.

Thanks for updating!

Compile errors have occurred, so can you fix them?
And I think we can eliminate similar redundancies in MERGE tab 
completion and I would like you to fix them.

For example,

	else if (TailMatches("WHEN", "MATCHED"))
		COMPLETE_WITH("THEN", "AND");
	else if (TailMatches("WHEN", "NOT", "MATCHED"))
		COMPLETE_WITH("THEN", "AND");

above statement can be converted to the statement below.

	else if (TailMatches("WHEN", "MATCHED") ||
			 TailMatches("WHEN", "NOT", "MATCHED"))
		COMPLETE_WITH("THEN", "AND");


-- 
Regards,

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

  5. Re: [PATCH]Feature improvement for MERGE tab completion

    bt22kawamotok <bt22kawamotok@oss.nttdata.com> — 2022-09-12T08:25:44Z

    > Thanks for updating!
> 
> Compile errors have occurred, so can you fix them?
> And I think we can eliminate similar redundancies in MERGE tab
> completion and I would like you to fix them.
> 
> For example,
> 
> 	else if (TailMatches("WHEN", "MATCHED"))
> 		COMPLETE_WITH("THEN", "AND");
> 	else if (TailMatches("WHEN", "NOT", "MATCHED"))
> 		COMPLETE_WITH("THEN", "AND");
> 
> above statement can be converted to the statement below.
> 
> 	else if (TailMatches("WHEN", "MATCHED") ||
> 			 TailMatches("WHEN", "NOT", "MATCHED"))
> 		COMPLETE_WITH("THEN", "AND");

Sorry for making such a silly mistake.
I create new path.
Please reviewing.

Kotaro Kawamoto

  6. Re: [PATCH]Feature improvement for MERGE tab completion

    bt22kawamotok <bt22kawamotok@oss.nttdata.com> — 2022-09-12T09:20:30Z

    Other than this correction, the parts that can be put together in OR 
were corrected in fix_tab_completion_merge_v4.patch.

Kotaro Kawamoto

  7. Re: [PATCH]Feature improvement for MERGE tab completion

    Shinya Kato <shinya11.kato@oss.nttdata.com> — 2022-09-13T04:56:44Z

    On 2022-09-12 18:20, bt22kawamotok wrote:
> Other than this correction, the parts that can be put together in OR
> were corrected in fix_tab_completion_merge_v4.patch.

When I tried to apply this patch, I got the following warning, please 
fix it.
Other than that, I think everything is fine.

$ git apply fix_tab_completion_merge_v4.patch
fix_tab_completion_merge_v4.patch:38: trailing whitespace.
         else if (TailMatches("USING", MatchAny, "ON", MatchAny) ||
fix_tab_completion_merge_v4.patch:39: indent with spaces.
                  TailMatches("USING", MatchAny, "AS", MatchAny, "ON", 
MatchAny) ||
fix_tab_completion_merge_v4.patch:40: indent with spaces.
                  TailMatches("USING", MatchAny, MatchAny, "ON", 
MatchAny))
fix_tab_completion_merge_v4.patch:53: trailing whitespace.
         else if (TailMatches("WHEN", "MATCHED") ||
warning: 4 lines add whitespace errors.

-- 
Regards,

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

  8. Re: [PATCH]Feature improvement for MERGE tab completion

    bt22kawamotok <bt22kawamotok@oss.nttdata.com> — 2022-09-14T05:08:07Z

    > When I tried to apply this patch, I got the following warning, please 
> fix it.
> Other than that, I think everything is fine.
> 
> $ git apply fix_tab_completion_merge_v4.patch
> fix_tab_completion_merge_v4.patch:38: trailing whitespace.
>         else if (TailMatches("USING", MatchAny, "ON", MatchAny) ||
> fix_tab_completion_merge_v4.patch:39: indent with spaces.
>                  TailMatches("USING", MatchAny, "AS", MatchAny, "ON",
> MatchAny) ||
> fix_tab_completion_merge_v4.patch:40: indent with spaces.
>                  TailMatches("USING", MatchAny, MatchAny, "ON", 
> MatchAny))
> fix_tab_completion_merge_v4.patch:53: trailing whitespace.
>         else if (TailMatches("WHEN", "MATCHED") ||
> warning: 4 lines add whitespace errors.

Thanks for reviewing.

I fixed the problem and make patch v5.
Please check it.

Regards,

Kotaro Kawamoto

  9. Re: [PATCH]Feature improvement for MERGE tab completion

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2022-09-14T08:15:02Z

    
On 2022/09/14 14:08, bt22kawamotok wrote:
>> When I tried to apply this patch, I got the following warning, please fix it.
>> Other than that, I think everything is fine.
>>
>> $ git apply fix_tab_completion_merge_v4.patch
>> fix_tab_completion_merge_v4.patch:38: trailing whitespace.
>>         else if (TailMatches("USING", MatchAny, "ON", MatchAny) ||
>> fix_tab_completion_merge_v4.patch:39: indent with spaces.
>>                  TailMatches("USING", MatchAny, "AS", MatchAny, "ON",
>> MatchAny) ||
>> fix_tab_completion_merge_v4.patch:40: indent with spaces.
>>                  TailMatches("USING", MatchAny, MatchAny, "ON", MatchAny))
>> fix_tab_completion_merge_v4.patch:53: trailing whitespace.
>>         else if (TailMatches("WHEN", "MATCHED") ||
>> warning: 4 lines add whitespace errors.
> 
> Thanks for reviewing.
> 
> I fixed the problem and make patch v5.
> Please check it.

Thanks for updating the patch!

+	else if (TailMatches("MERGE", "INTO", MatchAny, "USING") ||
+			 TailMatches("MERGE", "INTO", MatchAny, MatchAny, "USING") ||
+			 TailMatches("MERGE", "INTO", MatchAny, "AS", MatchAny, "USING"))
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables);

+	else if (TailMatches("MERGE", "INTO", MatchAny, "AS", MatchAny, "USING") ||
+			 TailMatches("MERGE", "INTO", MatchAny, MatchAny, "USING"))
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables);

The latter seems redundant and can be removed. The former seems to
cover all the cases where the latter covers.


Not only table but also view, foreign table, etc can be specified after
USING in MERGE command. So ISTM that Query_for_list_of_selectables
should be used at the above tab-completion, instead of Query_for_list_of_tables.
Thought?


+	else if (TailMatches("USING", MatchAny, "ON", MatchAny) ||
+			 TailMatches("USING", MatchAny, "ON", MatchAny, MatchAnyExcept("When"), MatchAnyExcept("When")) ||
+			 TailMatches("USING", MatchAny, "AS", MatchAny, "ON", MatchAny) ||
+			 TailMatches("USING", MatchAny, "AS", MatchAny, "ON", MatchAny, MatchAnyExcept("When"), MatchAnyExcept("When")) ||
+			 TailMatches("USING", MatchAny, MatchAny, "ON", MatchAny) ||
+			 TailMatches("USING", MatchAny, MatchAny, "ON", MatchAny, MatchAnyExcept("When"), MatchAnyExcept("When")))

"When" should be "WHEN"?


Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

  10. Re: [PATCH]Feature improvement for MERGE tab completion

    bt22kawamotok <bt22kawamotok@oss.nttdata.com> — 2022-09-14T09:12:52Z

    > +	else if (TailMatches("MERGE", "INTO", MatchAny, "USING") ||
> +			 TailMatches("MERGE", "INTO", MatchAny, MatchAny, "USING") ||
> +			 TailMatches("MERGE", "INTO", MatchAny, "AS", MatchAny, "USING"))
>  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables);
> 
> +	else if (TailMatches("MERGE", "INTO", MatchAny, "AS", MatchAny, 
> "USING") ||
> +			 TailMatches("MERGE", "INTO", MatchAny, MatchAny, "USING"))
>  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables);
> 
> The latter seems redundant and can be removed. The former seems to
> cover all the cases where the latter covers.

> +	else if (TailMatches("USING", MatchAny, "ON", MatchAny) ||
> +			 TailMatches("USING", MatchAny, "ON", MatchAny,
> MatchAnyExcept("When"), MatchAnyExcept("When")) ||
> +			 TailMatches("USING", MatchAny, "AS", MatchAny, "ON", MatchAny) ||
> +			 TailMatches("USING", MatchAny, "AS", MatchAny, "ON", MatchAny,
> MatchAnyExcept("When"), MatchAnyExcept("When")) ||
> +			 TailMatches("USING", MatchAny, MatchAny, "ON", MatchAny) ||
> +			 TailMatches("USING", MatchAny, MatchAny, "ON", MatchAny,
> MatchAnyExcept("When"), MatchAnyExcept("When")))
> 
> "When" should be "WHEN"?
> 
> 
> Regards,

Thanks for reviewing.

Sorry for making such a simple mistake.
I fixed it in v6.

> Not only table but also view, foreign table, etc can be specified after
> USING in MERGE command. So ISTM that Query_for_list_of_selectables
> should be used at the above tab-completion, instead of 
> Query_for_list_of_tables.
> Thought?

That's nice idea!
I took that in v6.

Regards,

Kotaro Kawamoto

  11. Re: [PATCH]Feature improvement for MERGE tab completion

    Shinya Kato <shinya11.kato@oss.nttdata.com> — 2022-09-15T00:57:55Z

    On 2022-09-14 18:12, bt22kawamotok wrote:

> I fixed it in v6.

Thanks for updating.

+		COMPLETE_WITH("UPDATE", "DELETE", "DO NOTHING");

"UPDATE" is always followed by "SET",  so why not complement it with 
"UPDATE SET"?

-- 
Regards,

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

  12. Re: [PATCH]Feature improvement for MERGE tab completion

    bt22kawamotok <bt22kawamotok@oss.nttdata.com> — 2022-09-16T02:46:14Z

    > Thanks for updating.
> 
> +		COMPLETE_WITH("UPDATE", "DELETE", "DO NOTHING");
> 
> "UPDATE" is always followed by "SET",  so why not complement it with
> "UPDATE SET"?

Thanks for reviewing.
That's a good idea!
I create new patch v7.

Regards,

Kotaro Kawamoto

  13. Re: [PATCH]Feature improvement for MERGE tab completion

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2022-09-18T05:29:30Z

    
On 2022/09/16 11:46, bt22kawamotok wrote:
>> Thanks for updating.
>>
>> +        COMPLETE_WITH("UPDATE", "DELETE", "DO NOTHING");
>>
>> "UPDATE" is always followed by "SET",  so why not complement it with
>> "UPDATE SET"?
> 
> Thanks for reviewing.
> That's a good idea!
> I create new patch v7.

Thanks for updating the patch!

I applied the following changes to the patch. Attached is the updated version of the patch.

The tab-completion code for MERGE was added in the middle of that for LOCK TABLE.
This would be an oversight of the commit that originally supported tab-completion
for MERGE. I fixed this issue.

+	else if (TailMatches("MERGE", "INTO", MatchAny, "AS", MatchAny) ||
+			 TailMatches("MERGE", "INTO", MatchAny, MatchAnyExcept("AS")))
  		COMPLETE_WITH("USING");

This can cause to complete "MERGE INTO <table> USING" with "USING" unexpectedly.
I fixed this issue by replacing MatchAnyExcept("AS") with MatchAnyExcept("USING|AS").

I added some comments.

"MERGE" was tab-completed with just after "EXPLAIN" or "EXPLAIN ANALYZE", etc.
Since "INTO" always follows "MERGE", it's better to complete with "MERGE INTO"
there. I replaced "MERGE" with "MERGE INTO" in those tab-completions.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

  14. Re: [PATCH]Feature improvement for MERGE tab completion

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-09-20T15:51:05Z

    On 2022-Sep-18, Fujii Masao wrote:

> The tab-completion code for MERGE was added in the middle of that for LOCK TABLE.
> This would be an oversight of the commit that originally supported tab-completion
> for MERGE. I fixed this issue.

Argh, thanks.

> "MERGE" was tab-completed with just after "EXPLAIN" or "EXPLAIN ANALYZE", etc.
> Since "INTO" always follows "MERGE", it's better to complete with "MERGE INTO"
> there. I replaced "MERGE" with "MERGE INTO" in those tab-completions.

OK, that would be similar to REFRESH MATERIALIZED VIEW.

The rules starting at line 4111 make me a bit nervous, since nowhere
we're restricting them to operating only on MERGE lines.  I don't think
it's a real problem since USING is not terribly common anyway.  Likewise
for the ones with WHEN [NOT] MATCHED.  I kinda wish we had a way to
search for stuff like "keyword MERGE appears earlier in the command",
but we don't have that.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"I'm always right, but sometimes I'm more right than other times."
                                                  (Linus Torvalds)

  15. Re: [PATCH]Feature improvement for MERGE tab completion

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2022-09-21T05:25:08Z

    
On 2022/09/21 0:51, Alvaro Herrera wrote:
> The rules starting at line 4111 make me a bit nervous, since nowhere
> we're restricting them to operating only on MERGE lines.  I don't think
> it's a real problem since USING is not terribly common anyway.  Likewise
> for the ones with WHEN [NOT] MATCHED.  I kinda wish we had a way to
> search for stuff like "keyword MERGE appears earlier in the command",
> but we don't have that.

Yeah, I was thinking the same when updating the patch.

How about adding something like PartialMatches() that checks whether
the keywords are included in the input string or not? If so, we can restrict
some tab-completion rules to operating only on MERGE, as follows. I attached
the WIP patch (0002 patch) that introduces PartialMatches().
Is this approach over-complicated? Thought?

+	else if (PartialMatches("MERGE", "INTO", MatchAny, "USING") ||
+			 PartialMatches("MERGE", "INTO", MatchAny, "AS", MatchAny, "USING") ||
+			 PartialMatches("MERGE", "INTO", MatchAny, MatchAny, "USING"))
+	{
+		/* Complete MERGE INTO ... ON with target table attributes */
+		if (TailMatches("INTO", MatchAny, "USING", MatchAny, "ON"))
+			COMPLETE_WITH_ATTR(prev4_wd);
+		else if (TailMatches("INTO", MatchAny, "AS", MatchAny, "USING", MatchAny, "AS", MatchAny, "ON"))
+			COMPLETE_WITH_ATTR(prev8_wd);
+		else if (TailMatches("INTO", MatchAny, MatchAny, "USING", MatchAny, MatchAny, "ON"))
+			COMPLETE_WITH_ATTR(prev6_wd);


Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

  16. Re: [PATCH]Feature improvement for MERGE tab completion

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-09-21T10:40:00Z

    On 2022-Sep-21, Fujii Masao wrote:

> How about adding something like PartialMatches() that checks whether
> the keywords are included in the input string or not? If so, we can restrict
> some tab-completion rules to operating only on MERGE, as follows. I attached
> the WIP patch (0002 patch) that introduces PartialMatches().
> Is this approach over-complicated? Thought?

I think it's fine to backpatch your 0001 to 15 and put 0002 in master.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"I can't go to a restaurant and order food because I keep looking at the
fonts on the menu.  Five minutes later I realize that it's also talking
about food" (Donald Knuth)

  17. Re: [PATCH]Feature improvement for MERGE tab completion

    vignesh C <vignesh21@gmail.com> — 2023-01-03T12:30:35Z

    On Wed, 21 Sept 2022 at 10:55, Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>
>
>
> On 2022/09/21 0:51, Alvaro Herrera wrote:
> > The rules starting at line 4111 make me a bit nervous, since nowhere
> > we're restricting them to operating only on MERGE lines.  I don't think
> > it's a real problem since USING is not terribly common anyway.  Likewise
> > for the ones with WHEN [NOT] MATCHED.  I kinda wish we had a way to
> > search for stuff like "keyword MERGE appears earlier in the command",
> > but we don't have that.
>
> Yeah, I was thinking the same when updating the patch.
>
> How about adding something like PartialMatches() that checks whether
> the keywords are included in the input string or not? If so, we can restrict
> some tab-completion rules to operating only on MERGE, as follows. I attached
> the WIP patch (0002 patch) that introduces PartialMatches().
> Is this approach over-complicated? Thought?
>
> +       else if (PartialMatches("MERGE", "INTO", MatchAny, "USING") ||
> +                        PartialMatches("MERGE", "INTO", MatchAny, "AS", MatchAny, "USING") ||
> +                        PartialMatches("MERGE", "INTO", MatchAny, MatchAny, "USING"))
> +       {
> +               /* Complete MERGE INTO ... ON with target table attributes */
> +               if (TailMatches("INTO", MatchAny, "USING", MatchAny, "ON"))
> +                       COMPLETE_WITH_ATTR(prev4_wd);
> +               else if (TailMatches("INTO", MatchAny, "AS", MatchAny, "USING", MatchAny, "AS", MatchAny, "ON"))
> +                       COMPLETE_WITH_ATTR(prev8_wd);
> +               else if (TailMatches("INTO", MatchAny, MatchAny, "USING", MatchAny, MatchAny, "ON"))
> +                       COMPLETE_WITH_ATTR(prev6_wd);

The patch does not apply on top of HEAD as in [1], please post a rebased patch:
=== Applying patches on top of PostgreSQL commit ID
e351f85418313e97c203c73181757a007dfda6d0 ===
=== applying patch ./v9-0001-psql-Improve-tab-completion-for-MERGE.patch
patching file src/bin/psql/tab-complete.c
Hunk #1 FAILED at 1669.
Hunk #2 FAILED at 3641.
Hunk #3 FAILED at 3660.
Hunk #4 FAILED at 4065.
4 out of 4 hunks FAILED -- saving rejects to file
src/bin/psql/tab-complete.c.rej

[1] - http://cfbot.cputube.org/patch_41_3890.log

Regards,
Vignesh

  18. Re: [PATCH]Feature improvement for MERGE tab completion

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2023-01-10T12:24:18Z

    On Tue, 3 Jan 2023 at 12:30, vignesh C <vignesh21@gmail.com> wrote:
>
> The patch does not apply on top of HEAD as in [1], please post a rebased patch:
>

This is because 0001 has been committed.
Re-uploading 0002, to keep the CF-bot happy.

Reviewing 0002...

I'm not entirely convinced that the PartialMatches() changes are
really necessary. As far as I can see USING followed by ON doesn't
appear anywhere else in the grammar, and the later completions
involving WHEN [NOT] MATCHED are definitely unique to MERGE.
Nonetheless, I guess it's not a bad thing to check that it really is a
MERGE. Also the new matching function might prove useful for other
cases.

Some more detailed code comments:

I find the name PartialMatches() a little off, since "partial" doesn't
really accurately describe what it's doing. HeadMatches() and
TailMatches() are also partial matches (matching the head and tail
parts). So perhaps MidMatches() would be a better name.

I also found the comment description of PartialMatchesImpl() misleading:

/*
 * Implementation of PartialMatches and PartialMatchesCS macros: do parts of
 * the words in previous_words match the variadic arguments?
 */

I think a more accurate description would be:

/*
 * Implementation of MidMatches and MidMatchesCS macros: do any N consecutive
 * words in previous_words match the variadic arguments?
 */

Similarly, instead of:

    /* Match N words on the line partially, case-insensitively. */

how about:

    /* Match N consecutive words anywhere on the line, case-insensitively. */

In PartialMatchesImpl()'s main loop:

        if (previous_words_count - startpos < narg)
        {
            va_end(args);
            return false;
        }

couldn't that just be built into the loop's termination clause (i.e.,
loop while startpos <= previous_words_count - narg)?

For the first block of changes using the new function:

    else if (PartialMatches("MERGE", "INTO", MatchAny, "USING") ||
             PartialMatches("MERGE", "INTO", MatchAny, "AS", MatchAny,
"USING") ||
             PartialMatches("MERGE", "INTO", MatchAny, MatchAny, "USING"))
    {
        /* Complete MERGE INTO ... ON with target table attributes */
        if (TailMatches("INTO", MatchAny, "USING", MatchAny, "ON"))
            COMPLETE_WITH_ATTR(prev4_wd);
        else if (TailMatches("INTO", MatchAny, "AS", MatchAny,
"USING", MatchAny, "AS", MatchAny, "ON"))
            COMPLETE_WITH_ATTR(prev8_wd);
        else if (TailMatches("INTO", MatchAny, MatchAny, "USING",
MatchAny, MatchAny, "ON"))
            COMPLETE_WITH_ATTR(prev6_wd);

wouldn't it be simpler to just include "MERGE" in the TailMatches()
arguments, and leave these 3 cases outside the new code block. I.e.:

    /* Complete MERGE INTO ... ON with target table attributes */
    else if (TailMatches("MERGE", "INTO", MatchAny, "USING", MatchAny, "ON"))
        COMPLETE_WITH_ATTR(prev4_wd);
    else if (TailMatches("MERGE", "INTO", MatchAny, "AS", MatchAny,
"USING", MatchAny, "AS", MatchAny, "ON"))
        COMPLETE_WITH_ATTR(prev8_wd);
    else if (TailMatches("MERGE", "INTO", MatchAny, MatchAny, "USING",
MatchAny, MatchAny, "ON"))
        COMPLETE_WITH_ATTR(prev6_wd);

Regards,
Dean

  19. Re: [PATCH]Feature improvement for MERGE tab completion

    Gregory Stark (as CFM) <stark.cfm@gmail.com> — 2023-03-28T18:55:54Z

    It looks like this remaining work isn't going to happen this CF and
therefore this release. There hasn't been an update since January when
Dean Rasheed posted a review.

Unless there's any updates soon I'll move this on to the next
commitfest or mark it returned with feedback.

-- 
Gregory Stark
As Commitfest Manager

  20. Re: [PATCH]Feature improvement for MERGE tab completion

    Gregory Stark (as CFM) <stark.cfm@gmail.com> — 2023-03-28T18:58:25Z

    Ah, another thread with a bouncing email address...
Please respond to to thread from this point to avoid bounces.



-- 
Gregory Stark
As Commitfest Manager

  21. Re: [PATCH]Feature improvement for MERGE tab completion

    Daniel Gustafsson <daniel@yesql.se> — 2023-07-04T12:34:28Z

    > On 28 Mar 2023, at 20:55, Gregory Stark (as CFM) <stark.cfm@gmail.com> wrote:
> 
> It looks like this remaining work isn't going to happen this CF and
> therefore this release. There hasn't been an update since January when
> Dean Rasheed posted a review.
> 
> Unless there's any updates soon I'll move this on to the next
> commitfest or mark it returned with feedback.

There are still no updates to this patch or thread, so I'm closing this as
Returned with Feedback.  Please feel free to resubmit to a future CF when there
is renewed interest in working on this.

--
Daniel Gustafsson