public inbox for [email protected]
help / color / mirror / Atom feed[PATCH] Fix pg_dump emitting OVERRIDING SYSTEM VALUE for tables with dropped identity columns
3+ messages / 2 participants
[nested] [flat]
* [PATCH] Fix pg_dump emitting OVERRIDING SYSTEM VALUE for tables with dropped identity columns
@ 2026-04-17 23:29 William Bernbaum <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: William Bernbaum @ 2026-04-17 23:29 UTC (permalink / raw)
To: [email protected] <[email protected]>
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.
Simple repro:
CREATE TABLE demo (
id BIGINT GENERATED ALWAYS AS IDENTITY PRIMARY KEY,
fk_a BIGINT NOT NULL,
fk_b BIGINT NOT NULL
);
INSERT INTO demo (fk_a, fk_b)
OVERRIDING SYSTEM VALUE VALUES (1, 2);
ALTER TABLE demo DROP COLUMN id;
ALTER TABLE demo ADD PRIMARY KEY (fk_a, fk_b);
pg_dump --data-only --inserts --table=demo mydb
Expected:
INSERT INTO public.demo VALUES (1, 2);
Actual:
INSERT INTO public.demo OVERRIDING SYSTEM VALUE VALUES (1, 2);
The clause is harmless, but it's misleading and causes noisy diffs.
In CI setups that rely on deterministic dumps, this can add significant friction
to reviews. We currently work around it by first clearing the stale markers:
UPDATE pg_catalog.pg_attribute a
SET attidentity = ''
FROM pg_catalog.pg_class c
JOIN pg_catalog.pg_namespace n
ON n.oid = c.relnamespace
WHERE a.attrelid = c.oid
AND a.attisdropped
AND a.attidentity <> ''
AND n.nspname = 'public'
AND c.relkind IN ('r', 'p');
Background:
After DROP COLUMN, the pg_attribute row is marked attisdropped = true,
but attidentity still has its old value ('a').
getTableAttrs() loops over all attributes (attnum > 0), including dropped
attributes, and does:
tbinfo->needs_override = tbinfo->needs_override ||
(tbinfo->attidentity[j] == ATTRIBUTE_IDENTITY_ALWAYS);
So a dropped column with attidentity = 'a' still flips needs_override to true,
and dumpTableData_insert() then always emits OVERRIDING SYSTEM VALUE
for the table.
The attached patch fixes this by ignoring dropped columns when setting
needs_override (i.e., checking !attisdropped).
I also considered clearing attidentity in DROP COLUMN, but that wouldn't
address the problem for already-stale catalog entries.
This seems to have been around since identity columns were added.
Patch attached.
Thoughts?
Attachments:
[application/octet-stream] pg_dump-identity-dropped-column.patch (3.5K, 3-pg_dump-identity-dropped-column.patch)
download | inline diff:
From: William Bernbaum <[email protected]>
Subject: [PATCH] Fix pg_dump emitting OVERRIDING SYSTEM VALUE for tables
with dropped identity columns
When a column with GENERATED ALWAYS AS IDENTITY is dropped via
ALTER TABLE ... DROP COLUMN, PostgreSQL marks the column as dropped
(attisdropped = true) but does not clear pg_attribute.attidentity.
In getTableAttrs(), pg_dump processes all attributes (attnum > 0),
including dropped ones. The needs_override flag is then set based
on attidentity without checking attisdropped, causing
dumpTableData_insert() to emit OVERRIDING SYSTEM VALUE for tables
that no longer have any identity column.
Fix by moving the attisdropped evaluation before the needs_override
calculation and gating the check on !attisdropped.
---
src/bin/pg_dump/pg_dump.c | 5 +++--
src/bin/pg_dump/t/008_pg_dump_dropped_identity.pl | 55 ++++++++++++++++++++++
2 files changed, 57 insertions(+), 2 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -9370,8 +9370,9 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
tbinfo->typstorage[j] = *(PQgetvalue(res, r, i_typstorage));
tbinfo->attidentity[j] = *(PQgetvalue(res, r, i_attidentity));
tbinfo->attgenerated[j] = *(PQgetvalue(res, r, i_attgenerated));
- tbinfo->needs_override = tbinfo->needs_override || (tbinfo->attidentity[j] == ATTRIBUTE_IDENTITY_ALWAYS);
tbinfo->attisdropped[j] = (PQgetvalue(res, r, i_attisdropped)[0] == 't');
+ tbinfo->needs_override = tbinfo->needs_override || (!tbinfo->attisdropped[j] && tbinfo->attidentity[j] == ATTRIBUTE_IDENTITY_ALWAYS);
tbinfo->attlen[j] = atoi(PQgetvalue(res, r, i_attlen));
tbinfo->attalign[j] = *(PQgetvalue(res, r, i_attalign));
tbinfo->attislocal[j] = (PQgetvalue(res, r, i_attislocal)[0] == 't');
diff --git a/src/bin/pg_dump/t/008_pg_dump_dropped_identity.pl b/src/bin/pg_dump/t/008_pg_dump_dropped_identity.pl
new file mode 100644
--- /dev/null
+++ b/src/bin/pg_dump/t/008_pg_dump_dropped_identity.pl
@@ -0,0 +1,55 @@
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# Verify that pg_dump does not emit OVERRIDING SYSTEM VALUE for a table
+# whose only identity column has been dropped.
+
+my $node = PostgreSQL::Test::Cluster->new('identity_override');
+$node->init;
+$node->start;
+
+# Create a table with an identity column, insert a row, then drop the
+# identity column and switch to a composite primary key.
+$node->safe_psql('postgres', q{
+ CREATE TABLE demo (
+ id BIGINT GENERATED ALWAYS AS IDENTITY PRIMARY KEY,
+ fk_a BIGINT NOT NULL,
+ fk_b BIGINT NOT NULL
+ );
+
+ INSERT INTO demo (fk_a, fk_b)
+ OVERRIDING SYSTEM VALUE VALUES (1, 2);
+
+ ALTER TABLE demo DROP COLUMN id;
+ ALTER TABLE demo ADD PRIMARY KEY (fk_a, fk_b);
+});
+
+# Dump data with INSERT statements
+my $dumpfile = $node->basedir . '/dump.sql';
+
+command_ok(
+ [
+ 'pg_dump',
+ '--data-only',
+ '--inserts',
+ '--table=demo',
+ '--file=' . $dumpfile,
+ 'postgres'
+ ],
+ 'pg_dump with --inserts runs');
+
+my $dump = slurp_file($dumpfile);
+
+# Ensure no spurious OVERRIDING SYSTEM VALUE is emitted
+unlike(
+ $dump,
+ qr/OVERRIDING SYSTEM VALUE/,
+ 'no OVERRIDING SYSTEM VALUE for dropped identity column'
+);
+
+# Ensure the row is still dumped correctly
+like(
+ $dump,
+ qr/insert into.*demo.*1.*2/i,
+ 'row is dumped correctly'
+);
+
+done_testing();
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: [PATCH] Fix pg_dump emitting OVERRIDING SYSTEM VALUE for tables with dropped identity columns
@ 2026-05-01 07:28 Andreas Karlsson <[email protected]>
parent: William Bernbaum <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Andreas Karlsson @ 2026-05-01 07:28 UTC (permalink / raw)
To: William Bernbaum <[email protected]>; [email protected] <[email protected]>
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
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: [PATCH] Fix pg_dump emitting OVERRIDING SYSTEM VALUE for tables with dropped identity columns
@ 2026-05-01 07:39 Andreas Karlsson <[email protected]>
parent: Andreas Karlsson <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Andreas Karlsson @ 2026-05-01 07:39 UTC (permalink / raw)
To: William Bernbaum <[email protected]>; [email protected] <[email protected]>
On 5/1/26 09:28, Andreas Karlsson wrote:
> On 4/18/26 01:29, William Bernbaum wrote:
>> Patch attached.
Forgot this: Maybe it is just a bit too early in the morning and I have
not had coffee yet but I struggled a bit to apply your patch. A
recommendation for making patches easy to apply for other developers is
to generate them with the "git format-patch" command which makes them
easy to apply with "git am". You can use the "-v" flag to "git
format-patch" if you want to add a version number to the generated file
names.
Also another question worth looking into is if this same bug affects
other things, e.g. the setting of hasdefaults.
Thanks again for the patch!
--
Andreas Karlsson
Percona
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-05-01 07:39 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-04-17 23:29 [PATCH] Fix pg_dump emitting OVERRIDING SYSTEM VALUE for tables with dropped identity columns William Bernbaum <[email protected]>
2026-05-01 07:28 ` Andreas Karlsson <[email protected]>
2026-05-01 07:39 ` Andreas Karlsson <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox