Lists: | pgsql-hackers |
---|
From: | Ian Barwick <ian(at)2ndquadrant(dot)com> |
---|---|
To: | Andreas Karlsson <andreas(at)proxel(dot)se>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | [REVIEW] psql tab completion for DROP TRIGGER/RULE and ALTER TABLE ... DISABLE/ENABLE |
Date: | 2014-06-17 11:36:16 |
Message-ID: | 53A02830.1080500@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Andreas Karlsson (andreas(at)proxel(dot)se) wrote:
> Hi,
>
> When benchmarking an application I got annoyed at how basic the tab
> completion for ALTER TABLE ... DISABLE/ENABLE TRIGGER and DROP TRIGGER
> is. So here is a patch improving the tab completion around triggers. For
> consistency I have also added the same completions to rules since their
> DDL is almost identical.
Thanks for this patch; I'm playing around with rules at the moment and it was
very useful. A quick review:
- applies cleanly to HEAD
- does what it claims, i.e. adds tab completion support for this syntax:
ALTER TABLE table { ENABLE | DISABLE } [ ALWAYS | REPLICA ] { RULE | TRIGGER } rule_or_trigger
DROP TRIGGER trigger ON relation { CASCADE | RESTRICT }
DROP RULE rule ON relation { CASCADE | RESTRICT }
- code style is consistent with the project style
One issue - the table's internal triggers will also be listed. which can result in
something like this:
database=> ALTER TABLE object_version DISABLE TRIGGER <TAB>
"RI_ConstraintTrigger_a_1916401" "RI_ConstraintTrigger_a_1916422" "RI_ConstraintTrigger_c_1916358"
"RI_ConstraintTrigger_a_1916402" "RI_ConstraintTrigger_c_1916238" "RI_ConstraintTrigger_c_1916359"
"RI_ConstraintTrigger_a_1916406" "RI_ConstraintTrigger_c_1916239" "RI_ConstraintTrigger_c_1916398"
"RI_ConstraintTrigger_a_1916407" "RI_ConstraintTrigger_c_1916263" "RI_ConstraintTrigger_c_1916399"
"RI_ConstraintTrigger_a_1916411" "RI_ConstraintTrigger_c_1916264" "RI_ConstraintTrigger_c_1916478"
"RI_ConstraintTrigger_a_1916412" "RI_ConstraintTrigger_c_1916298" "RI_ConstraintTrigger_c_1916479"
"RI_ConstraintTrigger_a_1916416" "RI_ConstraintTrigger_c_1916299" "RI_ConstraintTrigger_c_1916513"
"RI_ConstraintTrigger_a_1916417" "RI_ConstraintTrigger_c_1916328" "RI_ConstraintTrigger_c_1916514"
"RI_ConstraintTrigger_a_1916421" "RI_ConstraintTrigger_c_1916329" ts_vector_update
This is a bit of an extreme case, but I don't think manually manipulating
internal triggers (which can only be done as a superuser) is a common enough
operation to justify their inclusion. I suggest adding
'AND tgisinternal is FALSE' to 'Query_for_trigger_of_table' to hide them.
Regards
Ian Barwick
--
Ian Barwick http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From: | Andreas Karlsson <andreas(at)proxel(dot)se> |
---|---|
To: | Ian Barwick <ian(at)2ndquadrant(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [REVIEW] psql tab completion for DROP TRIGGER/RULE and ALTER TABLE ... DISABLE/ENABLE |
Date: | 2014-06-17 22:51:45 |
Message-ID: | 53A0C681.6050106@proxel.se |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On 06/17/2014 01:36 PM, Ian Barwick wrote:
> Thanks for this patch; I'm playing around with rules at the moment and
> it was
> very useful. A quick review:
>
> - applies cleanly to HEAD
>
> - does what it claims, i.e. adds tab completion support for this syntax:
>
> ALTER TABLE table { ENABLE | DISABLE } [ ALWAYS | REPLICA ] { RULE
> | TRIGGER } rule_or_trigger
> DROP TRIGGER trigger ON relation { CASCADE | RESTRICT }
> DROP RULE rule ON relation { CASCADE | RESTRICT }
>
> - code style is consistent with the project style
Thanks for the review.
> One issue - the table's internal triggers will also be listed. which can
> result in
> something like this:
>
> This is a bit of an extreme case, but I don't think manually manipulating
> internal triggers (which can only be done as a superuser) is a common
> enough
> operation to justify their inclusion. I suggest adding
> 'AND tgisinternal is FALSE' to 'Query_for_trigger_of_table' to hide them.
Good suggestion. I have attached a patch which filters out the internal
triggers, both for ALTER TABLE and DROP TRIGGER. I am not entirely sure
about the DROP TRIGGER case but I think I prefer no auto completion of
RI triggers.
--
Andreas Karlsson
Attachment | Content-Type | Size |
---|---|---|
tab-complete-trigger-rule-ddl-2.patch | text/x-patch | 7.2 KB |
From: | Ian Barwick <ian(at)2ndquadrant(dot)com> |
---|---|
To: | Andreas Karlsson <andreas(at)proxel(dot)se>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Re: [REVIEW] psql tab completion for DROP TRIGGER/RULE and ALTER TABLE ... DISABLE/ENABLE |
Date: | 2014-06-18 00:34:25 |
Message-ID: | 53A0DE91.6000403@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On 14/06/18 7:51, Andreas Karlsson wrote:
> On 06/17/2014 01:36 PM, Ian Barwick wrote:
>> One issue - the table's internal triggers will also be listed. which can
>> result in
>> something like this:
>>
>> This is a bit of an extreme case, but I don't think manually manipulating
>> internal triggers (which can only be done as a superuser) is a common
>> enough
>> operation to justify their inclusion. I suggest adding
>> 'AND tgisinternal is FALSE' to 'Query_for_trigger_of_table' to hide them.
>
> Good suggestion. I have attached a patch which filters out the internal triggers,
> both for ALTER TABLE and DROP TRIGGER. I am not entirely sure about the DROP TRIGGER
> case but I think I prefer no auto completion of RI triggers.
Thanks, looks good. Another reason for not autocompleting RI triggers is that
the names are all auto-generated; on the offchance you are manually manipulating
them individually, you'd have to have a pretty good idea of which ones you're
working with anyway.
Personally I think this patch could go into 9.4, as it's not introducing any
new features and doesn't depend on any 9.5 syntax.
Regards
Ian Barwick
--
Ian Barwick http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From: | Andreas Karlsson <andreas(at)proxel(dot)se> |
---|---|
To: | Ian Barwick <ian(at)2ndquadrant(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Re: [REVIEW] psql tab completion for DROP TRIGGER/RULE and ALTER TABLE ... DISABLE/ENABLE |
Date: | 2014-06-18 00:39:44 |
Message-ID: | 53A0DFD0.6020304@proxel.se |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On 06/18/2014 02:34 AM, Ian Barwick wrote:
> On 14/06/18 7:51, Andreas Karlsson wrote:
>> On 06/17/2014 01:36 PM, Ian Barwick wrote:
>>> One issue - the table's internal triggers will also be listed. which can
>>> result in
>>> something like this:
>>>
>>> This is a bit of an extreme case, but I don't think manually manipulating
>>> internal triggers (which can only be done as a superuser) is a common
>>> enough
>>> operation to justify their inclusion. I suggest adding
>>> 'AND tgisinternal is FALSE' to 'Query_for_trigger_of_table' to hide them.
>>
>> Good suggestion. I have attached a patch which filters out the internal triggers,
>> both for ALTER TABLE and DROP TRIGGER. I am not entirely sure about the DROP TRIGGER
>> case but I think I prefer no auto completion of RI triggers.
>
> Thanks, looks good. Another reason for not autocompleting RI triggers is that
> the names are all auto-generated; on the offchance you are manually manipulating
> them individually, you'd have to have a pretty good idea of which ones you're
> working with anyway.
Thanks, could you update the status of the patch in the commitfest app
to "Ready for Committer"?
> Personally I think this patch could go into 9.4, as it's not introducing any
> new features and doesn't depend on any 9.5 syntax.
I think the feature freeze is strict to avoid having to think about what
is an exception and what is not.
--
Andreas Karlsson
From: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> |
---|---|
To: | Andreas Karlsson <andreas(at)proxel(dot)se>, Ian Barwick <ian(at)2ndquadrant(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [REVIEW] psql tab completion for DROP TRIGGER/RULE and ALTER TABLE ... DISABLE/ENABLE |
Date: | 2014-06-23 21:12:22 |
Message-ID: | 53A89836.10203@vmware.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On 06/18/2014 03:39 AM, Andreas Karlsson wrote:
> On 06/18/2014 02:34 AM, Ian Barwick wrote:
>> On 14/06/18 7:51, Andreas Karlsson wrote:
>>> On 06/17/2014 01:36 PM, Ian Barwick wrote:
>>>> One issue - the table's internal triggers will also be listed. which can
>>>> result in
>>>> something like this:
>>>>
>>>> This is a bit of an extreme case, but I don't think manually manipulating
>>>> internal triggers (which can only be done as a superuser) is a common
>>>> enough
>>>> operation to justify their inclusion. I suggest adding
>>>> 'AND tgisinternal is FALSE' to 'Query_for_trigger_of_table' to hide them.
>>>
>>> Good suggestion. I have attached a patch which filters out the internal triggers,
>>> both for ALTER TABLE and DROP TRIGGER. I am not entirely sure about the DROP TRIGGER
>>> case but I think I prefer no auto completion of RI triggers.
>>
>> Thanks, looks good. Another reason for not autocompleting RI triggers is that
>> the names are all auto-generated; on the offchance you are manually manipulating
>> them individually, you'd have to have a pretty good idea of which ones you're
>> working with anyway.
>
> Thanks, could you update the status of the patch in the commitfest app
> to "Ready for Committer"?
Thanks, committed.
- Heikki