Re: partitioning and identity column

On Mon, Nov 13, 2023 at 3:51 PM Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
>
> On 27.10.23 13:32, Ashutosh Bapat wrote:
> > I think we should fix these anomalies as follows
> > 1. Allow identity columns to be added to the partitioned table
> > irrespective of whether they have partitions of not.
> > 2. Propagate identity property to partitions.
> > 3. Use the same underlying sequence for getting default value of an
> > identity column when INSERTing directly in a partition.
> > 4. Disallow attaching a partition with identity column.
> >
> > 1 will fix inconsistencies in Behaviour 3 and 4. 2 and 3 will fix
> > anomalies in Behaviour 1. 4 will fix Behaviour 2.
>
> This makes sense to me.

PFA WIP patches implementing/fixing identity support for partitioned
tables as outlined above.

A partitioned table is a single relation and thus an identity column
of a partitioned table should use the same identity space across all
the partitions. This means that the sequence underlying the identity
column will be shared by all the partitions of a partitioned table and
the column will have the same identity properties across all the
partitions. Thus
1. When a new partition is added or a table is attached as a
partition, it inherits the identity column along with the underlying
sequence from the partitioned table. It can not have an identity
column of its own.
2. Since a partition never had its own identity column, when detaching
a partition, it will loose identity property of any column that had
it. If it were to retain the identity property it can not use
underlying sequence. That's not possible anyway.

This is different from the way we treat identity in inheritance.
Children in inheritance hierarchy are independent enough to have
separate identity columns and sequences of their own. So the above
discussion applies only to partitioned table. The patches too deal
with only partitioned tables.

At this point I am looking for opinions on the above rules and whether
the implementation is on the right track.

The work consists of many small code changes. In order to know which
code change is associated with which SQL each patch has test changes
and associated code change. Each patch has a commit message explaining
the changes in detail (and some times repeating the above rules again,
sorry for the repetition). These patches will be merged into a single
patch or a couple patches at most. Here's what each patch does

0001 - change to get_partition_ancestors() prologue. Can be reviewed
and committed independent of other patches.

0002 - A new partition inherits identity column and uses the
underlying sequence for direct INSERTs

0004 - An attached partition inherits identity property and uses the
underlying sequence for direct INSERTs. When inheriting the identity
property it should also inherit the NOT NULL constraint, but that's a
TODO in this patch. We expect matching NOT NULL constraints to be
present in the partition being attached. I am not sure whether we want
to add NOT NULL constraints automatically for an identity column. We
require a NOT NULL constraint to be present when adding identity
property to a column. The behavior in the patch seems to be consistent
with this.

0006 - supports ADD COLUMN ... GENERATED AS IDENTITY on a partitioned
table. identity property is propagated down the partition hierarchy.

0008 - A TODO: that I need verify/address before finalizing these
patches. Any hints are welcome.

0009 - supports ALTER COLUMN ... ADD GENERATED AS IDENTITY on a
partitioned table. Propagates the identity property down the partition
hierarchy. It requires adding NOT NULL constraint before adding
identity property just like regular table.

0011 - dropping identity property of a column of a partitioned table
drops it from the corresponding columns of all of its partitions. But
the NOT NULL constraint is retained just like in case of regular
table.

0013 - detaching a partition, drops identity property from all the
columns of partition.

patches 0003, 0005, 0007, 0010, 0012, 0014 have detailed white box
tests testing the catalog changes for each SQL. But they are not meant
to be part of the final patch-set.

--
Best Wishes,
Ashutosh Bapat