broken fix: count just the right number of row #304

[artur-krueger]

This line leads to a broken behavior. https://github.com/moleculerjs/moleculer-db/pull/304/files#diff-35fc753ffb17451ff92b1bad2bff715aaa5ff65f4e08b7cce5290fe4e5003d4aR305

If includes are defined in the model, then this behavior is done automatically correctly by sequelize itself.

This is the relevant code from sequelize model.js, and you can see it's doing the settimg already automatically:

  static async count(options) {
    ...
    if (options.include) {
      col = `${this.name}.${options.col || this.primaryKeyField}`;
    }
    ...

By setting the col field in the options to 'tablename.primaryColumn', the resulting col will be composed to by sequelize to "tablename.tablename.primaryColumn" and later converted to 'tablename->tablename.primaryColumn'. The result is a broken sql query.

The right behavior is to set just the column name:

const strictCountRule = { distinct: true, col: `${this.model.primaryKeyAttribute}` };

or even better to skip this at all and let sequelize do it right.

[icebob]

@DenisFerrero could you agree on it?

[DenisFerrero]

Yeah, I'm aware of that. But when using the defaultScope the includes, in some way, are ignored and create a wrong SQL query for the count. The start of the query will be something like this:

SELECT COUNT("id")

and not as you expected

SELECT COUNT("table->id")

Just tested in my local environment by using your pattern.

The Sequelize adapter doesn't allow at the moment to decide which scope to call for the request, so the broken behavior that you raised should not occur yet.

Anyway.... thanks for the tip, I'll open an issue on the Sequelize repo, once it's fixed there I'll remove this part to make sure that in case the scopes selection would be implemented this feature won't broke everything

[DenisFerrero]

As I promised: https://github.com/sequelize/sequelize/issues/14816. I'll keep you updated :see_no_evil: