On Tue, Oct 11, 2022 at 7:00 PM Ajin Cherian <itsa...@gmail.com> wrote: >
I was going through 0001 patch and I have a few comments/suggestions. 1. @@ -6001,7 +6001,7 @@ getObjectIdentityParts(const ObjectAddress *object, transformType = format_type_be_qualified(transform->trftype); transformLang = get_language_name(transform->trflang, false); - appendStringInfo(&buffer, "for %s on language %s", + appendStringInfo(&buffer, "for %s language %s", transformType, transformLang); How is this change related to this patch? 2. +typedef struct ObjTree +{ + slist_head params; + int numParams; + StringInfo fmtinfo; + bool present; +} ObjTree; + +typedef struct ObjElem +{ + char *name; + ObjType objtype; + + union + { + bool boolean; + char *string; + int64 integer; + float8 flt; + ObjTree *object; + List *array; + } value; + slist_node node; +} ObjElem; It would be good to explain these structure members from readability pov. 3. + +bool verbose = true; + I do not understand the usage of such global variables. Even if it is required, add some comments to explain the purpose of it. 4. +/* + * Given a CollectedCommand, return a JSON representation of it. + * + * The command is expanded fully, so that there are no ambiguities even in the + * face of search_path changes. + */ +Datum +ddl_deparse_to_json(PG_FUNCTION_ARGS) +{ It will be nice to have a test case for this utility function. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com