Horton

Thread

Commits

  1. Simplify tab completion of extension versions, redux.

  2. Simplify tab completion of extension versions.

  3. Fix busted tab completion of extension versions.

  4. psql: improve tab-complete's handling of variant SQL names.

  1. 15beta1 tab completion of extension versions

    Jeff Janes <jeff.janes@gmail.com> — 2022-06-18T18:09:48Z

    Extension version strings need to be quoted.  Either double or single
quotes will work.  In released psql clients, tab completion offers double
quoted suggestions:

alter extension pg_trgm update TO <tab><tab>
"1.3"  "1.4"  "1.5"  "1.6"

But commit 02b8048ba5 broke that, it now offers unquoted version strings
which if used as offered then lead to syntax errors.

The code change seems to have been intentional, but I don't think the
behavior change was intended.  While the version string might not be an
identifier, it still needs to be treated as if it were one.
Putting pg_catalog.quote_ident back
into Query_for_list_of_available_extension_versions* fixes it, but might
not be the best way to fix it.

commit 02b8048ba5dc36238f3e7c3c58c5946220298d71 (HEAD, refs/bisect/bad)
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date:   Sun Jan 30 13:33:23 2022 -0500

    psql: improve tab-complete's handling of variant SQL names.


Cheer,

Jeff

  2. Re: 15beta1 tab completion of extension versions

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-06-18T23:49:28Z

    Jeff Janes <jeff.janes@gmail.com> writes:
> Extension version strings need to be quoted.  Either double or single
> quotes will work.  In released psql clients, tab completion offers double
> quoted suggestions:
> But commit 02b8048ba5 broke that, it now offers unquoted version strings
> which if used as offered then lead to syntax errors.

Ooops.

> The code change seems to have been intentional, but I don't think the
> behavior change was intended.

Given the comments about it, I'm sure I tested the behavior somewhere
along the line --- but I must not have done so with the final logic
of _complete_from_query.

> Putting pg_catalog.quote_ident back
> into Query_for_list_of_available_extension_versions* fixes it, but might
> not be the best way to fix it.

Yeah, that seems like the appropriate fix.  Done, thanks for the report!

			regards, tom lane

  3. Re: 15beta1 tab completion of extension versions

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-06-19T04:56:13Z

    I wrote:
> Jeff Janes <jeff.janes@gmail.com> writes:
>> Putting pg_catalog.quote_ident back
>> into Query_for_list_of_available_extension_versions* fixes it, but might
>> not be the best way to fix it.

> Yeah, that seems like the appropriate fix.  Done, thanks for the report!

Actually ... after further thought it seems like maybe we should
make this more like other cases rather than less so.  ISTM that much
of the issue here is somebody's decision that "TO version" should be
offered as a completion of "UPDATE", which is unlike the way we do this
anywhere else --- the usual thing is to offer "UPDATE TO" as a single
completion.  So I'm thinking about the attached.

This behaves a little differently from the old code.  In v14,
	alter extension pg_trgm upd<TAB>
gives you
	alter extension pg_trgm update<space>
and another <TAB> produces
	alter extension pg_trgm update TO "1.

With this,
	alter extension pg_trgm upd<TAB>
gives you
	alter extension pg_trgm update to<space>
and another <TAB> produces
	alter extension pg_trgm update to "1.

That seems more consistent with other cases, and it's the same
number of <TAB> presses.

			regards, tom lane

  4. Re: 15beta1 tab completion of extension versions

    Noah Misch <noah@leadboat.com> — 2022-07-03T08:32:17Z

    On Sun, Jun 19, 2022 at 12:56:13AM -0400, Tom Lane wrote:
> Actually ... after further thought it seems like maybe we should
> make this more like other cases rather than less so.  ISTM that much
> of the issue here is somebody's decision that "TO version" should be
> offered as a completion of "UPDATE", which is unlike the way we do this
> anywhere else --- the usual thing is to offer "UPDATE TO" as a single
> completion.  So I'm thinking about the attached.

Which are the older completions that offer "UPDATE TO"?  I don't see any.

> This behaves a little differently from the old code.  In v14,
> 	alter extension pg_trgm upd<TAB>
> gives you
> 	alter extension pg_trgm update<space>
> and another <TAB> produces
> 	alter extension pg_trgm update TO "1.
> 
> With this,
> 	alter extension pg_trgm upd<TAB>
> gives you
> 	alter extension pg_trgm update to<space>
> and another <TAB> produces
> 	alter extension pg_trgm update to "1.
> 
> That seems more consistent with other cases, and it's the same
> number of <TAB> presses.

I think it makes sense to send UPDATE TO as a single completion in places
where no valid command can have the UPDATE without the TO.  CREATE RULE foo AS
ON UPDATE TO is a candidate, though CREATE RULE completion doesn't do that
today.  "ALTER EXTENSION hstore UPDATE;" is a valid command (updates to the
control file default version).  Hence, I think the v14 behavior was better.

  5. Re: 15beta1 tab completion of extension versions

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-07-03T14:29:38Z

    Noah Misch <noah@leadboat.com> writes:
> I think it makes sense to send UPDATE TO as a single completion in places
> where no valid command can have the UPDATE without the TO.  CREATE RULE foo AS
> ON UPDATE TO is a candidate, though CREATE RULE completion doesn't do that
> today.  "ALTER EXTENSION hstore UPDATE;" is a valid command (updates to the
> control file default version).  Hence, I think the v14 behavior was better.

Hmm ... good point, let me think about that.

			regards, tom lane

  6. Re: 15beta1 tab completion of extension versions

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-07-03T18:00:59Z

    I wrote:
> Noah Misch <noah@leadboat.com> writes:
>> "ALTER EXTENSION hstore UPDATE;" is a valid command (updates to the
>> control file default version).  Hence, I think the v14 behavior was better.

> Hmm ... good point, let me think about that.

After consideration, my preferred solution is just this:

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 463cac9fb0..c5cafe6f4b 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1927,7 +1927,7 @@ psql_completion(const char *text, int start, int end)
 
 	/* ALTER EXTENSION <name> */
 	else if (Matches("ALTER", "EXTENSION", MatchAny))
-		COMPLETE_WITH("ADD", "DROP", "UPDATE TO", "SET SCHEMA");
+		COMPLETE_WITH("ADD", "DROP", "UPDATE", "SET SCHEMA");
 
 	/* ALTER EXTENSION <name> UPDATE */
 	else if (Matches("ALTER", "EXTENSION", MatchAny, "UPDATE"))

This will require one extra <TAB> when what you want is to update to
a specific version, but I doubt that that's going to bother anyone
very much.  I don't want to try to resurrect the v14 behavior exactly
because it's too much of a mess from a quoting standpoint.

			regards, tom lane

  7. Re: 15beta1 tab completion of extension versions

    Noah Misch <noah@leadboat.com> — 2022-07-03T19:11:22Z

    On Sun, Jul 03, 2022 at 02:00:59PM -0400, Tom Lane wrote:
> I wrote:
> > Noah Misch <noah@leadboat.com> writes:
> >> "ALTER EXTENSION hstore UPDATE;" is a valid command (updates to the
> >> control file default version).  Hence, I think the v14 behavior was better.
> 
> > Hmm ... good point, let me think about that.
> 
> After consideration, my preferred solution is just this:
> 
> diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
> index 463cac9fb0..c5cafe6f4b 100644
> --- a/src/bin/psql/tab-complete.c
> +++ b/src/bin/psql/tab-complete.c
> @@ -1927,7 +1927,7 @@ psql_completion(const char *text, int start, int end)
>  
>  	/* ALTER EXTENSION <name> */
>  	else if (Matches("ALTER", "EXTENSION", MatchAny))
> -		COMPLETE_WITH("ADD", "DROP", "UPDATE TO", "SET SCHEMA");
> +		COMPLETE_WITH("ADD", "DROP", "UPDATE", "SET SCHEMA");
>  
>  	/* ALTER EXTENSION <name> UPDATE */
>  	else if (Matches("ALTER", "EXTENSION", MatchAny, "UPDATE"))
> 
> This will require one extra <TAB> when what you want is to update to
> a specific version, but I doubt that that's going to bother anyone
> very much.  I don't want to try to resurrect the v14 behavior exactly
> because it's too much of a mess from a quoting standpoint.

Works for me, and I agree the patch implements that successfully.  "ALTER
EXTENSION x UPDATE;" is an infrequent command, and "ALTER EXTENSION x UPDATE
TO ..." is even less frequent.  It's not worth much special effort to shave
<TAB> steps.

  8. Re: 15beta1 tab completion of extension versions

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-07-03T19:28:15Z

    Noah Misch <noah@leadboat.com> writes:
> On Sun, Jul 03, 2022 at 02:00:59PM -0400, Tom Lane wrote:
>> This will require one extra <TAB> when what you want is to update to
>> a specific version, but I doubt that that's going to bother anyone
>> very much.  I don't want to try to resurrect the v14 behavior exactly
>> because it's too much of a mess from a quoting standpoint.

> Works for me, and I agree the patch implements that successfully.  "ALTER
> EXTENSION x UPDATE;" is an infrequent command, and "ALTER EXTENSION x UPDATE
> TO ..." is even less frequent.  It's not worth much special effort to shave
> <TAB> steps.

Done that way then.  Thanks for the report.

			regards, tom lane