Horton

Thread

Commits

  1. Don't treat complete_from_const as equivalent to complete_from_list.

  2. Fix tab completion of "SET variable TO|=" to not offer bogus completions.

  3. Stamp 9.6.13.

  1. psql UPDATE field [tab] expands to DEFAULT?

    Ken Tanzer <ken.tanzer@gmail.com> — 2019-06-17T22:03:11Z

    Hi.  If I'm using psql, and type for example:

UPDATE my_table SET my_field
(with a trailing space)

and then hit Tab, it will expand that to an =, and then another tab will
expand to DEFAULT, so that I then have:

UPDATE my_table SET my_field = DEFAULT

If I'm tabbing out in this situation, it's going to be after the =, and I
will have typed "myreal"[tab] in the vain hope that psql will complete that
to "myreallylongfieldname," but instead it gets replaced with DEFAULT.

So I'm curious if this is intended behavior, if it's considered useful,
and/or if it's a placeholder for something in the future that will be
useful.  Also, is this new, as I've never noticed it before?

Thanks in advance,
Ken

p.s.,  Version 9.6.13

-- 
AGENCY Software
A Free Software data system
By and for non-profits
*http://agency-software.org/ <http://agency-software.org/>*
*https://demo.agency-software.org/client
<https://demo.agency-software.org/client>*
ken.tanzer@agency-software.org
(253) 245-3801

Subscribe to the mailing list
<agency-general-request@lists.sourceforge.net?body=subscribe> to
learn more about AGENCY or
follow the discussion.

  2. Re: psql UPDATE field [tab] expands to DEFAULT?

    Adrian Klaver <adrian.klaver@aklaver.com> — 2019-06-17T23:24:43Z

    On 6/17/19 3:03 PM, Ken Tanzer wrote:
> Hi.  If I'm using psql, and type for example:
> 
> UPDATE my_table SET my_field
> (with a trailing space)
> 
> and then hit Tab, it will expand that to an =, and then another tab will 
> expand to DEFAULT, so that I then have:
> 
> UPDATE my_table SET my_field = DEFAULT
> 
> If I'm tabbing out in this situation, it's going to be after the =, and 
> I will have typed "myreal"[tab] in the vain hope that psql will complete 
> that to "myreallylongfieldname," but instead it gets replaced with DEFAULT.
> 
> So I'm curious if this is intended behavior, if it's considered useful, 
> and/or if it's a placeholder for something in the future that will be 
> useful.  Also, is this new, as I've never noticed it before?

Not sure how long that has been around.

My cheat for dealing with many/long column names is:

test=# \d up_test
               Table "public.up_test"
  Column |  Type   | Collation | Nullable | Default
--------+---------+-----------+----------+---------
  id     | integer |           |          |
  col1   | boolean |           |          |
  col2   | integer |           |          |



test=# \pset format unaligned
Output format is unaligned.
test=# \pset fieldsep ','
Field separator is ",".

select * from up_test limit 0;
id,col1,col2

Cut and paste above.

test=# \pset fieldsep '|'
Field separator is "|".

test=# \pset format 'aligned'
Output format is aligned.


> 
> Thanks in advance,
> Ken
> 
> p.s.,  Version 9.6.13
> 
> -- 
> AGENCY Software
> A Free Software data system
> By and for non-profits
> /http://agency-software.org//
> /https://demo.agency-software.org/client/
> ken.tanzer@agency-software.org <mailto:ken.tanzer@agency-software.org>
> (253) 245-3801
> 
> Subscribe to the mailing list 
> <mailto:agency-general-request@lists.sourceforge.net?body=subscribe> to
> learn more about AGENCY or
> follow the discussion.


-- 
Adrian Klaver
adrian.klaver@aklaver.com

  3. Re: psql UPDATE field [tab] expands to DEFAULT?

    Ken Tanzer <ken.tanzer@gmail.com> — 2019-06-17T23:33:44Z

    On Mon, Jun 17, 2019 at 4:24 PM Adrian Klaver <adrian.klaver@aklaver.com>
wrote:

> On 6/17/19 3:03 PM, Ken Tanzer wrote:
> >
> > So I'm curious if this is intended behavior, if it's considered useful,
> > and/or if it's a placeholder for something in the future that will be
> > useful.  Also, is this new, as I've never noticed it before?
>
> Not sure how long that has been around.
>
> My cheat for dealing with many/long column names is:
>
>
Thanks Adrian, though I wasn't really seeking tips for column names.  I was
instead trying to understand whether this particular tab expansion was
intentional and considered useful, and if so what that usefulness was,
because it's rather escaping me!

Cheers,
Ken


-- 
AGENCY Software
A Free Software data system
By and for non-profits
*http://agency-software.org/ <http://agency-software.org/>*
*https://demo.agency-software.org/client
<https://demo.agency-software.org/client>*
ken.tanzer@agency-software.org
(253) 245-3801

Subscribe to the mailing list
<agency-general-request@lists.sourceforge.net?body=subscribe> to
learn more about AGENCY or
follow the discussion.

  4. Re: psql UPDATE field [tab] expands to DEFAULT?

    Tim Cross <theophilusx@gmail.com> — 2019-06-18T00:09:11Z

    On Tue, 18 Jun 2019 at 09:34, Ken Tanzer <ken.tanzer@gmail.com> wrote:

> On Mon, Jun 17, 2019 at 4:24 PM Adrian Klaver <adrian.klaver@aklaver.com>
> wrote:
>
>> On 6/17/19 3:03 PM, Ken Tanzer wrote:
>> >
>> > So I'm curious if this is intended behavior, if it's considered useful,
>> > and/or if it's a placeholder for something in the future that will be
>> > useful.  Also, is this new, as I've never noticed it before?
>>
>> Not sure how long that has been around.
>>
>> My cheat for dealing with many/long column names is:
>>
>>
> Thanks Adrian, though I wasn't really seeking tips for column names.  I
> was instead trying to understand whether this particular tab expansion was
> intentional and considered useful, and if so what that usefulness was,
> because it's rather escaping me!
>
> Cheers,
> Ken
>
>
>
Have to say, I fid that behaviour unusual as well. I would expect that once
I've typed some characters, the completion mechanism would attempt to
complete based on the characters I've typed and if it cannot, to do
nothing. Instead, what happens is that what I have typed is replaced by
'default'.  For example, if I type

update my_table set my_col = other_t

and hit tab, 'other_t is replaced by 'default', which is of no use. What I
would expect is for tab to either complete (possibly only partially if
there is multiple candidates) what it could for candidates which start with
'other_t' e.g. 'other_table' or it would do nothing i.e. no completion
candidates found, telling me there is no match based on the prefix I've
typed.


-- 
regards,

Tim

--
Tim Cross

  5. Re: psql UPDATE field [tab] expands to DEFAULT?

    Adrian Klaver <adrian.klaver@aklaver.com> — 2019-06-18T00:22:53Z

    On 6/17/19 4:33 PM, Ken Tanzer wrote:
> On Mon, Jun 17, 2019 at 4:24 PM Adrian Klaver <adrian.klaver@aklaver.com 
> <mailto:adrian.klaver@aklaver.com>> wrote:
> 
>     On 6/17/19 3:03 PM, Ken Tanzer wrote:
>      >
>      > So I'm curious if this is intended behavior, if it's considered
>     useful,
>      > and/or if it's a placeholder for something in the future that
>     will be
>      > useful.  Also, is this new, as I've never noticed it before?
> 
>     Not sure how long that has been around.
> 
>     My cheat for dealing with many/long column names is:
> 
> 
> Thanks Adrian, though I wasn't really seeking tips for column names.  I 
> was instead trying to understand whether this particular tab expansion 
> was intentional and considered useful, and if so what that usefulness 

If I am following the below correctly it is intentional:

https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/bin/psql/tab-complete.c;h=68a2ba27aec22302625c5481a8f74cf866f4dc23;hb=d22ca701a39dfd03cdfa1ca238370d34f4bc4ac4

Line 2888

Useful, that is in the eye of the beholder:)

> was, because it's rather escaping me!
> 
> Cheers,
> Ken
> 
> 



-- 
Adrian Klaver
adrian.klaver@aklaver.com

  6. Re: psql UPDATE field [tab] expands to DEFAULT?

    Jeff Janes <jeff.janes@gmail.com> — 2019-06-18T00:23:47Z

    On Mon, Jun 17, 2019 at 6:03 PM Ken Tanzer <ken.tanzer@gmail.com> wrote:

> Hi.  If I'm using psql, and type for example:
>
> UPDATE my_table SET my_field
> (with a trailing space)
>
> and then hit Tab, it will expand that to an =, and then another tab will
> expand to DEFAULT, so that I then have:
>
> UPDATE my_table SET my_field = DEFAULT
>
> If I'm tabbing out in this situation, it's going to be after the =, and I
> will have typed "myreal"[tab] in the vain hope that psql will complete that
> to "myreallylongfieldname," but instead it gets replaced with DEFAULT.
>

Yeah, it is especially annoying to delete what I actually typed to replace
it with something else.  I've been irked by that before.  I think the
general behavior of replacing something already typed with (what it
believes to be) the only proper completion is part of the underlying
readline/libedit library, not something psql goes out of its way to do.


> So I'm curious if this is intended behavior, if it's considered useful,
> and/or if it's a placeholder for something in the future that will be
> useful.  Also, is this new, as I've never noticed it before?
>

The tab completion doesn't have a SQL parser/analyzer, it is just driven of
general rules of looking at the proceeding N words.  In this case, it is
hitting the rule for "SET anything TO", which is intended to catch the
setting of parameters, it is only accidentally hitting on the SET part of
UPDATE statements.

This goes back at least to 9.3.

We could improve it by making a higher priority rule which looks back a few
more words to:

UPDATE <tablename> SET <colname> TO

But what would we complete with?  Any expression can go there, and we can't
make it tab complete any arbitrary expression, like function names or
literals.  If we tab complete, but only with a restricted set of choices,
that could be interpreted as misleadingly suggesting no other things are
possible.  (Of course the current accidental behavior is also misleading,
then)

If we are willing to offer an incomplete list of suggestions, what would
they be?  NULL, DEFAULT, '(' and all the columnnames present in
<tablename>, with appropriate quotes where necessary?  But what to do with
<tablename> doesn't actually exist as the name of a table?

Or, we could have it implement the more precise higher priority rule, and
have it just refuse to offer any suggestions, but at least not delete what
is already there.

Cheers,

Jeff

>

  7. Re: psql UPDATE field [tab] expands to DEFAULT?

    Jeff Janes <jeff.janes@gmail.com> — 2019-06-18T00:34:33Z

    On Mon, Jun 17, 2019 at 8:23 PM Adrian Klaver <adrian.klaver@aklaver.com>
wrote:

> On 6/17/19 4:33 PM, Ken Tanzer wrote:
> >
> > Thanks Adrian, though I wasn't really seeking tips for column names.  I
> > was instead trying to understand whether this particular tab expansion
> > was intentional and considered useful, and if so what that usefulness
>
> If I am following the below correctly it is intentional:
>
>
> https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/bin/psql/tab-complete.c;h=68a2ba27aec22302625c5481a8f74cf866f4dc23;hb=d22ca701a39dfd03cdfa1ca238370d34f4bc4ac4
>
> Line 2888
>

But that portion doesn't offer the DEFAULT completion.  It stops at
offering '=', and goes no further.

It is at line 2859 which accidentally offers to complete DEFAULT, and that
is not part of the UPDATE-specific code.

Cheers,

Jeff

  8. Re: psql UPDATE field [tab] expands to DEFAULT?

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-06-18T00:39:21Z

    Tim Cross <theophilusx@gmail.com> writes:
> On Tue, 18 Jun 2019 at 09:34, Ken Tanzer <ken.tanzer@gmail.com> wrote:
>> Thanks Adrian, though I wasn't really seeking tips for column names.  I
>> was instead trying to understand whether this particular tab expansion was
>> intentional and considered useful, and if so what that usefulness was,
>> because it's rather escaping me!

> Have to say, I fid that behaviour unusual as well.

I don't think it's intentional.  A look into tab-complete.c shows that it
makes no attempt to offer completions beyond the "=" part of the syntax;
so there's room for improvement there.  But then what is producing the
"DEFAULT" completion?  After looking around a bit, I think it's
accidentally matching the pattern for a GUC "set" command:

    else if (TailMatches("SET", MatchAny, "TO|="))
    {
        /* special cased code for individual GUCs */
        ...
        else
            COMPLETE_WITH("DEFAULT");
    }

So perhaps that needs to look more like this other place where somebody
already noticed the conflict against UPDATE:

    else if (TailMatches("SET|RESET") && !TailMatches("UPDATE", MatchAny, "SET"))
        COMPLETE_WITH_QUERY(Query_for_list_of_set_vars);

More generally, though, I'm inclined to think that offering DEFAULT
and nothing else, which is what this code does if it doesn't recognize
the "GUC name", is just ridiculous.  If the word after SET is not a known
GUC name then we probably have misconstrued the context, as indeed is
happening in your example; and in any case DEFAULT is about the least
likely thing for somebody to be trying to enter here.  (They'd probably
have selected RESET not SET if they were trying to do that.)

			regards, tom lane

  9. Re: psql UPDATE field [tab] expands to DEFAULT?

    Tim Cross <theophilusx@gmail.com> — 2019-06-18T00:52:44Z

    On Tue, 18 Jun 2019 at 10:39, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Tim Cross <theophilusx@gmail.com> writes:
> > On Tue, 18 Jun 2019 at 09:34, Ken Tanzer <ken.tanzer@gmail.com> wrote:
> >> Thanks Adrian, though I wasn't really seeking tips for column names.  I
> >> was instead trying to understand whether this particular tab expansion
> was
> >> intentional and considered useful, and if so what that usefulness was,
> >> because it's rather escaping me!
>
> > Have to say, I fid that behaviour unusual as well.
>
> I don't think it's intentional.  A look into tab-complete.c shows that it
> makes no attempt to offer completions beyond the "=" part of the syntax;
> so there's room for improvement there.  But then what is producing the
> "DEFAULT" completion?  After looking around a bit, I think it's
> accidentally matching the pattern for a GUC "set" command:
>
>     else if (TailMatches("SET", MatchAny, "TO|="))
>     {
>         /* special cased code for individual GUCs */
>         ...
>         else
>             COMPLETE_WITH("DEFAULT");
>     }
>
> So perhaps that needs to look more like this other place where somebody
> already noticed the conflict against UPDATE:
>
>     else if (TailMatches("SET|RESET") && !TailMatches("UPDATE", MatchAny,
> "SET"))
>         COMPLETE_WITH_QUERY(Query_for_list_of_set_vars);
>
> More generally, though, I'm inclined to think that offering DEFAULT
> and nothing else, which is what this code does if it doesn't recognize
> the "GUC name", is just ridiculous.  If the word after SET is not a known
> GUC name then we probably have misconstrued the context, as indeed is
> happening in your example; and in any case DEFAULT is about the least
> likely thing for somebody to be trying to enter here.  (They'd probably
> have selected RESET not SET if they were trying to do that.)
>
>                         regards, tom lane
>


Given that without adding a full blown sql parser in order to identify
legitimate candidates following a '=' in an update statement, my suggestion
would be to refine the rules so that no completion is attempted after the
=. Would rather have tab do nothing over tab replacing what I've already
typed with 'default'.

-- 
regards,

Tim

--
Tim Cross

  10. Re: psql UPDATE field [tab] expands to DEFAULT?

    Ken Tanzer <ken.tanzer@gmail.com> — 2019-06-18T22:23:40Z

    On Mon, Jun 17, 2019 at 4:24 PM Adrian Klaver <adrian.klaver@aklaver.com>
wrote:

>
> My cheat for dealing with many/long column names is:
>
> test=# \d up_test
>                Table "public.up_test"
>   Column |  Type   | Collation | Nullable | Default
> --------+---------+-----------+----------+---------
>   id     | integer |           |          |
>   col1   | boolean |           |          |
>   col2   | integer |           |          |
>
>
>
> test=# \pset format unaligned
> Output format is unaligned.
> test=# \pset fieldsep ','
> Field separator is ",".
>
> select * from up_test limit 0;
> id,col1,col2
>
> Cut and paste above.
>
> test=# \pset fieldsep '|'
> Field separator is "|".
>
> test=# \pset format 'aligned'
> Output format is aligned.
>
>
Just curious, but if you really do that often, wouldn't you be better off
with something like this?

CREATE OR REPLACE FUNCTION field_list( name ) RETURNS text AS $$

SELECT array_to_string(array_agg(column_name::text ORDER BY
ordinal_position),',') FROM information_schema.columns WHERE table_name =
$1;

$$ LANGUAGE sql STABLE;

Cheers,
Ken



-- 
AGENCY Software
A Free Software data system
By and for non-profits
*http://agency-software.org/ <http://agency-software.org/>*
*https://demo.agency-software.org/client
<https://demo.agency-software.org/client>*
ken.tanzer@agency-software.org
(253) 245-3801

Subscribe to the mailing list
<agency-general-request@lists.sourceforge.net?body=subscribe> to
learn more about AGENCY or
follow the discussion.

  11. Re: psql UPDATE field [tab] expands to DEFAULT?

    Adrian Klaver <adrian.klaver@aklaver.com> — 2019-06-19T01:03:18Z

    On 6/18/19 3:23 PM, Ken Tanzer wrote:
> On Mon, Jun 17, 2019 at 4:24 PM Adrian Klaver <adrian.klaver@aklaver.com 
> <mailto:adrian.klaver@aklaver.com>> wrote:
> 
> 
>     My cheat for dealing with many/long column names is:
> 
>     test=# \d up_test
>                     Table "public.up_test"
>        Column |  Type   | Collation | Nullable | Default
>     --------+---------+-----------+----------+---------
>        id     | integer |           |          |
>        col1   | boolean |           |          |
>        col2   | integer |           |          |
> 
> 
> 
>     test=# \pset format unaligned
>     Output format is unaligned.
>     test=# \pset fieldsep ','
>     Field separator is ",".
> 
>     select * from up_test limit 0;
>     id,col1,col2
> 
>     Cut and paste above.
> 
>     test=# \pset fieldsep '|'
>     Field separator is "|".
> 
>     test=# \pset format 'aligned'
>     Output format is aligned.
> 
> 
> Just curious, but if you really do that often, wouldn't you be better 
> off with something like this?

I could/should I just don't do the above enough to get motivated to 
build a function. Most cases where I'm doing complicated updates I am 
not using psql I am building then in Python from a dict.

> 
> CREATE OR REPLACE FUNCTION field_list( name ) RETURNS text AS $$
> 
> SELECT array_to_string(array_agg(column_name::text ORDER BY 
> ordinal_position),',') FROM information_schema.columns WHERE table_name 
> = $1;
> 
> $$ LANGUAGE sql STABLE;
> 
> Cheers,
> Ken
> 



-- 
Adrian Klaver
adrian.klaver@aklaver.com

  12. Re: psql UPDATE field [tab] expands to DEFAULT?

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-06-19T14:39:24Z

    [ moving thread to -hackers ]

So I propose the attached patch for fixing the clear bugs that have
emerged in this discussion: don't confuse UPDATE ... SET ... with
GUC-setting commands, and don't offer just DEFAULT in contexts where
that's unlikely to be the only valid completion.

Nosing around in tab-complete.c, I notice a fair number of other
places where we're doing COMPLETE_WITH() with just a single possible
completion.  Knowing what we know now, in each one of those places
libreadline will suppose that that completion is the only syntactically
legal continuation, and throw away anything else the user might've typed.
We should probably inspect each of those places to see if that's really
desirable behavior ... but I didn't muster the energy to do that this
morning.

			regards, tom lane

  13. Re: psql UPDATE field [tab] expands to DEFAULT?

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-06-20T18:51:50Z

    I wrote:
> Nosing around in tab-complete.c, I notice a fair number of other
> places where we're doing COMPLETE_WITH() with just a single possible
> completion.  Knowing what we know now, in each one of those places
> libreadline will suppose that that completion is the only syntactically
> legal continuation, and throw away anything else the user might've typed.
> We should probably inspect each of those places to see if that's really
> desirable behavior ... but I didn't muster the energy to do that this
> morning.

I took a closer look and realized that this isn't some magic behavior of
arcane parts of libreadline; it's more like self-inflicted damage.  It
happens because tab-complete.c's complete_from_const() is doing exactly
what its comment says it does:

/*
 * This function returns one fixed string the first time even if it doesn't
 * match what's there, and nothing the second time. This should be used if
 * there is only one possibility that can appear at a certain spot, so
 * misspellings will be overwritten.  The string to be passed must be in
 * completion_charp.
 */

This is unlike complete_from_list(), which will only return completions
that match the text-string-so-far.

I have to wonder whether complete_from_const()'s behavior is really
a good idea; I think there might be an argument for getting rid of it
and using complete_from_list() even for one-element lists.

We certainly didn't do anybody any favors in the refactoring we did in
4f3b38fe2, which removed the source-code difference between calling
complete_from_const() and calling complete_from_list() with just one list
item.  But even before that, I really doubt that many people hacking on
tab-complete.c had internalized the idea that COMPLETE_WITH_CONST()
implied a higher degree of certainty than COMPLETE_WITH_LIST() with one
list item.  I'm pretty sure I'd never understood that.

Both of those functions go back to the beginnings of tab-complete.c,
so there's not much available in the history to explain the difference
in behavior (and the discussion of the original patch, if any, is lost
to the mists of time --- our archives for pgsql-patches only go back to
2000).  But my own feeling about this is that there's no situation in
which I'd expect tab completion to wipe out text I'd typed.

Thoughts?

			regards, tom lane

  14. Re: psql UPDATE field [tab] expands to DEFAULT?

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-06-25T15:59:56Z

    I wrote:
> I took a closer look and realized that this isn't some magic behavior of
> arcane parts of libreadline; it's more like self-inflicted damage.  It
> happens because tab-complete.c's complete_from_const() is doing exactly
> what its comment says it does:

> /*
>  * This function returns one fixed string the first time even if it doesn't
>  * match what's there, and nothing the second time. This should be used if
>  * there is only one possibility that can appear at a certain spot, so
>  * misspellings will be overwritten.  The string to be passed must be in
>  * completion_charp.
>  */

> This is unlike complete_from_list(), which will only return completions
> that match the text-string-so-far.

> I have to wonder whether complete_from_const()'s behavior is really
> a good idea; I think there might be an argument for getting rid of it
> and using complete_from_list() even for one-element lists.

I experimented with ripping out complete_from_const() altogether, and
soon found that there's still one place where we need it: down at the
end of psql_completion, where we've failed to find any useful completion.
If that instance of COMPLETE_WITH("") is implemented by complete_from_list
then readline will happily try to do filename completion :-(.
(I don't quite understand why we don't get the wiping-out behavior there;
maybe an empty-string result is treated differently from
not-empty-string?)

So I propose the attached instead, which doesn't get rid of
complete_from_const but ensures that it's only used in that one place.

This is independent of the other patch shown upthread.  I'm proposing
this one for HEAD only but would back-patch the other.

			regards, tom lane