| From: | Andreas Karlsson <andreas(at)proxel(dot)se> |
|---|---|
| To: | William Bernbaum <wbernbaum(at)dwdev(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: [PATCH] Fix pg_dump emitting OVERRIDING SYSTEM VALUE for tables with dropped identity columns |
| Date: | 2026-05-01 07:28:28 |
| Message-ID: | 49471fc0-5f07-4ed4-b33c-fc634aa301ef@proxel.se |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 4/18/26 01:29, William Bernbaum wrote:
> Hey hackers,
>
> I’ve encountered a small issue in pg_dump.
>
> It currently emits OVERRIDING SYSTEM VALUE in INSERTs for
>
> a table that doesn't have an identity column if it used to have
>
> a GENERATED ALWAYS AS IDENTITY column that was later dropped.
>
> [...]
>
> Patch attached.
>
> Thoughts?
Nicely spotted and thanks for the patch! Please add it to the currently
open commitfest (https://commitfest.postgresql.org/59/) so it is not lost.
I have two pieces of feedback:
1. I think the code would be easier to read as
if (!tbinfo->attisdropped[j])
tbinfo->needs_override = tbinfo->needs_override ||
tbinfo->attidentity[j] == ATTRIBUTE_IDENTITY_ALWAYS;
or even
if (tbinfo->attidentity[j] == ATTRIBUTE_IDENTITY_ALWAYS &&
!tbinfo->attisdropped[j])
tbinfo->needs_override = true;
since then we do not get such a long line.
2. While I am not personally a fan of that file it would be more
consistent if the new test was added as part of 002_pg_dump.pl if
possible. Plus then it would mean that we would not need to create and
tear down a PostgreSQL cluster.
Andreas
| From | Date | Subject | |
|---|---|---|---|
| Next Message | 'Alvaro Herrera' | 2026-05-01 07:30:27 | Re: Adding REPACK [concurrently] |
| Previous Message | Andreas Karlsson | 2026-05-01 06:41:55 | Re: meson/testwrap: support EXTRA_REGRESS_OPTS environment variable |