Skip to content

Enhance the function trigger__parse.#2

Closed
lizardoluis wants to merge 2 commits intoPostgres-Extensions:masterfrom
lizardoluis:0.1.5
Closed

Enhance the function trigger__parse.#2
lizardoluis wants to merge 2 commits intoPostgres-Extensions:masterfrom
lizardoluis:0.1.5

Conversation

@lizardoluis
Copy link
Copy Markdown

Add the return variables:
function_name: the procedure name called by the trigger.
table_name: the table which the trigger is associated.

Add the return variables:
function_name: the procedure name called by the trigger.
table_name: the table which the trigger is associated.
Copy link
Copy Markdown
Collaborator

@decibel decibel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the addition! Feel free to just reply to my comments; I can make the necessary changes.

Comment thread sql/cat_tools.sql
, OUT row_statement text
, OUT when_clause text
, OUT function_arguments text
, OUT function_name text
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this be better as regprocedure?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be!
I followed the name used in the PostgreSQL doc. But its up to you. It was just a suggestion.

Comment thread sql/cat_tools.sql
, OUT when_clause text
, OUT function_arguments text
, OUT function_name text
, OUT table_name text
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you describe your use case? I thought about adding this, but presumably if you already know the trigger OID then you already know what table it's on, no?

It should also be regclass instead of text.

Copy link
Copy Markdown
Author

@lizardoluis lizardoluis Sep 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I'm doing is what you helped me to do: http://stackoverflow.com/questions/38212882/postgresql-use-event-trigger-to-validate-a-trigger

The table_name was just a syntax sugar for me. I'm also not that experienced with pgSQL to deal with trigger OID and so. My idea is to use this function you created to parse a trigger and get all information of it. Regclass makes sense though!

I need the function_name to identify which trigger is, so that I can certify that it was created correctly. I wrote some trigger functions that applies some spatial integrity constraints on the database. But each function requires a certain type of trigger (BEFORE, AFTER, STATEMENT, ROW) to be created.

I'll make soon a second commit changing the function_arguments to array. I think its more practical then a string, and more helpful for me, because I need to check the arguments of the function as well.

@decibel
Copy link
Copy Markdown
Collaborator

decibel commented Sep 22, 2016 via email

Comment thread sql/cat_tools.sql
v_execute_clause := ' EXECUTE PROCEDURE ' || r_trigger.tgfoid::regproc || E'\\(';
v_array := regexp_split_to_array( v_work, v_execute_clause );
function_arguments := rtrim( v_array[2], ')' ); -- Yank trailing )
function_arguments := array_remove(regexp_split_to_array(rtrim( v_array[2], ')' ), '\W+'), '');
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to do this originally, but this will fail if you have any whitespace in any of the arguments themselves. I don't think it's really worth the (rather considerable) effort it would take to make this fool-proof.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true.
In my case works nice, because my trigger functions just receive table names or column names, but in your case, it will not work.
You can then just split the string by the comma (,) and trim lead and back whitespaces. But be careful not to split with comma inside strings. Then you'll have to check for quotes.

decibel pushed a commit that referenced this pull request Sep 22, 2016
Based on work by Luís Lizardo (#2)
@decibel
Copy link
Copy Markdown
Collaborator

decibel commented Sep 22, 2016

Here's what I'm thinking, though I haven't added trigger__get_oids yet: #3

@lizardoluis
Copy link
Copy Markdown
Author

If you're trying to find what trigger on a particular table uses a
particular trigger function, I think it would be better to have a
function that searches for that; something like

FUNCTION

cat_tools.trigger__get_oids(
trigger_table regclass
, trigger_function regproc
) RETURNS SETOF oid

You could then do

SELECT trigger__parse(oid) FROM trigger__get_oids(...) oid

Your function trigger__parse, with the modifications I did, was already enough. It solved all my problems.
Thanks a lot.

decibel pushed a commit that referenced this pull request Jun 11, 2017
Based on work by Luís Lizardo (#2)
decibel pushed a commit that referenced this pull request Jun 11, 2017
Based on work by Luís Lizardo (#2)
decibel pushed a commit that referenced this pull request Jun 11, 2017
Based on work by Luís Lizardo (#2)
@decibel
Copy link
Copy Markdown
Collaborator

decibel commented Jun 12, 2017

I've submitted changes to master that should handle everything you wanted here. Please check them ASAP and let me know if they make sense.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants