loopbackio/loopback-connector-postgresql

eq object (JSON columns) doesn't work

mgabeler-lee-6rs opened this issue · 6 comments

Steps to reproduce

  1. Create a model that has an object column, mapped as a JSON type in PostgreSQL
  2. Try to do a find searching for a full value in that property/column, e.g. const objectValue = {a: 1, b: 2}; and then repo.find({where: {objectProperty: objectValue}}) or repo.find({where: {objectProperty: {eq: objectValue}}})

Current Behavior

  1. The {objectProperty: {eq: value}} is translated down into {objectProperty: value}
  2. This line assumes that, if the value is an object, it must contain exactly one field that must be an operator: https://github.com/strongloop/loopback-connector-postgresql/blob/master/lib/postgresql.js#L654
  3. It tries to map e.g. a as an operator name
  4. The buildExpression operator switch hits its default clause which delegates to the base class in loopback-connector: https://github.com/strongloop/loopback-connector-postgresql/blob/master/lib/postgresql.js#L540-L543
  5. That base class method has a switch with no default clause, so it doesn't throw any errors and just concatenates the column name with the placeholder for the value: https://github.com/strongloop/loopback-connector/blob/master/lib/sql.js#L969
  6. And so it generates invalid SQL that looks like "columName"$1

Expected Behavior

  • I should be able to use object values in where clauses if the property contains object values

Link to reproduction sandbox

WIP -- NB: encountering this in an LB4 app

Additional information

  • Running on linux x64 12.22.1
  • npm ls doesn't work with rush, but using loopback-connector-postgresql v5.0.1, with loopback-connector v4.11.1, and the following LB4 components:
  • "@loopback/boot": "2.2.0"
  • "@loopback/context": "3.9.3"
  • "@loopback/core": "2.5.0"
  • "@loopback/metadata": "2.2.6"
  • "@loopback/openapi-v3": "3.3.1"
  • "@loopback/openapi-v3-types": "1.2.1"
  • "@loopback/repository": "2.4.0"
  • "@loopback/rest": "4.0.0"
  • "@loopback/rest-explorer": "2.2.0"

Related Issues

Haven't found any yet

Workaround

Create a custom class to represent the value, and then have the equality comparison value use that, e.g. something like this, but without the prototype pollution vulnerabilities:

class JSONWrapper {
  [k: string]: any
  constructor(value: any) {
    Object.assign(this, value)
  }
}

// elsewhere:
repo.find({where: {objectProperty: new JSONWrapper(objectValue)}});

This causes the expression.constructor === Object check to fail, and so it doesn't try to unwrap the value

stale commented

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.

I'm not gonna fight with stale-bot ... having something like that activate without the maintainers even visibly acknowledging the issue just encourages switching to a better supported framework :(

stale commented

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.

@dhmlau @bajtos any chance someone can look at this? having stale-bot auto-close things without the maintainers even seeming to look at them doesn't give your users good feels.

Hi @mgabeler-lee-6rs, apologies for not attending to the issue earlier. I've added this to my pipeline for review and will try to see if I can timebox this.

Currently, there's quite a bit of backlog at my end so unfortunately I may not be able to work on a fix asap; Though I'm open to merging community contributions.

stale commented

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.