-
Notifications
You must be signed in to change notification settings - Fork 177
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
Add partitioned tables support #2197
base: main
Are you sure you want to change the base?
Conversation
5e6a8f1
to
8aafed5
Compare
✅ Deploy Preview for electric-next ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
65582f7
to
b9e1bd0
Compare
|
b9e1bd0
to
b517c38
Compare
@robacourt situation recovered. review away. great start to the year! |
Nice work! We should document the two possible ways to subscribe to partitioned tables. |
@balegas I've added basic docs highlighting the behaviour of shapes on partitioned tables - pls review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! I've left a few comments.
I've also been thinking there might be an alternative implementation, whereby the replication steam is processed (by a PartitionTransformer for want of a better name) before it gets to the filter, and if it sees a change for a partition it inserts a change into the stream for the logical table. That way, all the partition logic can go in the PartitionTransformer. Shape, Filter, Consumer etc. would remain unchanged. Sure it will create some changes that might not be used, but that seems a good tradeoff for the simplicity. Is there a reason why this is not possible? Or undesirable?
{:ok, relation_info()} | {:error, String.t()} | ||
|
||
@callback load_column_info(relation(), opts :: term()) :: | ||
{:ok, [column_info()]} | :table_not_found | ||
|
||
# @callback introspect_relation() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing this should be removed?
# @callback introspect_relation() |
assert_affected(filter, insert, MapSet.new(["s1", "s2"])) | ||
end | ||
|
||
@tag :wip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Presumably this can be removed?
@tag :wip |
{:error, _} -> | ||
# just ignore errors here, they're unlikely anyway | ||
:ok |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surely the ShapeLogCollector should crash if there's an error? An error could be something as simple as loss of network connectivity to the database, which if we just swallow the error we won't clean the relation leading to stale cache and an inconsistent state, or have I missed something?
{:ok, _} -> | ||
# probably a malformed value from a test inspector | ||
:ok |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When would this happen?
@@ -56,6 +56,36 @@ This is the root table of the shape. All shapes must specify a table and it must | |||
|
|||
The value can be just a tablename like `projects`, or can be a qualified tablename prefixed by the database schema using a `.` delimiter, such as `foo.projects`. If you don't provide a schema prefix, then the table is assumed to be in the `public.` schema. | |||
|
|||
#### Partitioned Tables |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌
Allows for subscribing to partitions of a partitioned table (which you could already do) but also subscribing to the root, partitioned, table and receiving all updates across all partitions.
Fixes #2118
Major changes:
Filter
so that it can correctly pass on a change on a partition table to a shape on the partition root.key
s are not being re-written - I don't think this is an issue as the keys are guaranteed to be unique and that's enough.Minor:
{filter, shape_ids}
so that the filter's state can be updated