Skip to content

feat: postgrest 13 add maxaffected in client libraries #619

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

Draft
wants to merge 2 commits into
base: avallete/psql-436-feedback-request-postgrest-js-and-postgrest-v13-integration
Choose a base branch
from

Conversation

avallete
Copy link
Member

What kind of change does this PR introduce?

  • Implement the maxAffected postgrest 13 feature at the client level
  • Add tests, introduce a more generic way to handle Prefer header, can be refactor at other places in other PR

avallete added 2 commits May 19, 2025 09:26
…tgrest-v13-integration' into avallete/psql-372-postgrest-add-maxaffected-in-client-libraries
.maxAffected(2)
const { error } = result
expect(error).toBeDefined()
expect(error?.message).toBe('Query result exceeds max-affected preference constraint')
Copy link
Member

Choose a reason for hiding this comment

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

How about just matching the PostgREST error code? (PGRST124, ref)

This would allow us to change the error message without causing a test failure later. Right now the error message seems a bit specific to postgREST ("preference constraint" could be removed)

.select()

expect(error).toBeDefined()
expect(error?.message).toBe('Query result exceeds max-affected preference constraint')
Copy link
Member

Choose a reason for hiding this comment

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

ditto here, only match error code if possible.

@@ -0,0 +1,64 @@
export class HeaderManager {
Copy link
Member

Choose a reason for hiding this comment

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

Can we use the Headers API instead? Should be available on both web and Node

Copy link
Member

Choose a reason for hiding this comment

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

Great idea though and we should move to using that vs. plain objects/Records

*
* @param value - The maximum number of rows that can be affected
*/
maxAffected(
Copy link
Member

Choose a reason for hiding this comment

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

Let's move this to PostgrestTransformBuilder since this doesn't do horizontal filtering, but rather an assertion a la .single()

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.

3 participants