On Wed, Jun 11, 2025 at 8:06 AM Dmitry Koval <d.ko...@postgrespro.ru> wrote: > > >Do getAttributesList need to care about pg_attribute.attidentity? > >currently MERGE PARTITION seems to work fine with identity columns, > >this issue i didn't dig deeper. > > Probably after commit [3] partition's identity columns shares the > identity space (i.e. underlying sequence) as the corresponding > columns of the partitioned table. So call BuildDescForRelation in > createPartitionTable function should copy pg_attribute.attidentity > for new partition. > but BuildDescForRelation is based on getAttributesList, in getAttributesList, assign pg_attribute.attidentity to def->identity should be safe, IMHO.
+ <para> + If merged partitions have different owners, an error will be generated. + The owner of the merged partitions will be the owner of the new partition. + </para> + <para> + It is the user's responsibility to setup <acronym>ACL</acronym> on the + new partition. + </para> since they <para> are related, these two can be one <para>? + <para> + If merged partitions have individual constraints, those constraints will + be dropped because command uses partitioned table as a model to create + the constraints. + </para> I feel like it's not fully accurate, the following is what I can come up with: + <para> + When partitions are merged, any individual objects belonging to those + partitions, such as constraints or statistics will be dropped. This occurs + because ALTER TABLE MERGE PARTITIONS uses the partitioned table itself as the + template to define these objects. + </para> > > >I am wondering right after createPartitionTable, > >do we need a CommandCounterIncrement? > >because later moveMergedTablesRows will use the output of > >createPartitionTable. > > We call CommandCounterIncrement in createPartitionTable function right > after heap_create_with_catalog (same code in create_toast_table, > make_new_heap, DefineRelation functions). We need an additional > CommandCounterIncrement call in case we use objects created after this > point. But we probably don't use these objects (in function > moveMergedTablesRows too). > As mentioned in the previous thread [1], moveMergedTablesRows need latest relcache entry for newPartRel. so I guess, put one CommandCounterIncrement at the end of createPartitionTable should be fine, which I already did in [1]. [1]: https://postgr.es/m/cacjufxh3mfnyfhy9+dcuzphosmvrtjujbwu1vh248lg0ezj...@mail.gmail.com > 3. > >I only want to allow HEAP_TABLE_AM_OID to be used > >in the merge partition, > >I guess that would avoid unintended consequences. > > Thanks for the clarification. Isn't this limitation too strong? > It is very likely that the user will create an AM based on > HEAP_TABLE_AM_OID, in which case the code should work. > ok. if you looking at ATExecDetachPartition, we have: /* * Detaching the partition might involve TOAST table access, so ensure we * have a valid snapshot. */ PushActiveSnapshot(GetTransactionSnapshot()); /* Do the final part of detaching */ DetachPartitionFinalize(rel, partRel, concurrent, defaultPartOid); PopActiveSnapshot(); do we need do the same to the following DetachPartitionFinalize: foreach_ptr(RelationData, mergingPartition, mergingPartitionsList) { /* Remove the pg_inherits row first. */ RemoveInheritance(mergingPartition, rel, false); /* Do the final part of detaching. */ DetachPartitionFinalize(rel, mergingPartition, false, defaultPartOid); }
