Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[David Steele]

On 3/17/17 3:58 AM, Kyotaro HORIGUCHI wrote:
> At Mon, 13 Mar 2017 10:42:05 -0400, David Steele  wrote 
> in <1e8297fd-f7f2-feab-848d-5121e45c8...@pgmasters.net>
>> It has been a while since this thread has received any comments or a new
>> patch.  The general consensus seems to be that this feature is too large
>> a rewrite of tab completion considering a major rewrite was done for 9.6.
>>
>> Are you considering writing a localized patch for this feature as Tom
>> suggested?  If so, please post that by 2017-03-16 AoE.
>>
>> If no new patch is submitted by that date I will mark this submission
>> "Returned with Feedback".
> 
> It's a pity. I had to take a week's leave..
> 
> I understood that the 'localized fashion' means "without removing
> eles's", that is the core of this patch. So, if it is not
> acceptable this should be abandoned. I'll try put the inidividual
> enahancements other than refactoring in other shape, in the next
> commitfest.

I have marked this submission "Returned with Feedback".  Please feel
free to resubmit when you have a new version.

-- 
-David
da...@pgmasters.net


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

At Mon, 13 Mar 2017 10:42:05 -0400, David Steele  wrote in 
<1e8297fd-f7f2-feab-848d-5121e45c8...@pgmasters.net>
> It has been a while since this thread has received any comments or a new
> patch.  The general consensus seems to be that this feature is too large
> a rewrite of tab completion considering a major rewrite was done for 9.6.
> 
> Are you considering writing a localized patch for this feature as Tom
> suggested?  If so, please post that by 2017-03-16 AoE.
> 
> If no new patch is submitted by that date I will mark this submission
> "Returned with Feedback".

It's a pity. I had to take a week's leave..

I understood that the 'localized fashion' means "without removing
eles's", that is the core of this patch. So, if it is not
acceptable this should be abandoned. I'll try put the inidividual
enahancements other than refactoring in other shape, in the next
commitfest.

Thanks.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[David Steele]

Hello,

On 3/1/17 9:38 PM, Kyotaro HORIGUCHI wrote:

> At Tue, 28 Feb 2017 10:39:01 -0500, Stephen Frost  wrote 
> in <20170228153901.gh9...@tamriel.snowman.net>
>> * David Fetter (da...@fetter.org) wrote:
>>> On Mon, Feb 27, 2017 at 11:53:17PM -0500, Stephen Frost wrote:
 * Kyotaro HORIGUCHI (horiguchi.kyot...@lab.ntt.co.jp) wrote:
> I suppose it is for suggesting what kind of word should come
> there, or avoiding silence for a tab. Or for symmetry with other
> types of manipulation, like DROP. Another possibility is creating
> multiple objects with similar names, say CREATE TABLE employee_x1,
> CREATE TABLE employee_x2. Just trying to complete existing
> *schema* is one more another possible objective.

 I don't buy any of these arguments either.  I *really* don't want us
 going down some road where we try to make sure that hitting 'tab'
 never fails...
> 
> These suggestions exist before this patch. Whether to remove them
> would be another discussion. I was going to add some but finally
> I believe I have added no such things in this patchset.
> 
>>> Wouldn't that just be a correct, grammar-aware implementation of tab
>>> completion?  Why wouldn't you want that?
>>
>> No, it wouldn't, it would mean we have to provide something for cases
>> where it doesn't make sense to try and provide an answer, as being
>> discussed here for CREATE TABLE.
>>
>> We can't provide an answer based on tab-completion to what you want to
>> call your new table.
> 
> The difference seems to be that what we take this feature to
> be. If we see it as just a fast-path of entering a word where we
> know what words should come, silence is not a problem. If we see
> it as safety-wheels to guide users to the right way to go,
> silence would be bad. A silence during word completion suggests
> something wrong in the preceding words to me so it is a bit
> annoying.
> 
> As an analogous operation, mkdir on bash suggests existing
> directories. We can suggest existing tables for CREATE TABLE with
> the same basis.
> 
> Another possible way to go would be showing a 'suggestion' not a
> list of possibilities. If readline allows such operation, I
> imagine the following. But this is a quite diferrent discussion.
> 
> =# CREATE TABLE 
> <>
> =# CREATE TABLE table1 
> 
> regards,

It has been a while since this thread has received any comments or a new
patch.  The general consensus seems to be that this feature is too large
a rewrite of tab completion considering a major rewrite was done for 9.6.

Are you considering writing a localized patch for this feature as Tom
suggested?  If so, please post that by 2017-03-16 AoE.

If no new patch is submitted by that date I will mark this submission
"Returned with Feedback".

Thanks,
-- 
-David
da...@pgmasters.net


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Hello,

At Tue, 28 Feb 2017 10:39:01 -0500, Stephen Frost  wrote in 
<20170228153901.gh9...@tamriel.snowman.net>
> * David Fetter (da...@fetter.org) wrote:
> > On Mon, Feb 27, 2017 at 11:53:17PM -0500, Stephen Frost wrote:
> > > * Kyotaro HORIGUCHI (horiguchi.kyot...@lab.ntt.co.jp) wrote:
> > > > I suppose it is for suggesting what kind of word should come
> > > > there, or avoiding silence for a tab. Or for symmetry with other
> > > > types of manipulation, like DROP. Another possibility is creating
> > > > multiple objects with similar names, say CREATE TABLE employee_x1,
> > > > CREATE TABLE employee_x2. Just trying to complete existing
> > > > *schema* is one more another possible objective.
> > > 
> > > I don't buy any of these arguments either.  I *really* don't want us
> > > going down some road where we try to make sure that hitting 'tab'
> > > never fails...

These suggestions exist before this patch. Whether to remove them
would be another discussion. I was going to add some but finally
I believe I have added no such things in this patchset.

> > Wouldn't that just be a correct, grammar-aware implementation of tab
> > completion?  Why wouldn't you want that?
> 
> No, it wouldn't, it would mean we have to provide something for cases
> where it doesn't make sense to try and provide an answer, as being
> discussed here for CREATE TABLE.
> 
> We can't provide an answer based on tab-completion to what you want to
> call your new table.

The difference seems to be that what we take this feature to
be. If we see it as just a fast-path of entering a word where we
know what words should come, silence is not a problem. If we see
it as safety-wheels to guide users to the right way to go,
silence would be bad. A silence during word completion suggests
something wrong in the preceding words to me so it is a bit
annoying.

As an analogous operation, mkdir on bash suggests existing
directories. We can suggest existing tables for CREATE TABLE with
the same basis.

Another possible way to go would be showing a 'suggestion' not a
list of possibilities. If readline allows such operation, I
imagine the following. But this is a quite diferrent discussion.

=# CREATE TABLE 
<>
=# CREATE TABLE table1 

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Stephen Frost]

* David Fetter (da...@fetter.org) wrote:
> On Mon, Feb 27, 2017 at 11:53:17PM -0500, Stephen Frost wrote:
> > * Kyotaro HORIGUCHI (horiguchi.kyot...@lab.ntt.co.jp) wrote:
> > > I suppose it is for suggesting what kind of word should come
> > > there, or avoiding silence for a tab. Or for symmetry with other
> > > types of manipulation, like DROP. Another possibility is creating
> > > multiple objects with similar names, say CREATE TABLE employee_x1,
> > > CREATE TABLE employee_x2. Just trying to complete existing
> > > *schema* is one more another possible objective.
> > 
> > I don't buy any of these arguments either.  I *really* don't want us
> > going down some road where we try to make sure that hitting 'tab'
> > never fails...
> 
> Wouldn't that just be a correct, grammar-aware implementation of tab
> completion?  Why wouldn't you want that?

No, it wouldn't, it would mean we have to provide something for cases
where it doesn't make sense to try and provide an answer, as being
discussed here for CREATE TABLE.

We can't provide an answer based on tab-completion to what you want to
call your new table.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[David Fetter]

On Mon, Feb 27, 2017 at 11:53:17PM -0500, Stephen Frost wrote:
> * Kyotaro HORIGUCHI (horiguchi.kyot...@lab.ntt.co.jp) wrote:
> > I suppose it is for suggesting what kind of word should come
> > there, or avoiding silence for a tab. Or for symmetry with other
> > types of manipulation, like DROP. Another possibility is creating
> > multiple objects with similar names, say CREATE TABLE employee_x1,
> > CREATE TABLE employee_x2. Just trying to complete existing
> > *schema* is one more another possible objective.
> 
> I don't buy any of these arguments either.  I *really* don't want us
> going down some road where we try to make sure that hitting 'tab'
> never fails...

Wouldn't that just be a correct, grammar-aware implementation of tab
completion?  Why wouldn't you want that?

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Stephen Frost]

* Kyotaro HORIGUCHI (horiguchi.kyot...@lab.ntt.co.jp) wrote:
> I suppose it is for suggesting what kind of word should come
> there, or avoiding silence for a tab. Or for symmetry with other
> types of manipulation, like DROP. Another possibility is creating
> multiple objects with similar names, say CREATE TABLE
> employee_x1, CREATE TABLE employee_x2. Just trying to complete
> existing *schema* is one more another possible objective.

I don't buy any of these arguments either.  I *really* don't want us
going down some road where we try to make sure that hitting 'tab' never
fails...

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

At Mon, 27 Feb 2017 10:43:39 +0900, Michael Paquier  
wrote in 
> On Mon, Feb 27, 2017 at 10:20 AM, Tom Lane  wrote:
> > Michael Paquier  writes:
> >> On Mon, Feb 27, 2017 at 10:12 AM, Tom Lane  wrote:
> >>> BTW ... can anyone explain to me the reason why we offer to complete
> >>> CREATE OBJECT with the names of existing objects of that kind?
> >
> >> Isn't that to facilitate commands appended after CREATE SCHEMA? Say
> >> table foo is in schema1, and creating it in schema2 gets easier with
> >> tab completion?
> >
> > Seems like pretty much of a stretch.  I've never done anything like
> > that, have you?
> 
> Never, but that was the only reason I could think about. I recall
> reading something else on -hackers but I cannot put my finger on it,
> nor does a lookup at the archives help... Perhaps that's the one I
> just mentioned as well.

I suppose it is for suggesting what kind of word should come
there, or avoiding silence for a tab. Or for symmetry with other
types of manipulation, like DROP. Another possibility is creating
multiple objects with similar names, say CREATE TABLE
employee_x1, CREATE TABLE employee_x2. Just trying to complete
existing *schema* is one more another possible objective.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Michael Paquier]

On Mon, Feb 27, 2017 at 10:20 AM, Tom Lane  wrote:
> Michael Paquier  writes:
>> On Mon, Feb 27, 2017 at 10:12 AM, Tom Lane  wrote:
>>> BTW ... can anyone explain to me the reason why we offer to complete
>>> CREATE OBJECT with the names of existing objects of that kind?
>
>> Isn't that to facilitate commands appended after CREATE SCHEMA? Say
>> table foo is in schema1, and creating it in schema2 gets easier with
>> tab completion?
>
> Seems like pretty much of a stretch.  I've never done anything like
> that, have you?

Never, but that was the only reason I could think about. I recall
reading something else on -hackers but I cannot put my finger on it,
nor does a lookup at the archives help... Perhaps that's the one I
just mentioned as well.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Tom Lane]

Michael Paquier  writes:
> On Mon, Feb 27, 2017 at 10:12 AM, Tom Lane  wrote:
>> BTW ... can anyone explain to me the reason why we offer to complete
>> CREATE OBJECT with the names of existing objects of that kind?

> Isn't that to facilitate commands appended after CREATE SCHEMA? Say
> table foo is in schema1, and creating it in schema2 gets easier with
> tab completion?

Seems like pretty much of a stretch.  I've never done anything like
that, have you?

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Michael Paquier]

On Mon, Feb 27, 2017 at 10:12 AM, Tom Lane  wrote:
> Michael Paquier  writes:
>> On Mon, Feb 27, 2017 at 5:21 AM, Tom Lane  wrote:
>>> So I'd be a whole lot happier if it didn't do that.  Can we really not
>>> add the desired features in a more localized fashion?
>
>> As "if not exists" is defined after the object type if would not be
>> that complicated to add completion for IE/INE after the object type
>> with a set of THING_* flags in words_after_create. One missing piece
>> would be to add completion for the objects themselves after IE or INE
>> have been entered by the user, but I would think that tweaking the
>> checks on words_after_create[i] would be doable as well. And that
>> would be localized.
>
> BTW ... can anyone explain to me the reason why we offer to complete
> CREATE OBJECT with the names of existing objects of that kind?
> That seems pretty darn stupid.  I can see offering the names of existing
> schemas there, if the object type is one that has schema-qualified names,
> but completing with an existing object name is just setting up to fail
> isn't it?

Isn't that to facilitate commands appended after CREATE SCHEMA? Say
table foo is in schema1, and creating it in schema2 gets easier with
tab completion?
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Tom Lane]

Michael Paquier  writes:
> On Mon, Feb 27, 2017 at 5:21 AM, Tom Lane  wrote:
>> So I'd be a whole lot happier if it didn't do that.  Can we really not
>> add the desired features in a more localized fashion?

> As "if not exists" is defined after the object type if would not be
> that complicated to add completion for IE/INE after the object type
> with a set of THING_* flags in words_after_create. One missing piece
> would be to add completion for the objects themselves after IE or INE
> have been entered by the user, but I would think that tweaking the
> checks on words_after_create[i] would be doable as well. And that
> would be localized.

BTW ... can anyone explain to me the reason why we offer to complete
CREATE OBJECT with the names of existing objects of that kind?
That seems pretty darn stupid.  I can see offering the names of existing
schemas there, if the object type is one that has schema-qualified names,
but completing with an existing object name is just setting up to fail
isn't it?

If we dropped that behavior, seems like it would become much easier
to plug in IF NOT EXISTS at those spots.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Michael Paquier]

On Mon, Feb 27, 2017 at 5:21 AM, Tom Lane  wrote:
> Robert Haas  writes:
>> Yeah, maybe, but we'd need a committer to take more of an interest in
>> this patch series.  Personally, I'm wondering why we need a series of
>> 19 patches to add tab completion support for IF NOT EXISTS.  The
>> feature which is the subject of this thread arrives in patch 0017, and
>> a lot of the patches which come before that seem to change a lot of
>> stuff without actually improving much that would really benefit users.
>
> FWIW, one reason this committer hasn't jumped in is that we already
> rewrote tab-complete.c pretty completely in 9.6.  If we accept a patch
> that completely rewrites it again, we're going to be faced with
> maintaining three fundamentally different implementations for the next
> three-plus years (until 9.5 dies).  Admittedly, we don't back-patch
> fixes in tab-complete.c every week, but a look at the git history says
> we do need to do that several times a year.

Indeed, having worked on the 9.6 refactoring a bit as well... I'll
vote for not doing this again as HEAD is in a more readable shape
compared to the pre-9.5 area, and I am not convinced that it is worth
the trouble. There are a couple of things that can be extracted from
this set of patches, but I would vote for not doing the same level of
refactoring.

> Also, the nature of the primary refactoring (changing the big else-chain
> into standalone ifs, if I read it correctly) is particularly bad from a
> back-patching standpoint because all you have to do is insert an "else",
> or fail to insert one, to silently and almost completely break either
> one or the other branch.  And I don't really understand why that's a good
> idea anyway: surely we can return at most one set of completions, so how
> is turning the function into a lot of independent actions a win?
>
> So I'd be a whole lot happier if it didn't do that.  Can we really not
> add the desired features in a more localized fashion?

As "if not exists" is defined after the object type if would not be
that complicated to add completion for IE/INE after the object type
with a set of THING_* flags in words_after_create. One missing piece
would be to add completion for the objects themselves after IE or INE
have been entered by the user, but I would think that tweaking the
checks on words_after_create[i] would be doable as well. And that
would be localized.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Tom Lane]

Robert Haas  writes:
> Yeah, maybe, but we'd need a committer to take more of an interest in
> this patch series.  Personally, I'm wondering why we need a series of
> 19 patches to add tab completion support for IF NOT EXISTS.  The
> feature which is the subject of this thread arrives in patch 0017, and
> a lot of the patches which come before that seem to change a lot of
> stuff without actually improving much that would really benefit users.

FWIW, one reason this committer hasn't jumped in is that we already
rewrote tab-complete.c pretty completely in 9.6.  If we accept a patch
that completely rewrites it again, we're going to be faced with
maintaining three fundamentally different implementations for the next
three-plus years (until 9.5 dies).  Admittedly, we don't back-patch
fixes in tab-complete.c every week, but a look at the git history says
we do need to do that several times a year.

Also, the nature of the primary refactoring (changing the big else-chain
into standalone ifs, if I read it correctly) is particularly bad from a
back-patching standpoint because all you have to do is insert an "else",
or fail to insert one, to silently and almost completely break either
one or the other branch.  And I don't really understand why that's a good
idea anyway: surely we can return at most one set of completions, so how
is turning the function into a lot of independent actions a win?

So I'd be a whole lot happier if it didn't do that.  Can we really not
add the desired features in a more localized fashion?

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

2017-02-26 19:43 GMT+01:00 Robert Haas :

> On Wed, Feb 22, 2017 at 12:38 AM, Pavel Stehule 
> wrote:
> > Now first patch is broken :(
> >
> > It is pretty sensitive to any changes. Isn't possible to commit first
> four
> > patches first and separately maybe out of commitfest window?
>
> Yeah, maybe, but we'd need a committer to take more of an interest in
> this patch series.  Personally, I'm wondering why we need a series of
> 19 patches to add tab completion support for IF NOT EXISTS.  The
> feature which is the subject of this thread arrives in patch 0017, and
> a lot of the patches which come before that seem to change a lot of
> stuff without actually improving much that would really benefit users.
>
> 0001 seems like a lot of churn for no real benefit that I can immediately
> see.
> 0002 is a real feature, and probably a good one, though unrelated to
> the subject of this thread.  In the process, it changes many lines of
> code in fairly mechanical ways; does it need to do that?
> 0003 is infrastructure.
> 0004 adds a README.  Do we really need that?  It seems to be
> explaining things which are mostly fairly clear from just looking at
> the code.  If we add a README, we have to update it when we change
> things.  That's worthwhile if it helps people write code better, I'm
> not sure if it will do that.
>

it needs a separation to refactoring part and to new features part. The
refactoring looks well and I am sure so has sense.

about README - there are described fundamental things - that should be
stable. With last changes and this set of patches, the autocomplete is not
trivial and I am sure, so any documentation is better than nothing. Not all
developers has years of experience with PostgreSQL hacking.



> 0005 extends 0002.
> 0006 prevents incorrect completions in obscure circumstances.
> 0007 adds some kind of tab completion for CREATE RULE; I'm fuzzy on the
> details.
> 0008 improves tab completion after EXPLAIN.
> 0009-0014 uses the infrastructure from 0003 to improve tab completion
> for various commands.  They say they're merely simplifying tab
> completion for those things, but actually they're extending it to some
> obscure situations that aren't currently covered.
> 0015 adds completion for magic keywords like CURRENT_USER when role
> commands are used.
> 0016 refactors tab completion for ALTER DEFAULT PRIVILEGES, possibly
> improving it somehow.
> 0017 implements the titular feature.
> 0018 adds optional debugging output.
> 0019 improves things for CREATE OR REPLACE completion.
>
> Phew.  That's a lot of work for relatively obscure improvements to tab
> completion.  I grant that the result is probably better, but it's a
> lot of code change for what we get out of it.  I'm not saying we
> should reject it on that basis, but it may be the reason why nobody's
> jumped in to work on getting this committed.
>

These patches are big - but in the end it cleaning tab complete code, and
open a doors for more smarter completion.

Some features can be interesting for users too - repeated writing IF
EXISTS, IF NOT EXISTS or OR REPLACE is really scary - mainly so some other
parts of tab complete are friendly enough now.

Can be solution a splitting this set of patches to more independent parts?
We should to start with refactoring. Other patches can be processed
individually - with individual discussion.

Regards

Pavel



>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Robert Haas]

On Wed, Feb 22, 2017 at 12:38 AM, Pavel Stehule  wrote:
> Now first patch is broken :(
>
> It is pretty sensitive to any changes. Isn't possible to commit first four
> patches first and separately maybe out of commitfest window?

Yeah, maybe, but we'd need a committer to take more of an interest in
this patch series.  Personally, I'm wondering why we need a series of
19 patches to add tab completion support for IF NOT EXISTS.  The
feature which is the subject of this thread arrives in patch 0017, and
a lot of the patches which come before that seem to change a lot of
stuff without actually improving much that would really benefit users.

0001 seems like a lot of churn for no real benefit that I can immediately see.
0002 is a real feature, and probably a good one, though unrelated to
the subject of this thread.  In the process, it changes many lines of
code in fairly mechanical ways; does it need to do that?
0003 is infrastructure.
0004 adds a README.  Do we really need that?  It seems to be
explaining things which are mostly fairly clear from just looking at
the code.  If we add a README, we have to update it when we change
things.  That's worthwhile if it helps people write code better, I'm
not sure if it will do that.
0005 extends 0002.
0006 prevents incorrect completions in obscure circumstances.
0007 adds some kind of tab completion for CREATE RULE; I'm fuzzy on the details.
0008 improves tab completion after EXPLAIN.
0009-0014 uses the infrastructure from 0003 to improve tab completion
for various commands.  They say they're merely simplifying tab
completion for those things, but actually they're extending it to some
obscure situations that aren't currently covered.
0015 adds completion for magic keywords like CURRENT_USER when role
commands are used.
0016 refactors tab completion for ALTER DEFAULT PRIVILEGES, possibly
improving it somehow.
0017 implements the titular feature.
0018 adds optional debugging output.
0019 improves things for CREATE OR REPLACE completion.

Phew.  That's a lot of work for relatively obscure improvements to tab
completion.  I grant that the result is probably better, but it's a
lot of code change for what we get out of it.  I'm not saying we
should reject it on that basis, but it may be the reason why nobody's
jumped in to work on getting this committed.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

2017-02-14 11:51 GMT+01:00 Kyotaro HORIGUCHI <
horiguchi.kyot...@lab.ntt.co.jp>:

> Thank you for the comment.
>
> At Mon, 6 Feb 2017 17:10:43 +0100, Pavel Stehule 
> wrote in  mail.gmail.com>
> > > 0001-Refactoring-tab-complete-to-make-psql_completion-cod.patch
> > >   - Just a refactoring of psql_completion
> > >
> > > 0002-Make-keywords-case-follow-to-input.patch
> > >   - The letter case of additional suggestions for
> > > COMPLETION_WITH_XX follows input.
> > >
> > > 0003-Introduce-word-shift-and-removal-feature-to-psql-com.patch
> > >   - A feature to ignore preceding words. And a feature to remove
> > > intermediate words.
> > >
> > > 0004-Add-README-for-tab-completion.patch
> > >   - README
> > >
> > > 0005-Make-SET-RESET-SHOW-varialble-follow-input-letter-ca.patch
> > > 0006-Allow-complete-schema-elements-in-more-natural-way.patch
> > > 0007-Allow-CREATE-RULE-to-use-command-completion-recursiv.patch
> > > 0008-Allow-completing-the-body-of-EXPLAIN.patch
> > > 0009-Simpilfy-ALTER-TABLE-ALTER-COLUMN-completion.patch
> > > 0010-Simplify-completion-for-CLUSTER-VERBOSE.patch
> > > 0011-Simplify-completion-for-COPY.patch
> > > 0012-Simplify-completion-for-CREATE-INDEX.patch
> > > 0013-Simplify-completion-for-CREATE-SEQUENCE.patch
> > > 0014-Simplify-completion-for-DROP-INDEX.patch
> > > 0015-Add-CURRENT_USER-to-some-completions-of-role.patch
> > > 0016-Refactor-completion-for-ALTER-DEFAULT-PRIVILEGES.patch
> > > 0017-Add-suggestions-of-IF-NOT-EXISTS.patch
> > >   - A kind of sample refctoring (or augmenting) suggestion code
> > > based on the new infrastructure.
> > >
> > > 0018-Debug-output-of-psql-completion.patch
> > >   - Debug logging for psql_completion (described above)
> > >
> > > 0019-Add-suggestion-of-OR-REPLACE.patch
> > >   - Suggestion of CREATE OR REPLACE.
> > >
> > >
> > > # I hear the footsteps of another conflict..
> > >
> > The patch 0018 was not be applied.
>
> The fear came true. fd6cd69 conflicts with it but on a
> comment. The attached patch set applies on top of the current
> master head (ae0e550).
>

Now first patch is broken :(

It is pretty sensitive to any changes. Isn't possible to commit first four
patches first and separately maybe out of commitfest window?



>
> > Few other notes from testing - probably these notes should not be related
> > to your patch set
> >
> > 1. When we have set of keywords, then the upper or lower chars should to
> > follow previous keyword. Is it possible? It should to have impact only on
> > keywords.
>
> It sounds reasonable, more flexible than "upper"/"lower" of
> COMP_KEYWORD_CASE. The additional 20th(!) patch does that. It
> adds a new value 'follow-first' to COMP_KEYWORD_CASE. All
> keywords in a command line will be in the case of the first
> letter of the first word. ("CREATE" in the following case. I
> think it is enogh for the purpose.)
>
> postgres=# \set COMP_KEYWORD_CASE follow-first
> postgres=# CREATE in
>=># CREATE INDEX hoge 
>=># CREATE INDEX hoge ON emp
>=># CREATE INDEX hoge ON employee ..
> postgres=# create IN
>=># create index
>
> Typing tab at the first in a command line shows all available
> keywords in upper case.
>

It is great - from my perspective the best step in last years in this area.


>
> > 2. the list of possible functions after EXECUTE PROCEDURE in CREATE
> TRIGGER
> > statement should be reduced to trigger returns function only.
>
> Actually Query_for_list_of_trigger_functions returns too many
> candidates. The suggested restriction reduces them to a
> reasonable number. The 21th patch does that.
>
> > CREATE OR REPLACE FUNCTIONS works great, thank you!
>
> Thanks. It was easier than expected.
>
> As the result, 21 paches are attached to this message. 1 - 19th
> are described above and others are described below.
>
> 0020-New-COMP_KEYWORD_CASE-mode-follow-first.patch
>
>   - Add new COMP_KEYWORD_CASE mode "follow-first". The completion
> works with the case of the first word. (This doesn't rely on
> this patchset but works in more cases with 0002)
>
> 0021-Suggest-only-trigger-functions-for-CREAET-TRIGGER.EX.patch
>   - Restrict suggestion for the syntax to ones acutually usable
> there. (This relies on none of this patchset, though..)
>
> regards,
>

Thank you very much for this your work.

Regards

Pavel


>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Thank you for the comment.

At Mon, 6 Feb 2017 17:10:43 +0100, Pavel Stehule  
wrote in 
> > 0001-Refactoring-tab-complete-to-make-psql_completion-cod.patch
> >   - Just a refactoring of psql_completion
> >
> > 0002-Make-keywords-case-follow-to-input.patch
> >   - The letter case of additional suggestions for
> > COMPLETION_WITH_XX follows input.
> >
> > 0003-Introduce-word-shift-and-removal-feature-to-psql-com.patch
> >   - A feature to ignore preceding words. And a feature to remove
> > intermediate words.
> >
> > 0004-Add-README-for-tab-completion.patch
> >   - README
> >
> > 0005-Make-SET-RESET-SHOW-varialble-follow-input-letter-ca.patch
> > 0006-Allow-complete-schema-elements-in-more-natural-way.patch
> > 0007-Allow-CREATE-RULE-to-use-command-completion-recursiv.patch
> > 0008-Allow-completing-the-body-of-EXPLAIN.patch
> > 0009-Simpilfy-ALTER-TABLE-ALTER-COLUMN-completion.patch
> > 0010-Simplify-completion-for-CLUSTER-VERBOSE.patch
> > 0011-Simplify-completion-for-COPY.patch
> > 0012-Simplify-completion-for-CREATE-INDEX.patch
> > 0013-Simplify-completion-for-CREATE-SEQUENCE.patch
> > 0014-Simplify-completion-for-DROP-INDEX.patch
> > 0015-Add-CURRENT_USER-to-some-completions-of-role.patch
> > 0016-Refactor-completion-for-ALTER-DEFAULT-PRIVILEGES.patch
> > 0017-Add-suggestions-of-IF-NOT-EXISTS.patch
> >   - A kind of sample refctoring (or augmenting) suggestion code
> > based on the new infrastructure.
> >
> > 0018-Debug-output-of-psql-completion.patch
> >   - Debug logging for psql_completion (described above)
> >
> > 0019-Add-suggestion-of-OR-REPLACE.patch
> >   - Suggestion of CREATE OR REPLACE.
> >
> >
> > # I hear the footsteps of another conflict..
> >
> The patch 0018 was not be applied.

The fear came true. fd6cd69 conflicts with it but on a
comment. The attached patch set applies on top of the current
master head (ae0e550).

> Few other notes from testing - probably these notes should not be related
> to your patch set
> 
> 1. When we have set of keywords, then the upper or lower chars should to
> follow previous keyword. Is it possible? It should to have impact only on
> keywords.

It sounds reasonable, more flexible than "upper"/"lower" of
COMP_KEYWORD_CASE. The additional 20th(!) patch does that. It
adds a new value 'follow-first' to COMP_KEYWORD_CASE. All
keywords in a command line will be in the case of the first
letter of the first word. ("CREATE" in the following case. I
think it is enogh for the purpose.)

postgres=# \set COMP_KEYWORD_CASE follow-first
postgres=# CREATE in
   =># CREATE INDEX hoge 
   =># CREATE INDEX hoge ON emp
   =># CREATE INDEX hoge ON employee ..
postgres=# create IN
   =># create index

Typing tab at the first in a command line shows all available
keywords in upper case.

> 2. the list of possible functions after EXECUTE PROCEDURE in CREATE TRIGGER
> statement should be reduced to trigger returns function only.

Actually Query_for_list_of_trigger_functions returns too many
candidates. The suggested restriction reduces them to a
reasonable number. The 21th patch does that.

> CREATE OR REPLACE FUNCTIONS works great, thank you!

Thanks. It was easier than expected.

As the result, 21 paches are attached to this message. 1 - 19th
are described above and others are described below.

0020-New-COMP_KEYWORD_CASE-mode-follow-first.patch

  - Add new COMP_KEYWORD_CASE mode "follow-first". The completion
works with the case of the first word. (This doesn't rely on
this patchset but works in more cases with 0002)

0021-Suggest-only-trigger-functions-for-CREAET-TRIGGER.EX.patch
  - Restrict suggestion for the syntax to ones acutually usable
there. (This relies on none of this patchset, though..)

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center


if_not_exists_20170214.tar.gz
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

Hi

2017-02-03 9:17 GMT+01:00 Kyotaro HORIGUCHI :

> Hello. This is the new version of this patch.
>
> - Rebased to the current master (555494d)
>   PUBLICATION/SUBSCRIPTION stuff conflicted.
>
> - Fix a bug of CREATE INDEX(0012-Simplify-completion-for-CREATE-INDEX.
> patch).
>   CREATE INDEX ON no longer gets a suggestion of "ON".
>
> - Added logging feature (0018-Debug-output-of-psql-completion.patch)
>
>   This might be suitable to be a separate patch. psql completion
>   code is defficult to debug when it is uncertain what line did a
>   suggestion. This patch allows completion logs to psql log,
>   which is activated by -L option.
>
>   psql -L  
>
>   And the logs like the following will be printed.
>
>   | completion performed at tab-complete.c:1146 for "do"
>
> - OR REPLACE suggestion (0019-Add-suggestion-of-OR-REPLACE.patch)
>
> At Wed, 1 Feb 2017 09:42:54 +0100, Pavel Stehule 
> wrote in  jgbu2trrs...@mail.gmail.com>
> > 2017-02-01 9:37 GMT+01:00 Kyotaro HORIGUCHI <
> horiguchi.kyot...@lab.ntt.co.jp
> > the content of my last mail is a copy my mail from end of December.
> > Probably lot of changes there.
>
> Thanks for reposting.
>
> > > > 2. tab complete doesn't work well if I am manually writing "create
> index
> > > > on" - second "ON" is appended - it is a regression
> > >
> > > I'll fix it in the version.
> > >
> > > > I didn't find any other issues -
> > > >
> > > > note: not necessary to implement (nice to have) - I miss a support
> for OR
> > > > REPLACE flag - it is related to LANGUAGE, TRANSFORMATION,  FUNCTION
> and
> > > > RULE.
>
> Hmm. This patch perhaps should not 'add a feature' (how about the
> logging..). Anyway the last 19th patch does that.  The word
> removal framework works well for this case.
>
> After all, this patch is so large that I'd like to attach them as
> one compressed file. The content of the file is the following.
>
>
> 0001-Refactoring-tab-complete-to-make-psql_completion-cod.patch
>   - Just a refactoring of psql_completion
>
> 0002-Make-keywords-case-follow-to-input.patch
>   - The letter case of additional suggestions for
> COMPLETION_WITH_XX follows input.
>
> 0003-Introduce-word-shift-and-removal-feature-to-psql-com.patch
>   - A feature to ignore preceding words. And a feature to remove
> intermediate words.
>
> 0004-Add-README-for-tab-completion.patch
>   - README
>
> 0005-Make-SET-RESET-SHOW-varialble-follow-input-letter-ca.patch
> 0006-Allow-complete-schema-elements-in-more-natural-way.patch
> 0007-Allow-CREATE-RULE-to-use-command-completion-recursiv.patch
> 0008-Allow-completing-the-body-of-EXPLAIN.patch
> 0009-Simpilfy-ALTER-TABLE-ALTER-COLUMN-completion.patch
> 0010-Simplify-completion-for-CLUSTER-VERBOSE.patch
> 0011-Simplify-completion-for-COPY.patch
> 0012-Simplify-completion-for-CREATE-INDEX.patch
> 0013-Simplify-completion-for-CREATE-SEQUENCE.patch
> 0014-Simplify-completion-for-DROP-INDEX.patch
> 0015-Add-CURRENT_USER-to-some-completions-of-role.patch
> 0016-Refactor-completion-for-ALTER-DEFAULT-PRIVILEGES.patch
> 0017-Add-suggestions-of-IF-NOT-EXISTS.patch
>   - A kind of sample refctoring (or augmenting) suggestion code
> based on the new infrastructure.
>
> 0018-Debug-output-of-psql-completion.patch
>   - Debug logging for psql_completion (described above)
>
> 0019-Add-suggestion-of-OR-REPLACE.patch
>   - Suggestion of CREATE OR REPLACE.
>
>
> # I hear the footsteps of another conflict..
>
>
The patch 0018 was not be applied.

Few other notes from testing - probably these notes should not be related
to your patch set

1. When we have set of keywords, then the upper or lower chars should to
follow previous keyword. Is it possible? It should to have impact only on
keywords.

2. the list of possible functions after EXECUTE PROCEDURE in CREATE TRIGGER
statement should be reduced to trigger returns function only.

CREATE OR REPLACE FUNCTIONS works great, thank you!

Regards

Pavel








> regards,
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Hello. This is the new version of this patch. 

- Rebased to the current master (555494d)
  PUBLICATION/SUBSCRIPTION stuff conflicted.

- Fix a bug of CREATE INDEX(0012-Simplify-completion-for-CREATE-INDEX.patch).
  CREATE INDEX ON no longer gets a suggestion of "ON".

- Added logging feature (0018-Debug-output-of-psql-completion.patch)

  This might be suitable to be a separate patch. psql completion
  code is defficult to debug when it is uncertain what line did a
  suggestion. This patch allows completion logs to psql log,
  which is activated by -L option.

  psql -L  

  And the logs like the following will be printed.

  | completion performed at tab-complete.c:1146 for "do"

- OR REPLACE suggestion (0019-Add-suggestion-of-OR-REPLACE.patch)

At Wed, 1 Feb 2017 09:42:54 +0100, Pavel Stehule  
wrote in 
> 2017-02-01 9:37 GMT+01:00 Kyotaro HORIGUCHI  the content of my last mail is a copy my mail from end of December.
> Probably lot of changes there.

Thanks for reposting.

> > > 2. tab complete doesn't work well if I am manually writing "create index
> > > on" - second "ON" is appended - it is a regression
> >
> > I'll fix it in the version.
> >
> > > I didn't find any other issues -
> > >
> > > note: not necessary to implement (nice to have) - I miss a support for OR
> > > REPLACE flag - it is related to LANGUAGE, TRANSFORMATION,  FUNCTION and
> > > RULE.

Hmm. This patch perhaps should not 'add a feature' (how about the
logging..). Anyway the last 19th patch does that.  The word
removal framework works well for this case.

After all, this patch is so large that I'd like to attach them as
one compressed file. The content of the file is the following.


0001-Refactoring-tab-complete-to-make-psql_completion-cod.patch
  - Just a refactoring of psql_completion

0002-Make-keywords-case-follow-to-input.patch
  - The letter case of additional suggestions for
COMPLETION_WITH_XX follows input.

0003-Introduce-word-shift-and-removal-feature-to-psql-com.patch
  - A feature to ignore preceding words. And a feature to remove
intermediate words.

0004-Add-README-for-tab-completion.patch
  - README

0005-Make-SET-RESET-SHOW-varialble-follow-input-letter-ca.patch
0006-Allow-complete-schema-elements-in-more-natural-way.patch
0007-Allow-CREATE-RULE-to-use-command-completion-recursiv.patch
0008-Allow-completing-the-body-of-EXPLAIN.patch
0009-Simpilfy-ALTER-TABLE-ALTER-COLUMN-completion.patch
0010-Simplify-completion-for-CLUSTER-VERBOSE.patch
0011-Simplify-completion-for-COPY.patch
0012-Simplify-completion-for-CREATE-INDEX.patch
0013-Simplify-completion-for-CREATE-SEQUENCE.patch
0014-Simplify-completion-for-DROP-INDEX.patch
0015-Add-CURRENT_USER-to-some-completions-of-role.patch
0016-Refactor-completion-for-ALTER-DEFAULT-PRIVILEGES.patch
0017-Add-suggestions-of-IF-NOT-EXISTS.patch
  - A kind of sample refctoring (or augmenting) suggestion code
based on the new infrastructure.

0018-Debug-output-of-psql-completion.patch
  - Debug logging for psql_completion (described above)

0019-Add-suggestion-of-OR-REPLACE.patch
  - Suggestion of CREATE OR REPLACE.


# I hear the footsteps of another conflict..

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center


if_not_exists_20170203.tar.gz
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

2017-02-01 9:37 GMT+01:00 Kyotaro HORIGUCHI :

> Thank you for reviewing.
>
> At Tue, 31 Jan 2017 11:28:17 +0100, Pavel Stehule 
> wrote in 

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Thank you for reviewing.

At Tue, 31 Jan 2017 11:28:17 +0100, Pavel Stehule  
wrote in 

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

2017-01-31 11:10 GMT+01:00 Kyotaro HORIGUCHI <
horiguchi.kyot...@lab.ntt.co.jp>:

> At Tue, 31 Jan 2017 15:07:55 +0900, Michael Paquier <
> michael.paqu...@gmail.com> wrote in  8v0X6A4gKQb2Uc=mc+...@mail.gmail.com>
> > On Tue, Jan 31, 2017 at 2:58 PM, Pavel Stehule 
> wrote:
> > > I found a error - I sent mail only to author 2016-12-31 :( - It is my
> > > mistake. I am sorry
> >
> > Ah... Thanks for the update. No problem.
>
> Ouch. Sorry for missing you commnet. I'll dig my mail box for that.
>



I am doing a review of this set of patches:

1. There are no problem with patching, compiling - all regress tests passed

2. tab complete doesn't work well if I am manually writing "create index
on" - second "ON" is appended - it is a regression

I didn't find any other issues -

note: not necessary to implement (nice to have) - I miss a support for OR
REPLACE flag - it is related to LANGUAGE, TRANSFORMATION,  FUNCTION and
RULE.

Regards


>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>
>
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

At Tue, 31 Jan 2017 15:07:55 +0900, Michael Paquier  
wrote in 

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Michael Paquier]

On Tue, Jan 31, 2017 at 2:58 PM, Pavel Stehule  wrote:
> I found a error - I sent mail only to author 2016-12-31 :( - It is my
> mistake. I am sorry

Ah... Thanks for the update. No problem.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

2017-01-31 6:56 GMT+01:00 Michael Paquier :

> On Tue, Jan 31, 2017 at 2:53 PM, Pavel Stehule 
> wrote:
> > I tested new set of these patches and I found some regressions there -
> > mentioned in my last mail.
> >
> > Maybe I miss new update, bit I don't know about it.
>
> The last update I am aware of is that saying: "lot of patches. I hope
> I look on these patches this week.". Here is the message:
> https://www.postgresql.org/message-id/CAFj8pRD2qq6v0jm6kqmWMwo-
> yNSvn8Vvf+m=DBy3ps=y0_3...@mail.gmail.com
>
> And there is no sign of reviews on the new versions.
>

I found a error - I sent mail only to author 2016-12-31 :( - It is my
mistake. I am sorry



> --
> Michael
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

>
>
>
> 2017-01-31 6:51 GMT+01:00 Michael Paquier :
>
>>
>>
>> The current patch status was "waiting on author", but that's incorrect
>> as a new series of this patch has been sent. Please be careful with
>> the status of the CF app! I am moving it to next CF with "needs
>> review". Gerdan Santos has marked himself as a reviewer of the patch
>> but I saw no activity, so I have removed his name to not confuse
>> people looking for patches to review (that happens!).
>>
>
> I tested new set of these patches and I found some regressions there -
> mentioned in my last mail.
>
> Maybe I miss new update, bit I don't know about it.
>

Some infrastructure related patches (maybe 50%) from this set can be
committed now - the moving complete set of patches to next commitfest
generates generates lot of repeated useless work.

Regards

Pavel


> Regards
>
> Pavel
>
>> --
>> Michael
>>
>
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Michael Paquier]

On Tue, Jan 31, 2017 at 2:53 PM, Pavel Stehule  wrote:
> I tested new set of these patches and I found some regressions there -
> mentioned in my last mail.
>
> Maybe I miss new update, bit I don't know about it.

The last update I am aware of is that saying: "lot of patches. I hope
I look on these patches this week.". Here is the message:
https://www.postgresql.org/message-id/CAFj8pRD2qq6v0jm6kqmWMwo-yNSvn8Vvf+m=DBy3ps=y0_3...@mail.gmail.com

And there is no sign of reviews on the new versions.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

2017-01-31 6:51 GMT+01:00 Michael Paquier :

> On Tue, Dec 27, 2016 at 12:23 PM, Kyotaro HORIGUCHI
>  wrote:
> > Hello,
> >
> > At Mon, 26 Dec 2016 14:24:33 +0100, Pavel Stehule <
> pavel.steh...@gmail.com> wrote in  yNSvn8Vvf+m=DBy3ps=y0_3...@mail.gmail.com>
> > pavel.stehule> 2016-12-26 9:40 GMT+01:00 Kyotaro HORIGUCHI <
> horiguchi.kyot...@lab.ntt.co.jp
> >> >:
> >>
> >> > > Thanks for reviewing but I ran out of time for this CF..
> >> > >
> >> > > I'm going to move this to the next CF.
> >> >
> >> > I splitted the patch into small pieces. f3fd531 conflicted to
> >> > this so rebased onto the current master HEAD.
> >> >
> >> > 0001 is psql_completion refactoring.
> >> > 0002-0003 are patches prividing new infrastructures.
> >> > 0004 is README for the infrastructures.
> >> > 0005 is letter-case correction of SET/RESET/SHOW using 0002.
> >> > 0006-0008 are improvements of recursive syntaxes using 0001 and 0004.
> >> > 0009-0016 are simplifying (maybe) completion code per syntax.
> >> >
> >> > The last one (0017) is the IF(NOT)EXIST modifications. It
> >> > suggests if(not)exists for syntaxes already gets object
> >> > suggestion. So some kind of objects like operator, cast and so
> >> > don't get an if.. suggestion. Likewise, I intentionally didn't
> >> > modified siggestions for "TEXT SEARCH *".
> >> >
> >> >
> >> lot of patches. I hope I look on these patches this week.
> >
> > Thank you for looking this and sorry for the many files. But I
> > hople that they would be far easier to read.
>
> The current patch status was "waiting on author", but that's incorrect
> as a new series of this patch has been sent. Please be careful with
> the status of the CF app! I am moving it to next CF with "needs
> review". Gerdan Santos has marked himself as a reviewer of the patch
> but I saw no activity, so I have removed his name to not confuse
> people looking for patches to review (that happens!).
>

I tested new set of these patches and I found some regressions there -
mentioned in my last mail.

Maybe I miss new update, bit I don't know about it.

Regards

Pavel

> --
> Michael
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Michael Paquier]

On Tue, Dec 27, 2016 at 12:23 PM, Kyotaro HORIGUCHI
 wrote:
> Hello,
>
> At Mon, 26 Dec 2016 14:24:33 +0100, Pavel Stehule  
> wrote in 
> pavel.stehule> 2016-12-26 9:40 GMT+01:00 Kyotaro HORIGUCHI 
> > >:
>>
>> > > Thanks for reviewing but I ran out of time for this CF..
>> > >
>> > > I'm going to move this to the next CF.
>> >
>> > I splitted the patch into small pieces. f3fd531 conflicted to
>> > this so rebased onto the current master HEAD.
>> >
>> > 0001 is psql_completion refactoring.
>> > 0002-0003 are patches prividing new infrastructures.
>> > 0004 is README for the infrastructures.
>> > 0005 is letter-case correction of SET/RESET/SHOW using 0002.
>> > 0006-0008 are improvements of recursive syntaxes using 0001 and 0004.
>> > 0009-0016 are simplifying (maybe) completion code per syntax.
>> >
>> > The last one (0017) is the IF(NOT)EXIST modifications. It
>> > suggests if(not)exists for syntaxes already gets object
>> > suggestion. So some kind of objects like operator, cast and so
>> > don't get an if.. suggestion. Likewise, I intentionally didn't
>> > modified siggestions for "TEXT SEARCH *".
>> >
>> >
>> lot of patches. I hope I look on these patches this week.
>
> Thank you for looking this and sorry for the many files. But I
> hople that they would be far easier to read.

The current patch status was "waiting on author", but that's incorrect
as a new series of this patch has been sent. Please be careful with
the status of the CF app! I am moving it to next CF with "needs
review". Gerdan Santos has marked himself as a reviewer of the patch
but I saw no activity, so I have removed his name to not confuse
people looking for patches to review (that happens!).
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Hello,

At Mon, 26 Dec 2016 14:24:33 +0100, Pavel Stehule  
wrote in 
pavel.stehule> 2016-12-26 9:40 GMT+01:00 Kyotaro HORIGUCHI 
 >:
> 
> > > Thanks for reviewing but I ran out of time for this CF..
> > >
> > > I'm going to move this to the next CF.
> >
> > I splitted the patch into small pieces. f3fd531 conflicted to
> > this so rebased onto the current master HEAD.
> >
> > 0001 is psql_completion refactoring.
> > 0002-0003 are patches prividing new infrastructures.
> > 0004 is README for the infrastructures.
> > 0005 is letter-case correction of SET/RESET/SHOW using 0002.
> > 0006-0008 are improvements of recursive syntaxes using 0001 and 0004.
> > 0009-0016 are simplifying (maybe) completion code per syntax.
> >
> > The last one (0017) is the IF(NOT)EXIST modifications. It
> > suggests if(not)exists for syntaxes already gets object
> > suggestion. So some kind of objects like operator, cast and so
> > don't get an if.. suggestion. Likewise, I intentionally didn't
> > modified siggestions for "TEXT SEARCH *".
> >
> >
> lot of patches. I hope I look on these patches this week.

Thank you for looking this and sorry for the many files. But I
hople that they would be far easier to read.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

2016-12-26 9:40 GMT+01:00 Kyotaro HORIGUCHI :

> > Thanks for reviewing but I ran out of time for this CF..
> >
> > I'm going to move this to the next CF.
>
> I splitted the patch into small pieces. f3fd531 conflicted to
> this so rebased onto the current master HEAD.
>
> 0001 is psql_completion refactoring.
> 0002-0003 are patches prividing new infrastructures.
> 0004 is README for the infrastructures.
> 0005 is letter-case correction of SET/RESET/SHOW using 0002.
> 0006-0008 are improvements of recursive syntaxes using 0001 and 0004.
> 0009-0016 are simplifying (maybe) completion code per syntax.
>
> The last one (0017) is the IF(NOT)EXIST modifications. It
> suggests if(not)exists for syntaxes already gets object
> suggestion. So some kind of objects like operator, cast and so
> don't get an if.. suggestion. Likewise, I intentionally didn't
> modified siggestions for "TEXT SEARCH *".
>
>
lot of patches. I hope I look on these patches this week.

Regards

Pavel


> regards,
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Hello,

At Fri, 25 Nov 2016 06:51:43 +0100, Pavel Stehule  
wrote in 
> I am sure about benefit of all patches - but it is lot of changes in one
> moment, and it is not necessary in this moment.
> 
> patches 0004 and 0005 does some bigger mental changes, and the work can be
> separated.

The patches are collestions of sporadic changes on the same
basis. I agree that the result is too big too look at. (And the
code itself is confused) Please wait for a while for separated
patches.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

2016-11-25 2:24 GMT+01:00 Kyotaro HORIGUCHI :

> Hello,
>
> Thank you for looking this long-and-bothersome patch.
>
>
> At Wed, 23 Nov 2016 07:12:00 +0100, Pavel Stehule 
> wrote in  vd9fgpsgv...@mail.gmail.com>
> > Hi
> >
> > 2016-11-15 12:26 GMT+01:00 Kyotaro HORIGUCHI <
> horiguchi.kyot...@lab.ntt.co.
> > jp>:
> >
> > > Hello, I rebased this patch on the current master.
> > >
> > > At Mon, 31 Oct 2016 10:15:48 +0900 (Tokyo Standard Time), Kyotaro
> > > HORIGUCHI  wrote in <
> > > 20161031.101548.162143279.horiguchi.kyot...@lab.ntt.co.jp>
> > > > Anyway, I fixed space issues and addressed the
> > > > COMPLETE_WITH_QUERY()'s almost unused parameter problem. And
> > > > tried to write a README files.
> > >
> > > tab-complete.c have gotten some improvements after this time.
> > >
> > > 577f0bdd2b8904cbdfde6c98f4bda6fd93a05ffc psql: Tab completion for
> > > renaming enum values.
> > > 927d7bb6b120a2ca09a164898f887eb850b7a329 Improve tab completion for
> > > CREATE TRIGGER.
> > > 1d15d0db50a5f39ab69c1fe60f2d5dcc7e2ddb9c psql: Tab-complete LOCK
> [TABLE]
> > > ... IN {ACCESS|ROW|SHARE}.
> > >
> > > The attached patchset is rebsaed on the master including these
> > > patches.
> > >
> >
> > I checked patches 0001, 0002, 0003 patches
> >
> > There are no any problems with patching, compiling, it is working as
> > expected
> >
> > These patches can be committed separately - they are long, but the code
> is
> > almost mechanical
>
> Thanks.
>
> You're right. I haven't consider about relations among them.
>

I am sure about benefit of all patches - but it is lot of changes in one
moment, and it is not necessary in this moment.

patches 0004 and 0005 does some bigger mental changes, and the work can be
separated.

Regards

Pavel


>
>
> 0001 (if-else refactoring) does not anyting functionally. It is
> required by 0004(word-shift-and-removal) and 0005(if-not-exists).
>
> 0002 (keywords case improvement) is almost independent from all
> other patches in this patch set. And it brings an obvious
> improvement.
>
> 0003 (addition to 0002) is move embedded keywords out of defined
> queries. Functionally can be united to 0002 but separated for
> understandability
>
> 0004 (word-shift-and-removal) is quite arguable one. This
> introduces an ability to modify (or destroy) previous_words
> array. This reduces almost redundant matching predicates such as,
>
> >  if (TailMatches3("CREATE|UNIQUE", "INDEX", MatchAny) ||
> >  TailMatches4("CREATE|UNIQUE", "INDEX", "CONCURRENTLY", MatchAny))
>
> into
>
> >  if (Matches3("CREATE", "INDEX", MatchAnyExcept("ON")))
>
> by removing "CONCURRENTLY". This obviously simplifies the
> predicates literally but it the code implies history of
> modification. The implied history might be worse than the
> previous shape, especially for the simple cases like this. For a
> complex case of CREATE TRIGGER, it seems worse than the original
> shape... I'll consider this a bit more. Maybe match-and-collapse
> template should be written a predicate.
>
> 0005 (if-not-exists). I have admit that this is arguable
> feature...
>
> 0006 is the terder point:( but.
>
> > The README is perfect
>
> Thank you, I'm relieved by hearing that.
>
> regards,
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>
>
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Hello,

Thank you for looking this long-and-bothersome patch.


At Wed, 23 Nov 2016 07:12:00 +0100, Pavel Stehule  
wrote in 
> Hi
> 
> 2016-11-15 12:26 GMT+01:00 Kyotaro HORIGUCHI  jp>:
> 
> > Hello, I rebased this patch on the current master.
> >
> > At Mon, 31 Oct 2016 10:15:48 +0900 (Tokyo Standard Time), Kyotaro
> > HORIGUCHI  wrote in <
> > 20161031.101548.162143279.horiguchi.kyot...@lab.ntt.co.jp>
> > > Anyway, I fixed space issues and addressed the
> > > COMPLETE_WITH_QUERY()'s almost unused parameter problem. And
> > > tried to write a README files.
> >
> > tab-complete.c have gotten some improvements after this time.
> >
> > 577f0bdd2b8904cbdfde6c98f4bda6fd93a05ffc psql: Tab completion for
> > renaming enum values.
> > 927d7bb6b120a2ca09a164898f887eb850b7a329 Improve tab completion for
> > CREATE TRIGGER.
> > 1d15d0db50a5f39ab69c1fe60f2d5dcc7e2ddb9c psql: Tab-complete LOCK [TABLE]
> > ... IN {ACCESS|ROW|SHARE}.
> >
> > The attached patchset is rebsaed on the master including these
> > patches.
> >
> 
> I checked patches 0001, 0002, 0003 patches
> 
> There are no any problems with patching, compiling, it is working as
> expected
> 
> These patches can be committed separately - they are long, but the code is
> almost mechanical

Thanks. 

You're right. I haven't consider about relations among them.


0001 (if-else refactoring) does not anyting functionally. It is
required by 0004(word-shift-and-removal) and 0005(if-not-exists).

0002 (keywords case improvement) is almost independent from all
other patches in this patch set. And it brings an obvious
improvement.

0003 (addition to 0002) is move embedded keywords out of defined
queries. Functionally can be united to 0002 but separated for
understandability

0004 (word-shift-and-removal) is quite arguable one. This
introduces an ability to modify (or destroy) previous_words
array. This reduces almost redundant matching predicates such as,

>  if (TailMatches3("CREATE|UNIQUE", "INDEX", MatchAny) ||
>  TailMatches4("CREATE|UNIQUE", "INDEX", "CONCURRENTLY", MatchAny))

into

>  if (Matches3("CREATE", "INDEX", MatchAnyExcept("ON")))

by removing "CONCURRENTLY". This obviously simplifies the
predicates literally but it the code implies history of
modification. The implied history might be worse than the
previous shape, especially for the simple cases like this. For a
complex case of CREATE TRIGGER, it seems worse than the original
shape... I'll consider this a bit more. Maybe match-and-collapse
template should be written a predicate.

0005 (if-not-exists). I have admit that this is arguable
feature...

0006 is the terder point:( but.

> The README is perfect

Thank you, I'm relieved by hearing that.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

Hi

2016-11-15 12:26 GMT+01:00 Kyotaro HORIGUCHI :

> Hello, I rebased this patch on the current master.
>
> At Mon, 31 Oct 2016 10:15:48 +0900 (Tokyo Standard Time), Kyotaro
> HORIGUCHI  wrote in <
> 20161031.101548.162143279.horiguchi.kyot...@lab.ntt.co.jp>
> > Anyway, I fixed space issues and addressed the
> > COMPLETE_WITH_QUERY()'s almost unused parameter problem. And
> > tried to write a README files.
>
> tab-complete.c have gotten some improvements after this time.
>
> 577f0bdd2b8904cbdfde6c98f4bda6fd93a05ffc psql: Tab completion for
> renaming enum values.
> 927d7bb6b120a2ca09a164898f887eb850b7a329 Improve tab completion for
> CREATE TRIGGER.
> 1d15d0db50a5f39ab69c1fe60f2d5dcc7e2ddb9c psql: Tab-complete LOCK [TABLE]
> ... IN {ACCESS|ROW|SHARE}.
>
> The attached patchset is rebsaed on the master including these
> patches.
>

I checked patches 0001, 0002, 0003 patches

There are no any problems with patching, compiling, it is working as
expected

These patches can be committed separately - they are long, but the code is
almost mechanical

The README is perfect

Regards

Pavel





>
> regards,
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

At Thu, 29 Sep 2016 16:16:00 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI 
 wrote in 
<20160929.161600.224338668.horiguchi.kyot...@lab.ntt.co.jp>
> That looks better. I'll change the API as the following.
> 
> COMPLETE_WITH_QUERY(query);
> COMPLETE_WITH_QUERY_KW(query, kwlist);
> COMPLETE_WITH_SCHEMA_QUERY(squery);
> COMPLETE_WITH_SCHEMA_QUERY_KW(squery, kwlist);

Done on my environment.

> > >> 4. Introduce word shift and removal feature to psql-completion
> > >>
> > >>   This is the second core for the flexibility of completion code.
> > >>   The word shift feature is the ability to omit first several
> > >>   words in *MatchesN macros. For example this allows complete
> > >>   create-schema's schema elements in a natural code. (Currently
> > >>   those syntaxes that can be a schema elements are using
> > >>   TailMatches instead of Matches, as the result HeadMatches are
> > >>   not available there). The words removing feature is the ability
> > >>   to (desructively) clip multiple suceessive words in the
> > >>   previous_words list. This feature allows suceeding completion
> > >>   code not to care about the removed words, such like UNIQUE,
> > >>   CONCURRENTLY, VERBOSE and so on.
> > >>
> > >
> > I am thinking so commit's description should be inside README
> 
> Currently psql or tab-complete.c/psql_completion() have no such
> document. If this should be written as README, perhaps I should
> write about completion in general. On the other hand, per-macro
> explanations are written in tab-complete-macros.h but the usages
> are not. I'll try to write README.

Before proposing new patch including this. Since I'm totally
inconfident for my capability to write a documentation, I'd like
to ask anyone here of what shape we are to aim..

The following is the first part of the document I have written up
to now. Please help me by giving me any corrections, suggestions,
opinions, or anything else!

# The completion macro part would be better to be moved as
# comments for the macros in tab-complete-macros.h.

==
src/bin/psql/README.completion

Word completion of interactive psql
===

psql supports word completion on interactive input. The core function
of the feature is psql_completion_internal in tab-complete.c. A bunch
of macros are provided in order to make it easier to read and maintain
the completion code. The console input to refer is stored in char **
previous_words in reverse order but maintaiers of
psql_completion_internal don't need to be aware of the detail of
it. Most of the operation can be described using the provided macros.

Basic structure of the completion code
--

The main part of the function is just a series of completion
definitions, where the first match wins. Each definition basically is
in the following shape.

   if (*matching expression*)
  *corresponding completion, then return*

The matching expression checks against all input words before the word
currently being entered. Completion chooses the words prefixed by
letters already entered. For example, for "CREATE " the word list
to be matched is ["CREATE"] and the prefix for completion is
nothing. For "CREATE INDEX id", the list is ["CREATE", "INDEX"] and
the prefix is "id".


Matching expression macros
--
There are four types of matching expression macros.

- MatchesN(word1, word2 .. , wordN)

 true iff the word list is exactly the same as the paremeter.

- HeadMatchesN(word1, word2 .., wordN)

 true iff the first N words in the word list matches the parameter.

- TailMatchesN(word1, word2 .., wordN)

 true iff the last N words in the word list matches the parameter.

- MidMatchesN(pos, word1, word2 .., wordN)

 true iff N successive words starts from pos in the word list matches
 the parameter. The position is 1-based.


Completion macros
-
There are N types of completion macros.

- COMPLETE_WITH_QUERY(query), COMPLETE_WITH_QUERY_KW(query, addon)

  Suggest completion words acquired using the given query. The details
  of the query is seen in the comment for _complete_from_query(). Word
  matching is case-sensitive.

  The latter takes an additional parameter, which should be a fragment
  of query starts with " UNION " followed by a query string which
  gives some additional words. This addon can be ADDLISTN() macro for
  case-sensitive suggestion.

- COMPLETE_WITH_SCHEMA_QUERY(squery),
  COMPLETE_WITH_SCHEMA_QUERY_KW(squery, addon)

  Suggest based on a "schema query", which is a struct containing
  parameters. You will see the details in the comment for
  _complete_from_query(). Word maching is case-sensitive.

  Just same as COMPLETE_WITH_QUERY_KW, the latter form takes a
  fragment query same to that for COMPLETE_WITH_QUERY_KW.

- COMPLETE_WITH_LIST_CS(list)

  Suggest completion words given as an array of strings. Word matching
  is case-sensitive.

- 

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Hello,

At Tue, 20 Sep 2016 16:50:29 +0900, Michael Paquier  
wrote in 
> On Mon, Sep 19, 2016 at 6:11 PM, Pavel Stehule  
> wrote:
> > I am thinking so commit's description should be inside README
> 
> Horiguchi-san, your patch has some whitespace issues, you may want to
> get a run with git diff --check. Here are some things I have spotted:
> src/bin/psql/tab-complete.c:1074: trailing whitespace.
> +"MATERIALIZED VIEW",
> src/bin/psql/tab-complete.c:2621: trailing whitespace.
> +   COMPLETE_WITH_QUERY(Query_for_list_of_roles,

Thank you very much for pointing it out. I put a pre-commit hook
to check that not to do such a mistake again.

http://stackoverflow.com/questions/591923/make-git-automatically-remove-trailing-whitespace-before-committing/22704385#22704385

> This set of patches is making psql tab completion move into a better
> shape, particularly with 0001 that removes the legendary huge if-elif
> and just the routine return immediately in case of a keyword match.
> Things could be a little bit more shortened by for example not doing
> the refactoring of the tab macros because they are just needed in
> tab-complete.c. The other patches introduce further improvements for
> the existing infrastructure, but that's a lot of things just for
> adding IF [NOT] EXISTS to be honest.

It was the motive for this, but even excluding it, some syntaxes
with optional keywords can be simplified or enriched with the new
macros. CREATE SCHEMA's schema elements, CREATE INDEX and some
other syntaxes are simplified using the feature.

> Testing a bit, I have noticed that for example trying to after typing
> "create table if", if I attempt to do a tab completion "not exists"
> does not show up. I suspect that the other commands are failing at
> that as well.

I suppose it is "create table if ", with a space at the tail. It
is a general issue on combined keywords(?) suggestion in the
whole tab-completion mechanism (or readline's limitation). Some
sytaxes have explicit complition for such cases. For examle,
"create foreign " gets a suggestion of "DATA WRAPPER" since it
has an explcit suggestion step.

> /* ALTER FOREIGN */
> if (Matches2("ALTER", "FOREIGN"))
> COMPLETE_WITH_LIST2("DATA WRAPPER", "TABLE");

It is apparently solvable, but needs additional code to suggest
the rest words for every steps. It should be another issue.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Thank you for reviewing!

At Mon, 19 Sep 2016 11:11:03 +0200, Pavel Stehule  
wrote in 

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Hello,

At Wed, 28 Sep 2016 12:49:25 -0400, Robert Haas  wrote 
in 
> This patch hasn't been updated in over a week and we're just about out
> of time for this CommitFest, so I've marked it "Returned with
> Feedback" for now.  If it gets updated, it can be resubmitted for the
> next CommitFest.

Thanks, I will do it.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Robert Haas]

On Tue, Sep 20, 2016 at 3:50 AM, Michael Paquier
 wrote:
> On Mon, Sep 19, 2016 at 6:11 PM, Pavel Stehule  
> wrote:
>> I am thinking so commit's description should be inside README
>
> Horiguchi-san, your patch has some whitespace issues, you may want to
> get a run with git diff --check. Here are some things I have spotted:
> src/bin/psql/tab-complete.c:1074: trailing whitespace.
> +"MATERIALIZED VIEW",
> src/bin/psql/tab-complete.c:2621: trailing whitespace.
> +   COMPLETE_WITH_QUERY(Query_for_list_of_roles,
>
> This set of patches is making psql tab completion move into a better
> shape, particularly with 0001 that removes the legendary huge if-elif
> and just the routine return immediately in case of a keyword match.
> Things could be a little bit more shortened by for example not doing
> the refactoring of the tab macros because they are just needed in
> tab-complete.c. The other patches introduce further improvements for
> the existing infrastructure, but that's a lot of things just for
> adding IF [NOT] EXISTS to be honest.
>
> Testing a bit, I have noticed that for example trying to after typing
> "create table if", if I attempt to do a tab completion "not exists"
> does not show up. I suspect that the other commands are failing at
> that as well.

This patch hasn't been updated in over a week and we're just about out
of time for this CommitFest, so I've marked it "Returned with
Feedback" for now.  If it gets updated, it can be resubmitted for the
next CommitFest.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Michael Paquier]

On Mon, Sep 19, 2016 at 6:11 PM, Pavel Stehule  wrote:
> I am thinking so commit's description should be inside README

Horiguchi-san, your patch has some whitespace issues, you may want to
get a run with git diff --check. Here are some things I have spotted:
src/bin/psql/tab-complete.c:1074: trailing whitespace.
+"MATERIALIZED VIEW",
src/bin/psql/tab-complete.c:2621: trailing whitespace.
+   COMPLETE_WITH_QUERY(Query_for_list_of_roles,

This set of patches is making psql tab completion move into a better
shape, particularly with 0001 that removes the legendary huge if-elif
and just the routine return immediately in case of a keyword match.
Things could be a little bit more shortened by for example not doing
the refactoring of the tab macros because they are just needed in
tab-complete.c. The other patches introduce further improvements for
the existing infrastructure, but that's a lot of things just for
adding IF [NOT] EXISTS to be honest.

Testing a bit, I have noticed that for example trying to after typing
"create table if", if I attempt to do a tab completion "not exists"
does not show up. I suspect that the other commands are failing at
that as well.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

Hi



> Pavel
>
>
>> 2. Make keywords' case follow to input
>>
>>   Allow the keywords suggested along with databse objects to
>>   follow the input letter case. The core part of this patch is a
>>   new function additional_kw_query(), which dynamically generates
>>   additional query string with specified keywords in the desired
>>   letter case. COMPLETE_WITH_* macros are modified to accept the
>>   function.
>>
>>
second patch is working, but I don't think it is enough documented

what is addon in COMPLETE_WITH_QUERY(query, addon)? semantics, usage?

in 99% the addon is "" when macro
COMPLETE_WITH_SCHEMA_QUERY,COMPLETE_WITH_QUERY is used. Maybe a
introduction of new macros with nonempty addon parameter should be better.





> 3. Fix suggested keywords to follow input in tab-completion session 2
>>
>>   The 2nd patch above leaves some query string containing static
>>   keyword strings, which results in failure to follow input
>>   letter cases. Most of them are naturally removed but role names
>>   are a bother. This patch puts additional query strings for
>>   several usage of roles but it might be overdone.
>>
>
this patch looks well


>
>> 4. Introduce word shift and removal feature to psql-completion
>>
>>   This is the second core for the flexibility of completion code.
>>   The word shift feature is the ability to omit first several
>>   words in *MatchesN macros. For example this allows complete
>>   create-schema's schema elements in a natural code. (Currently
>>   those syntaxes that can be a schema elements are using
>>   TailMatches instead of Matches, as the result HeadMatches are
>>   not available there). The words removing feature is the ability
>>   to (desructively) clip multiple suceessive words in the
>>   previous_words list. This feature allows suceeding completion
>>   code not to care about the removed words, such like UNIQUE,
>>   CONCURRENTLY, VERBOSE and so on.
>>
>
I am thinking so commit's description should be inside README

Regards

Pavel


>
>> 5. Add suggestion for IF (NOT) EXISTS for some syntaxes
>>
>>   This adds IF (NOT) EXISTS suggestion, as a PoC.  This patch no
>>   loger covers all adoptable syntaces since the places where more
>>   than boilerplating is required are omitted.
>>
>> regards,
>>
>> --
>> Kyotaro Horiguchi
>> NTT Open Source Software Center
>>
>>
>>
>>
>>
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

2016-09-16 10:31 GMT+02:00 Kyotaro HORIGUCHI <
horiguchi.kyot...@lab.ntt.co.jp>:

> Hello, this is the new version.
>
> At Tue, 13 Sep 2016 10:50:13 +0900 (Tokyo Standard Time), Kyotaro
> HORIGUCHI  wrote in <
> 20160913.105013.65452566.horiguchi.kyot...@lab.ntt.co.jp>
> > > > This patch consists of the following files. Since these files are
> > > > splitted in strange criteria and order for historical reasons,
> > > > I'll reorganize this and post them later.
>
> The focus of this patch has changed. The first motivation was
> completing IF-EXISTS but the underlying issue was flexibility of
> psql_completion. And as Pavel's suggestion, keywords suggested
> along with database objects should follow the character case of
> input.
>
> For the purpose of resolving the issues, I reorganized the
> confused patch set. The attached patches are organized as the
> following.
>
> 1. Refactoring tab-complete to make psql_completion code
>
>   Does two things. One is moving out the macros that has grown to
>   be too large to stay in tab_completion.c to new file
>   tab-complete-macros.h The other is separating out the else-if
>   sequence in psql_completion() as a new function
>   psql_completion_internal(). This allows us to the following
>   things.
>
>   - Exit from arbitrary place in the former-else-if sequence just
> by return.
>
>   - Do other than "if(matching) { completion }" in anywhere
> convenient in the midst of the former-els...
>
>   - Recursively matching for sub syntaxes. EXPLAIN, RULE and
> others are using this feature. (Needs the 4th patch to do
> this, though)
>
>
This first patch looks well - although it is big patch - it doesn't do any
not trivial work. No problems with a patching or compilation. I didn't find
any issue.

Regards

Pavel


> 2. Make keywords' case follow to input
>
>   Allow the keywords suggested along with databse objects to
>   follow the input letter case. The core part of this patch is a
>   new function additional_kw_query(), which dynamically generates
>   additional query string with specified keywords in the desired
>   letter case. COMPLETE_WITH_* macros are modified to accept the
>   function.
>
> 3. Fix suggested keywords to follow input in tab-completion session 2
>
>   The 2nd patch above leaves some query string containing static
>   keyword strings, which results in failure to follow input
>   letter cases. Most of them are naturally removed but role names
>   are a bother. This patch puts additional query strings for
>   several usage of roles but it might be overdone.
>
> 4. Introduce word shift and removal feature to psql-completion
>
>   This is the second core for the flexibility of completion code.
>   The word shift feature is the ability to omit first several
>   words in *MatchesN macros. For example this allows complete
>   create-schema's schema elements in a natural code. (Currently
>   those syntaxes that can be a schema elements are using
>   TailMatches instead of Matches, as the result HeadMatches are
>   not available there). The words removing feature is the ability
>   to (desructively) clip multiple suceessive words in the
>   previous_words list. This feature allows suceeding completion
>   code not to care about the removed words, such like UNIQUE,
>   CONCURRENTLY, VERBOSE and so on.
>
> 5. Add suggestion for IF (NOT) EXISTS for some syntaxes
>
>   This adds IF (NOT) EXISTS suggestion, as a PoC.  This patch no
>   loger covers all adoptable syntaces since the places where more
>   than boilerplating is required are omitted.
>
> regards,
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>
>
>
>
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

Hi

2016-09-16 10:31 GMT+02:00 Kyotaro HORIGUCHI <
horiguchi.kyot...@lab.ntt.co.jp>:

> Hello, this is the new version.
>
> At Tue, 13 Sep 2016 10:50:13 +0900 (Tokyo Standard Time), Kyotaro
> HORIGUCHI  wrote in <
> 20160913.105013.65452566.horiguchi.kyot...@lab.ntt.co.jp>
> > > > This patch consists of the following files. Since these files are
> > > > splitted in strange criteria and order for historical reasons,
> > > > I'll reorganize this and post them later.
>
> The focus of this patch has changed. The first motivation was
> completing IF-EXISTS but the underlying issue was flexibility of
> psql_completion. And as Pavel's suggestion, keywords suggested
> along with database objects should follow the character case of
> input.
>
> For the purpose of resolving the issues, I reorganized the
> confused patch set. The attached patches are organized as the
> following.
>
> 1. Refactoring tab-complete to make psql_completion code
>
>   Does two things. One is moving out the macros that has grown to
>   be too large to stay in tab_completion.c to new file
>   tab-complete-macros.h The other is separating out the else-if
>   sequence in psql_completion() as a new function
>   psql_completion_internal(). This allows us to the following
>   things.
>
>   - Exit from arbitrary place in the former-else-if sequence just
> by return.
>
>   - Do other than "if(matching) { completion }" in anywhere
> convenient in the midst of the former-els...
>
>   - Recursively matching for sub syntaxes. EXPLAIN, RULE and
> others are using this feature. (Needs the 4th patch to do
> this, though)
>
> 2. Make keywords' case follow to input
>
>   Allow the keywords suggested along with databse objects to
>   follow the input letter case. The core part of this patch is a
>   new function additional_kw_query(), which dynamically generates
>   additional query string with specified keywords in the desired
>   letter case. COMPLETE_WITH_* macros are modified to accept the
>   function.
>
> 3. Fix suggested keywords to follow input in tab-completion session 2
>
>   The 2nd patch above leaves some query string containing static
>   keyword strings, which results in failure to follow input
>   letter cases. Most of them are naturally removed but role names
>   are a bother. This patch puts additional query strings for
>   several usage of roles but it might be overdone.
>
> 4. Introduce word shift and removal feature to psql-completion
>
>   This is the second core for the flexibility of completion code.
>   The word shift feature is the ability to omit first several
>   words in *MatchesN macros. For example this allows complete
>   create-schema's schema elements in a natural code. (Currently
>   those syntaxes that can be a schema elements are using
>   TailMatches instead of Matches, as the result HeadMatches are
>   not available there). The words removing feature is the ability
>   to (desructively) clip multiple suceessive words in the
>   previous_words list. This feature allows suceeding completion
>   code not to care about the removed words, such like UNIQUE,
>   CONCURRENTLY, VERBOSE and so on.
>
> 5. Add suggestion for IF (NOT) EXISTS for some syntaxes
>
>   This adds IF (NOT) EXISTS suggestion, as a PoC.  This patch no
>   loger covers all adoptable syntaces since the places where more
>   than boilerplating is required are omitted.
>

I am working on some initial tests - a compilation, a patching is ok.
Autocomplete for DROP TABLE IF EXISTS doesn't work. CREATE TABLE IF NOT
EXIST works well

Regards

Pavel



>
> regards,
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>
>
>
>
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Hello,

At Sat, 10 Sep 2016 07:40:16 +0200, Pavel Stehule  
wrote in 

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

2016-09-06 15:00 GMT+02:00 Kyotaro HORIGUCHI <
horiguchi.kyot...@lab.ntt.co.jp>:

> Hello, this is the new version of this patch. Rebased on the
> current master.
>
> At Tue, 06 Sep 2016 13:06:51 +0900 (Tokyo Standard Time), Kyotaro
> HORIGUCHI  wrote in <
> 20160906.130651.171572544.horiguchi.kyot...@lab.ntt.co.jp>
> > Thank you. I'll rebase the following patch and repost as soon as
> > possible.
> >
> > https://www.postgresql.org/message-id/20160407.211917.
> 147996130.horiguchi.kyot...@lab.ntt.co.jp
>
> This patch consists of the following files. Since these files are
> splitted in strange criteria and order for historical reasons,
> I'll reorganize this and post them later.
>

ok, can I start with testing and review with some from these files?

Regards

Pavel


>
> - 0001-Suggest-IF-NOT-EXISTS-for-tab-completion-of-psql.patch
>
>   Add suggestion of IF (NOT) EXISTS on master. This should be the
>   last patch in this patchset.
>
> - 0002-Make-added-keywords-for-completion-queries-follow-to.patch
>
>   Current suggestion mechanism doesn't distinguish object names
>   and keywords, which should be differently handled in
>   determining letter cases. This patch fixes that.
>
> - 0003-Make-COMPLETE_WITH_ATTR-to-accept-additional-keyword.patch
>
>   This patch apply the 0002 fix to COMPLET_WITH_ATTR.
>
> - 0004-Refactoring-tab-complete-to-make-psql_completion-cod.patch
>
>   By Tom's suggestion, in order to modify previous_words in more
>   sane way, transforming the else-if sequence in psql_completion
>   into a simple if sequence.
>
> regards,
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Hello,

At Sun, 4 Sep 2016 12:54:57 +0200, Pavel Stehule  
wrote in 

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

Hi

This patch needs rebase.

Regards

Pavel

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Robert Haas]

On Thu, Apr 7, 2016 at 8:19 AM, Kyotaro HORIGUCHI
 wrote:
> Thank you for looking this and for the comment.
>
> Since the end of this CF is quite soon and this seems in
> uncommittable state, feel free to move this to the next CF if any
> other patch with more priority.

Moved.  It's appropriate to consider this "needs review" at this
point, I think so added to 2016-09 that way.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Tom Lane]

Pavel Stehule  writes:
> 1. We want this patch - it increase a functionality of autocomplete

TBH, I do not think that is an agreed-to statement.  I concur with
Peter's comments upthread questioning how much use-case there is for
interactive completion of IF (NOT) EXISTS.  If it were a small and
uncomplicated patch, I wouldn't object ... but this is neither.

It increases the size of tab-complete.c by nearly 10%, and would
increase it more than that if it were adequately documented as to
what all these new macros and variables do.  (To take just the first
example, APPEND_COMP_CHARP and SET_COMP_CHARP not only lack any
documentation, but have been inserted between a comment documenting
some macros and those macros.  Another complaint in the same vein is
that MatchesN() no longer means even approximately what it did before,
but the comment for those macros hasn't been changed.)  On top of that,
it seems like there's an awful lot of klugery going on here.  An example
is the use of the COLLAPSE macro (hidden inside MidMatchAndRemoveN),
which seems like a seriously bad idea because it destroys data even if
the match the macro is embedded in ultimately fails.  That will create
order dependencies between match rules, which is not something we want
IMO, most especially when it's not clearly marked in the match rules
what's dependent on what.

Seeing things like "if (something-with-side-effects && false)" doesn't
fill me with any great admiration for the cleanliness of the code, either.

In short, I'm not sold that we need autocomplete for IF EXISTS,
and if the price we have to pay for it is klugery on this scale,
it's no sale.  I think this needs to be sent back for a considerable
amount of rethinking.

One thing that might be worth considering to get rid of this
side-effects-in-IF-conditions mess is to move the match rules into
a separate function so that after doing a successful match, we just
"return".  This could be implemented in most places by adding
return statements into the COMPLETE_WITH_FOO macros.  Then we would
not need the enormous else-if chain, but just simple independent
if statements, where we know a successful match will end with a
"return" instead of falling through to the next statement.  The
big advantage of that is that then you can do operations with
side-effects explicitly as separate statements, instead of having
to make them look like phony else-if cases.  So for example the
CREATE SCHEMA case might be handled like

if (Matches2("CREATE", "SCHEMA"))
{
... handle possible autocompletions of CREATE SCHEMA itself here ...

/* Else, move head match point past CREATE SCHEMA */
if ((n = find_last_index_of("CREATE")) > 0)
HEAD_SHIFT(n);
}

/*
 * Statements that can appear in CREATE SCHEMA should be considered here!
 */

if (Matches2("CREATE", "TABLE"))
... handle CREATE TABLE here ...

... handle other statements that can appear in CREATE SCHEMA here ...

After exhausting the possibilities for sub-statements of CREATE SCHEMA,
we could either return failure if we'd seen CREATE SCHEMA:

/*
 * Fail if we saw CREATE SCHEMA; no rules below here should be considered.
 */
if (head_shift > 0)
return false;

or reset the head match point before continuing with unrelated rules:

/*
 * Done considering CREATE SCHEMA sub-rules, so forget about
 * whether we saw CREATE SCHEMA.
 */
HEAD_SHIFT(0);

Immediately failing seems like the right thing for CREATE SCHEMA, but
maybe in other cases we just undo the head_shift change and keep trying.
This is still order dependent, but at least the places where we change
the match basis can be made to be fairly obvious actions instead of
being disguised as just-another-candidate-match.

I don't immediately have a better idea to replace COLLAPSE, but I really
don't like that as it stands.  I wonder whether we could dodge the need
for it by just increasing head_shift when deciding to skip over an
IF (NOT) EXISTS clause.  Otherwise, I think what I'd want to see is
some way to explicitly undo the effects of COLLAPSE when done with
rules that could benefit from it.  Or at least a coding convention
that makes it clear that you return failure once you've considered
all subsidiary rules, rather than continuing on with unrelated rules
that would have a risk of false match thanks to the data removal.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

Hi

2016-04-04 7:58 GMT+02:00 Kyotaro HORIGUCHI :

> Thank you for testing. That is a silly mistake, sorry.
>
> The attached is the fixed version.
>
> # Can I add a suffix to format-patche's output files?
>
> At Sat, 2 Apr 2016 07:18:32 +0200, Pavel Stehule 
> wrote in <
> cafj8pradf3rmq3y33aer1c7woi2qss65c8bbtirnqu0zwvw...@mail.gmail.com>
> > >> Finally I settled it by replacing comma expression with logical
> > >> OR or AND expresssion. gcc 4.9 compains for some unused variables
> > >> in flex output but it is the another issue.
> > >>
> > >> I forgot to address COMPLETE_WITH_ATTTR but it needed an overhaul
> > >> of some macros and changing the type of completion_charp. The
> > >> third patch does it but it might be unacceptable..
> > >>
> > >>
> > > something is wrong, autocomplete for CREATE TABLE IF NOT EXISTS doesn't
> > > work
> > >
> >
> > CREATE UNLOGGED/TEMP table is working.
>
> Mmm. I mitakenly refactored multi-step matching.
>
> >   else if (HeadMatches3("CREATE", MatchAny, "TABLE") &&
> >MidMatchAndRemove1(1, "TEMP|TEMPORARY|UNLOGGED") &&
> >Matches2("CREATE", "TABLE"))
> > COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables,
> >ADDLIST1("IF NOT EXISTS"));
>
> The completion runs only for CREATE AnyKeyword TABLE when
> AnyKeyword is removable. It is wrong to do so when any of
> prev_words[] that matches the last Matches() can be fileterd out
> by the first Headmatches(). The same kind of mistake was found in
> the following syntaxes. CREATE SEQENCE had one more mistake.
>
>
> "CREATE [UNIQUE] INDEX"
> "CREATE [TEMP] SEQUENCE"
> "CREATE [TEMP..] TABLE"
>
> It is arguable that it is proper to suggest existing object for
> CREATE statement, but most of the statement is suggested. It is
> semantically wrong but practically useful to know what kind of
> word should be there.
>

I tested this patch and I didn't find any problem.

1. We want this patch - it increase a functionality of autocomplete - IF
(NOT) EXISTS is relative long phrase and autocomplete is nice. - next
implementation can be CREATE "OR REPLACE" FUNCTION

2. The patch is possible to apply - no problems, no problems with compiling

3. All regress tests passed without problems

4. Patch respects PostgreSQL's codding style and it is enough commented

5. The regress tests are not possible - interactive process

6. The documentation is not necessary

7. It should not to have any impacts on SQL or performance


I'll mark this patch as ready for commiter

Thank you for the patch

Regards

Pavel



> regards,
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Thank you for testing. That is a silly mistake, sorry.

The attached is the fixed version.

# Can I add a suffix to format-patche's output files?

At Sat, 2 Apr 2016 07:18:32 +0200, Pavel Stehule  
wrote in 
> >> Finally I settled it by replacing comma expression with logical
> >> OR or AND expresssion. gcc 4.9 compains for some unused variables
> >> in flex output but it is the another issue.
> >>
> >> I forgot to address COMPLETE_WITH_ATTTR but it needed an overhaul
> >> of some macros and changing the type of completion_charp. The
> >> third patch does it but it might be unacceptable..
> >>
> >>
> > something is wrong, autocomplete for CREATE TABLE IF NOT EXISTS doesn't
> > work
> >
> 
> CREATE UNLOGGED/TEMP table is working.

Mmm. I mitakenly refactored multi-step matching.

>   else if (HeadMatches3("CREATE", MatchAny, "TABLE") &&
>MidMatchAndRemove1(1, "TEMP|TEMPORARY|UNLOGGED") &&
>Matches2("CREATE", "TABLE"))
> COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables,
>ADDLIST1("IF NOT EXISTS"));

The completion runs only for CREATE AnyKeyword TABLE when
AnyKeyword is removable. It is wrong to do so when any of
prev_words[] that matches the last Matches() can be fileterd out
by the first Headmatches(). The same kind of mistake was found in
the following syntaxes. CREATE SEQENCE had one more mistake.


"CREATE [UNIQUE] INDEX"
"CREATE [TEMP] SEQUENCE"
"CREATE [TEMP..] TABLE"

It is arguable that it is proper to suggest existing object for
CREATE statement, but most of the statement is suggested. It is
semantically wrong but practically useful to know what kind of
word should be there.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
>From eb7a91950dab394e6d6e0de9285b4f6002f22a09 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Fri, 5 Feb 2016 16:50:35 +0900
Subject: [PATCH 1/3] Suggest IF (NOT) EXISTS for tab-completion of psql

This patch lets psql to suggest "IF (NOT) EXISTS". Addition to that,
since this patch introduces some mechanism for syntactical robustness,
it allows psql completion to omit some optional part on matching.
---
 src/bin/psql/tab-complete.c | 630 
 1 file changed, 522 insertions(+), 108 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 688d92a..9b6e704 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -656,6 +656,10 @@ static const SchemaQuery Query_for_list_of_matviews = {
 "   FROM pg_catalog.pg_roles "\
 "  WHERE substring(pg_catalog.quote_ident(rolname),1,%d)='%s'"
 
+#define Query_for_list_of_rules \
+"SELECT pg_catalog.quote_ident(rulename) FROM pg_catalog.pg_rules "\
+" WHERE substring(pg_catalog.quote_ident(rulename),1,%d)='%s'"
+
 #define Query_for_list_of_grant_roles \
 " SELECT pg_catalog.quote_ident(rolname) "\
 "   FROM pg_catalog.pg_roles "\
@@ -763,6 +767,11 @@ static const SchemaQuery Query_for_list_of_matviews = {
 "SELECT pg_catalog.quote_ident(tmplname) FROM pg_catalog.pg_ts_template "\
 " WHERE substring(pg_catalog.quote_ident(tmplname),1,%d)='%s'"
 
+#define Query_for_list_of_triggers \
+"SELECT pg_catalog.quote_ident(tgname) FROM pg_catalog.pg_trigger "\
+" WHERE substring(pg_catalog.quote_ident(tgname),1,%d)='%s' AND "\
+"   NOT tgisinternal"
+
 #define Query_for_list_of_fdws \
 " SELECT pg_catalog.quote_ident(fdwname) "\
 "   FROM pg_catalog.pg_foreign_data_wrapper "\
@@ -907,7 +916,7 @@ static const pgsql_thing_t words_after_create[] = {
 	{"PARSER", Query_for_list_of_ts_parsers, NULL, THING_NO_SHOW},
 	{"POLICY", NULL, NULL},
 	{"ROLE", Query_for_list_of_roles},
-	{"RULE", "SELECT pg_catalog.quote_ident(rulename) FROM pg_catalog.pg_rules WHERE substring(pg_catalog.quote_ident(rulename),1,%d)='%s'"},
+	{"RULE", Query_for_list_of_rules},
 	{"SCHEMA", Query_for_list_of_schemas},
 	{"SEQUENCE", NULL, _for_list_of_sequences},
 	{"SERVER", Query_for_list_of_servers},
@@ -916,7 +925,7 @@ static const pgsql_thing_t words_after_create[] = {
 	{"TEMP", NULL, NULL, THING_NO_DROP},		/* for CREATE TEMP TABLE ... */
 	{"TEMPLATE", Query_for_list_of_ts_templates, NULL, THING_NO_SHOW},
 	{"TEXT SEARCH", NULL, NULL},
-	{"TRIGGER", "SELECT pg_catalog.quote_ident(tgname) FROM pg_catalog.pg_trigger WHERE substring(pg_catalog.quote_ident(tgname),1,%d)='%s' AND NOT tgisinternal"},
+	{"TRIGGER", Query_for_list_of_triggers},
 	{"TYPE", NULL, _for_list_of_datatypes},
 	{"UNIQUE", NULL, NULL, THING_NO_DROP},		/* for CREATE UNIQUE INDEX ... */
 	{"UNLOGGED", NULL, NULL, THING_NO_DROP},	/* for CREATE UNLOGGED TABLE
@@ -945,6 +954,7 @@ static char **complete_from_variables(const char *text,
 	const char *prefix, const char *suffix, bool need_value);
 static char *complete_from_files(const char *text, int state);
 
+static int find_last_index_of(char *w, char **previous_words, int 

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

2016-04-02 7:16 GMT+02:00 Pavel Stehule :

> Hi
>
> 2016-04-01 10:21 GMT+02:00 Kyotaro HORIGUCHI <
> horiguchi.kyot...@lab.ntt.co.jp>:
>
>> Hello, sorry for being a bit late.
>> The attatched are the new version of the patch.. set.
>>
>> 1. 0001-Suggest-IF-NOT-EXISTS-for-tab-completion-of-psql.patch
>>
>>  Adds IF (NOT) EXISTS completion. It doesn't fix the issue that
>>  the case of additional keywords don't follow the input.
>>
>> 2. 0002-Make-added-keywords-for-completion-queries-follow-to.patch
>>
>>  Fixes the case-don't-follow issue by introducing a new macro set
>>  ADDLISTn(). This leaves the issue for keywords along with
>>  attributes.
>>
>> 3. 0003-Make-COMPLETE_WITH_ATTR-to-accept-additional-keyword.patch
>>
>>   Fixes the issue left after 0002 patch.
>>   This patch does the following
>>   things.
>>
>>   1. Change completion_charp from const char * to PQExpBuffer.
>>
>>   2. Chnage COMPLETE_WITH_QUERY and COMPLETE_WITH_ATTR to accept
>>  an expression instead of string literal.
>>
>>   3. Replace all additional keyword lists in psql_copmletion with
>>  ADDLISTn() expression.
>>
>>
>> At Fri, 01 Apr 2016 11:52:03 +0900 (Tokyo Standard Time), Kyotaro
>> HORIGUCHI  wrote in <
>> 20160401.115203.98896697.horiguchi.kyot...@lab.ntt.co.jp>
>> > > > > I found new warning
>> > > > >
>> > > > >  tab-complete.c:1438:87: warning: right-hand operand of comma
>> expression
>> > > > > has no effect [-Wunused-value]
>> > > >
>> > > > Mmm. Google said me that gcc 4.9 does so. I'm using 4.8.5 so I
>> > > > haven't see the warning.
>> > > >
>> > > > https://gcc.gnu.org/gcc-4.9/porting_to.html
>> > > >
>> > > > 1436:   else if (HeadMatches2("CREATE", "SCHEMA") &&
>> > > > 1437:SHIFT_TO_LAST1("CREATE") &&
>> > > > 1438:false) {} /* FALL THROUGH */
>> ...
>> > > > But the right hand value (true) is actually "used" in the
>> > > > expression (even though not effective). Perhaps (true && false)
>> > > > was potimized as false and the true is regarded to be unused?
>> > > > That's stupid.. Using functions instead of macros seems to solve
>> > > > this but they needed to be wraped by macros as
>> > > > additional_kw_query(). That's a pain..
>> ...
>> > This needs to use gcc 4.9 to address, but CentOS7 doesn't have
>> > devtools-2 repo so now I'm building CentOS6 environment for this
>> > purpose. Please wait for a while.
>>
>> Finally I settled it by replacing comma expression with logical
>> OR or AND expresssion. gcc 4.9 compains for some unused variables
>> in flex output but it is the another issue.
>>
>> I forgot to address COMPLETE_WITH_ATTTR but it needed an overhaul
>> of some macros and changing the type of completion_charp. The
>> third patch does it but it might be unacceptable..
>>
>>
> something is wrong, autocomplete for CREATE TABLE IF NOT EXISTS doesn't
> work
>

CREATE UNLOGGED/TEMP table is working.

Pavel


>
> Regards
>
> Pavel
>
>
>> regards,
>>
>> --
>> Kyotaro Horiguchi
>> NTT Open Source Software Center
>>
>
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

Hi

2016-04-01 10:21 GMT+02:00 Kyotaro HORIGUCHI <
horiguchi.kyot...@lab.ntt.co.jp>:

> Hello, sorry for being a bit late.
> The attatched are the new version of the patch.. set.
>
> 1. 0001-Suggest-IF-NOT-EXISTS-for-tab-completion-of-psql.patch
>
>  Adds IF (NOT) EXISTS completion. It doesn't fix the issue that
>  the case of additional keywords don't follow the input.
>
> 2. 0002-Make-added-keywords-for-completion-queries-follow-to.patch
>
>  Fixes the case-don't-follow issue by introducing a new macro set
>  ADDLISTn(). This leaves the issue for keywords along with
>  attributes.
>
> 3. 0003-Make-COMPLETE_WITH_ATTR-to-accept-additional-keyword.patch
>
>   Fixes the issue left after 0002 patch.
>   This patch does the following
>   things.
>
>   1. Change completion_charp from const char * to PQExpBuffer.
>
>   2. Chnage COMPLETE_WITH_QUERY and COMPLETE_WITH_ATTR to accept
>  an expression instead of string literal.
>
>   3. Replace all additional keyword lists in psql_copmletion with
>  ADDLISTn() expression.
>
>
> At Fri, 01 Apr 2016 11:52:03 +0900 (Tokyo Standard Time), Kyotaro
> HORIGUCHI  wrote in <
> 20160401.115203.98896697.horiguchi.kyot...@lab.ntt.co.jp>
> > > > > I found new warning
> > > > >
> > > > >  tab-complete.c:1438:87: warning: right-hand operand of comma
> expression
> > > > > has no effect [-Wunused-value]
> > > >
> > > > Mmm. Google said me that gcc 4.9 does so. I'm using 4.8.5 so I
> > > > haven't see the warning.
> > > >
> > > > https://gcc.gnu.org/gcc-4.9/porting_to.html
> > > >
> > > > 1436:   else if (HeadMatches2("CREATE", "SCHEMA") &&
> > > > 1437:SHIFT_TO_LAST1("CREATE") &&
> > > > 1438:false) {} /* FALL THROUGH */
> ...
> > > > But the right hand value (true) is actually "used" in the
> > > > expression (even though not effective). Perhaps (true && false)
> > > > was potimized as false and the true is regarded to be unused?
> > > > That's stupid.. Using functions instead of macros seems to solve
> > > > this but they needed to be wraped by macros as
> > > > additional_kw_query(). That's a pain..
> ...
> > This needs to use gcc 4.9 to address, but CentOS7 doesn't have
> > devtools-2 repo so now I'm building CentOS6 environment for this
> > purpose. Please wait for a while.
>
> Finally I settled it by replacing comma expression with logical
> OR or AND expresssion. gcc 4.9 compains for some unused variables
> in flex output but it is the another issue.
>
> I forgot to address COMPLETE_WITH_ATTTR but it needed an overhaul
> of some macros and changing the type of completion_charp. The
> third patch does it but it might be unacceptable..
>
>
something is wrong, autocomplete for CREATE TABLE IF NOT EXISTS doesn't work

Regards

Pavel


> regards,
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Hello, sorry for being a bit late.
The attatched are the new version of the patch.. set.

1. 0001-Suggest-IF-NOT-EXISTS-for-tab-completion-of-psql.patch

 Adds IF (NOT) EXISTS completion. It doesn't fix the issue that
 the case of additional keywords don't follow the input.

2. 0002-Make-added-keywords-for-completion-queries-follow-to.patch

 Fixes the case-don't-follow issue by introducing a new macro set
 ADDLISTn(). This leaves the issue for keywords along with
 attributes.

3. 0003-Make-COMPLETE_WITH_ATTR-to-accept-additional-keyword.patch

  Fixes the issue left after 0002 patch. 
  This patch does the following
  things.
  
  1. Change completion_charp from const char * to PQExpBuffer.
  
  2. Chnage COMPLETE_WITH_QUERY and COMPLETE_WITH_ATTR to accept
 an expression instead of string literal.
  
  3. Replace all additional keyword lists in psql_copmletion with
 ADDLISTn() expression.


At Fri, 01 Apr 2016 11:52:03 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI 
 wrote in 
<20160401.115203.98896697.horiguchi.kyot...@lab.ntt.co.jp>
> > > > I found new warning
> > > >
> > > >  tab-complete.c:1438:87: warning: right-hand operand of comma expression
> > > > has no effect [-Wunused-value]
> > >
> > > Mmm. Google said me that gcc 4.9 does so. I'm using 4.8.5 so I
> > > haven't see the warning.
> > >
> > > https://gcc.gnu.org/gcc-4.9/porting_to.html
> > >
> > > 1436:   else if (HeadMatches2("CREATE", "SCHEMA") &&
> > > 1437:SHIFT_TO_LAST1("CREATE") &&
> > > 1438:false) {} /* FALL THROUGH */
...
> > > But the right hand value (true) is actually "used" in the
> > > expression (even though not effective). Perhaps (true && false)
> > > was potimized as false and the true is regarded to be unused?
> > > That's stupid.. Using functions instead of macros seems to solve
> > > this but they needed to be wraped by macros as
> > > additional_kw_query(). That's a pain..
...
> This needs to use gcc 4.9 to address, but CentOS7 doesn't have
> devtools-2 repo so now I'm building CentOS6 environment for this
> purpose. Please wait for a while.

Finally I settled it by replacing comma expression with logical
OR or AND expresssion. gcc 4.9 compains for some unused variables
in flex output but it is the another issue.

I forgot to address COMPLETE_WITH_ATTTR but it needed an overhaul
of some macros and changing the type of completion_charp. The
third patch does it but it might be unacceptable..

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
>From 3c2bbb46749cffc2fd78cdbfdc181128f3c1 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Fri, 5 Feb 2016 16:50:35 +0900
Subject: [PATCH 1/3] Suggest IF (NOT) EXISTS for tab-completion of psql

This patch lets psql to suggest "IF (NOT) EXISTS". Addition to that,
since this patch introduces some mechanism for syntactical robustness,
it allows psql completion to omit some optional part on matching.
---
 src/bin/psql/tab-complete.c | 625 
 1 file changed, 517 insertions(+), 108 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index eb592bb..a0808cf 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -656,6 +656,10 @@ static const SchemaQuery Query_for_list_of_matviews = {
 "   FROM pg_catalog.pg_roles "\
 "  WHERE substring(pg_catalog.quote_ident(rolname),1,%d)='%s'"
 
+#define Query_for_list_of_rules \
+"SELECT pg_catalog.quote_ident(rulename) FROM pg_catalog.pg_rules "\
+" WHERE substring(pg_catalog.quote_ident(rulename),1,%d)='%s'"
+
 #define Query_for_list_of_grant_roles \
 " SELECT pg_catalog.quote_ident(rolname) "\
 "   FROM pg_catalog.pg_roles "\
@@ -763,6 +767,11 @@ static const SchemaQuery Query_for_list_of_matviews = {
 "SELECT pg_catalog.quote_ident(tmplname) FROM pg_catalog.pg_ts_template "\
 " WHERE substring(pg_catalog.quote_ident(tmplname),1,%d)='%s'"
 
+#define Query_for_list_of_triggers \
+"SELECT pg_catalog.quote_ident(tgname) FROM pg_catalog.pg_trigger "\
+" WHERE substring(pg_catalog.quote_ident(tgname),1,%d)='%s' AND "\
+"   NOT tgisinternal"
+
 #define Query_for_list_of_fdws \
 " SELECT pg_catalog.quote_ident(fdwname) "\
 "   FROM pg_catalog.pg_foreign_data_wrapper "\
@@ -907,7 +916,7 @@ static const pgsql_thing_t words_after_create[] = {
 	{"PARSER", Query_for_list_of_ts_parsers, NULL, THING_NO_SHOW},
 	{"POLICY", NULL, NULL},
 	{"ROLE", Query_for_list_of_roles},
-	{"RULE", "SELECT pg_catalog.quote_ident(rulename) FROM pg_catalog.pg_rules WHERE substring(pg_catalog.quote_ident(rulename),1,%d)='%s'"},
+	{"RULE", Query_for_list_of_rules},
 	{"SCHEMA", Query_for_list_of_schemas},
 	{"SEQUENCE", NULL, _for_list_of_sequences},
 	{"SERVER", Query_for_list_of_servers},
@@ -916,7 +925,7 @@ static const pgsql_thing_t words_after_create[] = {
 	{"TEMP", NULL, NULL, THING_NO_DROP},		/* for CREATE TEMP TABLE ... */
 	{"TEMPLATE", 

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

2016-04-01 4:52 GMT+02:00 Kyotaro HORIGUCHI :

> At Thu, 31 Mar 2016 11:22:20 +0200, Pavel Stehule 
> wrote in <
> cafj8prd7vadunoiapb8exwc+c5ccis-rj2dphvzcwzkgxjb...@mail.gmail.com>
> > 2016-03-30 10:34 GMT+02:00 Kyotaro HORIGUCHI <
> > horiguchi.kyot...@lab.ntt.co.jp>:
> >
> > > Hi,
> > >
> > > At Wed, 30 Mar 2016 09:23:49 +0200, Pavel Stehule <
> pavel.steh...@gmail.com>
> > > wrote in <
> > > cafj8prbvka6ng4jwz2qmro7inudfjws5w0+demvgzznuf-h...@mail.gmail.com>
> > > > Hi
> > > >
> > > > ...
> > > > >> =# alter table if
> > > > >> =# alter table if exists
> > > > >> ==
> > > > >> =# alter table I
> > > > >> =# alter table IF EXISTS// "information_schema" doesn't match.
> > > > >>
> > > > >> Since this is another problem from IF (NOT) EXISTS, this is
> > > > >> in separate form.
> > > > >>
> > > > >> What do you think about this?
> > > > >>
> > > > >
> > > > > +1
> > > > >
> > > >
> > > > The new behave is much better.
> > >
> > > I'm glad to hear that.
> > >
> > > > I found new warning
> > > >
> > > >  tab-complete.c:1438:87: warning: right-hand operand of comma
> expression
> > > > has no effect [-Wunused-value]
> > >
> > > Mmm. Google said me that gcc 4.9 does so. I'm using 4.8.5 so I
> > > haven't see the warning.
> > >
> > > https://gcc.gnu.org/gcc-4.9/porting_to.html
> > >
> > > 1436:   else if (HeadMatches2("CREATE", "SCHEMA") &&
> > > 1437:SHIFT_TO_LAST1("CREATE") &&
> > > 1438:false) {} /* FALL THROUGH */
> > >
> > > > #define SHIFT_TO_LAST1(p1) \
> > > > (HEADSHIFT(find_last_index_of(p1, previous_words,
> > > previous_words_count)), \
> > > >  true)
> > >
> > > > #define HEADSHIFT(n) \
> > > > (head_shift += n, true)
> > >
> > > expanding the macros the lines at the error will be
> > >
> > > else if (HeadMatches2("CREATE", "SCHEMA") &&
> > >  (head_shift +=
> > >   find_last_index_of("CREATE", previous_words,
> > > previous_words_count),
> > >true) &&
> > >  false) {} /* FALL THROUGH */
> > >
> > > But the right hand value (true) is actually "used" in the
> > > expression (even though not effective). Perhaps (true && false)
> > > was potimized as false and the true is regarded to be unused?
> > > That's stupid.. Using functions instead of macros seems to solve
> > > this but they needed to be wraped by macros as
> > > additional_kw_query(). That's a pain..
> > >
> > > Any thougts?
> > >
> > > > There is small minor issue - I don't know if it is solvable.
> Autocomplete
> > > > is working only for "if" keyword. When I am writing "if " or "if "
> or "if
> > > > exi" - then autocomplete doesn't work. But this issue is exactly
> same for
> > > > other "multi words" completation like  "alter foreign data wrapper".
> So
> > > if
> > > > it is fixable, then it can be out of scope this patch.
> > >
> > > Yes.  It can be saved only by adding completion for every word,
> > > as some of the similar completion is doing.
> > >
> > > > anything else looks well.
> > >
> > > Thanks.
> > >
> >
> > please, final patch
>
> This needs to use gcc 4.9 to address, but CentOS7 doesn't have
> devtools-2 repo so now I'm building CentOS6 environment for this
> purpose. Please wait for a while.
>
>
sure, ok

regards

Pavel


> regards,
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>
>
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

At Thu, 31 Mar 2016 11:22:20 +0200, Pavel Stehule  
wrote in 
> 2016-03-30 10:34 GMT+02:00 Kyotaro HORIGUCHI <
> horiguchi.kyot...@lab.ntt.co.jp>:
> 
> > Hi,
> >
> > At Wed, 30 Mar 2016 09:23:49 +0200, Pavel Stehule 
> > wrote in <
> > cafj8prbvka6ng4jwz2qmro7inudfjws5w0+demvgzznuf-h...@mail.gmail.com>
> > > Hi
> > >
> > > ...
> > > >> =# alter table if
> > > >> =# alter table if exists
> > > >> ==
> > > >> =# alter table I
> > > >> =# alter table IF EXISTS// "information_schema" doesn't match.
> > > >>
> > > >> Since this is another problem from IF (NOT) EXISTS, this is
> > > >> in separate form.
> > > >>
> > > >> What do you think about this?
> > > >>
> > > >
> > > > +1
> > > >
> > >
> > > The new behave is much better.
> >
> > I'm glad to hear that.
> >
> > > I found new warning
> > >
> > >  tab-complete.c:1438:87: warning: right-hand operand of comma expression
> > > has no effect [-Wunused-value]
> >
> > Mmm. Google said me that gcc 4.9 does so. I'm using 4.8.5 so I
> > haven't see the warning.
> >
> > https://gcc.gnu.org/gcc-4.9/porting_to.html
> >
> > 1436:   else if (HeadMatches2("CREATE", "SCHEMA") &&
> > 1437:SHIFT_TO_LAST1("CREATE") &&
> > 1438:false) {} /* FALL THROUGH */
> >
> > > #define SHIFT_TO_LAST1(p1) \
> > > (HEADSHIFT(find_last_index_of(p1, previous_words,
> > previous_words_count)), \
> > >  true)
> >
> > > #define HEADSHIFT(n) \
> > > (head_shift += n, true)
> >
> > expanding the macros the lines at the error will be
> >
> > else if (HeadMatches2("CREATE", "SCHEMA") &&
> >  (head_shift +=
> >   find_last_index_of("CREATE", previous_words,
> > previous_words_count),
> >true) &&
> >  false) {} /* FALL THROUGH */
> >
> > But the right hand value (true) is actually "used" in the
> > expression (even though not effective). Perhaps (true && false)
> > was potimized as false and the true is regarded to be unused?
> > That's stupid.. Using functions instead of macros seems to solve
> > this but they needed to be wraped by macros as
> > additional_kw_query(). That's a pain..
> >
> > Any thougts?
> >
> > > There is small minor issue - I don't know if it is solvable. Autocomplete
> > > is working only for "if" keyword. When I am writing "if " or "if " or "if
> > > exi" - then autocomplete doesn't work. But this issue is exactly same for
> > > other "multi words" completation like  "alter foreign data wrapper". So
> > if
> > > it is fixable, then it can be out of scope this patch.
> >
> > Yes.  It can be saved only by adding completion for every word,
> > as some of the similar completion is doing.
> >
> > > anything else looks well.
> >
> > Thanks.
> >
> 
> please, final patch

This needs to use gcc 4.9 to address, but CentOS7 doesn't have
devtools-2 repo so now I'm building CentOS6 environment for this
purpose. Please wait for a while.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

2016-03-30 10:34 GMT+02:00 Kyotaro HORIGUCHI <
horiguchi.kyot...@lab.ntt.co.jp>:

> Hi,
>
> At Wed, 30 Mar 2016 09:23:49 +0200, Pavel Stehule 
> wrote in <
> cafj8prbvka6ng4jwz2qmro7inudfjws5w0+demvgzznuf-h...@mail.gmail.com>
> > Hi
> >
> > ...
> > >> =# alter table if
> > >> =# alter table if exists
> > >> ==
> > >> =# alter table I
> > >> =# alter table IF EXISTS// "information_schema" doesn't match.
> > >>
> > >> Since this is another problem from IF (NOT) EXISTS, this is
> > >> in separate form.
> > >>
> > >> What do you think about this?
> > >>
> > >
> > > +1
> > >
> >
> > The new behave is much better.
>
> I'm glad to hear that.
>
> > I found new warning
> >
> >  tab-complete.c:1438:87: warning: right-hand operand of comma expression
> > has no effect [-Wunused-value]
>
> Mmm. Google said me that gcc 4.9 does so. I'm using 4.8.5 so I
> haven't see the warning.
>
> https://gcc.gnu.org/gcc-4.9/porting_to.html
>
> 1436:   else if (HeadMatches2("CREATE", "SCHEMA") &&
> 1437:SHIFT_TO_LAST1("CREATE") &&
> 1438:false) {} /* FALL THROUGH */
>
> > #define SHIFT_TO_LAST1(p1) \
> > (HEADSHIFT(find_last_index_of(p1, previous_words,
> previous_words_count)), \
> >  true)
>
> > #define HEADSHIFT(n) \
> > (head_shift += n, true)
>
> expanding the macros the lines at the error will be
>
> else if (HeadMatches2("CREATE", "SCHEMA") &&
>  (head_shift +=
>   find_last_index_of("CREATE", previous_words,
> previous_words_count),
>true) &&
>  false) {} /* FALL THROUGH */
>
> But the right hand value (true) is actually "used" in the
> expression (even though not effective). Perhaps (true && false)
> was potimized as false and the true is regarded to be unused?
> That's stupid.. Using functions instead of macros seems to solve
> this but they needed to be wraped by macros as
> additional_kw_query(). That's a pain..
>
> Any thougts?
>
> > There is small minor issue - I don't know if it is solvable. Autocomplete
> > is working only for "if" keyword. When I am writing "if " or "if " or "if
> > exi" - then autocomplete doesn't work. But this issue is exactly same for
> > other "multi words" completation like  "alter foreign data wrapper". So
> if
> > it is fixable, then it can be out of scope this patch.
>
> Yes.  It can be saved only by adding completion for every word,
> as some of the similar completion is doing.
>
> > anything else looks well.
>
> Thanks.
>

please, final patch

Regards

Pavel


>
> regards,
>
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>
>
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Hi,

At Wed, 30 Mar 2016 09:23:49 +0200, Pavel Stehule  
wrote in 
> Hi
> 
> ...
> >> =# alter table if
> >> =# alter table if exists
> >> ==
> >> =# alter table I
> >> =# alter table IF EXISTS// "information_schema" doesn't match.
> >>
> >> Since this is another problem from IF (NOT) EXISTS, this is
> >> in separate form.
> >>
> >> What do you think about this?
> >>
> >
> > +1
> >
> 
> The new behave is much better.

I'm glad to hear that.

> I found new warning
> 
>  tab-complete.c:1438:87: warning: right-hand operand of comma expression
> has no effect [-Wunused-value]

Mmm. Google said me that gcc 4.9 does so. I'm using 4.8.5 so I
haven't see the warning.

https://gcc.gnu.org/gcc-4.9/porting_to.html

1436:   else if (HeadMatches2("CREATE", "SCHEMA") &&
1437:SHIFT_TO_LAST1("CREATE") &&
1438:false) {} /* FALL THROUGH */

> #define SHIFT_TO_LAST1(p1) \
> (HEADSHIFT(find_last_index_of(p1, previous_words, previous_words_count)), 
> \
>  true)

> #define HEADSHIFT(n) \
> (head_shift += n, true)

expanding the macros the lines at the error will be

else if (HeadMatches2("CREATE", "SCHEMA") &&
 (head_shift += 
  find_last_index_of("CREATE", previous_words, previous_words_count),
   true) &&
 false) {} /* FALL THROUGH */

But the right hand value (true) is actually "used" in the
expression (even though not effective). Perhaps (true && false)
was potimized as false and the true is regarded to be unused?
That's stupid.. Using functions instead of macros seems to solve
this but they needed to be wraped by macros as
additional_kw_query(). That's a pain..

Any thougts?

> There is small minor issue - I don't know if it is solvable. Autocomplete
> is working only for "if" keyword. When I am writing "if " or "if " or "if
> exi" - then autocomplete doesn't work. But this issue is exactly same for
> other "multi words" completation like  "alter foreign data wrapper". So if
> it is fixable, then it can be out of scope this patch.

Yes.  It can be saved only by adding completion for every word,
as some of the similar completion is doing.

> anything else looks well.

Thanks.

regards,


-- 
Kyotaro Horiguchi
NTT Open Source Software Center




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

Hi

...
>> =# alter table if
>> =# alter table if exists
>> ==
>> =# alter table I
>> =# alter table IF EXISTS// "information_schema" doesn't match.
>>
>> Since this is another problem from IF (NOT) EXISTS, this is
>> in separate form.
>>
>> What do you think about this?
>>
>
> +1
>

The new behave is much better.

I found new warning

 tab-complete.c:1438:87: warning: right-hand operand of comma expression
has no effect [-Wunused-value]
gcc -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute
-Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard
-g -O2 -I. -I. -I../../../src/interfaces/libpq -I../../../src/include
-D_GNU_SOURCE -I/usr/include/libxml2   -c -o sql_help.o sql_help.c

There is small minor issue - I don't know if it is solvable. Autocomplete
is working only for "if" keyword. When I am writing "if " or "if " or "if
exi" - then autocomplete doesn't work. But this issue is exactly same for
other "multi words" completation like  "alter foreign data wrapper". So if
it is fixable, then it can be out of scope this patch.

anything else looks well.

Regards

Pavel

>
> Regards
>
> Pavel
>
>
>
>>
>> regards,
>>
>> --
>> Kyotaro Horiguchi
>> NTT Open Source Software Center
>>
>
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

No one should care of this but to make shure..

At Tue, 29 Mar 2016 20:12:03 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI 
 wrote in 
<20160329.201203.78219296.horiguchi.kyot...@lab.ntt.co.jp>
> By the way, memory blocks that readline sees are freed by it but
> blocks allocated by the additional pstrdup seems abandoned
> without being freed. Actually, the address of newly allocated
> blocks seems increasing time by time slowly *even without this
> patch*..

psql doesn't leak memory. The increment was the result of new
history entry. src/common/exec.c does the following thing,

./common/exec.c:586:putenv(strdup(env_path));
./common/exec.c:597:putenv(strdup(env_path));

valgrind complains that the memory block allocated there is
definitely lost but it seems to be called once in lifetime of a
process.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

2016-03-30 5:43 GMT+02:00 Kyotaro HORIGUCHI :

> Hello,
>
> At Tue, 29 Mar 2016 13:12:06 +0200, Pavel Stehule 
> wrote in 

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Hello,

At Tue, 29 Mar 2016 13:12:06 +0200, Pavel Stehule  
wrote in 

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

2016-03-29 12:08 GMT+02:00 Kyotaro HORIGUCHI <
horiguchi.kyot...@lab.ntt.co.jp>:

> Hi, Pavel.
>
> At Tue, 29 Mar 2016 09:59:01 +0200, Pavel Stehule 
> wrote in <
> cafj8prdepggymz2axgtl33pud7x+xieao++wa+v9nqpqiyd...@mail.gmail.com>
> > > > On 3/18/16 3:22 AM, Pavel Stehule wrote:
> > > >
> > > > > I am looking this patch. It looks well, but this feature
> doesn't
> > > > > respect upper or lower chars. It enforce upper chars. This is
> not
> > > > > consistent with any other autocomplete.
> > >
> > > As mentioned before, upper-lower problem is an existing
> > > issue. The case of the words in a query result list cannot be
> > > edited since it may contain words that should not be changed,
> > > such as relation names. So we can address it only before issueing
> > > a query but I haven't found simple way to do it.
> > >
> >
> > This is unpleasant. I am sorry. I had very uncomfortable feeling from
> this
> > behave. I am thinking so it should be solvable - you have to convert only
> > keyword IF EXISTS or IF NOT EXISTS. Maybe there are not trivial solution,
> > but this should be fixed.
>
> I understand that and feel the same. But I don't want to put
> puzzling code. Defining a macro enable this by writing as the
> following.
>

puzzle is wrong, but nonconsistent behave is not acceptable

Regards

Pavel


>
> >else if (Matches2("ALTER", "TABLE"))
> >COMPLETE_WITH_SCHEMA_QUERY(
> >Query_for_list_of_tables,
> >ADDLIST("",
> >"('IF EXISTS'), ('ALL IN TABLESPACE')",
> >"('if exists'), ('all in tablespace')"));
> ...
> > else if (Matches2("ALTER", "POLICY"))
> > COMPLETE_WITH_QUERY(
> > ADDLIST(Query_for_list_of_policies,
> > "('IF EXISTS')", "('if exists')"));
>
> This will work as you expects but it looks quite redundant, but
> avoiding dynamic string (and/or malloc) operation would lead to
> the similar results. Integrating the ADDLIST into
> COMPLETE... might make the situation better.
>
> The attached patch does it only for "ALTER TABLE" and "ALTER
> POLICY".
>
> > > > > I checked it against sql help and these statements doesn't work
> > >
> > > Thank you very much.
> > >
> > > > > alter foreign table  drop column
> > > > > drop cast
> > > > > drop operator
> > > > > drop transform -- missing autocomplete completely
> > >
> > > These are not done. Each of them has issues to be addressed
> > > before adding completion of IF EXISTS.
> > >
> > > > > alter text search configuration jjj drop mapping
> > > > > alter type hhh drop attribute
> > > > > drop extension
> > >
> > > Done.
> > >
> > > > > drop text search
> > >
> > > I don't see the syntax "drop text search [if exists]".  drop text
> > > search (configuration|dictionary|parser|template) are already
> > > addressed.
> > >
> >
> > ok, probably my mistake. I am sorry.
> >
> >
> > >
> > > > > drop user mapping
> > >
> > > "drop user" was not completed with "mapping". I added it then
> > > addressed this. (This might be another issue.)
> > >
> > > > > alter table jjj add column
> > >
> > > Done if it is mentioning DROP COLUMN. But new two macros
> > > HeadMatches6 and 7 are introduced together.
> > >
> > > > > create temp sequence
> > > > > create sequence
> > >
> > > DROP SEQUENCE is already completed with IF EXISTS. CREATE [TEMP]
> > > SEQUENCE with IF NOT EXISTS is added.
> > >
> > > > Do you have an idea of when you will have a new patch ready?
> > >
> > > Sorry to to have been late. The attached is the revised version.
> > >
> >
> > I'll check it today.
>
> Thanks.
>
> regards,
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Hi, thank you for the suggestion.

At Tue, 29 Mar 2016 12:54:01 +0300, Artur Zakirov  
wrote in <56fa50b9.7000...@postgrespro.ru>
> On 29.03.2016 10:59, Pavel Stehule wrote:
> > > > I am looking this patch. It looks well, but this feature doesn't
> > > > respect upper or lower chars. It enforce upper chars. This is
> > > > not
> > > > consistent with any other autocomplete.
> >
> > As mentioned before, upper-lower problem is an existing
> > issue. The case of the words in a query result list cannot be
> > edited since it may contain words that should not be changed,
> > such as relation names. So we can address it only before issueing
> > a query but I haven't found simple way to do it.
> >
> >
> > This is unpleasant. I am sorry. I had very uncomfortable feeling from
> > this behave. I am thinking so it should be solvable - you have to
> > convert only keyword IF EXISTS or IF NOT EXISTS. Maybe there are not
> > trivial solution, but this should be fixed.
> >
> 
> Hello,
> 
> Can we do something like in the patch? This patch should be applied
> after the patch
> "0001-Suggest-IF-NOT-EXISTS-for-tab-completion-of-psql_v3.patch".

Unfortunately, all completions are shown in upper-case with it
for some cases (since COMP_KEYWORD_CASE is upper as the default
on my environment). This prevents names, that are not SQL
keywords, from getting further completion.

> =# alter table 
> ALL IN TABLESPACEPG_CATALOG.  PUBLIC.
> ALPHA.   PG_TEMP_1.   X
> IF EXISTSPG_TOAST.
> INFORMATION_SCHEMA.  PG_TOAST_TEMP_1. 

pg_strdup_keyword_case follows COMP_KEYWORD_CASE pset variable
which instructs how *SQL keywords* should be handled. It is not
for this usage. And as mentioned before, SQL keywords and
identifiers cannot be treated in the same way, so the place is
not suitable to do this.

By the way, memory blocks that readline sees are freed by it but
blocks allocated by the additional pstrdup seems abandoned
without being freed. Actually, the address of newly allocated
blocks seems increasing time by time slowly *even without this
patch*..

Of course, no one seems to be able to rush into out of memory by
this leak, though..

Anyway, using malloc is one reason of additional complexity and
it is the main cause that I avoided dynamic memory allocation.

Thoughts?

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Hi, Pavel.

At Tue, 29 Mar 2016 09:59:01 +0200, Pavel Stehule  
wrote in 
> > > On 3/18/16 3:22 AM, Pavel Stehule wrote:
> > >
> > > > I am looking this patch. It looks well, but this feature doesn't
> > > > respect upper or lower chars. It enforce upper chars. This is not
> > > > consistent with any other autocomplete.
> >
> > As mentioned before, upper-lower problem is an existing
> > issue. The case of the words in a query result list cannot be
> > edited since it may contain words that should not be changed,
> > such as relation names. So we can address it only before issueing
> > a query but I haven't found simple way to do it.
> >
> 
> This is unpleasant. I am sorry. I had very uncomfortable feeling from this
> behave. I am thinking so it should be solvable - you have to convert only
> keyword IF EXISTS or IF NOT EXISTS. Maybe there are not trivial solution,
> but this should be fixed.

I understand that and feel the same. But I don't want to put
puzzling code. Defining a macro enable this by writing as the
following.

>else if (Matches2("ALTER", "TABLE"))
>COMPLETE_WITH_SCHEMA_QUERY(
>Query_for_list_of_tables,
>ADDLIST("",
>"('IF EXISTS'), ('ALL IN TABLESPACE')",
>"('if exists'), ('all in tablespace')"));
...
> else if (Matches2("ALTER", "POLICY"))
> COMPLETE_WITH_QUERY(
> ADDLIST(Query_for_list_of_policies,
> "('IF EXISTS')", "('if exists')"));

This will work as you expects but it looks quite redundant, but
avoiding dynamic string (and/or malloc) operation would lead to
the similar results. Integrating the ADDLIST into
COMPLETE... might make the situation better.

The attached patch does it only for "ALTER TABLE" and "ALTER
POLICY".

> > > > I checked it against sql help and these statements doesn't work
> >
> > Thank you very much.
> >
> > > > alter foreign table  drop column
> > > > drop cast
> > > > drop operator
> > > > drop transform -- missing autocomplete completely
> >
> > These are not done. Each of them has issues to be addressed
> > before adding completion of IF EXISTS.
> >
> > > > alter text search configuration jjj drop mapping
> > > > alter type hhh drop attribute
> > > > drop extension
> >
> > Done.
> >
> > > > drop text search
> >
> > I don't see the syntax "drop text search [if exists]".  drop text
> > search (configuration|dictionary|parser|template) are already
> > addressed.
> >
> 
> ok, probably my mistake. I am sorry.
> 
> 
> >
> > > > drop user mapping
> >
> > "drop user" was not completed with "mapping". I added it then
> > addressed this. (This might be another issue.)
> >
> > > > alter table jjj add column
> >
> > Done if it is mentioning DROP COLUMN. But new two macros
> > HeadMatches6 and 7 are introduced together.
> >
> > > > create temp sequence
> > > > create sequence
> >
> > DROP SEQUENCE is already completed with IF EXISTS. CREATE [TEMP]
> > SEQUENCE with IF NOT EXISTS is added.
> >
> > > Do you have an idea of when you will have a new patch ready?
> >
> > Sorry to to have been late. The attached is the revised version.
> >
> 
> I'll check it today.

Thanks.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
>From c2e2469a72cbbd078a2fa3f300716f6e4b2ebc81 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Tue, 29 Mar 2016 19:01:13 +0900
Subject: [PATCH] Sample implement of case-sensitive additional list.

---
 src/bin/psql/tab-complete.c | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 73c5601..5d102d7 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -312,6 +312,10 @@ do { \
 	COMPLETE_WITH_LIST_CS(list); \
 } while (0)
 
+#define CHOOSE_CASE(b, e, u, l) (l && islower(text[0]) ? b l e : b u e)
+#define ADDLIST(q, u, l) \
+	CHOOSE_CASE(q "UNION SELECT * FROM (VALUES ", ") as x", u, l)
+
 /*
  * Assembly instructions for schema queries
  */
@@ -1437,9 +1441,12 @@ psql_completion(const char *text, int start, int end)
 
 	/* ALTER TABLE */
 	else if (Matches2("ALTER", "TABLE"))
-		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables,
-   "UNION SELECT 'IF EXISTS'"
-   "UNION SELECT 'ALL IN TABLESPACE'");
+		COMPLETE_WITH_SCHEMA_QUERY(
+			Query_for_list_of_tables,
+			ADDLIST("",
+	"('IF EXISTS'), ('ALL IN TABLESPACE')",
+	"('if exists'), ('all in tablespace')"));
+
 	/* Try ALTER TABLE after removing optional words IF EXISTS*/
 	else if (HeadMatches2("ALTER", "TABLE") &&
 			 MidMatchAndRemove2(2, "IF", "EXISTS") &&
@@ -1726,8 +1733,9 @@ psql_completion(const char *text, int start, int end)
 
 	/* ALTER POLICY */
 	else if (Matches2("ALTER", "POLICY"))
-		COMPLETE_WITH_QUERY(Query_for_list_of_policies
-			"UNION SELECT 'IF EXISTS'");
+	

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Artur Zakirov]

On 29.03.2016 10:59, Pavel Stehule wrote:

Hi

2016-03-29 8:43 GMT+02:00 Kyotaro HORIGUCHI
>:

Thank you Pavel, David.

Thank you for pointing syntaxes to be addressed. Most of the are
addressed in the attached patch.


At Tue, 22 Mar 2016 12:57:27 -0400, David Steele
> wrote in
<56f17977.8040...@pgmasters.net >
> Hi Kyotaro,
>
> On 3/18/16 3:22 AM, Pavel Stehule wrote:
>
> > I am looking this patch. It looks well, but this feature doesn't
> > respect upper or lower chars. It enforce upper chars. This is not
> > consistent with any other autocomplete.

As mentioned before, upper-lower problem is an existing
issue. The case of the words in a query result list cannot be
edited since it may contain words that should not be changed,
such as relation names. So we can address it only before issueing
a query but I haven't found simple way to do it.


This is unpleasant. I am sorry. I had very uncomfortable feeling from
this behave. I am thinking so it should be solvable - you have to
convert only keyword IF EXISTS or IF NOT EXISTS. Maybe there are not
trivial solution, but this should be fixed.



Hello,

Can we do something like in the patch? This patch should be applied 
after the patch 
"0001-Suggest-IF-NOT-EXISTS-for-tab-completion-of-psql_v3.patch".


--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 73c5601..ed4ff09 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -153,6 +153,7 @@ do { \
 do { \
 	completion_squery = &(query); \
 	completion_charp = addon; \
+	completion_case_sensitive = false; \
 	matches = completion_matches(text, complete_from_schema_query); \
 } while (0)
 
@@ -3754,7 +3755,17 @@ _complete_from_query(int is_schema_query, const char *text, int state)
 		while (list_index < PQntuples(result) &&
 			   (item = PQgetvalue(result, list_index++, 0)))
 			if (pg_strncasecmp(text, item, byte_length) == 0)
-return pg_strdup(item);
+			{
+if (completion_case_sensitive)
+	return pg_strdup(item);
+else
+
+	/*
+	 * If case insensitive matching was requested initially,
+	 * adjust the case according to setting.
+	 */
+	return pg_strdup_keyword_case(item, text);
+			}
 	}
 
 	/* If nothing matches, free the db structure and return null */

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

Hi

2016-03-29 8:43 GMT+02:00 Kyotaro HORIGUCHI :

> Thank you Pavel, David.
>
> Thank you for pointing syntaxes to be addressed. Most of the are
> addressed in the attached patch.
>
>
> At Tue, 22 Mar 2016 12:57:27 -0400, David Steele 
> wrote in <56f17977.8040...@pgmasters.net>
> > Hi Kyotaro,
> >
> > On 3/18/16 3:22 AM, Pavel Stehule wrote:
> >
> > > I am looking this patch. It looks well, but this feature doesn't
> > > respect upper or lower chars. It enforce upper chars. This is not
> > > consistent with any other autocomplete.
>
> As mentioned before, upper-lower problem is an existing
> issue. The case of the words in a query result list cannot be
> edited since it may contain words that should not be changed,
> such as relation names. So we can address it only before issueing
> a query but I haven't found simple way to do it.
>

This is unpleasant. I am sorry. I had very uncomfortable feeling from this
behave. I am thinking so it should be solvable - you have to convert only
keyword IF EXISTS or IF NOT EXISTS. Maybe there are not trivial solution,
but this should be fixed.


>
> > > I checked it against sql help and these statements doesn't work
>
> Thank you very much.
>
> > > alter foreign table  drop column
> > > drop cast
> > > drop operator
> > > drop transform -- missing autocomplete completely
>
> These are not done. Each of them has issues to be addressed
> before adding completion of IF EXISTS.
>
> > > alter text search configuration jjj drop mapping
> > > alter type hhh drop attribute
> > > drop extension
>
> Done.
>
> > > drop text search
>
> I don't see the syntax "drop text search [if exists]".  drop text
> search (configuration|dictionary|parser|template) are already
> addressed.
>

ok, probably my mistake. I am sorry.


>
> > > drop user mapping
>
> "drop user" was not completed with "mapping". I added it then
> addressed this. (This might be another issue.)
>
> > > alter table jjj add column
>
> Done if it is mentioning DROP COLUMN. But new two macros
> HeadMatches6 and 7 are introduced together.
>
> > > create temp sequence
> > > create sequence
>
> DROP SEQUENCE is already completed with IF EXISTS. CREATE [TEMP]
> SEQUENCE with IF NOT EXISTS is added.
>
> > Do you have an idea of when you will have a new patch ready?
>
> Sorry to to have been late. The attached is the revised version.
>

I'll check it today.

Regards

Pavel


>
> regards,
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Thank you Pavel, David.

Thank you for pointing syntaxes to be addressed. Most of the are
addressed in the attached patch.


At Tue, 22 Mar 2016 12:57:27 -0400, David Steele  wrote in 
<56f17977.8040...@pgmasters.net>
> Hi Kyotaro,
> 
> On 3/18/16 3:22 AM, Pavel Stehule wrote:
> 
> > I am looking this patch. It looks well, but this feature doesn't
> > respect upper or lower chars. It enforce upper chars. This is not
> > consistent with any other autocomplete.

As mentioned before, upper-lower problem is an existing
issue. The case of the words in a query result list cannot be
edited since it may contain words that should not be changed,
such as relation names. So we can address it only before issueing
a query but I haven't found simple way to do it.

> > I checked it against sql help and these statements doesn't work

Thank you very much.

> > alter foreign table  drop column
> > drop cast
> > drop operator
> > drop transform -- missing autocomplete completely

These are not done. Each of them has issues to be addressed
before adding completion of IF EXISTS.

> > alter text search configuration jjj drop mapping
> > alter type hhh drop attribute
> > drop extension

Done.

> > drop text search

I don't see the syntax "drop text search [if exists]".  drop text
search (configuration|dictionary|parser|template) are already
addressed.

> > drop user mapping

"drop user" was not completed with "mapping". I added it then
addressed this. (This might be another issue.)

> > alter table jjj add column

Done if it is mentioning DROP COLUMN. But new two macros
HeadMatches6 and 7 are introduced together.

> > create temp sequence
> > create sequence

DROP SEQUENCE is already completed with IF EXISTS. CREATE [TEMP]
SEQUENCE with IF NOT EXISTS is added.

> Do you have an idea of when you will have a new patch ready?

Sorry to to have been late. The attached is the revised version.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
>From 2f46c0aa00fd8fd6357dcb76a26e49e3a66e2572 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Fri, 5 Feb 2016 16:50:35 +0900
Subject: [PATCH] Suggest IF (NOT) EXISTS for tab-completion of psql

This patch lets psql to suggest "IF (NOT) EXISTS". Addition to that,
since this patch introduces some mechanism for syntactical robustness,
it allows psql completion to omit some optional part on matching.
---
 src/bin/psql/tab-complete.c | 626 
 1 file changed, 524 insertions(+), 102 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 6a81416..73c5601 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -656,6 +656,10 @@ static const SchemaQuery Query_for_list_of_matviews = {
 "   FROM pg_catalog.pg_roles "\
 "  WHERE substring(pg_catalog.quote_ident(rolname),1,%d)='%s'"
 
+#define Query_for_list_of_rules \
+"SELECT pg_catalog.quote_ident(rulename) FROM pg_catalog.pg_rules "\
+" WHERE substring(pg_catalog.quote_ident(rulename),1,%d)='%s'"
+
 #define Query_for_list_of_grant_roles \
 " SELECT pg_catalog.quote_ident(rolname) "\
 "   FROM pg_catalog.pg_roles "\
@@ -763,6 +767,11 @@ static const SchemaQuery Query_for_list_of_matviews = {
 "SELECT pg_catalog.quote_ident(tmplname) FROM pg_catalog.pg_ts_template "\
 " WHERE substring(pg_catalog.quote_ident(tmplname),1,%d)='%s'"
 
+#define Query_for_list_of_triggers \
+"SELECT pg_catalog.quote_ident(tgname) FROM pg_catalog.pg_trigger "\
+" WHERE substring(pg_catalog.quote_ident(tgname),1,%d)='%s' AND "\
+"   NOT tgisinternal"
+
 #define Query_for_list_of_fdws \
 " SELECT pg_catalog.quote_ident(fdwname) "\
 "   FROM pg_catalog.pg_foreign_data_wrapper "\
@@ -906,7 +915,7 @@ static const pgsql_thing_t words_after_create[] = {
 	{"PARSER", Query_for_list_of_ts_parsers, NULL, THING_NO_SHOW},
 	{"POLICY", NULL, NULL},
 	{"ROLE", Query_for_list_of_roles},
-	{"RULE", "SELECT pg_catalog.quote_ident(rulename) FROM pg_catalog.pg_rules WHERE substring(pg_catalog.quote_ident(rulename),1,%d)='%s'"},
+	{"RULE", Query_for_list_of_rules},
 	{"SCHEMA", Query_for_list_of_schemas},
 	{"SEQUENCE", NULL, _for_list_of_sequences},
 	{"SERVER", Query_for_list_of_servers},
@@ -915,7 +924,7 @@ static const pgsql_thing_t words_after_create[] = {
 	{"TEMP", NULL, NULL, THING_NO_DROP},		/* for CREATE TEMP TABLE ... */
 	{"TEMPLATE", Query_for_list_of_ts_templates, NULL, THING_NO_SHOW},
 	{"TEXT SEARCH", NULL, NULL},
-	{"TRIGGER", "SELECT pg_catalog.quote_ident(tgname) FROM pg_catalog.pg_trigger WHERE substring(pg_catalog.quote_ident(tgname),1,%d)='%s' AND NOT tgisinternal"},
+	{"TRIGGER", Query_for_list_of_triggers},
 	{"TYPE", NULL, _for_list_of_datatypes},
 	{"UNIQUE", NULL, NULL, THING_NO_DROP},		/* for CREATE UNIQUE INDEX ... */
 	{"UNLOGGED", NULL, NULL, THING_NO_DROP},	/* for CREATE UNLOGGED TABLE
@@ -944,6 +953,7 @@ static char **complete_from_variables(const char *text,
 	

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[David Steele]

Hi Kyotaro,

On 3/18/16 3:22 AM, Pavel Stehule wrote:

> I am looking this patch. It looks well, but this feature doesn't
> respect upper or lower chars. It enforce upper chars. This is not
> consistent with any other autocomplete.
> 
> 
> I checked it against sql help and these statements doesn't work
> 
> alter foreign table  drop column
> alter text search configuration jjj drop mapping
> alter type hhh drop attribute
> drop cast
> drop extension
> drop operator
> drop text search
> drop transform -- missing autocomplete completely
> drop user mapping
> alter table jjj add column
> create temp sequence
> create sequence

Do you have an idea of when you will have a new patch ready?

Thanks,
-- 
-David
da...@pgmasters.net


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Michael Paquier]

On Wed, Mar 16, 2016 at 2:42 AM, Robert Haas  wrote:
> On Fri, Feb 26, 2016 at 2:37 AM, Kyotaro HORIGUCHI
>  wrote:
>> Hello, this is the new version of this patch.
>
> The CommitFest entry for this patch is messed up.  It shows no author,
> even though I'm pretty sure that a patch has to have one by
> definition.

The CF app does not require an author name when the entry is first
created. The author needs to be added afterwards. A message-id, a
description and a category are the mandatory things.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

Hi

2016-03-17 21:02 GMT+01:00 Pavel Stehule :

> Hi
>
>
> I am looking this patch. It looks well, but this feature doesn't respect
> upper or lower chars. It enforce upper chars. This is not consistent with
> any other autocomplete.
>

I checked it against sql help and these statements doesn't work

alter foreign table  drop column
alter text search configuration jjj drop mapping
alter type hhh drop attribute
drop cast
drop extension
drop operator
drop text search
drop transform -- missing autocomplete completely
drop user mapping
alter table jjj add column
create temp sequence
create sequence

Regards

Pavel


> Regards
>
> Pavel
>
>
>
>>
>> regards,
>>
>> --
>> Kyotaro Horiguchi
>> NTT Open Source Software Center
>>
>>
>>
>>
>> --
>> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgsql-hackers
>>
>
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

Hi

2016-03-16 5:01 GMT+01:00 Kyotaro HORIGUCHI :

> Hello,
>
> # It seems that I have been forgotten in the recepient list..
>
> At Tue, 15 Mar 2016 22:09:59 -0400, Peter Eisentraut 
> wrote in <56e8c077.2000...@gmx.net>
> > On 2/5/16 3:09 AM, Kyotaro HORIGUCHI wrote:
> > > I considered how to make tab-completion robust for syntactical
> > > noises, in other words, optional words in syntax. Typically "IF
> > > (NOT) EXISTS", UNIQUE and TEMPORARY are words that don't affect
> > > further completion.
> >
> > To repeat the question I raised in the previous commit fest about tab
> > completion: Why do you want tab completion for IF NOT EXISTS?  When you
> > tab complete, the completion mechanism will show you whether the item in
> > question exists.  What is the use case?
>
> Ah, I think I understand you question. It's not about IF EXISTS,
> but only IF NOT EXSTS. It is needed when repeated execution of
> the same SQL statement will be done using command line
> history. Such stocks of commands in history is often
> convenient. And sometimes I rely on psql-completion to write a
> SQL script. The completions for such words seemingly useless on
> instant-execution will be handy to do that.
>
> Another thing I want to do by this patch is that we can get
> completion even after such optional words. I have been annoyed
> many times by this. Some of them, such as UNIQUE, TEMPORARY and
> CONCURRENTLY are treated but they just doubles the matching
> condition expressions.
>

I am looking this patch. It looks well, but this feature doesn't respect
upper or lower chars. It enforce upper chars. This is not consistent with
any other autocomplete.

Regards

Pavel



>
> regards,
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>
>
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Mmm. Have I broken the entry?

At Tue, 15 Mar 2016 13:55:24 -0400, David Steele  wrote in 
<56e84c8c.7060...@pgmasters.net>
> On 3/15/16 1:42 PM, Robert Haas wrote:
> > On Fri, Feb 26, 2016 at 2:37 AM, Kyotaro HORIGUCHI
> >  wrote:
> >> Hello, this is the new version of this patch.
> > 
> > The CommitFest entry for this patch is messed up.  It shows no author,
> > even though I'm pretty sure that a patch has to have one by
> > definition.
> > 
> > https://commitfest.postgresql.org/9/518/
> > 
> > Also, this patch was listed as Waiting on Author, but it's been
> > updated since it was last reviewed, so I switched it back to Needs
> > Review.  Can someone please review it and, if appropriate, mark it
> > Ready for Committer?
> 
> Author has been set to Kyotaro Horiguchi.

Thank you for repairing it.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Hello,

# It seems that I have been forgotten in the recepient list..

At Tue, 15 Mar 2016 22:09:59 -0400, Peter Eisentraut  wrote in 
<56e8c077.2000...@gmx.net>
> On 2/5/16 3:09 AM, Kyotaro HORIGUCHI wrote:
> > I considered how to make tab-completion robust for syntactical
> > noises, in other words, optional words in syntax. Typically "IF
> > (NOT) EXISTS", UNIQUE and TEMPORARY are words that don't affect
> > further completion.
> 
> To repeat the question I raised in the previous commit fest about tab
> completion: Why do you want tab completion for IF NOT EXISTS?  When you
> tab complete, the completion mechanism will show you whether the item in
> question exists.  What is the use case?

Ah, I think I understand you question. It's not about IF EXISTS,
but only IF NOT EXSTS. It is needed when repeated execution of
the same SQL statement will be done using command line
history. Such stocks of commands in history is often
convenient. And sometimes I rely on psql-completion to write a
SQL script. The completions for such words seemingly useless on
instant-execution will be handy to do that.

Another thing I want to do by this patch is that we can get
completion even after such optional words. I have been annoyed
many times by this. Some of them, such as UNIQUE, TEMPORARY and
CONCURRENTLY are treated but they just doubles the matching
condition expressions.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Peter Eisentraut]

On 2/5/16 3:09 AM, Kyotaro HORIGUCHI wrote:
> I considered how to make tab-completion robust for syntactical
> noises, in other words, optional words in syntax. Typically "IF
> (NOT) EXISTS", UNIQUE and TEMPORARY are words that don't affect
> further completion.

To repeat the question I raised in the previous commit fest about tab
completion: Why do you want tab completion for IF NOT EXISTS?  When you
tab complete, the completion mechanism will show you whether the item in
question exists.  What is the use case?




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[David Steele]

On 3/15/16 1:42 PM, Robert Haas wrote:
> On Fri, Feb 26, 2016 at 2:37 AM, Kyotaro HORIGUCHI
>  wrote:
>> Hello, this is the new version of this patch.
> 
> The CommitFest entry for this patch is messed up.  It shows no author,
> even though I'm pretty sure that a patch has to have one by
> definition.
> 
> https://commitfest.postgresql.org/9/518/
> 
> Also, this patch was listed as Waiting on Author, but it's been
> updated since it was last reviewed, so I switched it back to Needs
> Review.  Can someone please review it and, if appropriate, mark it
> Ready for Committer?

Author has been set to Kyotaro Horiguchi.

-- 
-David
da...@pgmasters.net


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Robert Haas]

On Fri, Feb 26, 2016 at 2:37 AM, Kyotaro HORIGUCHI
 wrote:
> Hello, this is the new version of this patch.

The CommitFest entry for this patch is messed up.  It shows no author,
even though I'm pretty sure that a patch has to have one by
definition.

https://commitfest.postgresql.org/9/518/

Also, this patch was listed as Waiting on Author, but it's been
updated since it was last reviewed, so I switched it back to Needs
Review.  Can someone please review it and, if appropriate, mark it
Ready for Committer?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Hello, this is the new version of this patch.

At Fri, 26 Feb 2016 13:17:26 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI 
 wrote in 
<20160226.131726.54224488.horiguchi.kyot...@lab.ntt.co.jp>
> Hello, thank you for the comments.
> 
> At Mon, 15 Feb 2016 15:43:57 +0300, Artur Zakirov  
> wrote in <56c1c80d.7020...@postgrespro.ru>
> > I did some tests with your patch. But I am not confident in
> > tab-complete.c.
> > 
> > And I have some notes:
> > 
> > 1 - I execute git apply command and get the following warning:
> > 
> > ../0001-Suggest-IF-NOT-EXISTS-for-tab-completion-of-psql.patch:302:
> > trailing whitespace.
> > /*
> > warning: 1 line adds whitespace errors.
> > 
> > This is because of superfluous whitespace I think.
> 
> Oops. I'll remove it.
> 
> > 2 - In psql I write "create table if" and press . psql adds the
> > following:
> > 
> > create table IF NOT EXISTS
> > 
> > I think psql should continue with lower case if user wrote query with
> > loser case text:
> 
> Good catch! Hmm. COMPLETE_WITH_SCHEMA_QUERY() does it. For
> example, the following existing completion behaves in the same
> way.
> 
> "drop index c" ==> "drop index CONCURRENTLY"
> 
> The results of schema queries should be treated in case-sensitive
> way so the additional keywords by 'UNION' are also treated
> case-sensitively.
> 
> COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_indexes,
>" UNION SELECT 'CONCURRENTLY'");
> 
> Fixing this is another problem. So I'd like to leave this alone
> here.
> 
> > create table if not exists
> > 
> > 3 - Same with "IF EXISTS". If a write "alter view if" and press 
> > psql writes:
> > 
> > alter view IF EXISTS

Finally, the only thing done in this new patch is removing one
dangling space.

reagrds,


-- 
Kyotaro Horiguchi
NTT Open Source Software Center
>From f9610a87d73629ac3fcc25fcf8e667c5ca3e921d Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Fri, 5 Feb 2016 16:50:35 +0900
Subject: [PATCH] Suggest IF (NOT) EXISTS for tab-completion of psql

This patch lets psql to suggest "IF (NOT) EXISTS". Addition to that,
since this patch introduces some mechanism for syntactical robustness,
it allows psql completion to omit some optional part on matching.
---
 src/bin/psql/tab-complete.c | 559 
 1 file changed, 463 insertions(+), 96 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 5f27120..d3eab8c 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -656,6 +656,10 @@ static const SchemaQuery Query_for_list_of_matviews = {
 "   FROM pg_catalog.pg_roles "\
 "  WHERE substring(pg_catalog.quote_ident(rolname),1,%d)='%s'"
 
+#define Query_for_list_of_rules \
+"SELECT pg_catalog.quote_ident(rulename) FROM pg_catalog.pg_rules "\
+" WHERE substring(pg_catalog.quote_ident(rulename),1,%d)='%s'"
+
 #define Query_for_list_of_grant_roles \
 " SELECT pg_catalog.quote_ident(rolname) "\
 "   FROM pg_catalog.pg_roles "\
@@ -763,6 +767,11 @@ static const SchemaQuery Query_for_list_of_matviews = {
 "SELECT pg_catalog.quote_ident(tmplname) FROM pg_catalog.pg_ts_template "\
 " WHERE substring(pg_catalog.quote_ident(tmplname),1,%d)='%s'"
 
+#define Query_for_list_of_triggers \
+"SELECT pg_catalog.quote_ident(tgname) FROM pg_catalog.pg_trigger "\
+" WHERE substring(pg_catalog.quote_ident(tgname),1,%d)='%s' AND "\
+"   NOT tgisinternal"
+
 #define Query_for_list_of_fdws \
 " SELECT pg_catalog.quote_ident(fdwname) "\
 "   FROM pg_catalog.pg_foreign_data_wrapper "\
@@ -906,7 +915,7 @@ static const pgsql_thing_t words_after_create[] = {
 	{"PARSER", Query_for_list_of_ts_parsers, NULL, THING_NO_SHOW},
 	{"POLICY", NULL, NULL},
 	{"ROLE", Query_for_list_of_roles},
-	{"RULE", "SELECT pg_catalog.quote_ident(rulename) FROM pg_catalog.pg_rules WHERE substring(pg_catalog.quote_ident(rulename),1,%d)='%s'"},
+	{"RULE", Query_for_list_of_rules},
 	{"SCHEMA", Query_for_list_of_schemas},
 	{"SEQUENCE", NULL, _for_list_of_sequences},
 	{"SERVER", Query_for_list_of_servers},
@@ -915,7 +924,7 @@ static const pgsql_thing_t words_after_create[] = {
 	{"TEMP", NULL, NULL, THING_NO_DROP},		/* for CREATE TEMP TABLE ... */
 	{"TEMPLATE", Query_for_list_of_ts_templates, NULL, THING_NO_SHOW},
 	{"TEXT SEARCH", NULL, NULL},
-	{"TRIGGER", "SELECT pg_catalog.quote_ident(tgname) FROM pg_catalog.pg_trigger WHERE substring(pg_catalog.quote_ident(tgname),1,%d)='%s' AND NOT tgisinternal"},
+	{"TRIGGER", Query_for_list_of_triggers},
 	{"TYPE", NULL, _for_list_of_datatypes},
 	{"UNIQUE", NULL, NULL, THING_NO_DROP},		/* for CREATE UNIQUE INDEX ... */
 	{"UNLOGGED", NULL, NULL, THING_NO_DROP},	/* for CREATE UNLOGGED TABLE
@@ -944,6 +953,7 @@ static char **complete_from_variables(const char *text,
 	const char *prefix, const char *suffix, bool need_value);
 static char *complete_from_files(const char *text, int state);
 

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Hello, thank you for the comments.

At Mon, 15 Feb 2016 15:43:57 +0300, Artur Zakirov  
wrote in <56c1c80d.7020...@postgrespro.ru>
> On 05.02.2016 11:09, Kyotaro HORIGUCHI wrote:
> > Hello,
> >
> > I considered how to make tab-completion robust for syntactical
> > noises, in other words, optional words in syntax. Typically "IF
> > (NOT) EXISTS", UNIQUE and TEMPORARY are words that don't affect
> > further completion. However, the current delimit-matching
> > mechanism is not so capable (or is complexty-prone) to live with
> > such noises. I have proposed to use regular expressions or
> > simplified one for the robustness but it was too complex to be
> > applied.
> >
> > This is another answer for the problem. Removal of such words
> > on-the-fly makes further matching more robust.
> >
> > Next, currently some CREATE xxx subsyntaxes of CREATE SCHEMA are
> > matched using TailMatching but it makes difficult the
> > options-removal operations, which needs forward matching.
> >
> > So I introduced two things to resolve them by this patch.
> >
> 
> I did some tests with your patch. But I am not confident in
> tab-complete.c.
> 
> And I have some notes:
> 
> 1 - I execute git apply command and get the following warning:
> 
> ../0001-Suggest-IF-NOT-EXISTS-for-tab-completion-of-psql.patch:302:
> trailing whitespace.
>   /*
> warning: 1 line adds whitespace errors.
> 
> This is because of superfluous whitespace I think.

Oops. I'll remove it.

> 2 - In psql I write "create table if" and press . psql adds the
> following:
> 
> create table IF NOT EXISTS
> 
> I think psql should continue with lower case if user wrote query with
> loser case text:

Good catch! Hmm. COMPLETE_WITH_SCHEMA_QUERY() does it. For
example, the following existing completion behaves in the same
way.

"drop index c" ==> "drop index CONCURRENTLY"

The results of schema queries should be treated in case-sensitive
way so the additional keywords by 'UNION' are also treated
case-sensitively.

COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_indexes,
   " UNION SELECT 'CONCURRENTLY'");

Fixing this is another problem. So I'd like to leave this alone
here.

> create table if not exists
> 
> 3 - Same with "IF EXISTS". If a write "alter view if" and press 
> psql writes:
> 
> alter view IF EXISTS

regards,

-- 
堀口恭太郎

日本電信電話株式会社 NTTオープンソースソフトウェアセンタ
Phone: 03-5860-5115 / Fax: 03-5463-5490




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Artur Zakirov]

On 05.02.2016 11:09, Kyotaro HORIGUCHI wrote:

Hello,

I considered how to make tab-completion robust for syntactical
noises, in other words, optional words in syntax. Typically "IF
(NOT) EXISTS", UNIQUE and TEMPORARY are words that don't affect
further completion. However, the current delimit-matching
mechanism is not so capable (or is complexty-prone) to live with
such noises. I have proposed to use regular expressions or
simplified one for the robustness but it was too complex to be
applied.

This is another answer for the problem. Removal of such words
on-the-fly makes further matching more robust.

Next, currently some CREATE xxx subsyntaxes of CREATE SCHEMA are
matched using TailMatching but it makes difficult the
options-removal operations, which needs forward matching.

So I introduced two things to resolve them by this patch.



I did some tests with your patch. But I am not confident in tab-complete.c.

And I have some notes:

1 - I execute git apply command and get the following warning:

../0001-Suggest-IF-NOT-EXISTS-for-tab-completion-of-psql.patch:302: 
trailing whitespace.

/*
warning: 1 line adds whitespace errors.

This is because of superfluous whitespace I think.

2 - In psql I write "create table if" and press . psql adds the 
following:


create table IF NOT EXISTS

I think psql should continue with lower case if user wrote query with 
loser case text:


create table if not exists

3 - Same with "IF EXISTS". If a write "alter view if" and press  
psql writes:


alter view IF EXISTS

--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers