Skip to content

Fix jsonGroupArray emitting redundant primary key check #113

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix jsonGroupArray emitting redundant primary key check #113

wants to merge 1 commit into from

Conversation

seanwoodward
Copy link
Contributor

Fix #109 and add test

@stephencelis
Copy link
Member

@seanwoodward Sorry for the delay, and thanks for explaining #109 a bit more with this PR.

I think we are inclined to keep the behavior as is. The jsonGroupArray that automatically unwraps values includes this filter automatically, so folks reaching for it should ideally omit the id.isNot(nil) check. If they do include it, the redundant SQL generated should be mostly harmless.

Let us know if you have other thoughts!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

jsonGroupArray emits redundant check for primaryKey not null if specified in filter argument
2 participants
@seanwoodward @stephencelis