Skip to content

Conversation

@iruizmar
Copy link
Contributor

@iruizmar iruizmar commented Jun 1, 2024

What kind of change does this PR introduce?

Feature

What is the current behavior?

The realtime API doesn't allow to have a multi-column PK.

What is the new behavior?

The realtime API now works with multi-column PKs.

Additional context

https://kotlinlang.slack.com/archives/C06QXPC7064/p1716652456870609

I couldn't really test this. What's the usual strategy to test the library?

@iruizmar iruizmar requested a review from jan-tennert as a code owner June 1, 2024 13:09
Copy link
Collaborator

@jan-tennert jan-tennert left a comment

Choose a reason for hiding this comment

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

Okay, it seems this might take more changes than expected. You can fix the selectAsFlow method by just taking the primary key values and combining them yourself. Something like this:

cache.remove(primaryKeys.map { key -> it.oldRecord[key.columnName]?.jsonPrimitive?.content }.joinToString(""))

(not sure if that works exactly).
For the single flow variant, I'm not sure if that's possible as we can only provide one filter for realtime

@iruizmar
Copy link
Contributor Author

iruizmar commented Jun 1, 2024

Okay, it seems this might take more changes than expected. You can fix the selectAsFlow method by just taking the primary key values and combining them yourself. Something like this:

cache.remove(primaryKeys.map { key -> it.oldRecord[key.columnName]?.jsonPrimitive?.content }.joinToString(""))

(not sure if that works exactly). For the single flow variant, I'm not sure if that's possible as we can only provide one filter for realtime

I made the changes. Can you test that again? 🙏🏻

Copy link
Collaborator

@jan-tennert jan-tennert left a comment

Choose a reason for hiding this comment

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

Yes, I think that should work. Can you add List<KProperty1<Data, Value>> overloads (like the other overload for PrimaryKey, so you can just use this syntax: selectAsFlow(listOf(Message::id, Message::channelId))

@iruizmar
Copy link
Contributor Author

iruizmar commented Jun 1, 2024

Yes, I think that should work. Can you add List<KProperty1<Data, Value>> overloads (like the other overload for PrimaryKey, so you can just use this syntax: selectAsFlow(listOf(Message::id, Message::channelId))

Oh, I don't know why I missed those. Done ✅.

@jan-tennert
Copy link
Collaborator

jan-tennert commented Jun 1, 2024

Also, I normally just use the blank test module which is included in the gradle project. (But in the gitignore, so not in the repo).
Alternatively, you can also publish the dependencies to maven local. I plan on improving this soon with the samples

Copy link
Collaborator

@jan-tennert jan-tennert left a comment

Choose a reason for hiding this comment

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

Great! I'm going to include this in the upcoming 2.5.0 beta, so you can try it out yourself!

@jan-tennert jan-tennert merged commit 9588c23 into supabase-community:master Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants