Re: Tab completion for CREATE SCHEMAAUTHORIZATION - Mailing list pgsql-hackers
| From | Suraj Khamkar |
|---|---|
| Subject | Re: Tab completion for CREATE SCHEMAAUTHORIZATION |
| Date | |
| Msg-id | CA+U=F9i3wayOrRK5__dAKipfNSsO53HbC0uE=NA1kOkj3S3mRQ@mail.gmail.com Whole thread Raw |
| In response to | Re: Tab completion for CREATE SCHEMAAUTHORIZATION (ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)) |
| List | pgsql-hackers |
Thanks Dagfinn for the updated patches.
I do not get these errors, neither with the patch file I still have
locally, or by saving the attachment from my original email. Are you
sure something in your download process hasn't converted it to Windows
line endings (\r\n), or otherwise mangled the whitespace?
No. I have downloaded patch on linux and patch also doesn't have any trailing
spaces, though it is throwing an error.
spaces, though it is throwing an error.
Here are few other comments,
- USER MAPPING does not have SESSION_USER as username in syntax
(though it works) but your changes provide the same. Also, we have USER in
list which is missing in current code changes.
CREATE USER MAPPING [ IF NOT EXISTS ] FOR { user_name | USER | CURRENT_ROLE | CURRENT_USER | PUBLIC }
SERVER server_name
[ OPTIONS ( option 'value' [ , ... ] ) ] - It might not be a scope of this ticket but as we are changing the query for ALTER GROUP,
we should complete the role_specification after ALTER GROUP.
postgres@17077=#ALTER GROUP
pg_database_owner pg_monitor pg_read_all_settings
pg_read_server_files pg_stat_scan_tables pg_write_server_files
surajkhamkar. pg_execute_server_program pg_read_all_data
pg_read_all_stats pg_signal_backend pg_write_all_data - Missing schema_elements after CREATE SCHEMA AUTHORIZATION username to tab-complete .
schema_elements might be CREATE, GRAND etc.
Thanks..
On Mon, Aug 9, 2021 at 11:30 PM Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:
Hi Suraj,
Suraj Khamkar <khamkarsuraj.b@gmail.com> writes:
> Hello Dagfinn,
>
> I had a look at your patch and below are my review comments.
> Please correct me if I am missing something.
>
> 1. For me the patch does not apply cleanly. I have been facing the error
> of trailing whitespaces.
I do not get these errors, neither with the patch file I still have
locally, or by saving the attachment from my original email. Are you
sure something in your download process hasn't converted it to Windows
line endings (\r\n), or otherwise mangled the whitespace?
> 2. We can remove space in before \ and below change
> +" UNION ALL SELECT 'PUBLIC'" \
>
> Should be,
> +" UNION ALL SELECT 'PUBLIC' "\
The patch doesn't add any lines that end with quote-space-backslash.
As for the space before the quote, that is not necessary either, since the
next line starts with a space after the quote. Either way, the updated
version of the patch doesn't add any new lines with continuation after a
string constant, so the point is moot.
> 3. role_specification has CURRENT_ROLE, CURRENT_USER and SESSION_USER.
> But current changes are missing CURRENT_ROLE.
Ah, I was looking at the documentation for 13, but CURRENT_ROLE is only
allowed in this context as of 14. Fixed.
> 4. I'm not sure about this but do we need to enable tab completion for IF
> NOT EXIST?
>
> 5. I think we are not handling IF NOT EXIST that's why it's not
> completing tab completion
> for AUTHORIZATION.
As you note, psql currently doesn't currently tab-complete IF NOT EXISTS
for any command, so that would be a subject for a separate patch.
> 6. As we are here we can also enable missing tab completion for ALTER
> SCHEMA.
> After OWNER TO we should also get CURRENT_ROLE, CURRENT_USER and
> SESSION_USER.
I did an audit of all the uses of Query_for_list_of_roles, and there
turned out be several more that accept CURRENT_ROLE, CURRENT_USER and
SESSION_USER that they weren't tab-completed for. I also renamed the
constant to Query_for_list_of_owner_roles, but I'm not 100% happy with
that name either.
> 7. Similarly, as we can drop multiple schemas' simultaneously, we should
> enable tab completion for
> comma with CASCADE and RESTRICT
> postgres@53724=#DROP SCHEMA sch
> CASCADE RESTRICT
The tab completion code for DROP is generic for all object types (see
the words_after_create array and the create_or_drop_command_generator
function), so that should be done genericallly, and is thus outside the
scope for this patch.
> Thanks.
Thanks for the review. Updated patch attached, with the CURRENT/SESSION
ROLE/USER changes for other commands separated out.
- ilmari
pgsql-hackers by date: