Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-11-07 Thread Philip Alger
Hello Hackers, I was thinking about the patch a little more, and I think some people may want the pretty formatted option. I was going to create another patch for pg_get_triggerdef to add that functionality, like pg_get_functiondef and pg_get_viewdef. but I think it will break it since pg_get_tri

Re: [PATCH] Add pretty formatting to pg_get_triggerdef

2025-11-06 Thread Philip Alger
Hi Tom, > This patch will cause psql's \d to fail hard against any pre-v19 > server. That's not acceptable. Note the comment at the top of > describe.c: > > * Support for the various \d ("describe") commands. Note that the current > * expectation is that all functions in this file will succ

Re: [PATCH] Add pretty formatting to pg_get_triggerdef

2025-11-05 Thread Philip Alger
I am attaching v3. I changed the name of the function that prints out the trigger using the \d command (below) from `pg_get_triggerdef_compact` to `pg_get_triggerdef_string`, which matches the index naming convention for a similar function. postgres=# \d child3 >Table "public.chil

Re: [PATCH] Add pretty formatting to pg_get_triggerdef

2025-11-05 Thread Philip Alger
Hi Chao, > > I think this is a mis-use of PRETTYFLAG_SCHEMA that is for printing > schema-qualified names but omitting schema. > > > > Yes, this is to omit the schema because the functions is used to print > out the triggers when using \d in psql, The current practice isn't to print > out a schem

Re: [PATCH] Add pretty formatting to pg_get_triggerdef

2025-11-04 Thread Philip Alger
Hi Chao, +/* > + * pg_get_triggerdef_compact > + * Returns trigger definition in a compact, single-line > format without > + * schema qualification designed for the psql \d command. > + */ > +Datum > +pg_get_triggerdef_compact(PG_FUNCTION_ARGS) > +{ > + Oid

[PATCH] Add pretty formatting to pg_get_triggerdef

2025-11-03 Thread Philip Alger
Hello Hackers, Currently, `pg_get_triggerdef` includes a "pretty" flag, but it does not actually format the `CREATE TRIGGER` statement in a "pretty" way. Unlike `pg_get_viewdef`, `pg_get_ruledef`, and `pg_get_indexdef`, the purpose of pretty formatting has been to remove the schema name so it can

Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-11-02 Thread Philip Alger
Hello, > > I think it’s better to pfree(res). > > Would you mind to share why pfree is needed? I tried to trace this > with Valgrind, but even pfree(res) was present or not, there was no > leak detected and both compiles without additional warnings. Wouldn't > be res "trashed" at the end of the

Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-11-02 Thread Philip Alger
Hi Josef, Would it make sense to rename trigger related variables to "trig" > prefix instead of "trg" as is done in other functions in the same file > (for example in function pg_get_triggerdef)? > Not sure it would be the same as triggerdef uses trig, while triggerdef_worker uses a mix of trig a

Re: [PATCH] Add pg_get_type_ddl() to retrieve the CREATE TYPE statement

2025-11-01 Thread Philip Alger
Hi Chao, Appreciate you pulling it apart: > 1 > Here when you call LookupTypeName(), you give the last parameter > “missing_ok” a value of “false”, so that it would “ereport” inside > LookupTypeName(), so your manual check of “if (typeTup == NULL)” will never > be satisfied. > Yeah, I changed

Re: [PATCH] Add pg_get_type_ddl() to retrieve the CREATE TYPE statement

2025-10-31 Thread Philip Alger
Hi Quan, > Found a small bug. MULTIRANGE_TYPE_NAME does not output schema. > > When outputting, the function "quote_qualified_identifier" should be > used instead of "quote_identifier". > > Similarly, the function names in print_range_type_def and > print_base_type_def should also be processed in

Re: [PATCH] Add pg_get_type_ddl() to retrieve the CREATE TYPE statement

2025-10-30 Thread Philip Alger
Hi Quan, This is part of a larger project as noted here: > I am submitting a patch as part of the Retail DDL functions project > > described here [1]. > > > 1. https://www.postgresql.org/message-id/945db7c5-be75-45bf-b55b- > > cb1e56f2e3e9%40dunslane.net >

[PATCH] Add pg_get_type_ddl() to retrieve the CREATE TYPE statement

2025-10-30 Thread Philip Alger
Hello Hackers, I am submitting a patch as part of the Retail DDL functions project described here [1]. This patch creates a function called pg_get_type_ddl designed to retrieve the DDL statement for CREATE TYPE. Users can get the DDL by providing a TYPE name like the following for the ENUM type:

Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-10-28 Thread Philip Alger
Hi > > I am fine with v8. > > ERROR: trigger name cannot be schema qualified > > > > I’m fine with changing it to the other error message: > > ERROR: trigger name \"%s\" for table \"%s\" does not exist > The extra check is for the user experience because the function accepts `text`. A user cou

Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-10-22 Thread Philip Alger
> > doc said trigger name can not be schema-qualified, >>> we can not do: >>> CREATE TRIGGER public.modified_a BEFORE UPDATE OF a ON main_table >>> FOR EACH ROW WHEN (OLD.a <> NEW.a) EXECUTE PROCEDURE >>> trigger_func('modified_a'); >> >> >> >>> + nameList = textToQualifiedNameList(trgName); >>> >>

Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-10-22 Thread Philip Alger
Hi Jian doc said trigger name can not be schema-qualified, >> we can not do: >> CREATE TRIGGER public.modified_a BEFORE UPDATE OF a ON main_table >> FOR EACH ROW WHEN (OLD.a <> NEW.a) EXECUTE PROCEDURE >> trigger_func('modified_a'); > > > >> + nameList = textToQualifiedNameList(trgName); >> >> I a

Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE POLICY statement

2025-10-22 Thread Philip Alger
>> The get_formatted_string function is needed. Instead of using multiple if > statements for the pretty flag, it’s better to have a generic function. > This will be useful if the pretty-format DDL implementation is accepted by > the community, as it can be reused by other pg_get__ddl() DDL > funct

Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-10-22 Thread Philip Alger
Hi Jian, > doc said trigger name can not be schema-qualified, > we can not do: > CREATE TRIGGER public.modified_a BEFORE UPDATE OF a ON main_table > FOR EACH ROW WHEN (OLD.a <> NEW.a) EXECUTE PROCEDURE > trigger_func('modified_a'); That's correct. The function wouldn't produce that output thoug

Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-10-21 Thread Philip Alger
> Just ignore it if you're pretty sure the error is unrelated. The > cfbot cycles through all open patches and will re-run CI for yours > in a day or three (not sure of the exact cycle time right now). > If the same error persists then you'd better look closer. > Thanks Tom. Actually, it's funny

Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-10-21 Thread Philip Alger
Hi Jim I attached v7 but it looks like that one of the CI tests failed, but I reran though my Github branch and they all passed. The error had nothing to do with the patch though. Wondering if I need to resubmit a v8 to kick it off again? Thoughts? -- Best, Phil Alger

Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-10-19 Thread Philip Alger
Hi Marcos, > In a multi tenant world this feature will be cool for clone or sync ddl of > two schemas. So, if I’m creating a new schema the way you did works but if > both exists and I want to update some ddls of a schema, sometimes I have to > DROP and CREATE or returned command should have CREAT

Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-10-18 Thread Philip Alger
Apologies, I forgot to add a new version of the patch with the documentation change. This is my first time doing this. > your documentation and the function's comment specifically say that the >> function take a trigger name and a table name, so it should not use >> regclass type, which allows O

Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-10-18 Thread Philip Alger
On Mon, Oct 13, 2025 at 9:28 PM jian he wrote: > > I just did a quick test. > > src1=# SELECT pg_get_trigger_ddl(2, 'foo_trigger'); > ERROR: trigger "foo_trigger" for table "(null)" does not exist > src1=# SELECT pg_get_trigger_ddl(0, 'foo_trigger'); > ERROR: trigger "foo_trigger" for table "(n

Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-10-18 Thread Philip Alger
Thank you Andrew. I think you should change the documentation. > Changed. I've updated v4, attached here. One thing I noted while testing was that pg_get_triggerdef does not put the statement terminator (;) at the end of the printed statement. pg_get_triggerdef ---

Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-10-18 Thread Philip Alger
Hi Jim, You're probably initialising the buffer a bit too early: > Attached is v7. Moved the `initStringInfo` as suggested and reran tests. -- Best, Phil Alger v7-0001-Add-pg_get_trigger_ddl-function.patch Description: Binary data

Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-10-18 Thread Philip Alger
> > select pg_get_trigger_ddl(-1, 'h'); > ERROR: relation with OID 4294967295 does not exist > > this error obviously is not good. > we can follow the approach used by pg_get_viewdef(oid) > But isn't that an edge case? Would a user really pass in an arbitrary number like -1? That seems counterin

Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-10-18 Thread Philip Alger
Hi Marcos, wouldn't it be better to have an option to drop them first or create if not > exists when available ? > > pg_get_trigger_ddl(oid, pretty, drop_first) > I’m not sure what you’re saying here. There is no pretty option for this one, and the intent is for the user to be able to input a t

Re: Improve coments on structures in trigger.c

2025-10-18 Thread Philip Alger
Hi Yugo, > I found that comments in trigger.c describing fields of three different > structures in a comment block. That makes sense to me. > I've attached a patch to improve the comments > by splitting it to three blocks. > I think you could also move the comment beginning on: * AfterT

[PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-10-18 Thread Philip Alger
Hello, I am submitting patch as a part of a larger Retail DDL functions project described by Andrew Dunstan here: https://www.postgresql.org/message-id/945db7c5-be75-45bf-b55b-cb1e56f2e3e9%40dunslane.net This patch creates a function pg_get_trigger_ddl, designed to retrieve the full DDL statement

Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-10-18 Thread Philip Alger
Hi Jim, > I can see how it can be more practical to not care about double quotes > when using pg_get_trigger_ddl(), but IMHO consistency and predictability > are more important in this particular case. If we do this, users would > need to know where to keep or remove the double quotes when using

Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-10-18 Thread Philip Alger
Hi Jim, Appreciate the feedback! > The function fails to look up triggers with quoted names > Not exactly. If you put "FOO" in the function pg_get_trigger_ddl('tbl', '"FOO"') it will error because you don't need the double quotes. They are already preserved. You just need the name, and pg_get_t

Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-10-18 Thread Philip Alger
Hi Jian, > maybe we can return NULL for > select pg_get_trigger_ddl(-1, 'h'); > Yes, I had the same idea last night. Running PG_RETURN_NULL would also be similar to how other functions handle it. Thanks, and I will make the change. -- Best, Phil Alger

Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE POLICY statement

2025-10-17 Thread Philip Alger
Hi Akshay, When applying the patch, I got a number of errors and the tests failed. I think it stems from: + targetTable = relation_open(tableID, NoLock); + relation_close(targetTable, NoLock); I changed them to use "AccessShareLock" and it worked, and it's probably relevant to change table_clos

Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-10-17 Thread Philip Alger
Hi Jim, One last thing: should we perhaps check for NULL before calling > appendStringInfo here? > > /* pg_get_triggerdef_worker retrieves the trigger definition */ > res = pg_get_triggerdef_worker(trgOid, false); > > appendStringInfo(&buf, "%s;", res); > Yes, you're correct. I've added that in

Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-10-17 Thread Philip Alger
Hi, your documentation and the function's comment specifically say that the > function take a trigger name and a table name, so it should not use > regclass type, which allows OID as input as well. > Thanks for pointing that out in the documentation. I think the regclass type actually makes it e

Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE POLICY statement

2025-10-17 Thread Philip Alger
Hi Akshay, >>> As for the statement terminator, it’s useful to include it, while >> running multiple queries together could result in a syntax error. In my >> opinion, there’s no harm in providing the statement terminator. >> > However, I’ve modified the logic to add the statement terminator at t

Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-10-17 Thread Philip Alger
Hi Jim, Just to add to this: > I don't think it's the expected behaviour. For instance, > pg_get_viewdef() sees it differently (opposite approach): > > postgres=# SELECT pg_get_viewdef('"MyView"'); > pg_get_viewdef > --- > SELECT 42 AS "?column?"; > (1 row) > > I

Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement

2025-10-17 Thread Philip Alger
Hi Jim, I am now wondering if introducing these new set of parameters to > pg_get_triggerdef() would be a better solution that creating a new > function. > > Doing so we keep it consistent with the other pg_get*def functions. What > do you think? > The rationale behind it is here: https://www.po