Skip to content
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

Seems like OR logic is messed up, wrong SQL is generated #895

Closed
thehappycoder opened this issue Feb 22, 2021 · 8 comments
Closed

Seems like OR logic is messed up, wrong SQL is generated #895

thehappycoder opened this issue Feb 22, 2021 · 8 comments
Labels
bug Something isn't working

Comments

@thehappycoder
Copy link
Contributor

Is it a known issue (hard to search something like that)? If not, I'll post more info.

Basically, SQL is generated with a OR b not enclosed in (), and when I add more filters with AND, this is what I get: a OR b AND c (in pseudocode) but should get (a OR b) AND c

@thehappycoder thehappycoder added the bug Something isn't working label Feb 22, 2021
@doug-martin
Copy link
Owner

@thehappycoder I'm not aware of this issue, if you could provide some more information that could help me track it down. If you could provide a complete example, along with info on the packages you are using I would appreciate it!

Thanks for reporting!

@thehappycoder
Copy link
Contributor Author

@doug-martin False alarm but maybe a good indicator that documentation should mention this gotcha. To "fix" this, I had to wrap "or" inside "and":

    "status":{"eq":"SUBMITTED"},
    "and": {
      "or": [
        {"prefColor":{"iLike":"%no preference%"}},
        {"prefColor":{"iLike":"%no real preference%"}},
        {"prefColor":{"iLike":"%no strong preference%"}},
        {"prefColor":{"iLike":"%any%"}},
        {"prefColor":{"iLike":"%open%"}}
      ]
    }
  }

@thehappycoder
Copy link
Contributor Author

I probably assumed that AND and OR logic worked like in Prisma, where wrapping or inside and isn't needed as far as I remember (https://www.prisma.io/docs/reference/api-reference/prisma-client-reference/#or), so I got hit by this gotcha.

@doug-martin
Copy link
Owner

@thehappycoder you are right it shouldn't be needed. Which persistence package are you using? Each one is different based on what's required to use the library.

@thehappycoder
Copy link
Contributor Author

@doug-martin I am using typeorm

doug-martin added a commit that referenced this issue Feb 26, 2021
* This ensures that groupings are correct
doug-martin added a commit that referenced this issue Feb 26, 2021
* This ensures that groupings are correct
@doug-martin
Copy link
Owner

@thehappycoder this has been addressed in v0.23.1. Thank you for reporting!

@vladmelnyk
Copy link

@doug-martin seems like v0.23.1 hasn't been published yet on npm
https://www.npmjs.com/package/@nestjs-query/core

@doug-martin
Copy link
Owner

@vladmelnyk it was just a patch for @nestjs-query/query-typeorm.

Let me know if that works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants