feat(postgres): add partitioned table support #297
Conversation
abhi-airspace-intelligence
changed the title
abhi-airspace-intelligence
force-pushed
the
abhi/add-partitioned-table-support
branch
4 times, most recently
from
e65c749 to
230f109
Compare
etl/src/replication/client.rs
Outdated
| "select schemaname, tablename from pg_publication_tables where pubname = {};", | ||
| "select pt.schemaname, pt.tablename from pg_publication_tables pt | ||
| join pg_class c on c.relname = pt.tablename | ||
| join pg_namespace n on n.oid = c.relnamespace AND n.nspname = pt.schemaname |
There was a problem hiding this comment.
Is there the need to perform this JOIN? Seems like we don't really need to validate namespaces.
There was a problem hiding this comment.
There wasn't, no. I think this was leftover code that I included attempting to fix the race issue I encountered. I cannot replicate it anymore.
abhi-airspace-intelligence
force-pushed
the
abhi/add-partitioned-table-support
branch
6 times, most recently
from
4670481 to
c983ec9
Compare
|
This MR has mostly been rewritten, as I discovered there wasn't an easy way to capture new partitions being created. I did discover |
|
Tests are broken again on the partial branch. Can be solved by this patch: uni-intelligence@c0ec81f |
abhi-airspace-intelligence
force-pushed
the
abhi/add-partitioned-table-support
branch
4 times, most recently
from
96cc8de to
6522d79
Compare
abhi-airspace-intelligence
force-pushed
the
abhi/add-partitioned-table-support
branch
5 times, most recently
from
67cc277 to
4893b04
Compare
abhi-airspace-intelligence
force-pushed
the
abhi/add-partitioned-table-support
branch
from
4893b04 to
6d5132b
Compare
Signed-off-by: Abhi Agarwal <abhi@airspace-intelligence.com>
Signed-off-by: Abhi Agarwal <abhi@airspace-intelligence.com>
Signed-off-by: Abhi Agarwal <abhi@airspace-intelligence.com>
Signed-off-by: Abhi Agarwal <abhi@airspace-intelligence.com>
Signed-off-by: Abhi Agarwal <abhi@airspace-intelligence.com>
abhi-airspace-intelligence
force-pushed
the
abhi/add-partitioned-table-support
branch
from
bf3af88 to
3524085
Compare
Signed-off-by: Abhi Agarwal <abhi@airspace-intelligence.com>
Signed-off-by: Abhi Agarwal <abhi@airspace-intelligence.com>
Signed-off-by: Abhi Agarwal <abhi@airspace-intelligence.com>
abhi-airspace-intelligence
force-pushed
the
abhi/add-partitioned-table-support
branch
from
9b70c19 to
82436de
Compare
|
This is ready for review! Note that I did cherry-pick #361 on top of this since I was running non-bigquery tests open, but happy to revert if desired. |
Thanks for the PR, once I have time, I will go through it. We are currently busy with other work so it might take some time. |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| pub_tables as ( | ||
| select r.prrelid as oid | ||
| from pg_publication_rel r | ||
| join pg_publication p on p.oid = r.prpubid | ||
| where p.pubname = {pub} and (select has from has_rel) | ||
| union all | ||
| select c.oid | ||
| from pg_publication_tables pt | ||
| join pg_class c on c.relname = pt.tablename | ||
| where pt.pubname = {pub} and not (select has from has_rel) |
There was a problem hiding this comment.
[P1] Join publication tables without schema
The new recursive query in PgReplicationClient::get_publication_table_ids now joins pg_publication_tables to pg_class only on relname. If a publication includes two tables with the same name in different schemas (e.g. sales.users and archive.users), this join can resolve to the wrong OID and the client will subscribe to or copy data from the wrong table. The previous implementation joined on both schema and table name and avoided this ambiguity.
Useful? React with 👍 / 👎.
What kind of change does this PR introduce?
Adds support for partitioned tables. It does this through a very convoluted sql query that I basically guess-and-checked until it worked. It basically uses a bunch of heuristics to confirm that if a table is a child table of a partitioned table, it's allowed to treat their PKs as its own.
What is the current behavior?
Fixes #296
What is the new behavior?
Partitioned tables are treated as a single table, and replicate as one unit.
Additional context
Note that this is stacked on top of my other MR because I found this bug after fixing that bug. I will also note that I happened to find a race condition in this PR where a replicate worker tries to push events while a sync worker is waiting for a schema. I solved this by simply blocking all workers until all schemas are fixed, but this is a suboptimal solution, to say the least.