Closed 4levels closed 6 years ago
@4levels Okay, the deeper I dig into this the clearer it is that the data needs to be restructured in order for this to work well. But again, this isn't a Lighthouse/GraphQL issue, this is a data structure issue. You're root problem is that you're trying to union two different tables w/ different data which isn't supported in Eloquent (or any ORM that I'm aware of).
If you were to union two different tables in SQL, you need to query for the same exact columns (totally unrelated to Eloquent). If that's the case, then you don't need a Relay connection with different types because they now they have the exact same fields from your query. You could run such a query and create a new GraphQL type with the combined fields, but I personally wouldn't go that route.
For example, let's say you have the following data types:
type Post {
title: String!
body: String!
}
type Video {
title: String!
url: String!
}
Since we have to query the same columns when doing a union
in SQL, your query would look something like the following:
(SELECT title, body as data FROM posts) UNION (SELECT title, url as data FROM videos);
Then we could do the following in GraphQL:
type Submittable {
title: String;
data: String;
}
However, this isn't the route I would go since you lose all your relationships and any other data that you might not be able to fit into a union
properly.
If you don't need pagination, then you could just do the following:
type Video implements Submittable {
# ...
}
type Post implements Submittable {
# ...
}
type Query {
submittable: [Submittable] @field(resolver: "MyResolver@submittable")
}
Then you're resolver would look something like this:
class MyResolver
{
public function submittable()
{
$videos = App\Video::all();
$posts = App\Post::all();
return $videos->merge($posts->all());
}
}
The only other two ways I can picture this working is here, and requires a new model/table or a single table for both models w/ a JSON column that holds the data that's unique for each type. But that would be the same regardless if you were working w/ GraphQL or event REST as you are attempting to paginate two different models w/ a single query. Either way, Lighthouse can handle both of those scenarios as I utilize both approaches in my project.
If you are able to create a single query w/ multiple models & unique columns (paginated) paste it here and we'll revisit this, but for now I'd strongly encourage the DB structure changes.
Hi @chrissm79,
thank you so much for your in depth explanation. Coming from a symfony/doctrine world, I guess these where the bits and pieces I couldn't figure out myself.
I'm quite familiar with hydration issues as Doctrine had quite some (performance) issues as well and I often had to fall back to using raw queries. Since I only started working with Lumen last year, I'm now wondering how the standard approach for Eloquent's union
would hydrate results (obviously without pagination - could be the reason why the 2 year old Eloquent query builder bug still exists). I'm going to give this a run tomorrowm as it's well over 2am over here.
I might consider refactoring the database, but as you've already guessed, there's a lot more going on with these models than one can see from my code snippets here and I'm pretty sure that I'll have to stick with the current separation, both in database and model land.
Did you find an explanation why the resolver in the @interface
directive never seems to be reached? Regardless of what records I return in a custom query, I'd expect at least the resolver would be hit once per found result. I'm still not 100% clear on how to correctly implement the @interface
directive (especially the Eloquent bits) as it seems like it's doing very similar things as @union
. Most of the info I found seems to suggest that they're almost interchangeable. I've already seen the exact same models / schema using both directives, hence my suspicion there might be still something off with the @interface
directive similar to the bug with the @union
directive. Commentable
as a polymorphic relation with @union
totally makes sence, but Searchable
doesn't seem to fit for this as I just can't imagine myself creating a dedicated table for search results with all the extra logic involved to keep it in sync.
Since I'm considering myself still a beginner when it comes to Eloquent, I'm still wondering how it's inheritance is supposed to work then. In Doctrine there were different ways of having polymorphic relationships with the materialized version being my favorite, since it would use separate tables with different columns for each type, yet allow inheritance of model relations, events and methods. Eg. a document model with a workflow logic alongside an invoice model, in a separate table, extending the document model, has proven to be very handy to say the least.
This still leaves me puzzled though about a very similar use case: search
Since search obviously doesn't require to store results as records in a database, I'll start with checking the various links you've posted so far to see how I can handle this gracefully. The end goal is still to use relay to page through a list of results, being of very different types. I don't want to add hydration logic on the client side so I'm definitely looking into getting GraphQL (with it's hydration aka fragment features) to do the heavy lifting here.
I'm familiar with Elasticsearch and keeping relational databases in sync with a more flat (and incredibly fast) external structure, but that will take me in a whole different direction (probably towards Laravel Scout). You can imagine I'm not feeling confident enough to implement this since Laravel has aparently still quite some secrets for me, left alone creating an Algolia replacement for Scout using Elastic. Personally I never even got why they chose Algolia over Elastic (we pbbly wouldn't be having this lenghty conversation if Elastic was the default driver), since it's a non-free service and Elastic is a no-brainer with it's huge knowledge base and proven stability.
Anyway, thanks again for your willingness to investigate my particular problem and shed some light on Laravel / Eloquent. If you already have references or posts on how you'd go about implementing search, with relay, please feel free let me know (or add them to the documentation.
Kind regards,
Erik
Hi @chrissm79,
after Googling a bit more, it seems like there really is no easy or efficient way to paginate through multiple models in Laravel / Eloquent and even the Scout package only allows for returning results of one type. So it seems Scout is handy to do the indexing with eg. Elastic (found some good looking driver projects), but when it comes to actually returning mixed records, no joy. Everyone seems to be dumping different collections into arrays (bummer), merging them (bigger bummer) and creating a new paginator from the result (even bigger bummer as it cannot use the database anymore).. this seems a terrible approach as every single record will be hydrated twice: once to dump it to an array and once to build the new paginator! This will definitely start failing once you're dealing with +10k records. I can't even begin to imagine how one would be able to paginate correctly considered sorting using this approach. So my initial thought that this would be an easy task seems very wrong to say the least.
Is there really no simple way to use a union query (with limit and orderby) that just returns id's and type names, and then have a paginator that can resolve the actual model for each result when needed? Like I said before: this seems a very simple process that seems easy to acomplish using the powers of both Lighthouse and Eloquent.
Still regarding the resolver of the @interface
directive: am I correct to assume that regardless of the Eloquent model relation behind the scenes, this method should at least be called once per returned result? I'm getting no errors and the resolver is simply ignored. To me this still sounds like a bug and I just can't handle searching for a needle in a haystack like I tried before with the @union
directive and you managed to fix this in a jiffy. If the resolve method would have been called, I'm sure I would have managed already using the forementioned union query approach and resolve each result to it's correct graphql type with Lighthouse. I'll be looking again at @kikoseijo 's approach as well.
So if you or anyone can just put a full @interface
example, including the required steps in Eloquent, in the documentation, this issue can die in peace :smile:
Or wait, I'll create a new issue regarding the @interface
resolve method not being called using Lumen / Relay and reference this issue there for further reading.
Feel free to close this issue if you've come this far. Thanks again!
Hi @chrissm79,
I think I'm getting very close! I'm getting mixed results from the resolve function in my query, it's just GraphQL not detecting the correct type, despite me returning the correct type in the Interface resolver.. So at least the InterfaceResolver is being called once now for each result, but somehow, because the paginator returns only Image records (known limitation of Eloquent), GraphQL somehow ignores this along the way and considers everything Images, despite me returning the correct type in the resolver.. maybe I'm still overlooking something or GraphQL / Lighthouse still uses the class from the paginator results instead of the resolved type class?
app/GraphQL/Queries/Search.php
namespace App\GraphQL\Queries;
use App\Models\Image;
use App\Models\Slogan;
use Nuwave\Lighthouse\Support\Schema\GraphQLQuery;
use Nuwave\Lighthouse\Support\Traits\HandlesGlobalId;
class Search extends GraphQLQuery
{
use HandlesGlobalId;
public function resolve()
{
$images = Image::query()->select('id', 'filename as data', 'created_at')
->selectRaw('"image" as type')
;
$slogans = Slogan::query()->select('id', 'slogan as data', 'created_at')
->selectRaw('"slogan" as type')
->union($images)
;
return $slogans->orderBy('created_at', 'DESC')->relayConnection($this->args);
}
}
app/GraphQL/SearchResultInterface.php
namespace App\GraphQL;
class SearchResultInterface
{
public function resolveType($value)
{
// use type from rawsql type value as Eloquent still casts all results as Images
// $value->type contains the actual type from the union query, eg "image" or "slogan"
// if error_logged to file the results are exactly what I need: images and slogans mixed, ordered by date
return schema()->instance(ucfirst($value->type));
}
}
app/GraphqQL/schema.graphql
interface SearchResult @interface(resolver: "App\\GraphQL\\SearchResultInterface@resolveType") {
id: ID! @globalId
created_at: DateTime
data: String!
type: String!
}
type Image implements SearchResult @model {
id: ID! @globalId
data: String! @rename(attribute: "filename")
type: String!
filename: String!
}
type Slogan implements SearchResult @model {
id: ID! @globalId
data: String! @rename(attribute: "slogan")
type: String!
slogan: String!
}
type Query {
viewer: User @auth
search (first: Int!, after: String): [SearchResult!]!
}
the query:
{
search (first: 100) {
id
created_at
type
...IFs
...SFs
}
}
fragment IFs on Image {
id
filename
}
fragment SFs on Slogan {
id
slogan
}
and results, which are somehow still casted as Images despite some of them being slogans
{
"data": {
"search": [
{
"id": "QXJ0d29yazo2Nzc=",
"created_at": "2018-05-27T14:00:05+02:00",
"type": "image",
"rating": null
},
{
"id": "QXJ0d29yazo2NzY=",
"created_at": "2018-05-23T10:00:05+02:00",
"type": "image",
"rating": null
},
...
}```
Strangly I do get the following GraphQL error if I define type: String! (required) in the Interace:
Error: SearchResult.type expects type "String!" but Image.type provides type "String".
The query actually loads, but the introspection failes. Removing the requirement from the SearchResult declaration in the schema solves this however..
I'll be still experimenting around, as this error doesn't seem to happen with regular interface fields..
IT WORKS!!!
I just had to use a different column than type
(I should have known since with all this magic happening underneath, type would pbbly clash with things..)
Still testing around as this is litteraly 20 seconds old :smile:
Too bad my solution still depends on the patch for the Eloquent query builder though.. I'll try to create another PR for it, but since my expectations with creating PR's for Laravel that will be accepted are pretty low, I might just resort to using a fork instead..
What a threat....
@4levels I beg it works, using type as a field name,,,, hahahahaha! You probably got it wrong from the base, as per architecture or data structure.
Why you need to call a morphed type by its own? they only should exist as a relationship to the models they belong to.
Come on! you can make it!
@4levels Looks like there's been some good progress lately, hopefully you're close to the finish line!!!
we pbbly wouldn't be having this lenghty conversation if Elastic was the default driver
This is so very true! I've yet to use ElasticSearch in a project but I see that it can provide a mixed set of results that you could probably hydrate back into the proper Laravel models and get your collection of mixed types. Depending on how that's done you still might need to go a special route to get it into a Relay connection but it shouldn't be too difficult. That would make for an amazing community plugin btw!!
Too bad my solution still depends on the patch for the Eloquent query builder though
Maybe you could create your own Query builder, pass in the two builders that need to be unioned and handle things as needed in there so you don't have to worry about changing the source? Of course that could much easier said than done and I haven't event looked at what's available on the builder that you need, but something along the line of:
// Inside resolver...
$imageBuilder = Image::query()->select('id', 'filename as data', 'created_at')
->selectRaw('"image" as type');
$sloganBuilder = Slogan::query()->select('id', 'slogan as data', 'created_at')
->selectRaw('"slogan" as type');
return UnionQueryBuilder::union($imageBuilder, $sloganBuilder)
->orderBy('created_at', 'DESC')
->relayConnection($this->args);
// inside UnionQueryBuilder...
class UnionQueryBuilder
{
public static function union($left, $right)
{
// based on your changes
// https://github.com/laravel/framework/issues/14837#issuecomment-391667831
$left->union($right);
return $left->newQuery()
->selectRaw("COUNT(*) as aggregate FROM (".$left->toSql().") as aggregate_query")
->mergeBindings($left);
}
}
Hi @chrissm79 and @kikoseijo,
I finally made it happen, but I had to use some old school tricks to trick Eloquent into believing me :smile: In a nutshell, I created a SearchResult eloquent model with table that will never contain any data, with the columns / attributes id, searchable_id, searchabletype, data and created at. I then used the forementioned trick to union the query builders and adding the expected searchable* field data for each model. I could then use the morphTo on the SearchResult model and add plain belongsTo relations to the Image and Slogan models under the name searchable. This results in me getting a basic search result where I can add arbitrary data using sql in the data column and have a searchable relation with the actual full record, all query-able and traversable by Relay! No time to paste code as I really need to hurry on with a deadline by tomorrow 4pm and I still have so much to do in RN! Expect a full example as a gist (or in the docs issue) anytime soon.
Thanks for bearing with me guys! Your support means a lot since there's virtually no one else to share this with..
Hi all,
I'm currently experimenting with the following use case, which seems like a perfect task for the
@union
directive.I have an Image and a Slogan model that have very similar properties and I need to have a mixed list exposed by graphql, combining the two types into one list. This should be a paginator from the Relay perspective.
From what I can read in the docs and unit tests, I'm not totally clear on how to go about the above situation. Eg. I don't want to specify a type as a requirement since I want to have both kind of results mixed in a single result set.
Maybe the
@interface
directive is the way to go? Any pointers in the right direction are greatly appreciated ;-)Kind regards,
Erik