psql UPDATE field [tab] expands to DEFAULT?

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

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/&gt;*
*https://demo.agency-software.org/client
<https://demo.agency-software.org/client&gt;*
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.

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

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

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

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

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

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

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/&gt;*
*https://demo.agency-software.org/client
<https://demo.agency-software.org/client&gt;*
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.

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

[ 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

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

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