loopbackio / loopback-connector-postgresql

PostgreSQL connector for LoopBack.
Other
117 stars 180 forks source link

neq in where filter does not include null values #340

Open hvlawren opened 6 years ago

hvlawren commented 6 years ago

Description/Steps to reproduce

  1. Create a model with String properties
  2. Insert instances with one of the string properties unset. (NULL in DB)
  3. Run a query for model {where: {stringProp: {neq: "value"}}}
  4. NULL entries are not included

Expected result

Null entries are not equal to value so should be included

Additional information

I notice that the output sql from the connector is !=, whereas IS DISTINCT FROM would work for NULL values. As far as I can tell, there is no operator that will invoke IS DISTINCT FROM. If this won't be fixed/is considered not an issue, having this at least documented somewhere would be useful.

bra1n commented 6 years ago

Adding to this, NULL values seem to be generally handled poorly in this module. Something else that should work, but does not: {where: {booleanProp: {inq: [null, false]}} will not return "unset" (= null) rows, but only false rows. Or, even simpler: {where: {booleanProp: {neq: true}} should turn into WHERE booleanprop IS NOT true but actually turns into WHERE booleanprop != true, which again excludes null values.

mightytyphoon commented 5 years ago

Adding to this, NULL values seem to be generally handled poorly in this module. Something else that should work, but does not: {where: {booleanProp: {inq: [null, false]}} will not return "unset" (= null) rows, but only false rows. Or, even simpler: {where: {booleanProp: {neq: true}} should turn into WHERE booleanprop IS NOT true but actually turns into WHERE booleanprop != true, which again excludes null values.

oh thanks I thought I was becoming crazy. Also ilike, like etc... doesn't work properly. Same if you give a or / and arrays.

I think a big part of this module needs to be corrected.

gopalakrishnan-subramani commented 5 years ago

Any solution for the same issue, we need this feature critically to maintain soft delete with deleted_at timestamp column, can you please confirm is this issue from pg node.js driver or loopback Postgresql driver, we can offer some help with PR instead of making work around for our larger solution.

When the attribute is present in the JavaScript object with null, it is evident that the driver should send query with null

bajtos commented 5 years ago

Here is the relevant code:

I suspect the problem is in PostgreSQL.prototype._buildWhere, it seems to handle only the following operators:

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

bra1n commented 5 years ago

not stale

bajtos commented 5 years ago

Thank you all for participating in the discussion. I am afraid our bandwidth is limited and this issue is unlikely to make it to our milestone backlog. We rely on you, the community of users, to contribute the changes necessary to fix the issues described above.

I am proposing the following approach:

This way we can preserve backwards compatibility for existing users and get more time to fix any edge cases that may be discovered when the new implementation is put into real-world use. Once we are confident about the new implementation, we can make the feature flag enabled by default and release a new semver-major version.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

zardilior commented 4 years ago

Yes this is still relevant and happening, I think that either a note should be added to the docs about a workaround or some attention should be given to it!

bajtos commented 4 years ago

I think that either a note should be added to the docs about a workaround or some attention should be given to it!

@zardilior could you please contribute that attention yourself and open a pull request with the necessary changes?

malek0512 commented 3 years ago

Hello @everyone, any news on this issue ?

natuan62 commented 3 years ago

same problem with me !!

JakobClausen commented 2 years ago

Is this still an issue? Can get this to work still with postgres.