Обсуждение: COMMENT ON, psql and access methods
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
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
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
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
>>> 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/
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
Вложения
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
Вложения
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
Вложения
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
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
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
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
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
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