Обсуждение: [Patch]Add tab completion for DELETE ... USING
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
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
Вложения
> 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/
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
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
Вложения
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
Hi!
Thank you for your review!
> 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.
I agree that not every tab-complete change requires tests, and we don't need to mirror all cases from tab-complete.in.c. I also think tests that provide little benefit compared to their maintenance cost can be omitted. However, since this patch implements a previously unimplemented feature (removing a XXX TODO comment), I believe the primary test cases should be kept to verify the core functionality and prevent regressions.
That said, I can reduce the test cases. The current 4 tests could be reduced to 2:
One for USING <TAB> (core functionality: table completion)
One for USING <table> <TAB> (keyword completion after table)
WDYT?
Regards,
Tatsuya Kawata
Thank you for your review!
> 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.
I agree that not every tab-complete change requires tests, and we don't need to mirror all cases from tab-complete.in.c. I also think tests that provide little benefit compared to their maintenance cost can be omitted. However, since this patch implements a previously unimplemented feature (removing a XXX TODO comment), I believe the primary test cases should be kept to verify the core functionality and prevent regressions.
That said, I can reduce the test cases. The current 4 tests could be reduced to 2:
One for USING <TAB> (core functionality: table completion)
One for USING <table> <TAB> (keyword completion after table)
WDYT?
Regards,
Tatsuya Kawata
Hi!
The updated v3 patch is attached as above.
The updated v3 patch is attached as above.
Just reduced test cases.
Regards,
Tatsuya Kawata
Regards,
Tatsuya Kawata
Вложения
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 Kirill-san,
Thank you for your review and for updating the CF entry status!
Regards,
Tatsuya Kawata
Thank you for your review and for updating the CF entry status!
Regards,
Tatsuya Kawata
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