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: remove unwrap() from stats related functions #289

Merged
merged 2 commits into from
Jun 26, 2024
Merged

Conversation

burmecia
Copy link
Member

@burmecia burmecia commented Jun 20, 2024

What kind of change does this PR introduce?

This PR is to remove unwrap() call from stats related functions. This also caused failure when using API to access foreign tables in some cases.

What is the current behavior?

Currently, it is hard to troubleshot when the panic is caused by unwrap() call.

What is the new behavior?

Replace unwrap() with expect() or remove it if possible.

Additional context

N/A

@burmecia burmecia requested a review from imor June 20, 2024 09:52
@burmecia burmecia added the bug Something isn't working label Jun 20, 2024
@burmecia
Copy link
Member Author

#290 is to fix the test failure

wrappers/src/stats.rs Outdated Show resolved Hide resolved
@burmecia burmecia merged commit 95a97d1 into main Jun 26, 2024
1 of 2 checks passed
@burmecia burmecia deleted the bo/fix/stats-unwrap branch June 26, 2024 07:11
@dohooo
Copy link

dohooo commented Jun 26, 2024

@burmecia, thanks for your work. Where can I follow the updates of the new release?

@burmecia
Copy link
Member Author

We're preparing a new release and it should be ready soon, you can check it on the release page. Just a note once the new version of Wrappers is release it sill needed to be included in Supabase Postgres image to available on Supabase platform, this will take some time.

@dohooo
Copy link

dohooo commented Jul 10, 2024

@burmecia I can confirm this problem still happens. I've tried to upgrade our service to the latest version(15.1.1.76). But I still need to execute the SQL in the studio client; otherwise, the call via JS SDK will fail.

image

@dohooo
Copy link

dohooo commented Jul 10, 2024

@burmecia Does this version(15.1.1.76) include this change?
image

@burmecia
Copy link
Member Author

@burmecia Does this version(15.1.1.76) include this change? image

not yet, that 15.1.1.74-wrapper tag was for integration testing, it cannot be used. But the latest Wrappers v0.4.1 has just been merged, it will be available on the next release of Supabase Postgres.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants