Skip to content

Conversation

@Vinzent03
Copy link
Collaborator

@Vinzent03 Vinzent03 commented Jul 14, 2023

Changes

  • add any and all version to like and ilike
  • set columns search parameter in bulk insert/upsert to allow missing column in one row
  • add defaultToNull to use default value instead of null for missing value in bulk insert/upsert

Ressources

@Vinzent03 Vinzent03 changed the title Feat/postgrest11 feat(postgrest): updates for postgREST 11 Jul 14, 2023
@Vinzent03 Vinzent03 marked this pull request as ready for review July 17, 2023 14:06
@Vinzent03
Copy link
Collaborator Author

With the update to postgREST v11 the test Default http client row level security error somehow fails now, because the call now returns null/ empty list when appending select() instead of throwing an error. I can't find this change in the changelog though. Should I just remove the test?

@Vinzent03 Vinzent03 requested a review from dshukertjr July 17, 2023 14:07
@dshukertjr
Copy link
Member

Thanks for this! I am a bit swamped today, but will take a look tomorrow!

Copy link
Member

@dshukertjr dshukertjr left a comment

Choose a reason for hiding this comment

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

The PR is spotless as usual 🚀 Just the one thing about the default value for defaultToNull parameter gets me. I have left a comment here, so let's see what the js team says.
https://github.com/supabase/postgrest-js/pull/417/files#r1268195130

Copy link
Member

@dshukertjr dshukertjr left a comment

Choose a reason for hiding this comment

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

The code looks great! Feel free to edit the comments to whatever you think best communicates the behavior to the users!

Copy link
Member

@dshukertjr dshukertjr left a comment

Choose a reason for hiding this comment

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

Oops, could you check why the tests might be failing? It seems like it has nothing to do with any of the changes in this PR though.

@Vinzent03
Copy link
Collaborator Author

@dshukertjr See #550 (comment) above for the test failing. With postgrest 9 or 10 it works, but 11 doesn't.

@dshukertjr
Copy link
Member

@Vinzent03
Sorry for missing your comment about the failing test. I think it's alright to just remove the test.

Copy link
Member

@dshukertjr dshukertjr left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the amazing work!

@Vinzent03 Vinzent03 merged commit 64d8eb5 into main Jul 23, 2023
@Vinzent03 Vinzent03 deleted the feat/postgrest11 branch July 23, 2023 21: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

Development

Successfully merging this pull request may close these issues.

2 participants