statamic / eloquent-driver

Provides support for storing your Statamic data in a database, rather than flat files.
https://statamic.dev/tips/storing-content-in-a-database
MIT License
104 stars 71 forks source link

Sorting counts null as the string 'null' #217

Closed Smef closed 8 months ago

Smef commented 8 months ago

When sorting on a field with null values, null values are treated as the string "null" and fall in between words starting with the letter 'm' and 'p'. I've changed from the eloquent driver to the file driver and found that the file driver does not have this issue.

ryanmitchell commented 8 months ago

So for example if you ->orderBy('title') but one of the titles is null, or does it have to be a field stored inside the JSON data?

If you could give the result of php please support:details that would be super useful.

Smef commented 8 months ago

Sorry, this is through the API with a query parameter like &sort=state_specific and results in data like this

{
    "data": [
        {
            "id": 442,
            "slug": "georgia-k-12-standards-success-grade-3-mathematics",
            "title": "Georgia K-12 Standards Success Grade 3 Mathematics",
            "price_base": 39,
            "state_specific": "ga"
        },
        {
            "id": 297,
            "slug": "algebra-i-foundations",
            "title": "Algebra I Foundations",
            "price_base": 41.75,
            "state_specific": null
        },
        {
            "id": 495,
            "slug": "oklahoma-ostp-success-grade-7-mathematics",
            "title": "Oklahoma OSTP Success Grade 7 Mathematics",
            "price_base": 39,
            "state_specific": "ok"
        },
]
}
Environment
Application Name: ABC CMS
Laravel Version: 10.28.0
PHP Version: 8.1.17
Composer Version: 2.5.8
Environment: local
Debug Mode: ENABLED
URL: cms.abc.test
Maintenance Mode: OFF

Cache
Config: NOT CACHED
Events: NOT CACHED
Routes: NOT CACHED
Views: CACHED

Drivers
Broadcasting: log
Cache: statamic
Database: mysql
Logs: stack / single
Mail: smtp
Queue: sync
Scout: algolia
Session: database

Sentry
Enabled: MISSING DSN
Environment: local
Laravel SDK Version: 3.8.2
PHP SDK Version: 3.21.0
Release: NOT SET
Sample Rate Errors: 100%
Sample Rate Performance Monitoring: NOT SET
Sample Rate Profiling: NOT SET
Send Default PII: DISABLED

Statamic
Addons: 1
Antlers: regex
Stache Watcher: Enabled
Static Caching: Disabled
Version: 4.27.0 PRO

Statamic Addons
statamic/eloquent-driver: 2.7.0

Statamic Eloquent Driver
Asset Containers: file
Assets: file
Blueprints: file
Collection Trees: file
Collections: file
Entries: eloquent
Forms: eloquent
Global Sets: file
Global Variables: file
Navigation Trees: file
Navigations: file
Revisions: file
Taxonomies: file
Terms: eloquent
ryanmitchell commented 8 months ago

It probably wont make any difference, but can you update eloquent-driver to 2.10.0 just to be sure you're on the latest version?

I can see its data held in the json field so that gives me something to work with.

Smef commented 8 months ago

Yes, it's actually a stored computed field in the JSON, in this case.

Updating to 2.10 did not help, unfortunately.

ryanmitchell commented 8 months ago

I've taken a quick look but I'm not sure theres a whole lot we can do here as we're deferring to the underlying database engine for sorting, and MySQL is choosing to treat it as an 'n' rather than blank.

Obviously if youre using a computed field you could blank it rather than null, but that doesn't help when its not a computed field.

Open to any suggestions.

Smef commented 8 months ago

MySQL normally puts null first if you're sorting strings in ascending order, so that doesn't sound like it's MySQL doing it. Is this JSON field related? Is there a sort happening on the text value somewhere?

ryanmitchell commented 8 months ago

Yeah it's when sorting on the json field. I can replicate the same sort order with a database query.

Smef commented 8 months ago

It looks like this is actually a known issue. https://bugs.mysql.com/bug.php?id=85755

Smef commented 8 months ago

I guess we can close this, since it's a MySQL issue and not with this driver. Thanks for your help!

Smef commented 8 months ago

I think our alternative to this is to create a new column in the entries table which can hold this value so that we can sort on there. Is that supported by this package?

Smef commented 8 months ago

Putting in a new column in the entries table would make that column for all entry types, so it would be even better if we could have a dedicated table for this one collection. I'd be happy to write the migrations to have a separate table for this collection if that is possible. Is that possible to do, or must all entries be in the same table?

ryanmitchell commented 8 months ago

It's not possible to use a seperate table. I'd suggest taking a look at Runway if you want that.

Smef commented 8 months ago

Yeah, I checked that out, but unfortunately it doesn't support the content API, which we make significant use of.

ryanmitchell commented 8 months ago

Maybe make your own api controller for that resource that extends Statamic's API controller? Looking at the asset controller it seems pretty straight forward?

ryanmitchell commented 8 months ago

@Smef Keep an eye on https://github.com/duncanmcclean/runway/pull/356

Smef commented 8 months ago

Thanks!