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