Re: partitioning and identity column

From: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
To: Peter Eisentraut <peter(at)eisentraut(dot)org>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: partitioning and identity column
Date: 2024-01-09 14:10:33
Message-ID: CAExHW5s+552sq=s2igOaFBuWFcRwL2uoL-O1gz2d0fDRf4bpvg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Dec 21, 2023 at 4:32 PM Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
>
> On 19.12.23 11:47, Ashutosh Bapat wrote:
> > At this point I am looking for opinions on the above rules and whether
> > the implementation is on the right track.
>
> This looks on the right track to me.

Thanks.

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

Thanks.

>
> > 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.
>
> I think it makes sense that the NOT NULL constraint must be added
> manually before attaching is allowed.
>
Ok. I have modified the test case to add NOT NULL constraint.

Here's complete patch-set.
0001 - fixes unrelated documentation style - can be committed
independently OR ignored
0002 - adds an Assert in related code - can be independently committed

On Mon, Nov 13, 2023 at 3:51 PM Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
> Note, here is a writeup about the behavior of generated columns with
> partitioning:
> https://www.postgresql.org/docs/devel/ddl-generated-columns.html. It
> would be useful if we documented the behavior of identity columns
> similarly. (I'm not saying the behavior has to match.)
0003 - addresses this request

0004 - 0011 - each patch contains code changes and SQL testing those
changes for ease of review. Each patch has commit message that
describes the changes and rationale, if any, behind those changes.
0012 - test changes
0013 - expected output change because of code changes
All these patches should be committed as a single commit finally.
Please let me know when I can squash those all together. We may commit
0003 separately or along with 0004-0013.

0014 and 0015 - pg_dump/restore and pg_upgrade tests. But these
patches are not expected to be committed for the reasons explained in
the commit message. Since identity columns of a partitioned table are
not marked as such in partitions in the older version, I tested their
upgrade from PG 14 through the changes in 0015. pg_dumpall_14.out
contains the dump file from PG 14 I used for this testing.

--
Best Wishes,
Ashutosh Bapat

Attachment Content-Type Size
0003-Add-Identity-Column-section-under-Data-Defi-20240109.patch text/x-patch 3.0 KB
0002-Assert-that-partition-inherits-from-only-on-20240109.patch text/x-patch 1.8 KB
0005-Attached-partition-inherits-identity-column-20240109.patch text/x-patch 8.5 KB
0004-A-newly-created-partition-inherits-indentit-20240109.patch text/x-patch 5.9 KB
0001-Decorate-PostgreSQL-with-productname-tag-20240109.patch text/x-patch 983 bytes
0006-Support-adding-indentity-column-to-a-partit-20240109.patch text/x-patch 5.4 KB
0007-Adding-identity-to-partitioned-table-adds-i-20240109.patch text/x-patch 7.8 KB
0009-Changing-Identity-column-of-a-partitioned-t-20240109.patch text/x-patch 7.7 KB
0008-DROP-IDENTITY-on-partitioned-table-recurses-20240109.patch text/x-patch 9.1 KB
0010-Drop-identity-property-when-detaching-parti-20240109.patch text/x-patch 5.4 KB
0011-Partitions-with-their-own-identity-columns--20240109.patch text/x-patch 6.2 KB
0012-Test-changing-some-properties-of-identity-c-20240109.patch text/x-patch 3.0 KB
0013-Fix-output-in-modules-test_ddl_deparse-20240109.patch text/x-patch 2.2 KB
0014-Test-dump-and-restore-NOT-FOR-FINAL-COMMIT-20240109.patch text/x-patch 3.1 KB
0015-Test-pg_upgrade-20240109.patch text/x-patch 3.3 KB
pg_dumpall_14.out application/octet-stream 4.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Christoph Berg 2024-01-09 14:35:56 Re: psql JSON output format
Previous Message Peter Eisentraut 2024-01-09 14:03:39 Re: add AVX2 support to simd.h