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

Inserts not emitted to listeners when using grouping statement #3743

Open
rustamsmax opened this issue Dec 21, 2022 · 10 comments
Open

Inserts not emitted to listeners when using grouping statement #3743

rustamsmax opened this issue Dec 21, 2022 · 10 comments
Labels

Comments

@rustamsmax
Copy link

SQLDelight Version

2.0.0-alpha02

Operating System

Android

Gradle Version

7.5

Kotlin Version

1.7.20

Dialect

default

AGP Version

No response

Describe the Bug

telegram-cloud-photo-size-2-5267409847173890973-y

telegram-cloud-photo-size-2-5267409847173890974-y

Stacktrace

No response

Gradle Build Script

No response

@rustamsmax rustamsmax added the bug label Dec 21, 2022
@rustamsmax
Copy link
Author

rustamsmax commented Dec 21, 2022

There is an extra code generated for normal insert statements

telegram-cloud-photo-size-2-5267409847173890979-y

@adrientetar
Copy link

I am hitting the same issue when using SQLite 3.35 RETURNING statement in an insert e.g.

INSERT INTO ...
VALUES(...)
RETURNING _id;

@AlecKazakova
Copy link
Collaborator

this is intentional, the alternative to whats there right now is returning an observable query which means it will execute the update/insert whenever the underlying table changes. I assume you only want to observe the SELECT or returned type, so you need to make a separate query for the reactive query

@AlecKazakova
Copy link
Collaborator

nvm misread

@AlecKazakova AlecKazakova reopened this Apr 8, 2023
@rustamsmax
Copy link
Author

Is there any plan to fix this issue?

@rustamsmax
Copy link
Author

Hi @griffio 9 months passed and nothing is done?
Why it's allowed to insert or upsert into a table if the changes are not notified to listeners?

@griffio
Copy link
Contributor

griffio commented Sep 11, 2023

🕊️ Sadly, I am not familiar with this issue. Though on quick inspection 🔍 the problem looks to be in SqlDelightQueriesFile

I can have a look and see if there is a fix - I would have to try a few things but can't promise 🎀

🤔 Looks like only when there is a SELECT statement in the group does it not emit the notification.
This means that calling notifyQueries only makes sense when used after transaction statements that don't return a QueryResult from a transactionWithResult(). For example - if you look at the generated source code and try manually adding the notifyQueries, where would it go?

@rustamsmax
Copy link
Author

Thank you for your answer. I know that if the query returns any data it won't notify listeners. But it's a bug.
It means "If you want to use a query with returning clause don't use flows".

@griffio
Copy link
Contributor

griffio commented Sep 13, 2023

Yes - indeed

Currently, that I have tried myself, only a grouping
used with a flow like this will work.

Any fix, would also have to work for INSERT INTO ...VALUES(...)RETURNING _id; - we can't generate "nested" transactions for a single statement - maybe it's possible to generate something like...

  transactionWithResult {
      driver.execute(...)
      ...
      driver.executeQuery(...)
    }.also {
            notifyQueries(...) { emit ->
                emit("subscription")
           }
    }

Maybe create a 📣 discussion question so other contributors can see it better ?
https://github.com/cashapp/sqldelight/discussions

@vitorhugods
Copy link
Contributor

I created a PR, #5006 that should fix it.

I notice now that I also came up with the same idea @griffio brought (using also). Let's see how it goes :)

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

No branches or pull requests

5 participants