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

chore: use borrowed row in iter_scan() #48

Merged
merged 1 commit into from
Jan 6, 2023
Merged

Conversation

burmecia
Copy link
Member

What kind of change does this PR introduce?

This PR is to use borrowed row in iter_scan() to improve performance, see #46 .

What is the current behavior?

Currently in each call of the iter_scan() function, an empty new row needs to be created and values are copied over to Postgres, this can be slow when reading large number of rows.

What is the new behavior?

The new iter_scan() function will pass a pre-allocated row and FDW can optionally use mem::replace() to directly move data in.

Additional context

See more details in #46 .

@burmecia burmecia requested a review from yrashk December 28, 2022 06:40
@burmecia burmecia added the enhancement New feature or request label Dec 28, 2022
@burmecia burmecia self-assigned this Dec 28, 2022
@burmecia burmecia marked this pull request as ready for review December 28, 2022 06:41
@burmecia burmecia requested a review from olirice December 30, 2022 01:05
@olirice
Copy link
Collaborator

olirice commented Dec 31, 2022

@burmecia I took a look but memory safety stuff is over my head. I pinged @yrashk to review when he gets a sec

concept sounds great!

@yrashk
Copy link
Contributor

yrashk commented Jan 1, 2023

No safety issues pop out for me here, but I think there may be further room for improvement. replace_with is still a byte copy of Row. Rows are still being created, moved, and copied.

I'd suggest establishing a benchmark test and iterating over this design further. Happy to assist there.

@burmecia
Copy link
Member Author

burmecia commented Jan 4, 2023

The byte copy of row is needed anyway, the saved cost is actually the repetitive row creation and data movement from fdw instance to supabase-wrappers, use borrowed row can remove that efforts.

@burmecia burmecia merged commit 4d52fd0 into main Jan 6, 2023
@burmecia burmecia deleted the chore/borrowed_row branch January 6, 2023 04:14
kamyshdm pushed a commit to dymium-io/supabase-wrappers that referenced this pull request Jun 17, 2024
…g-library

Tests with react testing library

Former-commit-id: f0eb3e59ecd907f31bb5798f5c3bdf5d8f167981 [formerly aff5f9a7c262992e1a0a63fe601b51183bf3b1bc]
Former-commit-id: aedad303cad2fdad4e819668ce76cca10dc00fe0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants