Re: Bug in pg_dump --table and --exclude-table for declarative partition table handling.

On Thu, May 11, 2017 at 2:06 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Tue, May 9, 2017 at 4:21 AM, Jeevan Ladhe
> <jeevan(dot)ladhe(at)enterprisedb(dot)com> wrote:
>>> Current pg_dump --exclude-table option excludes partitioned relation
>>> and dumps all its child relations and vice versa for --table option, which
>>> I think is incorrect.
>>>
>>> In this case we might need to explore all partitions and exclude or
>>> include
>>> from dump according to given pg_dump option, attaching WIP patch proposing
>>> same fix. Thoughts/Comments?
>>
>> +1.
>>
>> Also, I can see similar issue exists with inheritance.
>
> That's a pretty insightful comment which makes me think that this
> isn't properly categorized as a bug. Table partitioning is based on
> inheritance and is intended to behave like inheritance except when
> we've decided to make it behave otherwise. We made no such decision
> in this case, so it behaves like inheritance in this case. So, this
> is only a bug if the inheritance behavior is also a bug.

We have chosen inheritance as mechanism to implement partitioning, but
users will have different expectations from them. Partitioned table is
a "table" "containing" partitions. So, if we want to dump a table
which is partitioned, we better dump its partitions (which happen to
be tables by themselves) as well. I don't think we can say that
inheritance parent contains its children, esp. thinking in the context
of multiple inheritance. IOW users would expect us to dump partitioned
table with its partitions and not just the shell.

In case of DROP TABLE <partitioned table> we do drop all the
partitions without specifying CASCADE. To drop an inheritance parent
we need CASCADE to drop its children. So, we have already started
diverging from inheritance.

>
> And I think there's a pretty good argument that it isn't. Are we
> saying we think that it was always intended that dumping an
> inheritance parent would always dump its inheritance children as well,
> and the code accidentally failed to achieve that? Are we saying we'd
> back-patch a behavior change in this area to correct the wrong
> behavior we've had since time immemorial? I can't speak for anyone
> else, but I think the first is probably false and I would vote against
> the second.

I think the inheritance behaviour we have got, whether intentional or
unintentional, is acceptable since we do not consider an inheritance
parent alongwith its children a unit, esp. when multiple inheritance
exists.

>
> That's not to say that this isn't a possibly-useful behavior change.
> I can easily imagine that users would find it convenient to be able to
> dump a whole partitioning hierarchy by just selecting the parent table
> rather than having to list all of the partitions. But that's also
> true of inheritance.

I agree that its useful behaviour for inheritance but I am not sure
that it's a "feature" for partitioned tables.

> Is partitioning different enough from
> inheritance that the two should have different behaviors, perhaps
> because the partitioning parent can't contain data but the inheritance
> parent could?

Yes, most users would see them as different things, esp. those who
come from other DBMS backgrounds.

> Or should we change the behavior for inheritance as
> well, on the theory that the proposed new behavior is just plain
> better than the current behavior and everyone will want it?

Not necessarily, as is the inheritance behaviour looks sane to me.

> Or should
> we do nothing at all, so as to avoid breaking things for the user who
> says they want to dump JUST the parent and really means it? Even if
> the parent couldn't contain any rows, it's still got a schema that can
> be dumped. The option of changing partitioning in one patch and then
> having a separate patch later that makes a similar change for
> inheritance seems like the worst option, since then users might get
> any of three behaviors depending on which release they have. If we
> want to change this, let's change it all at once. But first we need
> to get clarity on exactly what we're changing and for what reason.
>
> There is a question of timing. If we accept that this is not a bug
> but a proposed behavior change, then it's not clear that it really
> qualifies as an open item for v10. I understand that there's an urge
> to keep tinkering and making things better, but it's far from clear to
> me that anything bad will happen if we just postpone changing anything
> until v11, especially if we decide to change both partitioning and
> inheritance. I am somewhat inclined to classify this proposed open
> item as a non-bug (i.e. a feature) but I'm also curious to hear what
> others think.

To me this looks like a bug, something to be fixed in v10 only for
partitioned tables. But we don't need to entangle that with
inheritance. Partitioned tables get dumped (or not dumped) as a whole
and inheritance parents as single units.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company