Migration to `@forestadmin/agent` - Records count is broken

[Matteo]

We’re migrating from forest-express-sequelize to @forestadmin/agent .

Records count is broken. When loading a collection in the list view, I should see the total counts of records as well as the number of pages.

This is currently broken, as the count query throws an error and this is logged.

To fix this we had to patch the broken module on your side (@forestadmin/datasource-sequelize). Here is the diff for /dist/utils/aggregation.js:

This way, it works:

Btw, you already know this problem, since there is a FIXME comment…
Please, fix it

Thank you.

Here is the “meta” section of the forest admin schema file:

  {
    "liana": "forest-nodejs-agent",
    "liana_version": "1.41.7",
    "liana_features": null,
    "stack": {"engine": "nodejs", "engine_version": "20.11.1"}
  }

and from package.json:

"sequelize": "^6.28.0"

cc @julien.jolles

[Nicolas.M]

Hello @Matteo
Thanks for your report.
Does this issue happen on all your collections ? Do you have anything particular about your SQL schema that could cause this ?
Do you encounter issues with the list query as well ? list and count should be very similar queries.
In order to help us reproduce and investigate this issue, could you please share the query that is generated by sequelize / SQL to perform the count (in private message if you prefer)?
Thanks,

@Nicolas.M

[Matteo]

Thank you @Nicolas.M

Answers:

Does this issue happen on all your collections ?

Yes

Do you have anything particular about your SQL schema that could cause this ?

No

Do you encounter issues with the list query as well ?

No

list and count should be very similar queries.

I understand, but in the count (in your code) you call the getOrder which is not involved in list (where you call getOrderFromSort).
This is a slight but important difference

This is your code for the list operation:

And this is your code form the count operation:

In order to help us reproduce and investigate this issue, could you please share the query that is generated by sequelize / SQL to perform the count (in private message if you prefer)?

This is the query (sequelize) you are generating. We got it by inserting a console.log here in your code, to see the query you’re generating:

NB: the above code comes from this file (on your side): forestadmin/datasource-sequelize/dist/collection.js

{
  "attributes": [
    [
      {
        "fn": "COUNT",
        "args": [
          {
            "col": "*"
          }
        ]
      },
      "__aggregate__"
    ]
  ],
  "where": {},
  "include": [],
  "order": [
    [
      {
        "col": "__aggregate__"
      },
      "DESC NULLS LAST"
    ]
  ],
  "subQuery": false,
  "raw": true
}

As you can see the "col": "__aggregate__" is obviously wrong !

In the sequelize lib thay state that col can be used only for DB-existing columns (not aliases). But youre using col on an alias.

@Nicolas.M
as I told in my first message in you file @forestadmin/datasource-sequelize/src/utils/aggregation.ts you already noticed that you have something wrong about the count. Here you have a “FIXME” comment!!!
And we tried to fix this for you, showing you the patch we’re suggesting (see my first message).

Please check our patch and fix this consistently with your lib

Thank you in advance
Matteo

[Nicolas.M]

Thanks for your detailed reply.
Can you please tell me what is your postgre version ?

[jeffladiray]

As discussed, this is super strange, as I’m able to make the exact same query as you without any issue (Pg14, Sequelize 6).

__aggregate__ should correctly be a part of the query result, so that’s surprising that Sequelize isn’t happy about it.

The discrepency of the mysql2@pg_hstore in the stack trace feels weird to me - especially because of the // FIXME you mentionned - because we would fall in 2 differents cases depending on the database driver that is actually used.

On my end, I currently don’t see anything specific updating this.col(this.aggregateFieldName) to this.aggregateFieldName, but we really want to make sure that this doesn’t break anything from our compatibility list of supported DBS.

I’ll check the fix against our test cases & general array of supported DBS, and I’ll get back to you once done.
If possible, in the meantime, I would be curious to get more informations about this mysql2@pg_hstore stacktrace, so if you find anything regarding this point, feel free to reach out

[Matteo]

Thank you @jeffladiray
Actually we have mysql drivers because in the past we interacted also with a second mysql DB. Now we’re not doing this any more, but the mysql lib was still in our code base, even if it was currently unused.

Our only DB is Postgresql.
And now we removed this dependency to remove any reasonable doubt.

But the problem is still there

Here is the updated stack trace, without any mysql lib possible pollution:

We also managed to deeper debug in order to see the SQL generated query, by inserting a console.log directly in the Sequelize code (in the pointed out file in the above stack trace screenshot), and here is the query:

…and in text (more readable):

SELECT COUNT(*) AS "_0" 
FROM "billing_profile" AS "BillingProfile" 
ORDER BY "__aggregate__" DESC NULLS LAST;

Can’t figure out what’s wrong but ORDER BY "__aggregate__" is oviously wrong.

Now we tried fiddling with all the options in our Sequelize setup to see if anything unusual came up…
…and we actually found that the Sequelize option minifyAliases that we use (to manage aliases longer than 64 char, that Postgres does not manage) is the guilty one.

If we remove this option, we have no problems here, like you.

But we can’t remove this option all over the code base, as Postgres has aggressive limits on identifier lenght:

(from Postgresql docs: PostgreSQL: Documentation: 16: Appendix K. PostgreSQL Limits)

That’s why you didn’t have the problem, but we still need to minify the Sequelize aliases
I apologize for not realizing this until now.

Help

Thank you,
Matteo

[jeffladiray]

Nice, thank you so much for these debug informations.

I’m now able to reproduce the issue, I’ll open a bug report on our end right away, and we’ll let you know once fixed.

[Matteo]

Hi @jeffladiray
we just saw you released a new version which fixes this problem:

fix: records count supports sequelize alias minifying (#1163)

I can confirm now the count looks fine on our side.

Thank you very much for the fix
Matteo

[Nicolas.M]

Sorry, I forgot to report that the fix went live.
I’m glad your use case is solved for you now.

have a nice day

@Nicolas.M