Re: [BUG] pg_dump does not properly deal with BEGIN ATOMIC function

> So it's lacking a rule to tell it what to do in this case, and the
> default is the wrong way around. I think we need to fix it in
> about the same way as the equivalent case for matviews, which
> leads to the attached barely-tested patch.

Thanks for the patch! A test on the initially reported use case
and some other cases show it does the expected.

Some minor comments I have:

1/

+ agginfo[i].aggfn.postponed_def = false;	/* might get set during sort */

This is probably not needed as it seems that we can only
get into this situation when function dependencies are tracked.
This is either the argument or results types of a function which
are already handled correctly, or when the function body is examined
as is the case with BEGIN ATOMIC.


2/

Instead of

+ * section.  This is sufficient to handle cases where a function depends on
+ * some unique index, as can happen if it has a GROUP BY for example.
+ */

The following description makes more sense. 

+ * section.  This is sufficient to handle cases where a function depends on
+ * some constraints, as can happen if a BEGIN ATOMIC function 
+ * references a constraint directly.


3/

The docs in https://www.postgresql.org/docs/current/ddl-depend.html
should be updated. The entire section after "For user-defined functions.... "
there is no mention of BEGIN ATOMIC as one way that the function body
can be examined for dependencies.

This could be tracked in a separate doc update patch. What do you think?


> BTW, now that I see a case the default printout here seems
> completely ridiculous. I think we need to do


> describeDumpableObject(loop[i], buf, sizeof(buf));
> - pg_log_info(" %s", buf);
> + pg_log_warning(" %s", buf);
> }

Not sure I like this more than what is there now.

The current indentation in " pg_dump:   " makes it obvious 
that these lines are details for the warning message. Additional
"warning" messages will be confusing.


Regards,

Sami Imseih
Amazon Web Services (AWS)