Skip to content

remove quotes #46

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

Conversation

spencerbart
Copy link
Contributor

Bug fix

#45 #42

@spencerbart
Copy link
Contributor Author

Screenshot 2022-11-18 at 2 16 22 PM

These are the same queries. On the left is the JavaScript SDK. On the right is the Rust SDK. The only difference is that Rust inserts unnecessary quotes that don't allow for numbers or foreign table queries.

@spencerbart
Copy link
Contributor Author

spencerbart commented Nov 18, 2022

Screenshot 2022-11-18 at 2 22 42 PM
Screenshot 2022-11-18 at 2 24 35 PM

The top is JavaScript SDK. The bottom is Rust SDK. Same queries but JavaScript works whereas Rust does not.

@spencerbart
Copy link
Contributor Author

Screenshot 2022-11-18 at 2 39 52 PM

Here's another example of a .gt query putting unnecessary quotes. On the left is JavaScript and on the right is Rust

src/filter.rs Outdated
@@ -4,7 +4,7 @@ use crate::Builder;

fn clean_param(param: &str) -> Cow<str> {
if ",.:()".chars().any(|c| param.contains(c)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also just remove the "." from the clean_param check.

@soedirgo
Copy link
Collaborator

Thanks! Do you mind addressing the failing check as well?

@spencerbart
Copy link
Contributor Author

@soedirgo I just fixed the clippy issue. I also realized that clear_params is now redundant so I removed it. I just checked through the JavaScript SDK (postgrest-js) and it appears that it doesn't have any function that clears the params.

This is in PostgrestFilterBuilder.ts
Screenshot 2022-11-21 at 11 18 39 PM

@soedirgo soedirgo merged commit 4dced30 into supabase-community:master Nov 22, 2022
@soedirgo
Copy link
Collaborator

Thanks again for the PR!

@spencerbart spencerbart deleted the spencer/remove_unnecessary_quote branch November 22, 2022 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants