> 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)