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

feat: Fallback to include whole table in replication if where clauses unsupported #2367

Merged
merged 3 commits into from
Feb 25, 2025

Conversation

msfstef
Copy link
Contributor

@msfstef msfstef commented Feb 24, 2025

Fixes #2360

This will make the running Electric fallback to replicating whole tables if it receives any shapes with unsupported where clauses (e.g. enums, varchar with IN checks, user-defined data types in general, and who knows what else).

There is no "recovery" mechanism to return to row-filtering as the Postgres error does not allow for an easy way to check which where clause caused the issue - once we go to relation-only filtering we stay there, like we would if an active shape had no where clause or if we were in PG14.

Ideally we would detect where clauses that are unsupported at the relation filter processing level, so we can fine tune that, but until then this fallback makes sure that Electric works even if an unsupported where clause is provided.

As discussed in this Discord thread, we could also have a configuration flag and better errors to avoid this sort of radical fallback, but we opted for an "always works" approach here. IIRC some benchmarking had shown that our filtering is fast enough that the PG level filtering might not be as important anyway, although the limiting of transmitted data is definitely nice (despite several issues we have with not being able to limit columns replicated etc).

This is related with #1778 , and I'm also referencing #1831 as we had encountered many limitations to row filtering which has led to this proposed change.

We can definitely improve this by detecting unsupported where clauses, checking filter diffs to know what caused the issue and reverting back after, periodically attempting to revert back to row-filtering, and an array of different approaches, but this allows all where clauses to be accepted and Electric to adjust accordingly.

@KyleAMathews
Copy link
Contributor

Love this. Make it right first (valid shape requests always work) and then make it fast as necessary (e.g. periodically retry to optimize publication slots). The later could be looked at with #1547 as when we GC is a good point to also consider other optimizations.

Copy link
Contributor

@icehaunter icehaunter left a comment

Choose a reason for hiding this comment

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

Great work & clean implementation

@msfstef msfstef merged commit f92d4b3 into main Feb 25, 2025
35 checks passed
@msfstef msfstef deleted the msfstef/fallback-to-include-whole-table-in-replication branch February 25, 2025 11:43
@marbemac
Copy link

thanks @msfstef!

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

Successfully merging this pull request may close these issues.

Replicate entire table if publication row filtering is not available
4 participants