supabase / supabase-flutter

Flutter integration for Supabase. This package makes it simple for developers to build secure and scalable products.
https://supabase.com/
MIT License
661 stars 154 forks source link

[Proposal] support for realtime multiple filters #869

Closed rickypid closed 3 months ago

rickypid commented 3 months ago

I've tried to implement support for adding multiple filters to a real-time subscription.

After conducting some tests locally, I don't see any major issues. I wanted to understand if it's a feature that can be implemented.

I need this function in some projects, I have seen several requests for this here

dshukertjr commented 3 months ago

Our Realtime server can only take one filter at the moment, so this update would not work unfortunately.

rickypid commented 3 months ago

Hi @dshukertjr ,

I tested it and it seemed to work:

    final query = client
        .schema(config.schema)
        .from(config.messagesTableName)
        .stream(primaryKey: ['id'])
        .eq('roomId', int.parse(room.id)).gt('createdAt', 1711479792862) //<-----
        .order('createdAt', ascending: false);
dshukertjr commented 3 months ago

@rickypid Currently, Supabase Realtime simply does not have multi-filter support, and you certainly cannot just join multiple filter strings with & to make it work. Supabase Realtime simply does not have the API to accept multiple filters joined by &. If you think it works, then you haven't done enough testing.

rickypid commented 3 months ago

@dshukertjr

I don't want to be insistent or suspicious but I would like to explain my situation to you.

To demonstrate how it works:

  1. I have a demo app for a chat, I create two rooms without messages.

image

  1. In both chats I send a message, in chat 1: Message from chat 1 and in chat 2: Message from chat 2

image

Chat 1:

image

Chat 2:

image

I have this function to get the message stream:

     final query = client
         .schema(config.schema)
         .from(config.messagesTableName)
         .stream(primaryKey: ['id'])
         .eq('roomId', int.parse(room.id)).gt('createdAt', 1711479792862)
         .order('createdAt', ascending: false);

I therefore have a filter for the roomId and one for the creation date createdAt greater than x.

Up to here everything works. 😄

If I try to remove the roomId filter the messages in the chats get mixed up (As I expect):

     final query = client
         .schema(config.schema)
         .from(config.messagesTableName)
         .stream(primaryKey: ['id'])
         /*.eq('roomId', int.parse(room.id))*/.gt('createdAt', 1711479792862)
         .order('createdAt', ascending: false);

The contents of chat 1:

image

Then both filters are applied.


I then carry out this test, restoring the original filters (So for both roomIdand createdAt) and insert a query into the database with a message with a date greater than the createdAt filter and the correct roomId, so I expect to receive the real-time event:

INSERT INTO chats.messages("createdAt", "roomId", "status", "type", "authorId", "text")
VALUES (1711531670000, 7, 'sent', 'text', '8af5a695-9e1f-4a0b-84d4-ec3ec1d1433d', 'Message from query');

Chat 1

image

Event received successfully!! 😄

Now I do the same thing with the date before the createdAt filter:

INSERT INTO chats.messages("createdAt", "roomId", "status", "type", "authorId", "text")
VALUES (1711479792700, 7, 'sent', 'text', '8af5a695-9e1f-4a0b-84d4-ec3ec1d1433d', 'Message from query');

Chat 1

image

Event not received because the date does not match the filter. 🚀

In my opinion I don't see any problems, I don't know where I'm going wrong but everything works correctly.

I hope I performed the tests correctly.

Just for completeness: I performed the same tests done with inserting the message with a query from another client by sending the message with the supabase-flutter client.

dshukertjr commented 3 months ago

@rickypid Try doing the same tests with only the .onPostgresChanges method and not the .stream() method.

rickypid commented 3 months ago

@dshukertjr

I tested with the tests in packages/realtime_client/test/mock_test.dart, I added:

image

and the tests pass:

image

if I change the data and they don't respect the filters they don't pass:

image

image

I hope I did it correctly.

dshukertjr commented 3 months ago

@rickypid The mock test that you used just has a mock server that emits the payload of Realtime server, and passing the wrong configuration could still result in events coming through, so it's not a good indicator of whether your PR works (TBH, it's probably not a very good test code).

I want you to try applying multiple filters on a live Supabase instance using the onPostgresChanges() and see if you get Realtime events. The .stream() method uses the REST API and the Realtime events, and probably the reason why you thought you were getting the correct results was because you were seeing the REST API, which supports multiple filters, fetch the correct results.

Supabase Realtime server only supports a single filter currently. If you had tried it against an actual Supabase instance, you would have seen that it doesn't work.

Here is a video explaining and demonstrating how the code in this PR fails to subscribe to Realtime changes.

https://github.com/supabase/supabase-flutter/assets/18113850/3078ae30-8ecb-45c1-9988-ea4c577cc336

I appreciate the gesture that you wanted to make this library better, but perhaps we can channel the energy into something else as there is nothing we can do no the Flutter SDK side of things to add support for multiple filters at the moment. Note that our Realtime team is aware that users want more complex filters, and they do have future plans of supporting complex queries.

rickypid commented 3 months ago

@dshukertjr

Thank you for the detailed explanation, trying it without the tests, I confirm that it is as you say.

However, this is a feature that is urgently needed as it is essential for most applications.

My use case is going to severely limit the ability to use Supabase as it is not possible to open a chat and load the entire list of messages, so I wanted to introduce timestamp filtering.

To have at least this functionality with .stream() do you have an applicable solution?

dshukertjr commented 3 months ago

@rickypid You probably could implement the chat feature without .stream(), and just use the combination of the REST API methods and onPostgresChanges method.

You can use the rest API methods to get the first

// Assuming you have the room ID
final myRoomId = 1;

// Get the past 20 messages in a chat room.
final chatData =  supabase.from('messages')
  .select()
  .eq('room_id', myRoomId)
  .order('created_at')
  .limit(20);

// Listen to new incoming chat messages
    supabase
        .channel('messages')
        .onPostgresChanges(
            event: PostgresChangeEvent.all,
            schema: 'public',
            table: 'messages',
            filter: PostgresChangeFilter(
              type: PostgresChangeFilterType.eq,
              column: 'room_id',
              value: myRoomId,
            ),
            callback: ((payload) {
              // Update state to add the new incoming messages to the UI
            }))
        .subscribe();

Then when the user scrolls up to see more past messages, you just need to get them using the REST API methods

final pastChatData =  supabase.from('messages')
  .select()
  .eq('room_id', myRoomId)
  .lt('created_at', chatData.last.createdAt)
  .order('created_at')
  .limit(20);
rickypid commented 3 months ago

@dshukertjr

I like it, I can handle this with a StreamController and create equivalent logic for myself.

Thank you so much for the suggestion and support you provided for this PR.