Thread: [REVIEW] psql tab completion for DROP TRIGGER/RULE and ALTER TABLE ... DISABLE/ENABLE
[REVIEW] psql tab completion for DROP TRIGGER/RULE and ALTER TABLE ... DISABLE/ENABLE
From
Ian Barwick
Date:
Andreas Karlsson (andreas@proxel.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 triggerON 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
Re: [REVIEW] psql tab completion for DROP TRIGGER/RULE and ALTER TABLE ... DISABLE/ENABLE
From
Andreas Karlsson
Date:
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
Re: Re: [REVIEW] psql tab completion for DROP TRIGGER/RULE and ALTER TABLE ... DISABLE/ENABLE
From
Ian Barwick
Date:
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
Re: Re: [REVIEW] psql tab completion for DROP TRIGGER/RULE and ALTER TABLE ... DISABLE/ENABLE
From
Andreas Karlsson
Date:
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
Re: [REVIEW] psql tab completion for DROP TRIGGER/RULE and ALTER TABLE ... DISABLE/ENABLE
From
Heikki Linnakangas
Date:
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