Thread: Improved tab completion for FDW DDL
Hi,
Here is a patch which adds the below missing tab completions for the FDW
DDL commands. I noticed these were missing while playing around with
FDWs a while ago.
"ALTER SERVER <name>" with "RENAME TO"
"CREATE SERVER <name> FOREIGN DATA WRAPPER" with fdw name
"CREATE|ALTER USER MAPPING FOR <name> SERVER <name>" with "OPTIONS ("
Another completion which is currently missing but I am not sure if we
should add or not is completing "ALTER|CREATE|DROP USER" with "MAPPING
FOR", but since it might interfere with completing to username for
"ALTER|DROP USER" I am not sure we want it. What do you think?
Andreas
Attachment
On Wed, Dec 30, 2015 at 01:21:06PM +0100, Andreas Karlsson wrote:
> Hi,
>
> Here is a patch which adds the below missing tab completions for the FDW DDL
> commands. I noticed these were missing while playing around with FDWs a
> while ago.
>
> "ALTER SERVER <name>" with "RENAME TO"
> "CREATE SERVER <name> FOREIGN DATA WRAPPER" with fdw name
> "CREATE|ALTER USER MAPPING FOR <name> SERVER <name>" with "OPTIONS ("
>
> Another completion which is currently missing but I am not sure if we should
> add or not is completing "ALTER|CREATE|DROP USER" with "MAPPING FOR", but
> since it might interfere with completing to username for "ALTER|DROP USER" I
> am not sure we want it. What do you think?
Is there a way to require some reasonable chunk--say, one that's
disambiguated from the name any known ROLE with LOGIN--of MAPPING
before completing with MAPPING FOR? I confess to not knowing how the
new system works in enough detail to know that off the top of my head.
Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
On Wed, Dec 30, 2015 at 9:21 PM, Andreas Karlsson <andreas@proxel.se> wrote:
> Hi,
>
> Here is a patch which adds the below missing tab completions for the FDW DDL
> commands. I noticed these were missing while playing around with FDWs a
> while ago.
>
> "ALTER SERVER <name>" with "RENAME TO"
> "CREATE SERVER <name> FOREIGN DATA WRAPPER" with fdw name
> "CREATE|ALTER USER MAPPING FOR <name> SERVER <name>" with "OPTIONS ("
>
> Another completion which is currently missing but I am not sure if we should
> add or not is completing "ALTER|CREATE|DROP USER" with "MAPPING FOR", but
> since it might interfere with completing to username for "ALTER|DROP USER" I
> am not sure we want it. What do you think?
You may want to use Matches() instead of TailMatches() for performance reasons.
--
Michael
On 01/04/2016 07:26 AM, Michael Paquier wrote: > You may want to use Matches() instead of TailMatches() for performance reasons. Probably, but if so should we not also change the surrounding rules to also use Matches()? I imitated the surrounding code to keep code consistency and avoid errors. I thought I saw some patch from you changing many of the rules from TailMatches() to Matches(). Perhaps I should just update my patch after your patch has been applied. Andreas
Andreas Karlsson <andreas@proxel.se> writes:
> I thought I saw some patch from you changing many of the rules from
> TailMatches() to Matches(). Perhaps I should just update my patch after
> your patch has been applied.
Yes, I think it's important to get that set of patches from Michael
into the tree ASAP so that they can be precedent for the other tab
completion patches that are floating around. I am under the gun for
9.5.0 release today but will review and hopefully push Michael's
changes soon.
regards, tom lane
On 01/04/2016 07:26 AM, Michael Paquier wrote: > You may want to use Matches() instead of TailMatches() for performance reasons. Here is an updated version which uses Matches(). Andreas
Attachment
On 01/04/2016 01:09 AM, David Fetter wrote: > On Wed, Dec 30, 2015 at 01:21:06PM +0100, Andreas Karlsson wrote: >> Another completion which is currently missing but I am not sure if we should >> add or not is completing "ALTER|CREATE|DROP USER" with "MAPPING FOR", but >> since it might interfere with completing to username for "ALTER|DROP USER" I >> am not sure we want it. What do you think? > > Is there a way to require some reasonable chunk--say, one that's > disambiguated from the name any known ROLE with LOGIN--of MAPPING > before completing with MAPPING FOR? I confess to not knowing how the > new system works in enough detail to know that off the top of my head. No, and while it would not be too hard to build it would not be worth doing just for this use case. Andreas
Andreas Karlsson <andreas@proxel.se> writes:
> On 01/04/2016 01:09 AM, David Fetter wrote:
>> On Wed, Dec 30, 2015 at 01:21:06PM +0100, Andreas Karlsson wrote:
>>> Another completion which is currently missing but I am not sure if we should
>>> add or not is completing "ALTER|CREATE|DROP USER" with "MAPPING FOR", but
>>> since it might interfere with completing to username for "ALTER|DROP USER" I
>>> am not sure we want it. What do you think?
>> Is there a way to require some reasonable chunk--say, one that's
>> disambiguated from the name any known ROLE with LOGIN--of MAPPING
>> before completing with MAPPING FOR? I confess to not knowing how the
>> new system works in enough detail to know that off the top of my head.
> No, and while it would not be too hard to build it would not be worth
> doing just for this use case.
The way we've solved other similar cases would translate like this:
instead of the "query for user names" just returning user names, add
on "UNION 'MAPPING FOR'". So if you do
# alter user <TAB>
where you're now offered
alice joe postgres
you'd instead get
alice joe postgres MAPPING FOR
Dunno if it's worth the trouble though.
regards, tom lane
The second part is not necessary, because there is already code that completes FDW names after "FOREIGN DATA WRAPPER". So this already works. The other two parts are valid improvements, but they should be done consistently across commands. So please - Also complete RENAME TO in ALTER FOREIGN DATA WRAPPER. - Also complete OPTIONS in FOREIGN DATA WRAPPER and SERVER commands.
On 01/11/2016 02:39 AM, Peter Eisentraut wrote: > The second part is not necessary, because there is already code that > completes FDW names after "FOREIGN DATA WRAPPER". So this already works. Good spot, thanks. I have no idea why I did not think it worked. Maybe it just did not work in 9.2 and I failed when trying to reproduce it on master. > - Also complete RENAME TO in ALTER FOREIGN DATA WRAPPER. Done. > - Also complete OPTIONS in FOREIGN DATA WRAPPER and SERVER commands. Done. Andreas
Attachment
On 1/18/16 8:36 PM, Andreas Karlsson wrote: > On 01/11/2016 02:39 AM, Peter Eisentraut wrote: >> The second part is not necessary, because there is already code that >> completes FDW names after "FOREIGN DATA WRAPPER". So this already works. > > Good spot, thanks. I have no idea why I did not think it worked. Maybe > it just did not work in 9.2 and I failed when trying to reproduce it on > master. > >> - Also complete RENAME TO in ALTER FOREIGN DATA WRAPPER. > > Done. > >> - Also complete OPTIONS in FOREIGN DATA WRAPPER and SERVER commands. > > Done. committed
On 01/23/2016 01:03 PM, Peter Eisentraut wrote: > committed Thanks! Andreas