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

Fix result expression for grouped statements #4378

Merged
merged 3 commits into from
Jul 17, 2023

Conversation

dellisd
Copy link
Collaborator

@dellisd dellisd commented Jul 14, 2023

Fixes #4374

@hfhbd
Copy link
Collaborator

hfhbd commented Jul 15, 2023

@dellisd I was curios why your fix didn't work so I tried it locally and fixed it. Are you okay with the commit?
Reason in async we generate this code:

override fun <R> execute(...): QueryResult<R> = QueryResult.AsyncValue {
  transactionWithResult {
    driver.execute(...).await()
    driver.executeQuery(...).await()
  }
}

To compare, in sync we return transactionWithResult directly:

override fun <R> execute(...): QueryResult<R> = transactionWithResult {
  driver.execute(...)
  driver.executeQuery(...)
}

Maybe we should use some suspend transaction directly?

@hfhbd hfhbd force-pushed the derekellis/2023-07-14/grouped-result-fix branch from 72b6bb3 to b2c82fb Compare July 15, 2023 15:46
@hfhbd hfhbd force-pushed the derekellis/2023-07-14/grouped-result-fix branch from b2c82fb to 2b25be5 Compare July 15, 2023 16:05
@hfhbd
Copy link
Collaborator

hfhbd commented Jul 15, 2023

We should add an async test too.

@hfhbd hfhbd force-pushed the derekellis/2023-07-14/grouped-result-fix branch from c1ce00f to 96cf33c Compare July 16, 2023 12:50
@AlecKazakova AlecKazakova merged commit 41ac02d into master Jul 17, 2023
5 checks passed
@AlecKazakova AlecKazakova deleted the derekellis/2023-07-14/grouped-result-fix branch July 17, 2023 10:16
@dellisd
Copy link
Collaborator Author

dellisd commented Jul 17, 2023

Thanks @hfhbd!

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

Successfully merging this pull request may close these issues.

[Bug] Type mismatch: inferred type is QueryResult<R> but R was expected when generateAsync is set to true.
3 participants