PG17 compatibility: account for identity columns in partitioned tables.

[colm-mchugh]

PG17 added support for identity columns in partitioned tables: https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=699586315 A consequence is that a table with an identity column cannot be attached as a partition. But Citus on Postgres 17 will generate identity column for the partitions if the parent table has one (or more) identity columns when propagating distributed table DDL to worker nodes, as happens in the generated_identity regress test in #7768:

 CREATE TABLE partitioned_table (
     a bigint CONSTRAINT myconname GENERATED BY DEFAULT AS IDENTITY (START WITH 10 INCREMENT BY 10),
     b bigint GENERATED ALWAYS AS IDENTITY (START WITH 10 INCREMENT BY 10),
     c int
 )
 PARTITION BY RANGE (c);
 CREATE TABLE partitioned_table_1_50 PARTITION OF partitioned_table FOR VALUES FROM (1) TO (50);
 CREATE TABLE partitioned_table_50_500 PARTITION OF partitioned_table FOR VALUES FROM (50) TO (1000);
 SELECT create_distributed_table('partitioned_table', 'a');
- create_distributed_table
----------------------------------------------------------------------
-
-(1 row)
-
+ERROR:  table "partitioned_table_1_50" being attached contains an identity column "a"
+DETAIL:  The new partition may not contain an identity column.

It is the Citus-generated ATTACH PARTITION statement that errors out, because the Citus-generated CREATE TABLE for the partitions included identity column definitions. The fix is straightforward - when propagating the CREATE TABLE ddl for a partition of a table with an identity column, don't include the identity column(s), they will be inherited on attaching the partition. In Citus on Postgres 16 (or less) partitions do not inherit identity; the partitions in the example would not have any identity columns so it was not an issue previously.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (release-13.0@89e0b53). Learn more about missing BASE report.

Additional details and impacted files

@@               Coverage Diff               @@
##             release-13.0    #7785   +/-   ##
===============================================
  Coverage                ?   89.64%           
===============================================
  Files                   ?      274           
  Lines                   ?    59595           
  Branches                ?     7436           
===============================================
  Hits                    ?    53425           
  Misses                  ?     4037           
  Partials                ?     2133           

[naisila]

This looks like a great fix. I don't see any immediate issues with previous PG version behavior. However, to be sure on PG17 behavior, we need to test on Citus all the cases in the PG commit:

• A newly created partition inherits identity property (this is currently done in pg17.sql for a distributed table, Citus local table not yet)
• Attached partition inherits identity column
• Drop identity property when detaching partition
• Partitions with their own identity columns are not allowed.
• The usual ALTER operations (add identity column, add identity
  property to existing column, alter properties of an indentity
  column, drop identity property) are supported for partitioned
  tables. Changing a column only in a partitioned table or a
  partition is not allowed; the change needs to be applied to the
  whole partition hierarchy.

Also, we need to test both distributed and Citus local partitioned tables, as they follow some different code paths for specific behaviors.

Note: I didn't test these locally myself, though I hope they all work. If there is any of these that doesn't work, we need to error out properly.

[colm-mchugh]

This looks like a great fix. I don't see any immediate issues with previous PG version behavior. However, to be sure on PG17 behavior, we need to test on Citus all the cases in the PG commit:

• A newly created partition inherits identity property (this is currently done in pg17.sql for a distributed table, Citus local table not yet)
• Attached partition inherits identity column
• Drop identity property when detaching partition
• Partitions with their own identity columns are not allowed.
• The usual ALTER operations (add identity column, add identity
  property to existing column, alter properties of an indentity
  column, drop identity property) are supported for partitioned
  tables. Changing a column only in a partitioned table or a
  partition is not allowed; the change needs to be applied to the
  whole partition hierarchy.

Also, we need to test both distributed and Citus local partitioned tables, as they follow some different code paths for specific behaviors.

The PR has been updated with tests for inheriting identity (create and attach partition) and drop identity (detach) for distributed table and local tables - thanks for the comprehensive list! Independently of partitioned tables, it looks like adding an identity column to a citus table is unsupported (there is a test for this in generated_identity, via #6738) as also is adding identity to or altering the identity of an existing column, and dropping identity property from an existing column; alter table preprocessing currently rejects Alter Table - Set Identity and Alter Table - Drop Identity commands. The PR has been updated with tests for ALTER operations, with the corresponding errors in the goldfile, to show the current status, and perhaps they can be amended if and when support is added in Citus ?

[naisila]

@colm-mchugh

The PR has been updated with tests for inheriting identity (create and attach partition) and drop identity (detach) for distributed table and local tables - thanks for the comprehensive list! Independently of partitioned tables, it looks like adding an identity column to a citus table is unsupported (there is a test for this in generated_identity, via #6738) as also is adding identity to or altering the identity of an existing column, and dropping identity property from an existing column; alter table preprocessing currently rejects Alter Table - Set Identity and Alter Table - Drop Identity commands. The PR has been updated with tests for ALTER operations, with the corresponding errors in the goldfile, to show the current status, and perhaps they can be amended if and when support is added in Citus ?

Thanks for addressing all of these cases, I had forgotten some of the Citus limitations. What you are saying makes total sense, and the current test suite is comprehensive for what we currently support.

[naisila]

LGTM, thanks!

Reminder to rebase to release-13.0 and drop the enable configure ... commit before merging.