Skip to content

Implement $exists #17

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

Merged
merged 1 commit into from
Jun 15, 2024
Merged

Implement $exists #17

merged 1 commit into from
Jun 15, 2024

Conversation

erikdubbelboer
Copy link
Member

Comment on lines +155 to +185
isNestedColumn := c.nestedColumn != ""
for _, exemption := range c.nestedExemptions {
if exemption == key {
isNestedColumn = false
break
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This code is duplicated in the case nil: as well, we could lift it above the switch.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah we can change those once the two pulls are both merged.

}
if !isNestedColumn {
// There is no way in Postgres to check if a column exists on a table.
return "", nil, fmt.Errorf("$exists operator not supported on non-nested jsonb columns")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Alternatively we can always return TRUE if it's not a nested column (or this could be an option).

Copy link
Member Author

Choose a reason for hiding this comment

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

But that's not true. The column can also not exist. The only way to actually check is selecting from information_schema.columns which seems dangerous and not possible since we don't know the table name.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's right, good point. 👍

koenbollen
koenbollen previously approved these changes Jun 10, 2024
Base automatically changed from more-test-data to main June 15, 2024 08:37
@erikdubbelboer erikdubbelboer dismissed koenbollen’s stale review June 15, 2024 08:37

The base branch was changed.

@erikdubbelboer erikdubbelboer merged commit 884c17f into main Jun 15, 2024
5 checks passed
@erikdubbelboer erikdubbelboer deleted the exists branch June 15, 2024 09:24
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