Обсуждение: [Patch]Add tab completion for DELETE ... USING

> On Dec 25, 2025, at 23:51, Tatsuya Kawata <kawatatatsuya0913@gmail.com> wrote:
>
> Hi,
>
> I noticed that tab completion for DELETE ... USING clause was not implemented. There was a TODO comment in
tab-complete.in.c:
>
> /* XXX: implement tab completion for DELETE ... USING */
> The attached patch implements this feature. The following completions are now supported:
>
> DELETE FROM <table> USING <TAB> -> suggests a list of tables
> DELETE FROM <table> USING <table> <TAB> -> suggests AS, WHERE
> DELETE FROM <table> USING <table> AS <alias> <TAB> -> suggests WHERE
> DELETE FROM <table> USING <table> <alias> <TAB> -> suggests WHERE (without AS keyword)
>
> I've also added tests for all these cases in 010_tab_completion.pl.
>
> Regards,
> Tatsuya Kawata
> <v1-0001-Add-tab-completion-for-DELETE-USING.patch>

Overall looks good to me. A very tiny nitpick:

Firs the first case:
```
+# check DELETE ... USING completion
+check_completion(
+    "DELETE FROM tab1 USING my\t",
+    qr/mytab/,
+    "complete DELETE FROM tab USING with table name");
```

Maybe just say “with tab”, because the rest test cases’ messages all say “USING tab”.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On Sat, 27 Dec 2025 at 05:50, Tatsuya Kawata
<kawatatatsuya0913@gmail.com> wrote:
>
> Hi Chao-san,
>
> Thank you very much for your review!
>
> > Firs the first case:
> > ```
> > +# check DELETE ... USING completion
> > +check_completion(
> > +       "DELETE FROM tab1 USING my\t",
> > +       qr/mytab/,
> > +       "complete DELETE FROM tab USING with table name");
> > ```
> >
> > Maybe just say “with tab”, because the rest test cases’ messages all say “USING tab”.
> I fixed it as you mentioned.
> The updated v2 is attached.
>
> Regards,
> Tatsuya Kawata


HI! What makes you think we need additional tab completion tests at all?
If we consult src/bin/psql changes history [0], it can be easily seen
that not every change to tab-complete.in.c is made with a new test.
This is (my own understanding, I may be wrong) because we only need to
test tab-complete patterns that are functionally different, not mirror
all cases from tab-complete.in.c  in the TAP test.

WDYT?

[0] https://github.com/postgres/postgres/commits/master/src/bin/psql

--
Best regards,
Kirill Reshke

On Sun, 11 Jan 2026 at 12:35, Tatsuya Kawata
<kawatatatsuya0913@gmail.com> wrote:
>
> Hi!
>
> The updated v3 patch is attached as above.
> Just reduced test cases.
>
> Regards,
> Tatsuya Kawata
>

Hi!
Sorry for the long response, no more complaints from my side.
I have changed CF entry status to Ready FC

-- 
Best regards,
Kirill Reshke

Hi all,

Thank you for the updated patch.

On Mon, Feb 16, 2026 at 10:06 AM Tatsuya Kawata
<kawatatatsuya0913@gmail.com> wrote:
>
> Hi Kirill-san,
>
> Thank you for your review and for updating the CF entry status!
>

I have gone through the discussions and found this as a relevant
patch. I manually tested the patch for Table completion, Keyword
completion after table, Alias case and Partial prefix completion and
the patch is working completely fine. And also make check passed
without regressions.
According to the patch, the implementation looks clean and consistent
with existing psql token-based completion patterns and LGTM.


Regards,
Soumya

On Mon, Feb 16, 2026 at 7:44 PM Soumya S Murali
<soumyamurali.work@gmail.com> wrote:
>
> Hi all,
>
> Thank you for the updated patch.
>
> On Mon, Feb 16, 2026 at 10:06 AM Tatsuya Kawata
> <kawatatatsuya0913@gmail.com> wrote:
> >
> > Hi Kirill-san,
> >
> > Thank you for your review and for updating the CF entry status!

Since this patch is marked ready-for-committer, I've started reviewing it.

+ /* Complete DELETE FROM <table> USING with a list of tables */
+ else if (TailMatches("DELETE", "FROM", MatchAny, "USING"))
+ COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables);

Should this use Query_for_list_of_selectables instead of
Query_for_list_of_tables? The USING clause can reference not only tables
but also views and other selectable objects. The documentation also says
that USING follows the same syntax as the FROM clause of a SELECT.

+ /* Complete DELETE FROM <table> USING <table> */
+ else if (TailMatches("DELETE", "FROM", MatchAny, "USING", MatchAny))
+ COMPLETE_WITH("AS", "WHERE");

Since multiple objects can follow USING, showing only AS and WHERE immediately
after USING <table> feels a bit odd. Also, if we want to support this
completion here, shouldn't RETURNING also be included?

That said, since tab-completion for SELECT ... FROM ... does not suggest
anything at that point, I would also be fine with not suggesting anything for
DELETE ... USING ....

Regards,

--
Fujii Masao

On Sat, Feb 28, 2026 at 1:38 PM Tatsuya Kawata
<kawatatatsuya0913@gmail.com> wrote:
>
> Hi Fujii-san, Soumya-san,
>
> Thank you very much for reviewing and testing my patch!
>
> > Should this use Query_for_list_of_selectables instead of
> > Query_for_list_of_tables? The USING clause can reference not only tables
> > but also views and other selectable objects. The documentation also says
> > that USING follows the same syntax as the FROM clause of a SELECT.
>
> You're absolutely right. Since the USING clause follows the same syntax as the FROM clause of a SELECT statement,
> it should support views and other selectable objects, not just tables.
> I've changed it to use Query_for_list_of_selectables.
>
> > Since multiple objects can follow USING, showing only AS and WHERE immediately
> > after USING <table> feels a bit odd. Also, if we want to support this
> > completion here, shouldn't RETURNING also be included?
> > That said, since tab-completion for SELECT ... FROM ... does not suggest
> > anything at that point, I would also be fine with not suggesting anything for
> > DELETE ... USING ....
>
> I agree with your second point. For consistency with SELECT ... FROM ... completion behavior,
> I think it's better not to suggest anything after USING <table>. This approach is cleaner
> and more consistent with existing patterns.
>
> I've prepared a v4 patch with the following changes:
>  - Use Query_for_list_of_selectables instead of Query_for_list_of_tables
>  - Remove the completion for USING <table> to maintain consistency with SELECT behavior
>
> The updated v4 patch is attached.

Thanks for updating the patch!

+ /* Complete DELETE FROM <table> USING with a list of selectables */

Would it be better to use "relations supporting SELECT" instead of
"selectables"? That wording is used in other tab-completion comments that
reference Query_for_list_of_selectables, so it might be better to stay
consistent.

As for the regression test, I agree with Kirill. I don't think it's necessary
to add a separate test for this specific tab-completion case. The existing
tests already cover tab-completion patterns based on
Query_for_list_of_selectables following specific keywords, which should be
sufficient.

Regards,

--
Fujii Masao

On Sun, Mar 1, 2026 at 3:07 PM Tatsuya Kawata
<kawatatatsuya0913@gmail.com> wrote:
>
> Hi Fujii-san,
>
> Thank you for your review!
>
> > Would it be better to use "relations supporting SELECT" instead of "selectables"?
>
> Good point. I've updated the comment to use "relations supporting SELECT" for consistency.
>
> > As for the regression test, I agree with Kirill.
>
> In my earlier discussion with Kirill, I argued that tests should be kept since this patch implements a new feature.
> However, I now understand that the existing tests already cover the Query_for_list_of_selectables pattern,
> so a separate test for DELETE ... USING would be redundant. I've removed the test in the updated patch.
>
> I've prepared a v5 patch with the following changes:
>  - Use "relations supporting SELECT" in the comment for consistency
>  - Remove the regression test
>
> The updated v5 patch is attached.

Thanks for updating the patch! I've pushed it.

Regards,

--
Fujii Masao

Hi Fujii-san, Tatsuya-san,

On Mon, Mar 2, 2026 at 7:39 AM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Sun, Mar 1, 2026 at 3:07 PM Tatsuya Kawata
> <kawatatatsuya0913@gmail.com> wrote:
> >
> > Hi Fujii-san,
> >
> > Thank you for your review!
> >
> > > Would it be better to use "relations supporting SELECT" instead of "selectables"?
> >
> > Good point. I've updated the comment to use "relations supporting SELECT" for consistency.
> >
> > > As for the regression test, I agree with Kirill.
> >
> > In my earlier discussion with Kirill, I argued that tests should be kept since this patch implements a new feature.
> > However, I now understand that the existing tests already cover the Query_for_list_of_selectables pattern,
> > so a separate test for DELETE ... USING would be redundant. I've removed the test in the updated patch.
> >
> > I've prepared a v5 patch with the following changes:
> >  - Use "relations supporting SELECT" in the comment for consistency
> >  - Remove the regression test
> >
> > The updated v5 patch is attached.
>
> Thanks for updating the patch! I've pushed it.


Thank you for the updates and for committing the patch.
I have reviewed the final changes in v5. Switching to
Query_for_list_of_selectables and aligning the behavior with SELECT
... FROM makes the implementation cleaner for me too and seems more
consistent with existing completion patterns. And also the removal of
the redundant test also makes sense given the existing coverage.
I appreciate the careful refinements and the clarity brought
throughout the review discussion.

Best regards,
Soumya