search

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 72 forks source link

Fix issues with selecting fields in the content API #183

Closed ryanmitchell closed 11 months ago

ryanmitchell commented 1 year ago

I don't love this approach, but it seems like it might be the only thing that works. Switch to always getting all data from the database, then limiting whats returned by selectedQueryFields in the Entry class.

Fixes https://github.com/statamic/eloquent-driver/issues/26

ryanmitchell commented 1 year ago

@jasonvarga could you sanity check this when you have some time. I really don’t like getting all the data every query but I feel like there may be no other option.

jasonvarga commented 11 months ago

What about if $columns is provided, we ensure that collection is added onto the list?

ryanmitchell commented 11 months ago

Yep that could work.

Taking a step back - can you see a reason why we shouldn't just get the full row each time? Getting a full row each time closes https://github.com/statamic/eloquent-driver/issues/96 and also prevents duplicate queries (eg on a normal page load we get id,uri for routing, then a seperate query for * for the data)

jasonvarga commented 11 months ago

can you see a reason why we shouldn't just get the full row each time

Performance, I guess. The same reason you don't just use select * all the time.

As for #96, we could add origin_id along with collection. Clearly collection isn't the only required field.

ryanmitchell commented 11 months ago

As for https://github.com/statamic/eloquent-driver/issues/96, we could add origin_id along with collection. Clearly collection isn't the only required field.

I think you'd get the same error on slug or collection or whatever other field we didn't retrieve. I think that issue just can't be fixed without getting all the row data.

I'll refactor this to always include collection - I agree re: performance.

jasonvarga commented 11 months ago

Sorry to be clear, initially I suggested "enforce the collection column is selected". What I really mean is that we need "enforce a list of columns".

If you do $query->get('title'), we should merge on collection, origin_id, maybe slug, etc.

jasonvarga commented 11 months ago

Er wait I see what you're saying. The rest of the custom fields are all stored inside data. I forgot it worked that way! The important stuff is all dedicated columns. Ok I guess we just get them all 🤷

ryanmitchell commented 11 months ago

Glad it has you going round in circles too! I cant decide whats best either.