Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sorting files on filesize in app sorts alphabetically #7677

Closed
3 tasks done
MoltenCoffee opened this issue Aug 29, 2021 · 8 comments
Closed
3 tasks done

Sorting files on filesize in app sorts alphabetically #7677

MoltenCoffee opened this issue Aug 29, 2021 · 8 comments
Labels

Comments

@MoltenCoffee
Copy link
Contributor

Preflight Checklist

Describe the Bug

In the admin app > file library, when sorting files on file size, files are sorted alphabetically.

This does not seem an issue when visiting the library from the menu, but it does when changing the sorting direction.

To Reproduce

  1. Upload several files of various sizes
  2. Go to the file library
  3. Change sorting direction

What version of Directus are you using?

v9.0.0-rc.91

What version of Node.js are you using?

16.8.0

What database are you using?

Postgres 14

What browser are you using?

Chrome 92

What operating system are you using?

Windows 10

How are you deploying Directus?

Node app on server

@Nitwel
Copy link
Member

Nitwel commented Aug 30, 2021

Seems to be working for me. Could you provide more information on how to reproduce this.
If possible, could you provide a database dump as it might help reproduce this issue.

@rijkvanzanten
Copy link
Member

@Nitwel IIRC bigIntegers are returned as a string (postgres is one that does it for sure), as they can theoretically hold bigger numbers than JS itself:

9,223,372,036,854,775,807 (Postgres)
    9,007,199,254,740,992 (JavaScript)

When sorting datasets that fit on the page, we sort on the client side for optimized performance. However, that sorts on the string value, which makes it alphabetical.

This matches the reported

This does not seem an issue when visiting the library from the menu, but it does when changing the sorting direction.

as that first load is done with a sort query in the API request, and therefore sorts on the database level.

@rijkvanzanten
Copy link
Member

With that in mind, the question becomes: Is that something we can fix in the first place? We can't simply convert the value to a number first and then sort, as that can crash javascript altogether. One possible solution is to drop the client-side sorting altogether in favor of always using the API, but that's a bit of a shame as it makes sorting way slower for small data sets 🤔

@nickrum
Copy link
Member

nickrum commented Aug 30, 2021

Could we maybe use Javascript's BigInt to solve this?

@rijkvanzanten
Copy link
Member

Could we maybe use Javascript's BigInt to solve this?

Yeah, though I'm wondering how that interop works with JSON parse/stringify 🤔

Also, curiously enough, my Node runtime/Safari seem to handle massive int's just fine, so I'm wondering if BigInt is just an implementation detail under the hood we don't even have to think about?

CleanShot 2021-08-30 at 11 03 14@2x

CleanShot 2021-08-30 at 11 03 43@2x

@rijkvanzanten
Copy link
Member

Maybe the solution is as simple as:

UPDATE-2

Version 9.3.0 of the library added support for the native BigInt type, which now you can use, if you are running Node.js v10.4.0 or later.

To make the driver automatically use BigInt for BIGINT + BIGSERIAL:

pgp.pg.types.setTypeParser(20, BigInt); // Type Id 20 = BIGINT | BIGSERIAL
For more details, see BigInt Manual in the project's WiKi.

https://stackoverflow.com/a/39176670/4859211

@nickrum
Copy link
Member

nickrum commented Aug 30, 2021

Yeah, though I'm wondering how that interop works with JSON parse/stringify 🤔

Good point! Seems like JSON doesn't support BigInt. There are workarounds like converting to a string and back.

Also, curiously enough, my Node runtime/Safari seem to handle massive int's just fine, so I'm wondering if BigInt is just an implementation detail under the hood we don't even have to think about?

Massive int's will work but they will loose precision (as any number in js is just a float). In your screenshots, the number litteral in code and the printed number aren't equivalent. You have to append an n to the number to use a BigInt explicitly.

Also, BigInts are ES2020, so we might be getting a little bit ahead of ourselves.

@joselcvarela
Copy link
Member

Hello all,

I believe this is fixed in #8570

More specifically on this chunk https://github.com/directus/directus/pull/8570/files#diff-9898b066ab280a7cd156ae0d26bea7373f99c17d512e2a20d9899fe8c57ed907R70-R91

Basically the solution was to do the sort on server side.

If you think this is not solved yet, feel free to reach us in order to reopen.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants