Thanks for the review, Andreas! > 1. I think the code would be easier to read as [...]
Agreed. I went with your second form.
> 2. [...] it would be more consistent if the new test was added as part
> of 002_pg_dump.pl if possible.
Done, and the separate 006_pg_dump_dropped_identity.pl file is gone, so
no extra cluster is created. v2 adds five entries: one that creates a table
whose only identity column is then dropped, one for the COPY output, two
for the --inserts and --column-inserts forms, and one with "like => {}"
asserting that OVERRIDING SYSTEM VALUE never shows up for that table.
> Also another question worth looking into is if this same bug affects
> other things, e.g. the setting of hasdefaults.
Good question. I checked, and hasdefaults is not affected.
RemoveAttributeById() explicitly clears attgenerated and attnotnull, and
atthasdef is cleared by RemoveAttrDefaultById(), which dependency.c
invokes when the column's pg_attrdef entry is dropped along with the
column (so the table's pg_attrdef rows go away too).
> A recommendation for making patches easy to apply [...] git format-patch
Sorry about that, and thanks for the pointer. Attached v2 is generated
with git format-patch -v2 against current master, so git am should be
happy with it.
Now added to the open commitfest:
https://commitfest.postgresql.org/patch/7246/
Best,
Will
v2-0001-pg_dump-Don-t-emit-OVERRIDING-SYSTEM-VALUE-for-dr.patch
Description: v2-0001-pg_dump-Don-t-emit-OVERRIDING-SYSTEM-VALUE-for-dr.patch
