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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add for-await support for streaming #6

Closed
wants to merge 1 commit into from

Conversation

Minigugus
Copy link
Contributor

Fix #1

This PR is a very simple and naive implementation of for-await for consuming stream. I added tests and updated the README.

Feel free to update or close if you have a better idea.

Otherwise, awesome project, thank you 馃槃

let result = 0
for await (const { generate_series: x } of sql`select * from generate_series(1, 100000)`)
  result += x;
return [4699408878, result]

@porsager
Copy link
Owner

porsager commented Jan 8, 2020

Hey @Minigugus ... Thanks a lot for the kind words and the PR.

As you might have seen in the #1 issue I've already got a working branch here: https://github.com/porsager/postgres/tree/cursor which I realize I should have made a PR for so that it was visible work was already under way.

With regards to your implementation here, while it works, it will fill up memory with all the rows coming from the database which I don't feel is what a user will expect if using for await.

I'll be finishing up my work in the cursor branch this or next week which will also include for await support, so I hope you're ok with me closing out this PR?

@Minigugus
Copy link
Contributor Author

Argh, too fast, my bad 馃槄

Your implementation will obviously be better than mine, I'm closing this PR 馃槈

Keep up the great work 馃槃

@Minigugus Minigugus closed this Jan 8, 2020
@porsager
Copy link
Owner

porsager commented Jan 8, 2020

Cool :)

And thanks, I'll do my best.

@Minigugus Minigugus deleted the feature/for-await branch January 15, 2020 19:06
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.

Stream speed management using async/await (cursor)
2 participants