How do I put sequelize.literal to .get() or .patch() call query?

[1valdis]

I want to put a sequelize.literal query under Op.and symbol before get and patch method to attach an EXISTS query. But at the same time, seems like because of the Object.assign there (duplicated below), the id is lost from the query and only my sequelize.literal remains.

where: Object.assign({
        [this.Op.and]: { [this.id]: id }
      }, where)

In patch method it works vice-versa: my [Op.and] gets replaced with id query. This isn't right because I want to filter what is getting patched.

Is there any way to accomplish including sequelize.literal in a way that works across methods without too much mess (or in case of patch, accomplish that at all)?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Apologies if the issue could not be resolved. FeathersJS ecosystem modules are community maintained so there may be a chance that there isn't anybody available to address the issue at the moment. For other ways to get help see here.

[1valdis]

The issue wasn't solved yet. I want to see how to do this without awful workarounds.

[DaddyWarbucks]

@1valdis Can you post and example of the query that you are trying to use?

[1valdis]

@DaddyWarbucks It was pretty long ago, and I couldn't find the related code in my repos. If I remember correctly though, it was something as such. In before get/patch hook I had this line of code:

context.query[Op.and] = sequelize.literal('EXISTS ...');

I expect it to filter by literal as well as by the context.id.

For one of the listed methods (get, patch), this line of code resulted to query filtering only by literal, without taking context.id into account; for the other method in turn, this resulted in filtering only by context.id, without taking literal into account.

This was incredibly confusing and I resorted to setting context.id to null for both methods and manually assigning id to context.query.

[DaddyWarbucks]

OK. I think I see why this is happening. You are assigning the whole object of context.query[Op.and] to be a literal, essentially opting out of the object query syntax.

The sequelize.literal returns a Literal instance, which looks something like Literal { val: 'EXISTS ...' } and is a way to tell sequelize not to parse and escape that part of the query. By assigning this to the whole $and query you are telling sequelize that you are taking care of the whole and query. Subsequently when that tries to get merged with the ID, it is part object syntax and part Literal...which is likely what lead to your different results.

There are a couple of ways to get what you want, but you will need to play in Sequelize's playground more. For example, using includes, belongsTo, etc instead of relying on the the literal escape hatch.

Something like this should work

const byExists = context => {
  const seqOptions = context.params.sequelize || {};
  seqOptions.include = [
    {
      model: otherModal,
      as: 'otherThing',
      where: { $and: [sequelize.literal('EXISTS ...')] }
    }
  ];
  context.params.sequelize = seqOptions;
};

[1valdis]

assigning the whole object of context.query[Op.and] to be a literal, essentially opting out of the object query syntax

@DaddyWarbucks how so? A query of such form:

{
  id: 1,
  [Op.and]: sequelize.literal('EXISTS ...')
}

If passed to Sequelize, would produce such SQL:
SELECT * FROM ... WHERE id=1 AND (EXISTS ...)
This is what I was expecting to happen, given that context.id would be assigned to context.query.id.

[DaddyWarbucks]

Right, but that is not what

feathers-sequelize/lib/index.js

Line 172 in 208cdfb

where: Object.assign({

is doing.

// You have assigned $and to be your literal. Not an object like `{ $and: { name: 'Bob' } }`
const where = { $and: sequelize.literal('EXISTS ...') };

// So now $and is an instance of a Literal and is no longer a plain old javascript object.
where === { $and: Literal{ val: 'EXISTS...'  } }. // Literal is a class instance

// Then the code you linked (below)  tries to merge a plain old JS object 
// and a class instance for the $and. So I think that is the root of the problem
Object.assign({
   [this.Op.and]: { [this.id]: id }
}, where)

// That is what I meant about having assigned "the whole $and to be a literal".
// Sequelize doesn't know what "val" is for because it was originally meant
// to be for the the entire $and query, not it merged with something else.
where = {  $and: { id: 123, val: 'EXISTS...' }  }

I did a little googling and it seems like most examples of using the Literal it is assigned to $and. That makes sense because you don't have to define the property in the query and you can use a generic literal to do whatever you want. I also noticed I do the same in one of my codebases similar to the byExists hook in my previous comment.

It seems like id could be merged right onto the query like your code example above. But I can also see a number of good reasons it is on the $and too. I don't think it's a good idea to overwrite the id property directly because it will step on the users' toes more. For example I often use things like { id: { $in: [...] } }

In regards to your comments about it behaving differently for get and patch, I know the code has been updated since your original comment such that the two behave more similarly. Note sure if that fixes the issue, but worth noting.

I agree this is confusing and worth some documentation. Examples of using the literal generally show it on the $and, even this sequelize/sequelize#9410 (comment) comment. Some docs telling the user why this is happening and an example hook seems like a good solution.

Would you mind sharing your hook that you used?

[1valdis]

@DaddyWarbucks Unless you have some kind of modified Object.assign in the codebase, I don't think it "tries to merge a plain old JS object and a class instance for the $and". Object.assign does not merge values of keys of two objects; it simply replaces values of first object with values of the other object.

// doing that
Object.assign({
   [this.Op.and]: { [this.id]: id }
}, {
   [this.Op.and]: Literal{ val: 'EXISTS...'  }
})
// will result in
{
   [this.Op.and]: Literal{ val: 'EXISTS...'  }
}

That's what I meant by phrases "id is lost from the query" or "id is overwritten".

I appreciate that you understand that this usage of assigning Literal (or, generally speaking, anything) to [Op.and], while recommended and mentioned by many, was at the time of writing giving such unpredictable results at first.

Considering code update after the issue was raised: I'm now using, and back then was using [email protected]. Has the situation changed considerably since then for me to try something similar on the newest 6.2.0?

Would you mind sharing your hook that you used?

I will try to find it later today but no guarantee. If I can't, I'll try to come up with some example.

[DaddyWarbucks]

You are totally right on the Object.assign. I got lost in the trees on that one. Sorry about that.
I am not a maintainer or the original author BTW. I have made a couple contributions and know the code base well and try to help out where I can. But, yea, you are totally right about the Object.assign. I misread the code. I think your example of

{
  id: 1,
  [Op.and]: sequelize.literal('EXISTS ...')
}

threw me off a bit that you were thinking that was what was happening and I was trying to explain thats not what was happening...by wrongly explaining what was also not happening haha. Sorry.

But, at the end of the day, feathers-seq has to assign the id somewhere (for that feature to work) and it does that on the $and (aka [Op.and]) and there are some good reasons it is assigning it there.

[DaddyWarbucks]

I am not quit sure if/how feathers-seq can handle this any better. My comments up to this point I have not tried to reproduce the issue and have been speculation. I will spin up the repo and throw some code at it tomorrow.

Regarding your question about the version bump, I made some changes to how update/patch work to where they work more similarly to get (that was not the goal, but was a result of the changes). I doubt it made any noticeable change for this issue though.

[1valdis]

There are two simple use cases which I found.

First. I have my own (currently logged in) User and another User. They both have some Group (many-to-many). I want to retrieve and patch a User by id, but only if he has the same group which I have. So I use a hook:

(context) => {
  context.params.query[Op.and] = literal(`exists (select * from groups where ...)`);
}

Naturally I'd expect that, if even such User exists by id, but doesn't belong to my Group, this will throw me beautiful standard 404 if I try to get or patch it. But because of the reasons above (either id or literal get lost), to make it work somewhat reliably for both methods, I have to mess with context.sequelize.

Second. I have an Event, which I want to be able to again retrieve and patch by id, but only if it's due to a date which is not further in the past than 1 day. So I'd have such hook:

(context) => {
  context.params.query[Op.and] = literal("date >= now() - interval '1 day'");
}

Same thing as with above, either my literal or context.id would be lost.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Apologies if the issue could not be resolved. FeathersJS ecosystem modules are community maintained so there may be a chance that there isn't anybody available to address the issue at the moment. For other ways to get help see here.

[1valdis]

Not resolved yet (unless I missed something).

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Apologies if the issue could not be resolved. FeathersJS ecosystem modules are community maintained so there may be a chance that there isn't anybody available to address the issue at the moment. For other ways to get help see here.

[DaddyWarbucks]

I just stumbled across this again and have another idea on the problem. Sequelize operators support all kinds of values for operators. For example,

// POJO
$and: { prop: 'value' }

// array
$and: [{ prop: 'value' }]

// literal
$and: literal(`exists (select * from groups where ...)`)

And I think that is the core of the problem. And also what lead to some of my comments above about not being able to "merge" a literal and a POJO. feathers-sequelize needs to "merge" it's id query with the user's $and query instead of replacing it, but that is difficult without knowing the type.

Perhaps something like this would work

const applyIdQuery = ($and, idQuery) => {
  if (!$and) {
    return idQuery;
  }

  if (Array.isArray($and)) {
    return [...$and, idQuery];
  }

  return [$and, idQuery];
};

where: Object.assign({
  [this.Op.and]: applyIdQuery(where[this.Op.and], { [this.id]: id })
}, where)

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Apologies if the issue could not be resolved. FeathersJS ecosystem modules are community maintained so there may be a chance that there isn't anybody available to address the issue at the moment. For other ways to get help see here.