Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix schema synchronization for partitioned tables with PostgreSQL 12+ #6780

Merged
merged 2 commits into from Oct 7, 2020

Conversation

nlenepveu
Copy link
Contributor

@nlenepveu nlenepveu commented Sep 24, 2020

Consider partitioned tables as regular tables

Typeorm currently excludes partitioned tables from schema synchronization. We could also take them into account as their behavior regarding schema modifications is the same as regular tables.

Ignore foreign keys on partitions of the partitioned tables when syncing schema on PostgreSQL 12+

When a table has foreign keys referencing a partitioned table, the PostgresQueryRunner will load the constraints from every partition.
We need to ignore those foreign keys as they are not managed by the user.

Example:
table_A has a foreign key constraint referencing table_B which is a partitioned table on key1 and key2.

The command typeorm schema:log will return this kind of list of queries to remove the FK constraints from table_A.

------------------------------------------------------------------
-- Schema syncronization will execute following sql queries (220):
------------------------------------------------------------------
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2__fkey";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey1";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey2";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey3";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey4";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey5";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey6";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey7";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey8";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey9";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey10";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey11";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey12";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey13";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey14";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey15";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey16";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey17";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey18";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey19";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey20";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey21";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey22";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey23";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey24";
ALTER TABLE "table_A" DROP CONSTRAINT "table_A_table_B_key1_key2_fkey25";
...

Typeorm should ignore those constraints.

@@ -1434,6 +1434,10 @@ export class PostgresQueryRunner extends BaseQueryRunner implements QueryRunner
}
return `("ns"."nspname" = '${schema}' AND "cl"."relname" = '${name}')`;
}).join(" OR ");

const hasIsPartition = await this.hasPartitionedTables();
const isPartitionCondition = hasIsPartition ? ` AND "cl"."relispartition" = 'f'` : "";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if we wouldn't have hasIsPartition check here? according to this check it looks like we do need items with "AND "cl"."relispartition" = 't' ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pleerock the column relispartition does not exist in PostgreSQL versions that have no support for partitioned tables (prior to version 10). This is why we need this check here.
Then, when using a version of PostgreSQL with partitioned tables, we need to ignore the partitions and only check the constraints on the "main" table ie: "relispartition" = "f" (which are not partitions).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should rename hasPartitionedTables to hasSupportForPartitionedTables to make it clearer?

@nlenepveu
Copy link
Contributor Author

@pleerock let me know if anything is unclear

@nlenepveu nlenepveu changed the title Ignore foreign keys on partitions of partitioned table when syncing schema on postgresql 12+ Fix schema synchronization for partitioned tables with PostgreSQL 12+ Oct 6, 2020
@pleerock pleerock merged commit 990442e into typeorm:master Oct 7, 2020
@pleerock
Copy link
Member

pleerock commented Oct 7, 2020

Looks good, thanks

@imnotjames imnotjames added the hacktoberfest-accepted label hacktoberfest label Oct 9, 2020
zaro pushed a commit to zaro/typeorm that referenced this pull request Jan 12, 2021
…2+ (typeorm#6780)

* feat: ignore foreign keys on partitions of partitioned tables when syncing schema on postgresql 12+

* feat: consider partitionned tables as regular tables in order to keep them in sync
@prevostc
Copy link

Hi there sorry to interrupt, this seem to be the only issue related to pg partitioning.
I couldn't find in the documentation how to add PARTITION BY RANGE (x,y) to the table definition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted label hacktoberfest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants