Passive support for partitioned tables on Postgres

[nikophil]

Q	A
Type	improvement

Partitioned tables is a complex topic which is not fully supported by Doctrine.

Currently, with PostgreSQL, Doctrine does not "see" partitioned tables, and only "sees" partitions, which is problematic and should be done the other way. This PR fixes this in PostgreSQLSchemaManager::selectTableColumns().

Before this PR:

• does not see at all "partitioned tables" (which are the actual tables related to an entity)
• doctrine suggest to remove all "partitions"

After this PR:

• doctrine:schema:validate is happy, even if tables have been partitioned manually

see #4657 (comment)

Note: I'd want to see if maintainers folks are willing to accept such modification before digging into the tests for this

[derrabus]

Thank you for your PR!

I'd want to see if maintainers folks are willing to accept such modification before digging into the tests for this

For me it's the other way round. I'd like to see functional tests before I can decide if we accept a certain change. 😓

If I got your PR right, you want the schema manager to detect a partitioned table without introducing the concept of partitioned tables itself to DBAL's schema management. That means, a partitioned table would appear along other non-partitioned tables with no way of telling them apart. Does that sound like your plan?

If so, does the change detection work properly on those tables? And would the schema manager be able to create a proper diff including correct SQL/DDL for partitioned tables?

[nikophil]

hi, thanks for your quick answer :)

For me it's the other way round. I'd like to see functional tests before I can decide if we accept a certain change. 😓

no prob, I'll work on that in few times

If I got your PR right, you want the schema manager to detect a partitioned table without introducing the concept of partitioned tables itself to DBAL's schema management

this is exact

That means, a partitioned table would appear along other non-partitioned tables with no way of telling them apart. Does that sound like your plan?

yep, a partitioned table is the main table, its partitions should be transparent to doctrine.
For instance, in PHPStorm's PostgreSQL client, you only see partitions as in a folder "partitions" under the partitioned table, while you see the partitioned table as a regular table:
https://blog.jetbrains.com/wp-content/uploads/2019/08/datagrip-PArtititons.png

Moreover, I was told that this is the current behavior of doctrine with MySQL (meaning: you can change the table as partition table in your migration, and the schema manager still sees it as a normal table, and does not see any partition)

If so, does the change detection work properly on those tables? And would the schema manager be able to create a proper diff including correct SQL/DDL for partitioned tables?

yes. With this tiny change, everything is OK, any modification to the entity will create a diff

[derrabus]

If so, does the change detection work properly on those tables? And would the schema manager be able to create a proper diff including correct SQL/DDL for partitioned tables?

yes. With this tiny change, everything is OK, any modification to the entity will create a diff

I need a functional test that covers this exact scenario: A manually created partitioned table on which we detect a modification (column/index added/changed/removed). We execute the generated DDL statements and afterwards we still have a partitioned table, but with the necessary modifications applied.

[nikophil]

@derrabus I've added the test

and afterwards we still have a partitioned table

Not sure how to test this 🤔 should I require on table pg_class to check that my table is still of realkind: 'p'?

[derrabus]

Not sure how to test this 🤔 should I require on table pg_class to check that my table is still of realkind: 'p'?

Yes. Use Postgres' own tooling to see if the table is still a partitioned table and if it's partitions are still there. I want to make sure that a partitioned table is not accidentally destroyed by any DDL we emit.

[derrabus]

The test failures seem to be related to your changes.

[nikophil]

yeah postgreSQL version problem: primary key are only supported on partitioned tables since version 11. I'll see this tomorrow

[derrabus]

We intend to drop support for Postgres 10 in DBAL 5, but that's not helping you right now, is it. 😓

[nikophil]

I removed the PK, I thought it was needed but it seems it is not, so it might work for all versions

[nikophil]

ok, I need to run the tests locally on pg 10 😅

[nikophil]

done: the test is passing in both versions 10 & 15 of pg

[nikophil]

cool CI green 🎉

the last failure seems not related to my PR 😅

[derrabus]

Awesome, thank you very much.

[Gwemox]

Thank you for your work !