supabase / walrus

Applying RLS to PostgreSQL WAL
Apache License 2.0
119 stars 9 forks source link

Realtime events not emitted when used with `eq` filter and the row no longer meets the `eq` condition due to update #64

Open dshukertjr opened 1 year ago

dshukertjr commented 1 year ago

Bug report

Describe the bug

Original issue: https://github.com/supabase/supabase-flutter/issues/319

When listening to row level changes using the eq filter, the update or delete events where the column of eq filter changes.

To Reproduce

  1. Create a table with at least one additional column (I set it to type boolean and called it col)
  2. Add some mock data
  3. Use the SDK to retrieve data with an .eq filter: col=eq.true
  4. Lets say there are 2 rows where col is true. These two are delivered to the device and an event is registered everytime something changes in these rows.
  5. If a row A that previously had col = true gets set to col = false, there is no event emitted.

Expected behavior

I have a feeling that this behavior is the intended behavior, or there are some known limitations, but would it be better if there was one last event emitted in the above example when the col was updated to false so that client can react to the fact that the data no longer meets the eq condition?

w3b6x9 commented 1 year ago

@dshukertjr there's no guarantee that replica identity is full which means it won't always be possible if the filter is on a non-primary key column. We could check previous and new in update cases so if a table has replica identity set to full it could send along changes when previous fulfills the filter.

@olirice would like your take on this.

dshukertjr commented 1 year ago

@w3b6x9 I think the fact that there is no guarantee that the replica identity is set to full is fine. We just need to make sure it is well documented. It would be amazing if we could also check the previous value to send the changes if it fulfills the filter 🚀

olirice commented 1 year ago

yeah, I agree. we would not be able to apply filters consistently using old_record because it usually only includes the primary key information.

but, as Tyler pointed out, we could work around it by trying the filter i.e.

if 
  filter_is_valid(ud_filter, old_record.column_x) 
  or
  filter_is_valid(ud_filter, new_record.column_x)

where users could opt to the behavior by setting replica identity full for a given table.


I can see that there are valid use-cases but (imo) including the event where a filter becomes false would be a little unintuitive for users and could potentially cause weird things to happen in their applications. It's also not backwards compatible.


If the functionality is needed, I think a clearer way to communicate it with users would be to make it explicit in the filter e.g.

old_record.col=eq.true

but thats also a bigger feature in realtime and in walrus


let me know how you want to play it

dshukertjr commented 1 year ago

Just throwing some ideas out there just because I have in my mind, but maybe for such update where the old record meets the filter but the new record does not, we can send down the old record, but not the new record. To me, this feels like an intuitive behavior. I'm sure it will be breaking changes but...

w3b6x9 commented 1 year ago

@dshukertjr thanks, definitely something to ponder. We can revisit this when Enable filters for delete events supabase/realtime#53 is prioritized. I'll move this over to the walrus repo since the changes will take place there.

KaushikGupta007 commented 2 months ago

Facing same issue.