Re: Support logical replication of DDLs - Mailing list pgsql-hackers

On Mon, Apr 17, 2023 at 5:32 PM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
> Attach the new version patch set which include the following changes:
>

Few comments for ddl_deparse.c in patch dated April17:

1) expand_jsonval_string()
I think we need to assert if jsonval is neither jbvString nor jbvBinary.

2) expand_jsonval_strlit()
same here, assert if not jbvString (like we do in expand_jsonval_number and expand_jsonval_identifier etc)

3) expand_jsonb_array()
arrayelem is allocated as below, but not freed.
initStringInfo(&arrayelem)

4) expand_jsonb_array(),
we initialize iterator as below which internally does palloc
it = JsonbIteratorInit(container);
Shall this be freed at the end? I see usage of this function in other files. At a few places, it is freed while not freed at other places.

5) deparse_ddl_json_to_string(char *json_str, char** owner)
str = palloc(value->val.string.len + 1);
we do  palloc here and return allocated memory to caller as 'owner'. Caller sets this 'owner' using SetConfigOption() which internally allocates new memory and copies 'owner' to that. So the memory allocated in deparse_ddl_json_to_string is never freed. Better way should be the caller passing this allocated memory to deparse_ddl_json_to_string() and freeing it when done. Thoughts?

6)expand_fmt_recursive():
value = findJsonbValueFromContainer(container, JB_FOBJECT, &key);