Theget_formatted_stringfunction is needed. Instead of using multipleifstatements for theprettyflag, 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_<object>_ddl() DDL functions added in the future.
in pg_get_triggerdef_worker, I found the below code pattern:
/*
* In non-pretty mode, always schema-qualify the target table name for
* safety. In pretty mode, schema-qualify only if not visible.
*/
appendStringInfo(&buf, " ON %s ",
pretty ?
generate_relation_name(trigrec->tgrelid, NIL) :
generate_qualified_relation_name(trigrec->tgrelid));
maybe we can apply it too while construct query string:
"CREATE POLICY %s ON %s",
In my opinion, the table name should always be schema-qualified, which I have addressed in the v4 patch. The implementation of theprettyflag here differs frompg_get_triggerdef_worker, which is used only for the target table name. In my patch, theprettyflag adds\tand\nto each different clause (example AS, FOR, USING ...)
I think that's where the confusion lies with adding `pretty` to the DDL functions. The `pretty` flag set to `true` on other functions is used to "not" show the schema name. But in the case of this function, it is used to format the statement. I wonder if the word `format` or `pretty_format` is a better name? `pretty` itself in the codebase isn't a very clear name regardless.
In my opinion, we should first decide whether we want the DDL statement to be in a 'pretty' format or not. Personally, I believe it should be, since parsing a one-line DDL statement can be quite complex and difficult for users of this function. From a user’s perspective, having the entire DDL in a single line makes it harder to read and understand. The flag allows users to disable the pretty format by passing false.
--Best,Phil Alger