Make use of new flexibility for Views field handlers

There is already a handler for date fields – if someone wanted to improve it, and the improvements are not for their specific use case, it can of course go into the Search API project. The same goes for other general improvements for the standard types.
Things like location handling, on the other hand, which aren't part of the standard types, should go into the other module.

That's a good question (although unrelated to this issue). As the Date module probably wouldn't want it, either: probably in a third module, that could maybe be kind of a collection of such extensions.

Thanks, that comes in handy! Seems to be fine, too, I'll use it in my eventual patch.
And I can't blame you for not diving into the advanced rendering stuff – it's really a mess, and I fear having to deal with it, myself.

First off: The patch doesn't correctly extract the score.
Then, I don't really see the benefit of this approach versus storing the field information in the "entity" key. That's the way "returning properties in the search result" is currently supported across the Search API, and this would already be possible without the need for another patch. And it doesn't touch the database either. So, please an explanation why this would be "incredibly useful" and can't be already done.
Also, even if we should allow that, I don't think the patch does this the right way. As I see it, the fields are still all extracted and take precedence over the data returned in the search results? Shouldn't only the fields not already set be extracted, and then only be used if the property wasn't set directly? Otherwise, this would require dedicated handlers that check themselves whether the data is available directly on the results instead of using get_value(). Or am I misreading the code?

Ah, yes, that does make sense.
I'll keep it in mind when doing this.

Also should keep this one in mind: #1135516: "Hide if empty" doesn't work with field rewrite.

Oh, nice! Thanks for spotting this!

In my opinion, we should just use all field values that are provided in $result['fields] and only use the entity and wrapper to extract those values that aren't present there. If this isn't, what you meant (not sure). Not having to implement get_value() is especially important since after #1077148: add an entity-view row plugin lands I might even remove my own field handlers in favor of those, so there wouldn't be the possibility to implement special logic there.

But thanks for reminding me, have to document this …

As far as I can see, this would still load all entities and extract all their fields, even if all necessary fields are set in the search results.

Good point. Here's an updated patch with a check for whether all the requested fields have been returned in the search result; it will only attempt to extract missing fields.

Thanks, but this still loads all entities. You should first check which fields are provided for which result, and then only load those entities that don't have all fields set.

Also: this could handle the special case added in #1147466: Provide excerpt in Search pages and Views, at least for solr, if 'search_api_excerpt' were mapped to 'excerpt' in the service's result handling.

That doesn't really help, as we would still need to handle it for all other backends and exceprt sources. Apart from Solr not returning an "excerpt" field.

I noticed that the 'excerpt' key isn't documented as part of the search result array; perhaps it should be?

It is, you must have overlooked it.

Ah, you're right of course. That would mess up the "Entity view" row style, and maybe other things too.
Sorry, my mistake. We should of course load the entities in any case. The gain from not extracting all the properties should probably still be noticable, though.

My plan was to wait for #1077148: add an entity-view row plugin to land and then see how we could use those field handlers.
However, maybe we can commit the existing patch beforehand and then retouch it after the other patch has landed. I just have especially little time this week, so didn't get round to reviewing the patch again to see if it could already be commited.
Otherwise, I'd suggest your help would probably be most appreciated in the other issue, which seems a bit stuck. How urgent is this issue for you?

I've talked to fago, and he thinks about not requiring query backends to put the entity in a certain place, but just introducing a method on the query class to return the entity. This would allow us to skip loading the entities when otherwise unnecessary and just store their IDs.

So, if this isn't urgent for you either, let's just wait for this to clear up and then do the whole revision at once.
But apart from this, your patch would be good to go, as far as I can see.

OK, latest development: This has now become urgent for me, so gonna commit this soon.

However, as I see it, now that we'll probably get an extra method for retrieving the entities, we can avoid loading them, unless necessary. The attached patch should do this. (Already tested with various combinations of "fields" and "entity" values.) It only breaks compatibility with something that wasn't committed yet – and probably won't be, either.
Also, it re-adds some fields that I just realized were missing in your patch (excerpt and score).

Yes, that's not optimal, but as you say, it's only provisional. Hopefully, we can soon change that to a unified system across all backends.

Committed the patch, but leaving active as there is of course still some work to be done.

Moving discussion over from #1154116-5: Search API Solr retrieving search results data directly from SOLR, avoid going through MySQL:

@drunken monkey: perhaps the 'retrieve_data' flag from your patch in #4 should be used to trigger the new Search API Views behavior?

No – we can hardly make the Views integration depend on such a backend-specific setting.

The rest is a valid point, though, we should really think about fixing this.
On the other hand, extracting the data out of the entities still takes some time and we have to assume that people who want to load the data directly from the server know what they are doing, and what they want.
Additionally, the "fields" key is specified as containing extracted field data, not necessarily from the search server. So we can't really assume it in the Views integration, even if this is at the moment the case for the only (known) usage of this key.
In the end, I think we have to assume that if there are fields returned along with the results, this is what the user (i.e., site admin) wants to be used as the field data.
It might even be that the field data returned is already formatted in some way the user wishes, so replacing it with fresh entity data is really undesirable. It could also impede integration of non-entity searches later, which the "fields" key seems to support quite well.

Yes, thanks. Now I just have to find the time, in September. The plan has actually expanded quite a bit, now I'm looking at implementing entity base tables in the Entity API, along with the field handlers, and then use that to implement field handlers and relationships (#1231512: Use real Relationships instead of level magic in Views integration) in the Search API.
So, quite some work still to do—and before the 1.0 stable release, too. :-/

I think, the new field handlers would be generic enough to live in the Entity API. I created an issue there with my (half-baked) plans: #1266036: Add generic Views entity tables with fields and relationships.

After that is done, it's just a matter of using this with the Search API, maybe extending field handlers where necessary, and even maybe-er writing an update function for this stuff.

Even though this issue was here first, all work is now happening over in #1231512: Use real Relationships instead of level magic in Views integration, so finally marking this one as a duplicate to avoid confusion.