Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate the triggers in 2 nodes: "Triggers" and "Database Triggers" #48

Closed
luronumen opened this issue Aug 10, 2020 · 11 comments
Closed

Comments

@luronumen
Copy link

Please separate the triggers in 2 nodes: "Triggers" for table triggers and "Database Triggers" for the database triggers (See RedExpert tool as reference):

RedExpert

@Jdochoa
Copy link
Contributor

Jdochoa commented Aug 22, 2020

I'm separated DB Trigger, DDL Trigger and Trigger, but can't generate script for DDL Trigger because rdb$types not defined this rdb$trigger_types
See https://github.com/Jdochoa/flamerobin/tree/FB3

@arvanus
Copy link
Collaborator

arvanus commented Aug 22, 2020

Just to congrat @Jdochoa
image
It's looking great!

@luronumen
Copy link
Author

luronumen commented Aug 22, 2020

My suggestions is:

  • Rename FunctionSQLs for just Functions
  • Rename Global temporary tables for just Global Temporaries
  • Hide System tables and System domains
  • Add Indexes node
  • Rename Generators to Sequences (in the future with this issue Rename all GENERATOR references to SEQUENCE #54)

Here the full list:

  • Database Triggers
  • DDL Triggers
  • Domains
  • Exceptions
  • Functions
  • Global Temporaries
  • Indexes
  • Packages
  • Procedures
  • Roles
  • Sequences
  • Tables
  • Triggers
  • UDFs
  • Views

Here is the Red Expert as reference:

RedExpert

@PizzaProgram
Copy link

* Hide **System tables** and **System domains**
  • Why would that be good to anyone?
  • How would you check / set field-domains in a <1.5 version DB without them?
  • How could you learn / analyse how the FB system works?

@PizzaProgram
Copy link

* Rename _Generators_ to **Sequences**

After 20 years of working with FireBird > It would be a pain looking for "Generators" in the list, but not finding them.
But maybe it would be OK, to enhance it:
GENERATORS / SEQUENCES or:
GENERATORS (SEQUENCES)

Maybe later there would be an option at the global-settings of FlameRobin:
[ x ] "Use the word "sequence" instead of generator.

if someone is checking that >> everything would be replaced visually. But that would also create lot's of problems. (Like: opening an old DB, or someone is trying to write "create sequence ...", also the auto-script system...)

@PizzaProgram
Copy link

* Add **Indexes** node

That's a great idea! 👍

@PizzaProgram
Copy link

PizzaProgram commented Aug 23, 2020

I'm separated DB Trigger, DDL Trigger and Trigger...

@Jdochoa
Why not putting them in a SUB-tree folder ? (And create +1 folder in the 1. place to show ALL.)

[-] Triggers
   [-] ALL Triggers
      CONN1_TR
      ...
   [+] Database Triggers
   [+] DDL Triggers
   [+] Table Triggers

So everyone would see ALL, making it easy to find one by name in ABC order.

  • That would also maintain similar view with older users who are used to this program.
  • What if later a NEW type appears in FB5 which does not fit to any category? ("ALL Triggers" would still show it!)

Separating them so "far from each other" makes much more difficult to find ONE trigger by name!
(Also, at first sight nobody can see there are more folders that would normally need somehow "stick together".)

By default > if you click [+] sign next to the Main folder >> the "ALL Triggers" sub-tree should auto-open to show ALL.

If it's too difficult to make sub-sub tree-folders, at least rename them, so these be sorted after each other:
TRIGGERS - ALL
TRIGGERS for Database
...

@arvanus
Copy link
Collaborator

arvanus commented Aug 23, 2020

Just my thoughts:
I liked this idea:

[-] Triggers
   [-] ALL Triggers
      CONN1_TR
      ...
   [+] Database Triggers
   [+] DDL Triggers
   [+] Table Triggers

Maybe I would just change the subfolder pattern, you don't need to reinforce that "All" , "Database", "DDL" and "Table" refers to triggers, they are already inside triggers, doesn't change a lot, but "less is more" ;)

[-] Triggers
   [-] ALL
      CONN1_TR
      ...
   [+] Database
   [+] DDL
   [+] Table

I agree with Rename FunctionSQLs for just Functions, but maybe for now, keep like this because "Functions" where used for UDFs and this kind of changes goes easy for a bug/miss-renaming or something like that if you don't take care enough

System Tables exists from along time, I disagree with removing it
... for now, it's it :)
Thanks for your effort helping Flamerobin

@Jdochoa
Copy link
Contributor

Jdochoa commented Aug 24, 2020

Hi all

- Hide System tables and System domains: This is configurable in preferences database.
imagen

- Add Indexes node: It's a good idea, I'll work on it
- Rename FunctionSQLs for just Functions: I agree.
- Put them in a SUB-tree folder: I agree.
- Rename Global temporary tables for just Global Temporaries: I agree, i change it.
- Rename Generators to Sequences: I'm not sure. I'm think we should keep backward compatibility with FB and FR.

@luronumen
Copy link
Author

Hi @PizzaProgram , @arvanus and @Jdochoa

Let's leave GENERATORS for now instead of SEQUENCES.
We think better about this change in the bug #54

For a better organization of the triggers we can do as PizzaProgram suggested:

[-] Triggers
[+] Database
[+] DDL
[+] Table

Best Regards,
Luciano

@Jdochoa Jdochoa mentioned this issue Jan 25, 2021
Merged
Jdochoa added a commit to Jdochoa/flamerobin that referenced this issue Mar 1, 2021
- Some fixes DB Trigger, DML Trigger and DDL Trigger mariuz#48, mariuz#70, mariuz#73 and  mariuz#154
- Some fixes Package implementation mariuz#55
- Unable to change Field Datatype mariuz#165
- "Functions" and "Global Temporaries" context popup menu mariuz#162
- Index node experimental.
- Initial implementation node users mariuz#149
Jdochoa added a commit to Jdochoa/flamerobin that referenced this issue Mar 17, 2021
@Jdochoa Jdochoa mentioned this issue May 10, 2021
Merged
@luronumen
Copy link
Author

Retest result on FlameRobin 0.9.3.8 Snapshot: Passed!

Thank you very much for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants