On 2017/03/27 23:30, Robert Haas wrote:
> On Fri, Feb 17, 2017 at 3:23 AM, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> In certain cases, pg_dump's dumpTableSchema() emits a separate ALTER TABLE
>> command for those schema elements of a table that could not be included
>> directly in the CREATE TABLE command for the table.
>>
>> For example:
>>
>> create table p (a int, b int) partition by range (a);
>> create table p1 partition of p for values from (1) to (10) partition by
>> range (b);
>> create table p11 partition of p1 for values from (1) to (10);
>>
>> pg_dump -s gives:
>>
>> CREATE TABLE p (
>> a integer NOT NULL,
>> b integer
>> )
>> PARTITION BY RANGE (a);
>>
>> CREATE TABLE p1 PARTITION OF p
>> FOR VALUES FROM (1) TO (10)
>> PARTITION BY RANGE (b);
>> ALTER TABLE ONLY p1 ALTER COLUMN a SET NOT NULL;
>> ALTER TABLE ONLY p1 ALTER COLUMN b SET NOT NULL;
>>
>> <snip>
>>
>> Note the ONLY in the above emitted command. Now if I run the above
>> commands in another database, the following error occurs:
>>
>> ERROR: constraint must be added to child tables too
>>
>> That's because specifying ONLY for the AT commands on partitioned tables
>> that must recurse causes an error.
>>
>> Attached patch fixes that - it prevents emitting ONLY for those ALTER
>> TABLE commands, which if run, would cause an error like the one above.
>
> Isn't it bogus that this is generating ALTER TABLE .. SET NOT NULL
> columns at all? You didn't say anything like that when setting up the
> database, so why should it be there when dumping?
So we should find a way for the NOT NULL constraints added for the range
partition key columns to not be emitted *separately*? Like when a table
has primary key:
--
-- Name: foo; Type: TABLE; Schema: public; Owner: amit
--
CREATE TABLE foo ( a integer NOT NULL
);
ALTER TABLE foo OWNER TO amit;
--
-- Name: foo foo_pkey; Type: CONSTRAINT; Schema: public; Owner: amit
--
ALTER TABLE ONLY foo ADD CONSTRAINT foo_pkey PRIMARY KEY (a);
The NOT NULL constraint is emitted with CREATE TABLE, not separately.
Thanks,
Amit