feat: AND filters in Postgres Changes#1912
Conversation
97e25f6 to
5e3c423
Compare
|
Coverage is 91.479% — feat/postgres-changes-and-filters-real-826 into main. No base build found for |
leandrocp
left a comment
leandrocp
left a comment
There was a problem hiding this comment.
Non-blocking comments, just a question about empty filters.
| empty or whitespace-only filter string is treated as no filter: | ||
|
|
||
| iex> parse_subscription_params(%{"schema" => "public", "table" => "messages", "filter" => ""}) | ||
| {:ok, {"*", "public", "messages", []}} |
There was a problem hiding this comment.
Empty filters fail on main:
iex> Extensions.PostgresCdcRls.Subscriptions.parse_subscription_params(%{"schema" => "public", "table" => "messages", "filter" => ""})
{:error, "Error parsing `filter` params: [\"\"]"}
Should this change be considered a soft-breaking change or is this fine?
There was a problem hiding this comment.
not sure honestly... IMO we could see this as a bug we had and fix it or we can see it as properly informing the user of this error AS an error. wdyt?
There was a problem hiding this comment.
IMO an empty filter means... no filter so this change looks correct and since it used to return error, the impact of that change should be minimal.
But my question was also to understand if such changes should be listed on https://supabase.com/changelog or any other comm.
There was a problem hiding this comment.
the and filter will also need to be shared as this will potentially reduce data sent so it has that benefit for us also
There was a problem hiding this comment.
To answer my own question, I think we should update realtime docs (new AND operator and this empty filter behavior change) and also announce on /changelog
There was a problem hiding this comment.
will get the docs PR ready so we can merge almost at the same time
There was a problem hiding this comment.
|
🎉 This PR is included in version 2.100.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
3 participants
What kind of change does this PR introduce?
Implement AND filter support for Postgres Changes with comma separated parameters