Horton

Thread

Commits

  1. Fix loose ends for SQL ACCESS METHOD objects

  1. COMMENT ON, psql and access methods

    Michael Paquier <michael.paquier@gmail.com> — 2016-05-27T11:53:38Z

    Hi all,

As far as I can see, COMMENT ON has no support for access methods.
Wouldn't we want to add it as it is created by a command? On top of
that, perhaps we could have a backslash command in psql to list the
supported access methods, like \dam[S]? The system methods would be in
this case all the in-core ones.

Regards,
-- 
Michael

  2. Re: COMMENT ON, psql and access methods

    Tom Lane <tgl@sss.pgh.pa.us> — 2016-05-27T13:53:56Z

    Michael Paquier <michael.paquier@gmail.com> writes:
> As far as I can see, COMMENT ON has no support for access methods.
> Wouldn't we want to add it as it is created by a command? On top of
> that, perhaps we could have a backslash command in psql to list the
> supported access methods, like \dam[S]? The system methods would be in
> this case all the in-core ones.

I'm a bit skeptical that we need this yet.  Maybe if custom access methods
become so popular that everybody's got one, it'd be worth the trouble.

			regards, tom lane

  3. Re: COMMENT ON, psql and access methods

    Robert Haas <robertmhaas@gmail.com> — 2016-05-31T19:52:35Z

    On Fri, May 27, 2016 at 7:53 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> As far as I can see, COMMENT ON has no support for access methods.
> Wouldn't we want to add it as it is created by a command? On top of
> that, perhaps we could have a backslash command in psql to list the
> supported access methods, like \dam[S]? The system methods would be in
> this case all the in-core ones.

+1.

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

  4. Re: COMMENT ON, psql and access methods

    Michael Paquier <michael.paquier@gmail.com> — 2016-06-01T00:24:11Z

    On Wed, Jun 1, 2016 at 4:52 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Fri, May 27, 2016 at 7:53 AM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> As far as I can see, COMMENT ON has no support for access methods.
>> Wouldn't we want to add it as it is created by a command? On top of
>> that, perhaps we could have a backslash command in psql to list the
>> supported access methods, like \dam[S]? The system methods would be in
>> this case all the in-core ones.
>
> +1.

Are there other opinions? That's not a 9.6 blocker IMO, so I could get
patches out for 10.0 if needed.
-- 
Michael

  5. Re: COMMENT ON, psql and access methods

    Teodor Sigaev <teodor@sigaev.ru> — 2016-06-01T11:25:49Z

    >>> As far as I can see, COMMENT ON has no support for access methods.
>>> Wouldn't we want to add it as it is created by a command? On top of
>>> that, perhaps we could have a backslash command in psql to list the
>>> supported access methods, like \dam[S]? The system methods would be in
>>> this case all the in-core ones.
>>
>> +1.
>
> Are there other opinions? That's not a 9.6 blocker IMO, so I could get
> patches out for 10.0 if needed.

I missed that on review process, but I'm agree it should be implemented.

-- 
Teodor Sigaev                                   E-mail: teodor@sigaev.ru
                                                    WWW: http://www.sigaev.ru/

  6. Re: COMMENT ON, psql and access methods

    Michael Paquier <michael.paquier@gmail.com> — 2016-06-02T04:00:19Z

    On Wed, Jun 1, 2016 at 8:25 PM, Teodor Sigaev <teodor@sigaev.ru> wrote:
>>>> As far as I can see, COMMENT ON has no support for access methods.
>>>> Wouldn't we want to add it as it is created by a command? On top of
>>>> that, perhaps we could have a backslash command in psql to list the
>>>> supported access methods, like \dam[S]? The system methods would be in
>>>> this case all the in-core ones.
>>>
>>>
>>> +1.
>>
>>
>> Are there other opinions? That's not a 9.6 blocker IMO, so I could get
>> patches out for 10.0 if needed.
>
>
> I missed that on review process, but I'm agree it should be implemented.

So, I have taken the time to hack that, and finished with the patch
attached, that is less intrusive than I thought first:
- COMMENT support is added for access methods
- Added meta-command \dA in psql to list the available access methods:
=# \dA
List of access methods
  Name  | Type
--------+-------
 bloom  | Index
 brin   | Index
 btree  | Index
 gin    | Index
 gist   | Index
 hash   | Index
 spgist | Index
(7 rows)
=# \dA+
                        List of access methods
  Name  | Type  |   Handler   |              Description
--------+-------+-------------+----------------------------------------
 bloom  | Index | blhandler   | bloom index access method
 brin   | Index | brinhandler | block range index (BRIN) access method
 btree  | Index | bthandler   | b-tree index access method
 gin    | Index | ginhandler  | GIN index access method
 gist   | Index | gisthandler | GiST index access method
 hash   | Index | hashhandler | hash index access method
 spgist | Index | spghandler  | SP-GiST index access method
(7 rows)
- Fixed a missing tab completion for DROP ACCESS METHOD.

I have added an open item for 9.6 regarding this patch, that would be
good to complete this work in this release for consistency with the
other objects.
Regards,
-- 
Michael

  7. Re: COMMENT ON, psql and access methods

    Michael Paquier <michael.paquier@gmail.com> — 2016-06-02T06:42:42Z

    On Thu, Jun 2, 2016 at 1:00 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> I have added an open item for 9.6 regarding this patch, that would be
> good to complete this work in this release for consistency with the
> other objects.

Doh. I forgot to update psql --help.
-- 
Michael

  8. Re: COMMENT ON, psql and access methods

    Michael Paquier <michael.paquier@gmail.com> — 2016-06-02T07:00:33Z

    On Thu, Jun 2, 2016 at 3:42 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Thu, Jun 2, 2016 at 1:00 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> I have added an open item for 9.6 regarding this patch, that would be
>> good to complete this work in this release for consistency with the
>> other objects.
>
> Doh. I forgot to update psql --help.

And Query_for_list_of_access_methods was defined more or less twice,
the one of my patch having an error...
-- 
Michael

  9. Re: COMMENT ON, psql and access methods

    Noah Misch <noah@leadboat.com> — 2016-06-04T21:54:16Z

    On Thu, Jun 02, 2016 at 04:00:33PM +0900, Michael Paquier wrote:
> On Thu, Jun 2, 2016 at 3:42 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
> > On Thu, Jun 2, 2016 at 1:00 PM, Michael Paquier
> > <michael.paquier@gmail.com> wrote:
> >> I have added an open item for 9.6 regarding this patch, that would be
> >> good to complete this work in this release for consistency with the
> >> other objects.
> >
> > Doh. I forgot to update psql --help.
> 
> And Query_for_list_of_access_methods was defined more or less twice,
> the one of my patch having an error...

[Action required within 72 hours.  This is a generic notification.]

The above-described topic is currently a PostgreSQL 9.6 open item.  Álvaro,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
9.6 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within 72 hours of this
message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping 9.6rc1.  Consequently, I will appreciate your
efforts toward speedy resolution.  Thanks.

[1] http://www.postgresql.org/message-id/20160527025039.GA447393@tornado.leadboat.com

  10. Re: COMMENT ON, psql and access methods

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2016-06-06T14:21:33Z

    Noah Misch wrote:

> The above-described topic is currently a PostgreSQL 9.6 open item.  Álvaro,
> since you committed the patch believed to have created it, you own this open
> item.  If some other commit is more relevant or if this does not belong as a
> 9.6 open item, please let us know.  Otherwise, please observe the policy on
> open item ownership[1] and send a status update within 72 hours of this
> message.  Include a date for your subsequent status update.  Testers may
> discover new open items at any time, and I want to plan to get them all fixed
> well in advance of shipping 9.6rc1.  Consequently, I will appreciate your
> efforts toward speedy resolution.  Thanks.

I'll have this patch reviewed, and barring serious problems, committed
no later than EOB Friday 10th.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  11. Re: COMMENT ON, psql and access methods

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2016-06-06T22:35:01Z

    Michael Paquier wrote:
> On Thu, Jun 2, 2016 at 3:42 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
> > On Thu, Jun 2, 2016 at 1:00 PM, Michael Paquier
> > <michael.paquier@gmail.com> wrote:
> >> I have added an open item for 9.6 regarding this patch, that would be
> >> good to complete this work in this release for consistency with the
> >> other objects.
> >
> > Doh. I forgot to update psql --help.
> 
> And Query_for_list_of_access_methods was defined more or less twice,
> the one of my patch having an error...

In looking at the DROP ACCESS METHOD completion I noticed that the
words_after_create gadget is a bit buggy: for things with more than one
word in the thing name (such as MATERIALIZED VIEW, USER MAPPING FOR,
EVENT TRIGGER among others) the "query/squery"-based completion isn't
triggered, because the loop at the end of psql_completion only considers
a single word (using strcmp against prev_wd), which obviously doesn't
match the multiple-word specifier in the struct.  Some things such as
EVENT TRIGGER and MATERIALIZED VIEW have specialized code that does the
actual work; the latter specifies a query in words_after_create, but
it's dead code.  As a probably separate but related bug, CREATE USER
MAPPING FOR stops working after you tab-complete the USER in it.
Lastly, there is an entry for CONFIGURATION which also doesn't work: if
you enter "DROP <tab>" it doesn't complete CONFIGURATION, but if you
enter "DROP CONFIGURATION <tab>" then it shows a list of text search
configurations, which is not a valid command.

To conclude, so far as I can tell, your patch (for DROP AM completion)
is fine, but the existing code has some minor flags which we could just
as well ignore for now, but could be improved in the future.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  12. Re: COMMENT ON, psql and access methods

    Michael Paquier <michael.paquier@gmail.com> — 2016-06-07T03:56:42Z

    On Tue, Jun 7, 2016 at 7:35 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> In looking at the DROP ACCESS METHOD completion I noticed that the
> words_after_create gadget is a bit buggy: for things with more than one
> word in the thing name (such as MATERIALIZED VIEW, USER MAPPING FOR,
> EVENT TRIGGER among others) the "query/squery"-based completion isn't
> triggered, because the loop at the end of psql_completion only considers
> a single word (using strcmp against prev_wd), which obviously doesn't
> match the multiple-word specifier in the struct.  Some things such as
> EVENT TRIGGER and MATERIALIZED VIEW have specialized code that does the
> actual work; the latter specifies a query in words_after_create, but
> it's dead code.  As a probably separate but related bug, CREATE USER
> MAPPING FOR stops working after you tab-complete the USER in it.

Yes, that's not new...

> Lastly, there is an entry for CONFIGURATION which also doesn't work:
> if you enter "DROP <tab>" it doesn't complete CONFIGURATION, but if you
> enter "DROP CONFIGURATION <tab>" then it shows a list of text search
> configurations, which is not a valid command.

This is not a new issue as well. Even before the tab completion
refactoring things are behaving this way. There is much room for
improvements. The refactoring makes back-patching a bit more
difficult, so we may just want to get those improvements in 9.6~ based
on the lack of complaints regarding that.

> To conclude, so far as I can tell, your patch (for DROP AM completion)
> is fine, but the existing code has some minor flags which we could just
> as well ignore for now, but could be improved in the future.

Thanks!
-- 
Michael

  13. Re: COMMENT ON, psql and access methods

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2016-06-07T22:06:26Z

    Michael Paquier wrote:
> On Thu, Jun 2, 2016 at 3:42 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
> > On Thu, Jun 2, 2016 at 1:00 PM, Michael Paquier
> > <michael.paquier@gmail.com> wrote:
> >> I have added an open item for 9.6 regarding this patch, that would be
> >> good to complete this work in this release for consistency with the
> >> other objects.
> >
> > Doh. I forgot to update psql --help.
> 
> And Query_for_list_of_access_methods was defined more or less twice,
> the one of my patch having an error...

Thanks, I have pushed this.  I only added a "translate_columns" option
to printQuery.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  14. Re: COMMENT ON, psql and access methods

    Michael Paquier <michael.paquier@gmail.com> — 2016-06-07T22:59:52Z

    On Wed, Jun 8, 2016 at 7:06 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> Michael Paquier wrote:
>> On Thu, Jun 2, 2016 at 3:42 PM, Michael Paquier
>> <michael.paquier@gmail.com> wrote:
>> > On Thu, Jun 2, 2016 at 1:00 PM, Michael Paquier
>> > <michael.paquier@gmail.com> wrote:
>> >> I have added an open item for 9.6 regarding this patch, that would be
>> >> good to complete this work in this release for consistency with the
>> >> other objects.
>> >
>> > Doh. I forgot to update psql --help.
>>
>> And Query_for_list_of_access_methods was defined more or less twice,
>> the one of my patch having an error...
>
> Thanks, I have pushed this.  I only added a "translate_columns" option
> to printQuery.

Thanks.
-- 
Michael