From: | "Imseih (AWS), Sami" <simseih(at)amazon(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: [BUG] pg_dump does not properly deal with BEGIN ATOMIC function |
Date: | 2023-06-02 21:26:50 |
Message-ID: | 0AA8F2B1-7855-4F25-BD6C-83CB85722120@amazon.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> 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)
From | Date | Subject | |
---|---|---|---|
Next Message | Marco Atzeri | 2023-06-03 05:35:51 | Re: PostgreSQL 16 Beta 1 Released! |
Previous Message | Daniel Gustafsson | 2023-06-02 21:23:19 | Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~? |